From d0936fb0224ad048c01f34d39f13061aae62bbc6 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Fri, 22 May 2026 11:36:57 -0500 Subject: [PATCH] Implemented default config/campaign discovery for narratio session init --- docs/cli.md | 8 +- docs/operations.md | 4 +- internal/app/config_loader.go | 73 +++++++---- internal/app/operator_helpers.go | 30 ++--- internal/app/operator_helpers_test.go | 178 ++++++++++++++++++++++++++ 5 files changed, 240 insertions(+), 53 deletions(-) diff --git a/docs/cli.md b/docs/cli.md index 12a5298..fc0e104 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -148,8 +148,8 @@ Valid stage names: ### `session init` -- `--config `: required. -- `--campaign `: required. +- `--config `: optional explicit `pipeline.yml` path. +- `--campaign `: optional explicit `campaign.yml` path. - `--session-id `: required. - `--output `: local `session.yml` target; mutually exclusive with `--remote`. - `--remote`: write remote `session.yml` to the canonical session prefix; mutually exclusive with `--output`. @@ -302,12 +302,14 @@ Purpose: Syntax: ```bash -narratio session init --config --campaign --session-id --output ./session.yml +narratio session init --session-id --output ./session.yml +narratio session init --session-id --remote narratio session init --config --campaign --session-id --remote ``` Behavior: - exactly one of `--output` or `--remote` is required. +- `--config` and `--campaign` are optional overrides; omitted values use normal default config discovery. - remote writes target `{root_prefix}/campaigns/{campaign}/sessions/{session_id}/session.yml`. - existing local or remote targets fail unless `--force` is passed. - remote writes use existence checks, not compare-and-swap. diff --git a/docs/operations.md b/docs/operations.md index 9fadc1a..509a531 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -26,10 +26,10 @@ Notes: Initialize a remote session skeleton: ```bash -narratio session init --config /etc/narratio/pipeline.yml --campaign /etc/narratio/campaign.yml --session-id 2026-04-04 --remote +narratio session init --session-id 2026-04-04 --remote ``` -Remote init writes `{root_prefix}/campaigns/{campaign}/sessions/{session_id}/session.yml`. It fails if the object already exists unless `--force` is passed. +Remote init uses normal default config discovery and writes `{root_prefix}/campaigns/{campaign}/sessions/{session_id}/session.yml`. Pass `--config` and `--campaign` when testing non-system config files. It fails if the object already exists unless `--force` is passed. Validate before running: diff --git a/internal/app/config_loader.go b/internal/app/config_loader.go index a7f1396..4a4f90f 100644 --- a/internal/app/config_loader.go +++ b/internal/app/config_loader.go @@ -12,18 +12,21 @@ import ( "gitea.maximumdirect.net/eric/narratio/internal/config" ) +type pipelineCampaignConfig struct { + PipelinePath string + CampaignPath string + Pipeline *config.PipelineConfig + Campaign *config.CampaignConfig +} + func loadCommandConfig(ctx context.Context, pipelineFlag, campaignFlag, sessionFlag string, sessionOpts config.SessionLoadOptions) (*config.Config, error) { - resolvedPipelinePath, err := resolvePipelineConfigPath(pipelineFlag) - if err != nil { - return nil, err - } - resolvedCampaignPath, err := resolveCampaignConfigPath(campaignFlag) + base, err := loadPipelineCampaignConfig(pipelineFlag, campaignFlag) if err != nil { return nil, err } if explicitSession := strings.TrimSpace(sessionFlag); explicitSession != "" { - return config.LoadWithSessionOptions(resolvedPipelinePath, resolvedCampaignPath, explicitSession, sessionOpts) + return config.LoadWithSessionOptions(base.PipelinePath, base.CampaignPath, explicitSession, sessionOpts) } discoveredSession, err := discoverSessionConfigPathWithCandidates(config.DefaultSessionConfigSearchPaths) @@ -31,16 +34,7 @@ func loadCommandConfig(ctx context.Context, pipelineFlag, campaignFlag, sessionF return nil, err } if discoveredSession.Path != "" { - return config.LoadWithSessionOptions(resolvedPipelinePath, resolvedCampaignPath, discoveredSession.Path, sessionOpts) - } - - pipelineCfg, err := config.LoadPipeline(resolvedPipelinePath) - if err != nil { - return nil, err - } - campaignCfg, err := config.LoadCampaign(resolvedCampaignPath) - if err != nil { - return nil, err + return config.LoadWithSessionOptions(base.PipelinePath, base.CampaignPath, discoveredSession.Path, sessionOpts) } sessionID := strings.TrimSpace(sessionOpts.SessionID) @@ -48,13 +42,13 @@ func loadCommandConfig(ctx context.Context, pipelineFlag, campaignFlag, sessionF return nil, missingSessionConfigError(discoveredSession.Searched, "remote session loading requires --session-id") } - sessionPrefix := artifacts.S3SessionPrefix(pipelineCfg.Storage.S3.RootPrefix, campaignCfg.Campaign, sessionID) + sessionPrefix := artifacts.S3SessionPrefix(base.Pipeline.Storage.S3.RootPrefix, base.Campaign.Campaign, sessionID) remoteKey := artifacts.S3SessionConfigKey(sessionPrefix) partialCfg := &config.Config{ - Pipeline: pipelineCfg, - Campaign: campaignCfg, - PipelinePath: resolvedPipelinePath, - CampaignPath: resolvedCampaignPath, + Pipeline: base.Pipeline, + Campaign: base.Campaign, + PipelinePath: base.PipelinePath, + CampaignPath: base.CampaignPath, } store, err := newCommandObjectStore(ctx, partialCfg, nil) if err != nil { @@ -73,22 +67,22 @@ func loadCommandConfig(ctx context.Context, pipelineFlag, campaignFlag, sessionF if err != nil { return nil, fmt.Errorf("read downloaded remote session %q: %w", sessionTempPath, err) } - sessionCfg, err := config.LoadSessionBytesWithOptions("s3://"+s3BucketName(pipelineCfg)+"/"+remoteKey, sessionBytes, sessionOpts) + sessionCfg, err := config.LoadSessionBytesWithOptions("s3://"+s3BucketName(base.Pipeline)+"/"+remoteKey, sessionBytes, sessionOpts) if err != nil { return nil, err } return config.Resolve( - resolvedPipelinePath, - pipelineCfg, - resolvedCampaignPath, - campaignCfg, + base.PipelinePath, + base.Pipeline, + base.CampaignPath, + base.Campaign, sessionTempPath, sessionCfg, config.SessionSource{ Source: "session_config.s3", LocalPath: sessionTempPath, - S3Bucket: s3BucketName(pipelineCfg), + S3Bucket: s3BucketName(base.Pipeline), S3Key: remoteKey, S3Size: sessionInfo.Size, S3ETag: sessionInfo.ETag, @@ -97,6 +91,31 @@ func loadCommandConfig(ctx context.Context, pipelineFlag, campaignFlag, sessionF ) } +func loadPipelineCampaignConfig(pipelineFlag, campaignFlag string) (*pipelineCampaignConfig, error) { + resolvedPipelinePath, err := resolvePipelineConfigPath(pipelineFlag) + if err != nil { + return nil, err + } + resolvedCampaignPath, err := resolveCampaignConfigPath(campaignFlag) + if err != nil { + return nil, err + } + pipelineCfg, err := config.LoadPipeline(resolvedPipelinePath) + if err != nil { + return nil, err + } + campaignCfg, err := config.LoadCampaign(resolvedCampaignPath) + if err != nil { + return nil, err + } + return &pipelineCampaignConfig{ + PipelinePath: resolvedPipelinePath, + CampaignPath: resolvedCampaignPath, + Pipeline: pipelineCfg, + Campaign: campaignCfg, + }, nil +} + func findRemoteSessionConfig(ctx context.Context, store storage.ObjectStore, sessionPrefix, remoteKey string) (storage.ObjectInfo, error) { objects, err := store.List(ctx, sessionPrefix) if err != nil { diff --git a/internal/app/operator_helpers.go b/internal/app/operator_helpers.go index 39c5d84..bf88a3c 100644 --- a/internal/app/operator_helpers.go +++ b/internal/app/operator_helpers.go @@ -258,8 +258,8 @@ func SessionInit(ctx context.Context, args []string, out io.Writer) error { fs.SetOutput(io.Discard) var pipelinePath, campaignPath, sessionID, previousSessionID, date, title, output, audioS3Prefix, audioDir string var remote, force bool - fs.StringVar(&pipelinePath, "config", "", "path to pipeline.yml") - fs.StringVar(&campaignPath, "campaign", "", "path to campaign.yml") + fs.StringVar(&pipelinePath, "config", "", "path to pipeline.yml (optional; defaults searched)") + fs.StringVar(&campaignPath, "campaign", "", "path to campaign.yml (optional; defaults searched)") fs.StringVar(&sessionID, "session-id", "", "session identifier") fs.StringVar(&previousSessionID, "previous-session-id", "", "previous session identifier") fs.StringVar(&date, "date", "", "session date") @@ -275,8 +275,8 @@ func SessionInit(ctx context.Context, args []string, out io.Writer) error { if fs.NArg() != 0 { return fmt.Errorf("session init: unexpected positional arguments") } - if strings.TrimSpace(pipelinePath) == "" || strings.TrimSpace(campaignPath) == "" || strings.TrimSpace(sessionID) == "" { - return fmt.Errorf("session init: --config, --campaign, and --session-id are required") + if strings.TrimSpace(sessionID) == "" { + return fmt.Errorf("session init: --session-id is required") } if (strings.TrimSpace(output) == "") == !remote { return fmt.Errorf("session init: specify exactly one target: --output or --remote") @@ -285,24 +285,12 @@ func SessionInit(ctx context.Context, args []string, out io.Writer) error { return fmt.Errorf("session init: --audio-dir and --audio-s3-prefix are mutually exclusive") } - resolvedPipeline, err := resolvePipelineConfigPath(pipelinePath) - if err != nil { - return fmt.Errorf("session init: %w", err) - } - resolvedCampaign, err := resolveCampaignConfigPath(campaignPath) - if err != nil { - return fmt.Errorf("session init: %w", err) - } - pipelineCfg, err := config.LoadPipeline(resolvedPipeline) - if err != nil { - return fmt.Errorf("session init: %w", err) - } - campaignCfg, err := config.LoadCampaign(resolvedCampaign) + base, err := loadPipelineCampaignConfig(pipelinePath, campaignPath) if err != nil { return fmt.Errorf("session init: %w", err) } - data, err := buildSessionYAML(campaignCfg.Campaign, sessionID, previousSessionID, date, title, audioS3Prefix, audioDir) + data, err := buildSessionYAML(base.Campaign.Campaign, sessionID, previousSessionID, date, title, audioS3Prefix, audioDir) if err != nil { return fmt.Errorf("session init: %w", err) } @@ -317,7 +305,7 @@ func SessionInit(ctx context.Context, args []string, out io.Writer) error { if err != nil { return fmt.Errorf("session init: %w", err) } - cfg, err := config.Resolve(resolvedPipeline, pipelineCfg, resolvedCampaign, campaignCfg, label, sessionCfg, config.SessionSource{Source: "session_config", LocalPath: label}) + cfg, err := config.Resolve(base.PipelinePath, base.Pipeline, base.CampaignPath, base.Campaign, label, sessionCfg, config.SessionSource{Source: "session_config", LocalPath: label}) if err != nil { return fmt.Errorf("session init: %w", err) } @@ -337,7 +325,7 @@ func SessionInit(ctx context.Context, args []string, out io.Writer) error { if err != nil { return fmt.Errorf("session init: %w", err) } - sessionPrefix := artifacts.S3SessionPrefix(pipelineCfg.Storage.S3.RootPrefix, campaignCfg.Campaign, sessionID) + sessionPrefix := artifacts.S3SessionPrefix(base.Pipeline.Storage.S3.RootPrefix, base.Campaign.Campaign, sessionID) key := artifacts.S3SessionConfigKey(sessionPrefix) exists, err := store.Exists(ctx, key) if err != nil { @@ -362,7 +350,7 @@ func SessionInit(ctx context.Context, args []string, out io.Writer) error { if _, err := store.Upload(ctx, tmpPath, key, storage.UploadOptions{ContentType: "application/x-yaml; charset=utf-8"}); err != nil { return fmt.Errorf("session init: upload remote session %q: %w", key, err) } - _, err = fmt.Fprintf(out, "narratio session init: wrote s3://%s/%s\n", s3BucketName(pipelineCfg), key) + _, err = fmt.Fprintf(out, "narratio session init: wrote s3://%s/%s\n", s3BucketName(base.Pipeline), key) return err } diff --git a/internal/app/operator_helpers_test.go b/internal/app/operator_helpers_test.go index 538df6c..ed6012f 100644 --- a/internal/app/operator_helpers_test.go +++ b/internal/app/operator_helpers_test.go @@ -48,6 +48,172 @@ func TestExecuteSessionInitRemoteWritesCanonicalSessionConfig(t *testing.T) { } } +func TestExecuteSessionInitRemoteUsesDefaultConfigDiscovery(t *testing.T) { + workspaceRoot := t.TempDir() + pipelinePath, campaignPath, _ := writeValidConfigFiles(t, workspaceRoot) + withDefaultPipelineCampaignConfigs(t, pipelinePath, campaignPath) + fake := &storage.FakeBackend{} + var storeInitCalls int + restoreAppConfigTestGlobals(t, fake, &storeInitCalls, []string{filepath.Join(t.TempDir(), "session.yml")}) + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := Execute([]string{ + "session", "init", + "--session-id", "2026-06-07", + "--remote", + }, &stdout, &stderr) + if code != 0 { + t.Fatalf("exit code = %d, want 0; stderr=%q", code, stderr.String()) + } + key := artifacts.S3SessionConfigKey(artifacts.S3SessionPrefix("dnd", "sample-campaign", "2026-06-07")) + if _, ok := fake.Objects[key]; !ok { + t.Fatalf("remote session key %q not uploaded; objects=%v", key, fake.Objects) + } + if storeInitCalls != 1 { + t.Fatalf("object store init calls = %d, want 1", storeInitCalls) + } +} + +func TestExecuteSessionInitLocalUsesDefaultConfigDiscovery(t *testing.T) { + workspaceRoot := t.TempDir() + pipelinePath, campaignPath, _ := writeValidConfigFiles(t, workspaceRoot) + withDefaultPipelineCampaignConfigs(t, pipelinePath, campaignPath) + outputPath := filepath.Join(t.TempDir(), "session.yml") + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := Execute([]string{ + "session", "init", + "--session-id", "2026-06-07", + "--output", outputPath, + }, &stdout, &stderr) + if code != 0 { + t.Fatalf("exit code = %d, want 0; stderr=%q", code, stderr.String()) + } + data, err := os.ReadFile(outputPath) + if err != nil { + t.Fatalf("read generated session: %v", err) + } + if !strings.Contains(string(data), `session_id: "2026-06-07"`) || !strings.Contains(string(data), "prefix: audio/") { + t.Fatalf("generated session = %q", string(data)) + } +} + +func TestExecuteSessionInitExplicitConfigWinsOverDefaults(t *testing.T) { + workspaceRoot := t.TempDir() + defaultPipeline, defaultCampaign, _ := writeValidConfigFiles(t, workspaceRoot) + withDefaultPipelineCampaignConfigs(t, defaultPipeline, defaultCampaign) + + explicitDir := t.TempDir() + explicitCampaign := filepath.Join(explicitDir, "campaign.yml") + if err := os.WriteFile(explicitCampaign, []byte(`campaign: explicit-campaign +inputs: + speakers_file: ./speakers.yml + autocorrect_file: ./autocorrect.yml + glossary_file: ./glossary.yml +`), 0o644); err != nil { + t.Fatalf("write explicit campaign: %v", err) + } + mustWriteTestFile(t, filepath.Join(explicitDir, "speakers.yml"), "match:\n - speaker: Alice\n match: [\"alice\"]\n") + mustWriteTestFile(t, filepath.Join(explicitDir, "autocorrect.yml"), "[]\n") + mustWriteTestFile(t, filepath.Join(explicitDir, "glossary.yml"), "[]\n") + + fake := &storage.FakeBackend{} + var storeInitCalls int + restoreAppConfigTestGlobals(t, fake, &storeInitCalls, []string{filepath.Join(t.TempDir(), "session.yml")}) + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := Execute([]string{ + "session", "init", + "--config", defaultPipeline, + "--campaign", explicitCampaign, + "--session-id", "2026-06-07", + "--remote", + }, &stdout, &stderr) + if code != 0 { + t.Fatalf("exit code = %d, want 0; stderr=%q", code, stderr.String()) + } + explicitKey := artifacts.S3SessionConfigKey(artifacts.S3SessionPrefix("dnd", "explicit-campaign", "2026-06-07")) + if _, ok := fake.Objects[explicitKey]; !ok { + t.Fatalf("explicit campaign remote key %q not uploaded; objects=%v", explicitKey, fake.Objects) + } + defaultKey := artifacts.S3SessionConfigKey(artifacts.S3SessionPrefix("dnd", "sample-campaign", "2026-06-07")) + if _, ok := fake.Objects[defaultKey]; ok { + t.Fatalf("default campaign key %q uploaded despite explicit campaign override", defaultKey) + } +} + +func TestExecuteSessionInitRequiresSessionID(t *testing.T) { + workspaceRoot := t.TempDir() + pipelinePath, campaignPath, _ := writeValidConfigFiles(t, workspaceRoot) + withDefaultPipelineCampaignConfigs(t, pipelinePath, campaignPath) + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := Execute([]string{"session", "init", "--remote"}, &stdout, &stderr) + if code == 0 { + t.Fatal("exit code = 0, want non-zero") + } + if !strings.Contains(stderr.String(), "session init: --session-id is required") { + t.Fatalf("stderr = %q, want session-id required error", stderr.String()) + } +} + +func TestExecuteSessionInitMissingDefaultConfigReportsSearchedPaths(t *testing.T) { + origPipelineDefaults := append([]string(nil), config.DefaultPipelineConfigSearchPaths...) + origCampaignDefaults := append([]string(nil), config.DefaultCampaignConfigSearchPaths...) + config.DefaultPipelineConfigSearchPaths = []string{filepath.Join(t.TempDir(), "missing-pipeline.yml")} + config.DefaultCampaignConfigSearchPaths = []string{filepath.Join(t.TempDir(), "missing-campaign.yml")} + t.Cleanup(func() { + config.DefaultPipelineConfigSearchPaths = origPipelineDefaults + config.DefaultCampaignConfigSearchPaths = origCampaignDefaults + }) + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := Execute([]string{"session", "init", "--session-id", "2026-06-07", "--remote"}, &stdout, &stderr) + if code == 0 { + t.Fatal("exit code = 0, want non-zero") + } + if !strings.Contains(stderr.String(), "session init: no pipeline config path provided and no default pipeline config found; searched:") { + t.Fatalf("stderr = %q, want default pipeline searched-path error", stderr.String()) + } +} + +func TestExecuteSessionInitRemoteLoadsSecretsBeforeObjectStoreInit(t *testing.T) { + workspaceRoot := t.TempDir() + pipelinePath, campaignPath, _ := writeValidConfigFiles(t, workspaceRoot) + withDefaultPipelineCampaignConfigs(t, pipelinePath, campaignPath) + accessKeyEnv := "NARRATIO_TEST_SESSION_INIT_OBJECT_KEY_ID" + secretKeyEnv := "NARRATIO_TEST_SESSION_INIT_OBJECT_SECRET" + restoreEnvAfterTest(t, accessKeyEnv, secretKeyEnv) + secretsDir := t.TempDir() + mustWriteTestFile(t, filepath.Join(secretsDir, accessKeyEnv), "test-key-id\n") + mustWriteTestFile(t, filepath.Join(secretsDir, secretKeyEnv), "test-secret\n") + addSecretsToPipelineConfig(t, pipelinePath, secretsDir, accessKeyEnv, secretKeyEnv) + + fake := &storage.FakeBackend{} + origStoreFn := newObjectStoreFromConfigFn + newObjectStoreFromConfigFn = func(context.Context, *config.Config) (storage.ObjectStore, error) { + if os.Getenv(accessKeyEnv) != "test-key-id" || os.Getenv(secretKeyEnv) != "test-secret" { + return nil, fmt.Errorf("secrets were not loaded before object store init") + } + return fake, nil + } + t.Cleanup(func() { + newObjectStoreFromConfigFn = origStoreFn + }) + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := Execute([]string{"session", "init", "--session-id", "2026-06-07", "--remote"}, &stdout, &stderr) + if code != 0 { + t.Fatalf("exit code = %d, want 0; stderr=%q", code, stderr.String()) + } +} + func TestExecuteSessionValidateLoadsSecretsBeforeObjectStoreInit(t *testing.T) { workspaceRoot := t.TempDir() pipelinePath, campaignPath, sessionPath := writeValidConfigFiles(t, workspaceRoot) @@ -308,6 +474,18 @@ func TestExecuteTopLevelLockAndUnlockAreRemoved(t *testing.T) { } } +func withDefaultPipelineCampaignConfigs(t *testing.T, pipelinePath, campaignPath string) { + t.Helper() + origPipelineDefaults := append([]string(nil), config.DefaultPipelineConfigSearchPaths...) + origCampaignDefaults := append([]string(nil), config.DefaultCampaignConfigSearchPaths...) + config.DefaultPipelineConfigSearchPaths = []string{pipelinePath} + config.DefaultCampaignConfigSearchPaths = []string{campaignPath} + t.Cleanup(func() { + config.DefaultPipelineConfigSearchPaths = origPipelineDefaults + config.DefaultCampaignConfigSearchPaths = origCampaignDefaults + }) +} + func TestExecuteArtifactsListRemoteReportsPromotedAvailability(t *testing.T) { workspaceRoot := t.TempDir() pipelinePath, campaignPath, sessionPath := writeValidConfigFiles(t, workspaceRoot)