From 7324c5a686aa193dc19fd9465a9ffd1d8f43c55f Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Fri, 22 May 2026 17:38:23 -0500 Subject: [PATCH] Session configuration templates are now proceeded by narratio session init; all other commands require concrete configuration --- docs/cli.md | 20 +- docs/config.md | 49 +++-- docs/operations.md | 2 +- docs/roadmap/operations.md | 52 ------ docs/troubleshooting.md | 12 +- examples/campaign.yml | 1 + internal/app/operator_helpers.go | 115 +++++++++++- internal/app/operator_helpers_test.go | 227 +++++++++++++++++++++++ internal/app/plan.go | 4 +- internal/app/remote_session_test.go | 46 ++++- internal/app/restore.go | 4 +- internal/app/resume.go | 4 +- internal/app/run.go | 4 +- internal/app/run_stage.go | 12 +- internal/app/session_cli_test.go | 16 +- internal/config/campaign_config_test.go | 15 ++ internal/config/config.go | 5 +- internal/config/load.go | 103 +++------- internal/config/load_validate_test.go | 19 +- internal/config/session_template_test.go | 139 ++++---------- 20 files changed, 550 insertions(+), 299 deletions(-) delete mode 100644 docs/roadmap/operations.md diff --git a/docs/cli.md b/docs/cli.md index fc0e104..c1de003 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -10,6 +10,8 @@ This command uses default system discovery for `pipeline.yml`, `campaign.yml`, a Default discovery checks system config locations only. Pass `--config`, `--campaign`, and `--session` to use files from the current working directory. +Ordinary local and remote `session.yml` files must be concrete YAML. Templates belong to `narratio session init`, which renders a configured campaign template before writing the concrete file. + ## Command Overview Implemented commands: @@ -39,8 +41,8 @@ For config semantics, see [docs/config.md](./config.md). For operator lifecycle - `--config `: optional explicit `pipeline.yml` path. - `--campaign `: optional explicit `campaign.yml` path. - `--session `: optional explicit `session.yml` path. -- `--session-id `: session template variable value. -- `--previous-session-id `: previous-session template variable value. +- `--session-id `: expected session identifier and remote session lookup value. +- `--previous-session-id `: expected previous session identifier. - `--force`: force stage execution. - `--artifacts `: analyze artifact keys to execute (repeatable or comma-separated). @@ -160,6 +162,8 @@ Valid stage names: - `--audio-dir `: local audio directory; mutually exclusive with `--audio-s3-prefix`. - `--force`: overwrite existing local or remote target. +When `campaign.yml` sets `session_template_file`, `session init` renders that template before writing the concrete session file. Template variables are supplied by these flags: `--session-id`, `--previous-session-id`, `--date`, `--title`, `--audio-s3-prefix`, and `--audio-dir`. + ### `artifacts list` - `--config ` @@ -175,7 +179,7 @@ Valid stage names: - `--config `: optional explicit `pipeline.yml` path. - `--campaign `: optional explicit `campaign.yml` path. - `--session `: optional explicit `session.yml` path. -- `--previous-session-id `: optional session template value. +- `--previous-session-id `: optional expected previous session identifier. - `add `: add a remote lock for one artifact or transcript source. - `add --reason `: record an optional remote lock reason. - `add --force`: update the reason for an existing remote lock. @@ -200,7 +204,8 @@ Success output: Common failure cases: - missing system default config/campaign/session paths when flags omitted. - missing local session plus missing/unavailable remote `session.yml`. -- invalid template/rendered session mismatch. +- templated `session.yml`; run `narratio session init` to generate concrete YAML. +- concrete session identity mismatch. - unknown/invalid `--artifacts` value. - `--artifacts` with unknown configured artifact key. @@ -241,7 +246,7 @@ Success output: - or `narratio resume: session ; executed= skipped=; manifest=` Common failure cases: -- same discovery/template/validation failures as `run`. +- same discovery and validation failures as `run`. - manifest load errors when existing manifest is unreadable. - invalid or unknown artifact selections. @@ -297,7 +302,7 @@ Warnings do not fail the command. Any `ERROR` finding exits non-zero. ### `session init` Purpose: -- Create a strict-decoded session skeleton locally or in object storage. +- Create a strict-decoded concrete `session.yml` locally or in object storage. Syntax: @@ -310,6 +315,9 @@ narratio session init --config --campaign --sessio Behavior: - exactly one of `--output` or `--remote` is required. - `--config` and `--campaign` are optional overrides; omitted values use normal default config discovery. +- if `campaign.yml` sets `session_template_file`, the template path is resolved relative to `campaign.yml` and rendered from init flags. +- if no session template is configured, a minimal concrete session file is generated directly. +- template variables must be supplied by matching flags, and supplied template-related flags must be used by the template. - 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/config.md b/docs/config.md index 0383c19..114812e 100644 --- a/docs/config.md +++ b/docs/config.md @@ -19,11 +19,11 @@ These commands load and validate all three files before running: Behavior: - strict YAML decode is enabled (`KnownFields(true)`): unknown fields fail. -- session templates render before session YAML decode. -- remote `session.yml` uses the same strict decode and template behavior as local `session.yml`. +- ordinary local and remote `session.yml` files must be concrete YAML; template placeholders are rejected. - defaults are applied for optional pipeline fields. - campaign-level stable input paths fill missing session input paths. - session-level stable input paths override campaign-level input paths. +- campaign config may point `session init` to a session template. - validation enforces required fields, value formats, and cross-field constraints. ## 2. Config file discovery @@ -66,18 +66,29 @@ Session config lookup: ## 3. Session templating -Template behavior for local and remote `session.yml`: +Template behavior for local and remote `session.yml` loaded by downstream commands: -- supported placeholders: - - `{{session_id}}` +- downstream commands do not render templates. +- local and remote `session.yml` must be concrete. +- any `{{ ... }}` placeholder in loaded `session.yml` fails with guidance to run `narratio session init`. +- if concrete `session_id` mismatches `--session-id`, load fails. +- if concrete `previous_session_id` mismatches `--previous-session-id`, load fails. + +Template behavior for `narratio session init`: + +- `campaign.yml` may set `session_template_file`. +- relative template paths resolve relative to `campaign.yml`. +- supported init template variables: - `{{ session_id }}` - - `{{previous_session_id}}` - `{{ previous_session_id }}` -- `--session-id ` supplies the placeholder value. -- `--previous-session-id ` supplies the previous-session placeholder value. -- unresolved placeholders fail load. -- if rendered `session_id` mismatches `--session-id`, load fails. -- if rendered `previous_session_id` mismatches `--previous-session-id`, load fails. + - `{{ date }}` + - `{{ title }}` + - `{{ audio_s3_prefix }}` + - `{{ audio_dir }}` +- each template variable must be supplied by the matching `session init` flag. +- template-related flags such as `--date`, `--title`, `--audio-s3-prefix`, `--audio-dir`, and `--previous-session-id` fail if the configured template does not use them. +- rendered output is strict-decoded and validated before it is written locally or remotely. +- if `session_template_file` is omitted, `session init` generates the minimal concrete session YAML directly. ## 4. Minimal config set @@ -98,6 +109,7 @@ Why this is sufficient: ```yaml campaign: sample-campaign +session_template_file: ./session.template.yml inputs: speakers_file: ./speakers.yml autocorrect_file: ./autocorrect.yml @@ -112,14 +124,14 @@ Why this is sufficient: ### `session.yml` ```yaml -session_id: "{{ session_id }}" +session_id: 2026-05-03 inputs: audio_dir: ./audio ``` Why this is sufficient: -- `session_id` is required and can be rendered from `--session-id`. +- `session_id` is required. - `campaign` can be omitted because it is supplied by `campaign.yml`. - stable input paths can be omitted because `campaign.yml` supplies defaults. - local `audio_dir` resolves relative to `session.yml`. @@ -133,8 +145,8 @@ narratio run --config /path/to/pipeline.yml --campaign ./campaign.yml --session Previous-session-enabled variant: ```yaml -session_id: "{{ session_id }}" -previous_session_id: "{{ previous_session_id }}" +session_id: 2026-05-03 +previous_session_id: 2026-04-26 inputs: audio_dir: ./audio ``` @@ -214,8 +226,8 @@ inputs: ### Local `session.yml` ```yaml -session_id: "{{ session_id }}" -previous_session_id: "{{ previous_session_id }}" +session_id: 2026-05-03 +previous_session_id: 2026-04-26 date: 2026-05-03 title: The Black Cabin inputs: @@ -416,11 +428,12 @@ Restore-related implications: | Path | Type | Required | Default | | --- | --- | --- | --- | | `campaign.campaign` | string | Yes | none | +| `campaign.session_template_file` | string | No | none | | `campaign.inputs.speakers_file` | string | Yes | none | | `campaign.inputs.autocorrect_file` | string | Yes | none | | `campaign.inputs.glossary_file` | string | Yes | none | -Campaign input paths may be absolute or relative. Relative paths resolve from the directory containing `campaign.yml`. +Campaign input paths and `campaign.session_template_file` may be absolute or relative. Relative paths resolve from the directory containing `campaign.yml`. ## 8. Full session reference diff --git a/docs/operations.md b/docs/operations.md index 509a531..fb2ab93 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -29,7 +29,7 @@ Initialize a remote session skeleton: narratio session init --session-id 2026-04-04 --remote ``` -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. +Remote init uses normal default config discovery and writes `{root_prefix}/campaigns/{campaign}/sessions/{session_id}/session.yml`. If `campaign.yml` sets `session_template_file`, init renders that template from the supplied flags and writes concrete YAML. 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/docs/roadmap/operations.md b/docs/roadmap/operations.md deleted file mode 100644 index c86e06e..0000000 --- a/docs/roadmap/operations.md +++ /dev/null @@ -1,52 +0,0 @@ -# Roadmap: Operator Helper Commands - -## Status - -Implemented. - -The operator helper command set is no longer conceptual. Current behavior is documented in: - -- `docs/cli.md` -- `docs/operations.md` -- `docs/config.md` -- `docs/internal/artifacts.md` -- `docs/internal/stage-archive.md` - -## Implemented Commands - -- `narratio session validate` -- `narratio status --manifest ` -- `narratio status --session-id ` -- `narratio session init --output ` -- `narratio session init --remote` -- `narratio artifacts list` -- `narratio artifacts list --remote` -- `narratio locks` -- `narratio locks add ` -- `narratio locks remove ` - -## Implemented Decisions - -- Helper output is text-only. No JSON schema exists yet. -- `status` remains a top-level command. -- `session validate`, `session init`, and `artifacts list` are nested helper commands. -- `locks` is the single top-level command for listing, adding, and removing archive promotion locks. -- Remote session initialization requires explicit `--remote`. -- Local session initialization requires `--output`. -- Remote artifact availability is opt-in with `artifacts list --remote`. -- Mutable locks are source-based and stored at `{session_prefix}/locks.yml`. -- The remote lock store uses strict YAML with top-level `locks`. -- Static `pipeline.archive.locks` and remote locks are merged; static locks win on duplicate sources. -- `locks remove` removes only remote locks. -- Ordinary execution `--force` does not override locks. -- Remote lock writes use existence checks and `--force` for updates; there is no compare-and-swap protection. - -## Remaining Future Enhancements - -These are intentionally not implemented: - -- `--json` output for helper commands. -- Optimistic concurrency or ETag compare-and-swap for remote lock mutations. -- Rich remote artifact availability across historical run-local objects. -- Session-lock acquisition for remote mutation helpers. -- Broader campaign helper commands such as `campaign validate` or `campaign publish`. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 604d3c8..ae02c8f 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -28,14 +28,14 @@ Links: - [docs/config.md](./config.md) - [docs/cli.md](./cli.md) -## Session template rendering failure +## Templated session file rejected Symptom: -- load fails with unresolved placeholder or `session_id` mismatch. +- load fails with a message that `session.yml must be concrete`. Likely Cause: -- templated `session.yml` used without `--session-id`. -- rendered `session_id` differs from passed `--session-id`. +- a template authoring file such as `session.template.yml` was passed to `--session` or uploaded as remote `session.yml`. +- `session.yml` still contains `{{ ... }}` placeholders. Diagnostics: @@ -44,8 +44,8 @@ narratio plan --config /path/to/pipeline.yml --campaign /path/to/campaign.yml -- ``` Safe Fix: -- pass `--session-id` when template placeholders are present. -- ensure rendered `session_id` matches intended run session id. +- generate concrete YAML with `narratio session init`. +- pass the generated concrete `session.yml` to downstream commands or upload it through `session init --remote`. Links: - [docs/config.md](./config.md) diff --git a/examples/campaign.yml b/examples/campaign.yml index 226624e..148c663 100644 --- a/examples/campaign.yml +++ b/examples/campaign.yml @@ -1,4 +1,5 @@ campaign: sample-campaign +session_template_file: ./session.template.yml inputs: speakers_file: ./speakers.yml autocorrect_file: ./autocorrect.yml diff --git a/internal/app/operator_helpers.go b/internal/app/operator_helpers.go index bf88a3c..0cc5687 100644 --- a/internal/app/operator_helpers.go +++ b/internal/app/operator_helpers.go @@ -44,8 +44,8 @@ func addCommonConfigFlags(fs *flag.FlagSet, flags *commonConfigFlags) { fs.StringVar(&flags.pipelinePath, "config", "", "path to pipeline.yml (optional; defaults searched)") fs.StringVar(&flags.campaignPath, "campaign", "", "path to campaign.yml (optional; defaults searched)") fs.StringVar(&flags.sessionPath, "session", "", "path to session.yml") - fs.StringVar(&flags.sessionID, "session-id", "", "session identifier for session.yml templates") - fs.StringVar(&flags.previousSessionID, "previous-session-id", "", "previous session identifier for session.yml templates") + fs.StringVar(&flags.sessionID, "session-id", "", "expected session identifier and remote session lookup value") + fs.StringVar(&flags.previousSessionID, "previous-session-id", "", "expected previous session identifier") } func (f commonConfigFlags) sessionOptions() config.SessionLoadOptions { @@ -290,7 +290,18 @@ func SessionInit(ctx context.Context, args []string, out io.Writer) error { return fmt.Errorf("session init: %w", err) } - data, err := buildSessionYAML(base.Campaign.Campaign, sessionID, previousSessionID, date, title, audioS3Prefix, audioDir) + input := sessionInitInput{ + Campaign: base.Campaign.Campaign, + CampaignPath: base.CampaignPath, + TemplateFile: base.Campaign.SessionTemplateFile, + SessionID: sessionID, + PreviousSessionID: previousSessionID, + Date: date, + Title: title, + AudioS3Prefix: audioS3Prefix, + AudioDir: audioDir, + } + data, err := buildSessionInitYAML(input) if err != nil { return fmt.Errorf("session init: %w", err) } @@ -588,6 +599,104 @@ func buildSessionYAML(campaign, sessionID, previousSessionID, date, title, audio return data, nil } +type sessionInitInput struct { + Campaign string + CampaignPath string + TemplateFile string + SessionID string + PreviousSessionID string + Date string + Title string + AudioS3Prefix string + AudioDir string +} + +func buildSessionInitYAML(in sessionInitInput) ([]byte, error) { + if strings.TrimSpace(in.TemplateFile) == "" { + return buildSessionYAML(in.Campaign, in.SessionID, in.PreviousSessionID, in.Date, in.Title, in.AudioS3Prefix, in.AudioDir) + } + templatePath := resolveSessionInitTemplatePath(in.CampaignPath, in.TemplateFile) + templateBytes, err := os.ReadFile(templatePath) + if err != nil { + return nil, fmt.Errorf("read session template %q: %w", templatePath, err) + } + rendered, err := renderSessionInitTemplate(string(templateBytes), in) + if err != nil { + return nil, fmt.Errorf("render session template %q: %w", templatePath, err) + } + return []byte(rendered), nil +} + +func resolveSessionInitTemplatePath(campaignPath, templateFile string) string { + templateFile = strings.TrimSpace(templateFile) + if filepath.IsAbs(templateFile) { + return filepath.Clean(templateFile) + } + return filepath.Clean(filepath.Join(filepath.Dir(campaignPath), templateFile)) +} + +var sessionInitTemplatePattern = regexp.MustCompile(`\{\{\s*([a-zA-Z_][a-zA-Z0-9_]*)\s*\}\}`) + +func renderSessionInitTemplate(content string, in sessionInitInput) (string, error) { + values := map[string]string{ + "session_id": strings.TrimSpace(in.SessionID), + "previous_session_id": strings.TrimSpace(in.PreviousSessionID), + "date": strings.TrimSpace(in.Date), + "title": strings.TrimSpace(in.Title), + "audio_s3_prefix": strings.TrimSpace(in.AudioS3Prefix), + "audio_dir": strings.TrimSpace(in.AudioDir), + } + used := map[string]struct{}{} + unknown := map[string]struct{}{} + missing := map[string]struct{}{} + rendered := sessionInitTemplatePattern.ReplaceAllStringFunc(content, func(match string) string { + parts := sessionInitTemplatePattern.FindStringSubmatch(match) + if len(parts) < 2 { + return match + } + name := parts[1] + value, ok := values[name] + if !ok { + unknown[name] = struct{}{} + return match + } + used[name] = struct{}{} + if value == "" { + missing[name] = struct{}{} + return match + } + return value + }) + if len(unknown) > 0 { + return "", fmt.Errorf("unsupported template variable(s): %s", sortedStringSet(unknown)) + } + if len(missing) > 0 { + return "", fmt.Errorf("missing required template variable value(s): %s", sortedStringSet(missing)) + } + unused := map[string]struct{}{} + for _, name := range []string{"previous_session_id", "date", "title", "audio_s3_prefix", "audio_dir"} { + if values[name] == "" { + continue + } + if _, ok := used[name]; !ok { + unused[name] = struct{}{} + } + } + if len(unused) > 0 { + return "", fmt.Errorf("unused template variable value(s): %s", sortedStringSet(unused)) + } + return rendered, nil +} + +func sortedStringSet(set map[string]struct{}) string { + items := make([]string, 0, len(set)) + for item := range set { + items = append(items, item) + } + sort.Strings(items) + return strings.Join(items, ", ") +} + func renderFindings(out io.Writer, campaign, sessionID string, findings []finding) error { if campaign != "" || sessionID != "" { fmt.Fprintf(out, "Campaign: %s\n", campaign) diff --git a/internal/app/operator_helpers_test.go b/internal/app/operator_helpers_test.go index ed6012f..61f0cb0 100644 --- a/internal/app/operator_helpers_test.go +++ b/internal/app/operator_helpers_test.go @@ -214,6 +214,209 @@ func TestExecuteSessionInitRemoteLoadsSecretsBeforeObjectStoreInit(t *testing.T) } } +func TestExecuteSessionInitLocalRendersCampaignTemplate(t *testing.T) { + workspaceRoot := t.TempDir() + pipelinePath, campaignPath, _ := writeValidConfigFiles(t, workspaceRoot) + writeSessionInitTemplate(t, campaignPath, `session_id: "{{ session_id }}" +previous_session_id: "{{ previous_session_id }}" +date: "{{ date }}" +title: "{{ title }}" +inputs: + audio_s3: + prefix: "{{ audio_s3_prefix }}" +`) + outputPath := filepath.Join(t.TempDir(), "session.yml") + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := Execute([]string{ + "session", "init", + "--config", pipelinePath, + "--campaign", campaignPath, + "--session-id", "2026-06-07", + "--previous-session-id", "2026-05-31", + "--date", "2026-06-07", + "--title", "The Black Cabin", + "--audio-s3-prefix", "audio/", + "--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) + } + got := string(data) + for _, want := range []string{ + `session_id: "2026-06-07"`, + `previous_session_id: "2026-05-31"`, + `date: "2026-06-07"`, + `title: "The Black Cabin"`, + `prefix: "audio/"`, + } { + if !strings.Contains(got, want) { + t.Fatalf("generated session = %q, want %q", got, want) + } + } + if strings.Contains(got, "{{") { + t.Fatalf("generated session still contains template placeholder: %q", got) + } +} + +func TestExecuteSessionInitRemoteRendersCampaignTemplate(t *testing.T) { + workspaceRoot := t.TempDir() + pipelinePath, campaignPath, _ := writeValidConfigFiles(t, workspaceRoot) + writeSessionInitTemplate(t, campaignPath, `session_id: "{{ session_id }}" +inputs: + audio_s3: + prefix: audio/ +`) + 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", pipelinePath, + "--campaign", campaignPath, + "--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")) + obj, ok := fake.Objects[key] + if !ok { + t.Fatalf("remote session key %q not uploaded; objects=%v", key, fake.Objects) + } + if strings.Contains(string(obj.Data), "{{") || !strings.Contains(string(obj.Data), `session_id: "2026-06-07"`) { + t.Fatalf("remote session data = %q, want rendered concrete session", string(obj.Data)) + } +} + +func TestExecuteSessionInitTemplatePathIsCampaignRelative(t *testing.T) { + workspaceRoot := t.TempDir() + pipelinePath, campaignPath, _ := writeValidConfigFiles(t, workspaceRoot) + templateDir := filepath.Join(filepath.Dir(campaignPath), "templates") + if err := os.MkdirAll(templateDir, 0o755); err != nil { + t.Fatalf("mkdir template dir: %v", err) + } + templatePath := filepath.Join(templateDir, "session.template.yml") + if err := os.WriteFile(templatePath, []byte(`session_id: "{{ session_id }}" +inputs: + audio_dir: ./audio +`), 0o644); err != nil { + t.Fatalf("write session template: %v", err) + } + addSessionTemplateToCampaign(t, campaignPath, "./templates/session.template.yml") + outputPath := filepath.Join(t.TempDir(), "session.yml") + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := Execute([]string{ + "session", "init", + "--config", pipelinePath, + "--campaign", campaignPath, + "--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"`) { + t.Fatalf("generated session = %q, want campaign-relative template output", string(data)) + } +} + +func TestExecuteSessionInitTemplateMissingVariableFails(t *testing.T) { + workspaceRoot := t.TempDir() + pipelinePath, campaignPath, _ := writeValidConfigFiles(t, workspaceRoot) + writeSessionInitTemplate(t, campaignPath, `session_id: "{{ session_id }}" +date: "{{ date }}" +inputs: + audio_s3: + prefix: audio/ +`) + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := Execute([]string{ + "session", "init", + "--config", pipelinePath, + "--campaign", campaignPath, + "--session-id", "2026-06-07", + "--remote", + }, &stdout, &stderr) + if code == 0 { + t.Fatal("exit code = 0, want non-zero") + } + if !strings.Contains(stderr.String(), "missing required template variable value(s): date") { + t.Fatalf("stderr = %q, want missing date variable", stderr.String()) + } +} + +func TestExecuteSessionInitTemplateUnusedFlagFails(t *testing.T) { + workspaceRoot := t.TempDir() + pipelinePath, campaignPath, _ := writeValidConfigFiles(t, workspaceRoot) + writeSessionInitTemplate(t, campaignPath, `session_id: "{{ session_id }}" +inputs: + audio_s3: + prefix: audio/ +`) + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := Execute([]string{ + "session", "init", + "--config", pipelinePath, + "--campaign", campaignPath, + "--session-id", "2026-06-07", + "--title", "Unused Title", + "--remote", + }, &stdout, &stderr) + if code == 0 { + t.Fatal("exit code = 0, want non-zero") + } + if !strings.Contains(stderr.String(), "unused template variable value(s): title") { + t.Fatalf("stderr = %q, want unused title variable", stderr.String()) + } +} + +func TestExecuteSessionInitTemplateStrictDecodeFailure(t *testing.T) { + workspaceRoot := t.TempDir() + pipelinePath, campaignPath, _ := writeValidConfigFiles(t, workspaceRoot) + writeSessionInitTemplate(t, campaignPath, `session_id: "{{ session_id }}" +unknown: true +inputs: + audio_s3: + prefix: audio/ +`) + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := Execute([]string{ + "session", "init", + "--config", pipelinePath, + "--campaign", campaignPath, + "--session-id", "2026-06-07", + "--remote", + }, &stdout, &stderr) + if code == 0 { + t.Fatal("exit code = 0, want non-zero") + } + if !strings.Contains(stderr.String(), "strict decode failed") { + t.Fatalf("stderr = %q, want strict decode error", stderr.String()) + } +} + func TestExecuteSessionValidateLoadsSecretsBeforeObjectStoreInit(t *testing.T) { workspaceRoot := t.TempDir() pipelinePath, campaignPath, sessionPath := writeValidConfigFiles(t, workspaceRoot) @@ -486,6 +689,30 @@ func withDefaultPipelineCampaignConfigs(t *testing.T, pipelinePath, campaignPath }) } +func writeSessionInitTemplate(t *testing.T, campaignPath, templateYAML string) { + t.Helper() + templatePath := filepath.Join(filepath.Dir(campaignPath), "session.template.yml") + if err := os.WriteFile(templatePath, []byte(templateYAML), 0o644); err != nil { + t.Fatalf("write session template: %v", err) + } + addSessionTemplateToCampaign(t, campaignPath, "./session.template.yml") +} + +func addSessionTemplateToCampaign(t *testing.T, campaignPath, templateFile string) { + t.Helper() + data, err := os.ReadFile(campaignPath) + if err != nil { + t.Fatalf("read campaign config: %v", err) + } + if strings.Contains(string(data), "session_template_file:") { + t.Fatalf("campaign config already has session_template_file: %q", string(data)) + } + updated := "session_template_file: " + templateFile + "\n" + string(data) + if err := os.WriteFile(campaignPath, []byte(updated), 0o644); err != nil { + t.Fatalf("write campaign config: %v", err) + } +} + func TestExecuteArtifactsListRemoteReportsPromotedAvailability(t *testing.T) { workspaceRoot := t.TempDir() pipelinePath, campaignPath, sessionPath := writeValidConfigFiles(t, workspaceRoot) diff --git a/internal/app/plan.go b/internal/app/plan.go index 04e8457..27619eb 100644 --- a/internal/app/plan.go +++ b/internal/app/plan.go @@ -28,8 +28,8 @@ func Plan(ctx context.Context, args []string, out io.Writer) error { fs.StringVar(&pipelinePath, "config", "", "path to pipeline.yml (optional; defaults searched)") fs.StringVar(&campaignPath, "campaign", "", "path to campaign.yml (optional; defaults searched)") fs.StringVar(&sessionPath, "session", "", "path to session.yml") - fs.StringVar(&sessionID, "session-id", "", "session identifier for session.yml templates") - fs.StringVar(&previousSessionID, "previous-session-id", "", "previous session identifier for session.yml templates") + fs.StringVar(&sessionID, "session-id", "", "expected session identifier and remote session lookup value") + fs.StringVar(&previousSessionID, "previous-session-id", "", "expected previous session identifier") fs.BoolVar(&force, "force", false, "force stage execution (reserved for future behavior)") if err := fs.Parse(args); err != nil { diff --git a/internal/app/remote_session_test.go b/internal/app/remote_session_test.go index d08e69c..81323df 100644 --- a/internal/app/remote_session_test.go +++ b/internal/app/remote_session_test.go @@ -19,7 +19,7 @@ func TestExecuteRemoteSessionFallbackLoadsFromObjectStore(t *testing.T) { workspaceRoot := t.TempDir() pipelinePath, campaignPath, _ := writeValidConfigFiles(t, workspaceRoot) fake := &storage.FakeBackend{} - remoteKey := seedRemoteSessionConfig(t, fake, "2026-05-03", `session_id: "{{ session_id }}" + remoteKey := seedRemoteSessionConfig(t, fake, "2026-05-03", `session_id: 2026-05-03 inputs: audio_s3: prefix: audio/ @@ -56,7 +56,7 @@ func TestExecuteRemoteSessionFallbackLoadsSecretsBeforeObjectStoreInit(t *testin addSecretsToPipelineConfig(t, pipelinePath, secretsDir, accessKeyEnv, secretKeyEnv) fake := &storage.FakeBackend{} - seedRemoteSessionConfig(t, fake, "2026-05-03", `session_id: "{{ session_id }}" + seedRemoteSessionConfig(t, fake, "2026-05-03", `session_id: 2026-05-03 inputs: audio_s3: prefix: audio/ @@ -205,6 +205,48 @@ func TestExecuteRemoteSessionMalformedYAMLFailsStrictDecode(t *testing.T) { } } +func TestExecuteRemoteSessionTemplateFailsConcreteSessionCheck(t *testing.T) { + workspaceRoot := t.TempDir() + pipelinePath, campaignPath, _ := writeValidConfigFiles(t, workspaceRoot) + fake := &storage.FakeBackend{} + seedRemoteSessionConfig(t, fake, "2026-05-03", `session_id: "{{ session_id }}" +inputs: + audio_s3: + prefix: audio/ +`) + 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{"plan", "--config", pipelinePath, "--campaign", campaignPath, "--session-id", "2026-05-03"}, &stdout, &stderr) + if code == 0 { + t.Fatal("exit code = 0, want non-zero") + } + if !strings.Contains(stderr.String(), "session.yml must be concrete") || !strings.Contains(stderr.String(), "run narratio session init") { + t.Fatalf("stderr = %q, want concrete session guidance", stderr.String()) + } +} + +func TestExecuteRemoteSessionMismatchFails(t *testing.T) { + workspaceRoot := t.TempDir() + pipelinePath, campaignPath, _ := writeValidConfigFiles(t, workspaceRoot) + fake := &storage.FakeBackend{} + seedRemoteSessionConfig(t, fake, "2026-05-03", "session_id: 2026-05-04\ninputs:\n audio_s3:\n prefix: audio/\n") + 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{"plan", "--config", pipelinePath, "--campaign", campaignPath, "--session-id", "2026-05-03"}, &stdout, &stderr) + if code == 0 { + t.Fatal("exit code = 0, want non-zero") + } + if !strings.Contains(stderr.String(), "session_id mismatch") { + t.Fatalf("stderr = %q, want session_id mismatch", stderr.String()) + } +} + func restoreAppConfigTestGlobals(t *testing.T, fake *storage.FakeBackend, storeInitCalls *int, sessionDefaults []string) { t.Helper() origStoreFn := newObjectStoreFromConfigFn diff --git a/internal/app/restore.go b/internal/app/restore.go index 8408b91..ca211d1 100644 --- a/internal/app/restore.go +++ b/internal/app/restore.go @@ -36,8 +36,8 @@ func Restore(ctx context.Context, args []string, out io.Writer) error { fs.StringVar(&pipelinePath, "config", "", "path to pipeline.yml (optional; defaults searched)") fs.StringVar(&campaignPath, "campaign", "", "path to campaign.yml (optional; defaults searched)") fs.StringVar(&sessionPath, "session", "", "path to session.yml") - fs.StringVar(&sessionID, "session-id", "", "session identifier for session.yml templates") - fs.StringVar(&previousSessionID, "previous-session-id", "", "previous session identifier for session.yml templates") + fs.StringVar(&sessionID, "session-id", "", "expected session identifier and remote session lookup value") + fs.StringVar(&previousSessionID, "previous-session-id", "", "expected previous session identifier") fs.BoolVar(&dryRun, "dry-run", false, "plan restore actions without writing local files") fs.BoolVar(&force, "force", false, "overwrite local conflicts with remote state") fs.BoolVar(&includeAudio, "include-audio", false, "include archived session-level audio objects") diff --git a/internal/app/resume.go b/internal/app/resume.go index f09b45f..20d0233 100644 --- a/internal/app/resume.go +++ b/internal/app/resume.go @@ -26,8 +26,8 @@ func Resume(ctx context.Context, args []string, out io.Writer) error { fs.StringVar(&pipelinePath, "config", "", "path to pipeline.yml (optional; defaults searched)") fs.StringVar(&campaignPath, "campaign", "", "path to campaign.yml (optional; defaults searched)") fs.StringVar(&sessionPath, "session", "", "path to session.yml") - fs.StringVar(&sessionID, "session-id", "", "session identifier for session.yml templates") - fs.StringVar(&previousSessionID, "previous-session-id", "", "previous session identifier for session.yml templates") + fs.StringVar(&sessionID, "session-id", "", "expected session identifier and remote session lookup value") + fs.StringVar(&previousSessionID, "previous-session-id", "", "expected previous session identifier") fs.BoolVar(&force, "force", false, "force stage execution") fs.Var(&selectedArtifacts, "artifacts", "artifact names to execute during analyze (comma-separated or repeatable)") diff --git a/internal/app/run.go b/internal/app/run.go index 410b6f9..b87d7d5 100644 --- a/internal/app/run.go +++ b/internal/app/run.go @@ -24,8 +24,8 @@ func Run(ctx context.Context, args []string, out io.Writer) error { fs.StringVar(&pipelinePath, "config", "", "path to pipeline.yml (optional; defaults searched)") fs.StringVar(&campaignPath, "campaign", "", "path to campaign.yml (optional; defaults searched)") fs.StringVar(&sessionPath, "session", "", "path to session.yml") - fs.StringVar(&sessionID, "session-id", "", "session identifier for session.yml templates") - fs.StringVar(&previousSessionID, "previous-session-id", "", "previous session identifier for session.yml templates") + fs.StringVar(&sessionID, "session-id", "", "expected session identifier and remote session lookup value") + fs.StringVar(&previousSessionID, "previous-session-id", "", "expected previous session identifier") fs.BoolVar(&force, "force", false, "force stage execution (reserved for future behavior)") fs.Var(&selectedArtifacts, "artifacts", "artifact names to execute during analyze (comma-separated or repeatable)") diff --git a/internal/app/run_stage.go b/internal/app/run_stage.go index 62b0928..ca64d5f 100644 --- a/internal/app/run_stage.go +++ b/internal/app/run_stage.go @@ -24,8 +24,8 @@ func RunStage(ctx context.Context, args []string, out io.Writer) error { fs.StringVar(&pipelinePath, "config", "", "path to pipeline.yml (optional; defaults searched)") fs.StringVar(&campaignPath, "campaign", "", "path to campaign.yml (optional; defaults searched)") fs.StringVar(&sessionPath, "session", "", "path to session.yml") - fs.StringVar(&sessionID, "session-id", "", "session identifier for session.yml templates") - fs.StringVar(&previousSessionID, "previous-session-id", "", "previous session identifier for session.yml templates") + fs.StringVar(&sessionID, "session-id", "", "expected session identifier and remote session lookup value") + fs.StringVar(&previousSessionID, "previous-session-id", "", "expected previous session identifier") fs.BoolVar(&force, "force", false, "force stage execution (reserved for future behavior)") fs.Var(&selectedArtifacts, "artifacts", "artifact names to execute during analyze (comma-separated or repeatable)") @@ -85,8 +85,8 @@ func Analyze(ctx context.Context, args []string, out io.Writer) error { fs.StringVar(&pipelinePath, "config", "", "path to pipeline.yml (optional; defaults searched)") fs.StringVar(&campaignPath, "campaign", "", "path to campaign.yml (optional; defaults searched)") fs.StringVar(&sessionPath, "session", "", "path to session.yml") - fs.StringVar(&sessionID, "session-id", "", "session identifier for session.yml templates") - fs.StringVar(&previousSessionID, "previous-session-id", "", "previous session identifier for session.yml templates") + fs.StringVar(&sessionID, "session-id", "", "expected session identifier and remote session lookup value") + fs.StringVar(&previousSessionID, "previous-session-id", "", "expected previous session identifier") fs.Var(&selectedArtifacts, "artifacts", "artifact names to execute during analyze (comma-separated or repeatable)") if err := fs.Parse(args); err != nil { @@ -138,8 +138,8 @@ func Publish(ctx context.Context, args []string, out io.Writer) error { fs.StringVar(&pipelinePath, "config", "", "path to pipeline.yml (optional; defaults searched)") fs.StringVar(&campaignPath, "campaign", "", "path to campaign.yml (optional; defaults searched)") fs.StringVar(&sessionPath, "session", "", "path to session.yml") - fs.StringVar(&sessionID, "session-id", "", "session identifier for session.yml templates") - fs.StringVar(&previousSessionID, "previous-session-id", "", "previous session identifier for session.yml templates") + fs.StringVar(&sessionID, "session-id", "", "expected session identifier and remote session lookup value") + fs.StringVar(&previousSessionID, "previous-session-id", "", "expected previous session identifier") if err := fs.Parse(args); err != nil { return fmt.Errorf("publish: invalid flags: %w", err) diff --git a/internal/app/session_cli_test.go b/internal/app/session_cli_test.go index 70f5a6a..def1049 100644 --- a/internal/app/session_cli_test.go +++ b/internal/app/session_cli_test.go @@ -10,7 +10,7 @@ import ( "gitea.maximumdirect.net/eric/narratio/internal/config" ) -func TestPlanUsesDiscoveredSessionTemplateWithSessionIDs(t *testing.T) { +func TestPlanRejectsDiscoveredSessionTemplate(t *testing.T) { workspaceRoot := t.TempDir() pipelinePath, campaignPath, sessionPath := writeValidConfigFiles(t, workspaceRoot) @@ -32,16 +32,20 @@ inputs: t.Cleanup(func() { config.DefaultSessionConfigSearchPaths = origSessionDefaults }) var out bytes.Buffer - if err := Plan(context.Background(), []string{ + err := Plan(context.Background(), []string{ "--config", pipelinePath, "--campaign", campaignPath, "--session-id", "2026-04-04", "--previous-session-id", "2026-03-28", - }, &out); err != nil { - t.Fatalf("Plan() error = %v", err) + }, &out) + if err == nil { + t.Fatal("expected error, got nil") } - if !strings.Contains(out.String(), "narratio plan: workdir prepared") { - t.Fatalf("output = %q, want plan output", out.String()) + if !strings.Contains(err.Error(), "session.yml must be concrete") { + t.Fatalf("error = %q, want concrete session guidance", err.Error()) + } + if !strings.Contains(err.Error(), "run narratio session init") { + t.Fatalf("error = %q, want session init guidance", err.Error()) } } diff --git a/internal/config/campaign_config_test.go b/internal/config/campaign_config_test.go index bbab106..ed41d43 100644 --- a/internal/config/campaign_config_test.go +++ b/internal/config/campaign_config_test.go @@ -37,6 +37,21 @@ func TestCampaignStrictDecodeRejectsUnknownFields(t *testing.T) { } } +func TestCampaignStrictDecodeAcceptsSessionTemplateFile(t *testing.T) { + pipelinePath, campaignPath, sessionPath := writeCampaignConfigTestFiles(t, + "campaign: sample-campaign\nsession_template_file: ./session.template.yml\ninputs:\n speakers_file: ./speakers.yml\n autocorrect_file: ./autocorrect.yml\n glossary_file: ./glossary.yml\n", + "session_id: 2026-05-03\ninputs:\n audio_dir: ./audio\n", + ) + + cfg, err := LoadWithSessionOptions(pipelinePath, campaignPath, sessionPath, SessionLoadOptions{}) + if err != nil { + t.Fatalf("LoadWithSessionOptions() error = %v", err) + } + if cfg.Campaign.SessionTemplateFile != "./session.template.yml" { + t.Fatalf("SessionTemplateFile = %q, want ./session.template.yml", cfg.Campaign.SessionTemplateFile) + } +} + func TestCampaignSessionMergeFillsStableInputs(t *testing.T) { pipelinePath, campaignPath, sessionPath := writeCampaignConfigTestFiles(t, "campaign: sample-campaign\ninputs:\n speakers_file: ./campaign-speakers.yml\n autocorrect_file: ./campaign-autocorrect.yml\n glossary_file: ./campaign-glossary.yml\n", diff --git a/internal/config/config.go b/internal/config/config.go index 11cbd1e..fe4b56a 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -34,8 +34,9 @@ type PipelineConfig struct { // CampaignConfig contains stable campaign-level identity and input defaults. type CampaignConfig struct { - Campaign string `yaml:"campaign"` - Inputs CampaignInputsConfig `yaml:"inputs"` + Campaign string `yaml:"campaign"` + SessionTemplateFile string `yaml:"session_template_file"` + Inputs CampaignInputsConfig `yaml:"inputs"` } // CampaignInputsConfig contains stable campaign-level input file references. diff --git a/internal/config/load.go b/internal/config/load.go index 298d058..0d884a4 100644 --- a/internal/config/load.go +++ b/internal/config/load.go @@ -36,14 +36,14 @@ func LoadSession(path string) (*SessionConfig, error) { return LoadSessionWithOptions(path, SessionLoadOptions{}) } -// SessionLoadOptions configures session template rendering behavior. +// SessionLoadOptions configures expected session identity checks. type SessionLoadOptions struct { SessionID string PreviousSessionID string } // LoadSessionWithOptions loads session configuration from a YAML file with -// strict field checking after template rendering. +// strict field checking. func LoadSessionWithOptions(path string, opts SessionLoadOptions) (*SessionConfig, error) { sessionBytes, err := os.ReadFile(path) if err != nil { @@ -53,20 +53,19 @@ func LoadSessionWithOptions(path string, opts SessionLoadOptions) (*SessionConfi } // LoadSessionBytesWithOptions loads session configuration from YAML bytes with -// strict field checking after template rendering. +// strict field checking. func LoadSessionBytesWithOptions(label string, data []byte, opts SessionLoadOptions) (*SessionConfig, error) { - rendered, err := renderSessionTemplate(string(data), opts) - if err != nil { + if err := rejectSessionTemplatePlaceholders(label, string(data)); err != nil { return nil, fmt.Errorf("load session config: %w", err) } var cfg SessionConfig - if err := decodeStrictYAMLFromReader("session", label, strings.NewReader(rendered), &cfg); err != nil { + if err := decodeStrictYAMLFromReader("session", label, strings.NewReader(string(data)), &cfg); err != nil { return nil, fmt.Errorf("load session config: %w", err) } if strings.TrimSpace(opts.SessionID) != "" && strings.TrimSpace(cfg.SessionID) != "" && strings.TrimSpace(cfg.SessionID) != strings.TrimSpace(opts.SessionID) { return nil, fmt.Errorf( - "load session config: session file %q: session_id mismatch: --session-id %q does not match rendered session_id %q", + "load session config: session file %q: session_id mismatch: --session-id %q does not match session_id %q", label, strings.TrimSpace(opts.SessionID), strings.TrimSpace(cfg.SessionID), @@ -76,7 +75,7 @@ func LoadSessionBytesWithOptions(label string, data []byte, opts SessionLoadOpti strings.TrimSpace(cfg.PreviousSessionID) != "" && strings.TrimSpace(cfg.PreviousSessionID) != strings.TrimSpace(opts.PreviousSessionID) { return nil, fmt.Errorf( - "load session config: session file %q: previous_session_id mismatch: --previous-session-id %q does not match rendered previous_session_id %q", + "load session config: session file %q: previous_session_id mismatch: --previous-session-id %q does not match previous_session_id %q", label, strings.TrimSpace(opts.PreviousSessionID), strings.TrimSpace(cfg.PreviousSessionID), @@ -124,7 +123,7 @@ func Load(pipelinePath string, paths ...string) (*Config, error) { } // LoadWithSessionOptions loads and resolves combined pipeline, campaign, and -// session configuration with session template options. +// session configuration with expected session identity checks. func LoadWithSessionOptions(pipelinePath, campaignPath, sessionPath string, sessionOpts SessionLoadOptions) (*Config, error) { pipelineCfg, err := LoadPipeline(pipelinePath) if err != nil { @@ -275,75 +274,33 @@ func decodeStrictYAMLFromReader(kind, path string, r io.Reader, out any) error { return nil } -var sessionTemplatePattern = regexp.MustCompile(`\{\{\s*([a-zA-Z_][a-zA-Z0-9_]*)\s*\}\}`) +var sessionTemplatePlaceholderPattern = regexp.MustCompile(`\{\{[^}]*\}\}`) +var sessionTemplateVariablePattern = regexp.MustCompile(`\{\{\s*([a-zA-Z_][a-zA-Z0-9_]*)\s*\}\}`) -func renderSessionTemplate(content string, opts SessionLoadOptions) (string, error) { - sessionID := strings.TrimSpace(opts.SessionID) - previousSessionID := strings.TrimSpace(opts.PreviousSessionID) - rendered := content - if sessionID != "" { - rendered = replaceTemplateVariable(rendered, "session_id", sessionID) +func rejectSessionTemplatePlaceholders(label, content string) error { + placeholders := sessionTemplatePlaceholderPattern.FindAllString(content, -1) + if len(placeholders) == 0 { + return nil } - if previousSessionID != "" { - rendered = replaceTemplateVariable(rendered, "previous_session_id", previousSessionID) - } - - unresolved := sessionTemplatePattern.FindAllStringSubmatch(rendered, -1) - if len(unresolved) > 0 { - seenVars := map[string]struct{}{} - vars := make([]string, 0, len(unresolved)) - for _, m := range unresolved { - if len(m) > 1 { - name := m[1] - if _, ok := seenVars[name]; ok { - continue - } - seenVars[name] = struct{}{} - vars = append(vars, name) - } - } - sort.Strings(vars) - if len(vars) > 0 { - hints := unresolvedTemplateHints(vars) - return "", fmt.Errorf( - "session file template rendering failed: unresolved template variable(s): %s%s", - strings.Join(vars, ", "), - hints, - ) - } - return "", fmt.Errorf("session file template rendering failed: unresolved template placeholders remain") - } - - return rendered, nil -} - -func replaceTemplateVariable(content, name, value string) string { - rendered := strings.ReplaceAll(content, "{{"+name+"}}", value) - rendered = strings.ReplaceAll(rendered, "{{ "+name+" }}", value) - return rendered -} - -func unresolvedTemplateHints(vars []string) string { seen := map[string]struct{}{} - flags := make([]string, 0, 2) - for _, name := range vars { - switch name { - case "session_id": - if _, ok := seen["--session-id"]; !ok { - seen["--session-id"] = struct{}{} - flags = append(flags, "--session-id") - } - case "previous_session_id": - if _, ok := seen["--previous-session-id"]; !ok { - seen["--previous-session-id"] = struct{}{} - flags = append(flags, "--previous-session-id") - } + vars := make([]string, 0, len(placeholders)) + for _, placeholder := range placeholders { + name := strings.TrimSpace(placeholder) + if match := sessionTemplateVariablePattern.FindStringSubmatch(placeholder); len(match) > 1 { + name = match[1] } + if _, ok := seen[name]; ok { + continue + } + seen[name] = struct{}{} + vars = append(vars, name) } - if len(flags) == 0 { - return "" - } - return "; pass " + strings.Join(flags, " and ") + " when using those template variable(s)" + sort.Strings(vars) + return fmt.Errorf( + "session file %q contains template placeholder(s): %s; session.yml must be concrete; run narratio session init to generate it", + label, + strings.Join(vars, ", "), + ) } func shortName(path, fallback string) string { diff --git a/internal/config/load_validate_test.go b/internal/config/load_validate_test.go index daed348..3ee8b9e 100644 --- a/internal/config/load_validate_test.go +++ b/internal/config/load_validate_test.go @@ -934,7 +934,6 @@ func TestExamplesLoadAndValidate(t *testing.T) { name string pipelineFile string sessionFile string - sessionOpts SessionLoadOptions }{ { name: "minimal pipeline with local audio session", @@ -951,14 +950,6 @@ func TestExamplesLoadAndValidate(t *testing.T) { pipelineFile: "pipeline.full.annotated.yml", sessionFile: "session.local-audio.yml", }, - { - name: "template session renders with session_id option", - pipelineFile: "pipeline.minimal.yml", - sessionFile: "session.template.yml", - sessionOpts: SessionLoadOptions{ - SessionID: "2026-05-03", - }, - }, } for _, tt := range tests { @@ -967,15 +958,7 @@ func TestExamplesLoadAndValidate(t *testing.T) { campaignPath := filepath.Join(examplesDir, "campaign.yml") sessionPath := filepath.Join(examplesDir, tt.sessionFile) - var ( - cfg *Config - err error - ) - if strings.TrimSpace(tt.sessionOpts.SessionID) == "" { - cfg, err = Load(pipelinePath, sessionPath) - } else { - cfg, err = LoadWithSessionOptions(pipelinePath, campaignPath, sessionPath, tt.sessionOpts) - } + cfg, err := Load(pipelinePath, campaignPath, sessionPath) if err != nil { t.Fatalf("load example config error = %v", err) } diff --git a/internal/config/session_template_test.go b/internal/config/session_template_test.go index afb94db..c41f005 100644 --- a/internal/config/session_template_test.go +++ b/internal/config/session_template_test.go @@ -7,7 +7,7 @@ import ( "testing" ) -func TestLoadSessionWithOptionsRendersCompactPlaceholder(t *testing.T) { +func TestLoadSessionWithOptionsRejectsCompactPlaceholder(t *testing.T) { dir := t.TempDir() sessionPath := filepath.Join(dir, "session.yml") sessionYAML := `session_id: "{{session_id}}" @@ -22,40 +22,14 @@ inputs: t.Fatalf("write session.yml: %v", err) } - cfg, err := LoadSessionWithOptions(sessionPath, SessionLoadOptions{SessionID: "2026-04-04"}) - if err != nil { - t.Fatalf("LoadSessionWithOptions() error = %v", err) - } - if cfg.SessionID != "2026-04-04" { - t.Fatalf("SessionID = %q, want 2026-04-04", cfg.SessionID) + _, err := LoadSessionWithOptions(sessionPath, SessionLoadOptions{SessionID: "2026-04-04"}) + if err == nil { + t.Fatal("expected error, got nil") } + assertConcreteSessionTemplateError(t, err, "session_id") } -func TestLoadSessionWithOptionsRendersSpacedPlaceholder(t *testing.T) { - dir := t.TempDir() - sessionPath := filepath.Join(dir, "session.yml") - sessionYAML := `session_id: "{{ session_id }}" -campaign: sample-campaign -inputs: - audio_dir: ./audio - speakers_file: ./speakers.yml - autocorrect_file: ./autocorrect.yml - glossary_file: ./glossary.yml -` - if err := os.WriteFile(sessionPath, []byte(sessionYAML), 0o644); err != nil { - t.Fatalf("write session.yml: %v", err) - } - - cfg, err := LoadSessionWithOptions(sessionPath, SessionLoadOptions{SessionID: "2026-04-04"}) - if err != nil { - t.Fatalf("LoadSessionWithOptions() error = %v", err) - } - if cfg.SessionID != "2026-04-04" { - t.Fatalf("SessionID = %q, want 2026-04-04", cfg.SessionID) - } -} - -func TestLoadSessionWithOptionsRendersPreviousSessionPlaceholder(t *testing.T) { +func TestLoadSessionWithOptionsRejectsSpacedPlaceholder(t *testing.T) { dir := t.TempDir() sessionPath := filepath.Join(dir, "session.yml") sessionYAML := `session_id: "{{ session_id }}" @@ -71,74 +45,14 @@ inputs: t.Fatalf("write session.yml: %v", err) } - cfg, err := LoadSessionWithOptions(sessionPath, SessionLoadOptions{ + _, err := LoadSessionWithOptions(sessionPath, SessionLoadOptions{ SessionID: "2026-04-04", PreviousSessionID: "2026-03-28", }) - if err != nil { - t.Fatalf("LoadSessionWithOptions() error = %v", err) - } - if cfg.PreviousSessionID != "2026-03-28" { - t.Fatalf("PreviousSessionID = %q, want 2026-03-28", cfg.PreviousSessionID) - } -} - -func TestLoadSessionWithOptionsUnresolvedPlaceholderFails(t *testing.T) { - dir := t.TempDir() - sessionPath := filepath.Join(dir, "session.yml") - sessionYAML := `session_id: "{{ session_id }}" -campaign: sample-campaign -inputs: - audio_dir: ./audio - speakers_file: ./speakers.yml - autocorrect_file: ./autocorrect.yml - glossary_file: ./glossary.yml -` - if err := os.WriteFile(sessionPath, []byte(sessionYAML), 0o644); err != nil { - t.Fatalf("write session.yml: %v", err) - } - - _, err := LoadSessionWithOptions(sessionPath, SessionLoadOptions{}) if err == nil { t.Fatal("expected error, got nil") } - if !strings.Contains(err.Error(), "unresolved template variable") { - t.Fatalf("error = %q, want unresolved-variable context", err.Error()) - } - if !strings.Contains(err.Error(), "session_id") { - t.Fatalf("error = %q, want session_id variable", err.Error()) - } -} - -func TestLoadSessionWithOptionsUnresolvedPreviousSessionPlaceholderFails(t *testing.T) { - dir := t.TempDir() - sessionPath := filepath.Join(dir, "session.yml") - sessionYAML := `session_id: 2026-05-03 -previous_session_id: "{{ previous_session_id }}" -campaign: sample-campaign -inputs: - audio_dir: ./audio - speakers_file: ./speakers.yml - autocorrect_file: ./autocorrect.yml - glossary_file: ./glossary.yml -` - if err := os.WriteFile(sessionPath, []byte(sessionYAML), 0o644); err != nil { - t.Fatalf("write session.yml: %v", err) - } - - _, err := LoadSessionWithOptions(sessionPath, SessionLoadOptions{}) - if err == nil { - t.Fatal("expected error, got nil") - } - if !strings.Contains(err.Error(), "unresolved template variable") { - t.Fatalf("error = %q, want unresolved-variable context", err.Error()) - } - if !strings.Contains(err.Error(), "previous_session_id") { - t.Fatalf("error = %q, want previous_session_id variable", err.Error()) - } - if !strings.Contains(err.Error(), "--previous-session-id") { - t.Fatalf("error = %q, want previous-session-id guidance", err.Error()) - } + assertConcreteSessionTemplateError(t, err, "session_id", "previous_session_id") } func TestLoadSessionWithOptionsMismatchFails(t *testing.T) { @@ -163,6 +77,9 @@ inputs: if !strings.Contains(err.Error(), "session_id mismatch") { t.Fatalf("error = %q, want mismatch context", err.Error()) } + if strings.Contains(err.Error(), "rendered") { + t.Fatalf("error = %q, should not mention rendered session", err.Error()) + } } func TestLoadSessionWithOptionsPreviousSessionMismatchFails(t *testing.T) { @@ -191,12 +108,15 @@ inputs: if !strings.Contains(err.Error(), "previous_session_id mismatch") { t.Fatalf("error = %q, want mismatch context", err.Error()) } + if strings.Contains(err.Error(), "rendered") { + t.Fatalf("error = %q, should not mention rendered session", err.Error()) + } } -func TestLoadSessionWithOptionsUnknownFieldStillRejectedAfterRendering(t *testing.T) { +func TestLoadSessionWithOptionsUnknownFieldStillRejected(t *testing.T) { dir := t.TempDir() sessionPath := filepath.Join(dir, "session.yml") - sessionYAML := `session_id: "{{ session_id }}" + sessionYAML := `session_id: 2026-04-04 campaign: sample-campaign unknown_field: true inputs: @@ -242,7 +162,7 @@ inputs: } } -func TestLoadSessionBytesWithOptionsUsesSameTemplateAndStrictDecode(t *testing.T) { +func TestLoadSessionBytesWithOptionsRejectsPlaceholder(t *testing.T) { sessionYAML := []byte(`session_id: "{{ session_id }}" campaign: sample-campaign inputs: @@ -250,7 +170,15 @@ inputs: prefix: audio/ `) - cfg, err := LoadSessionBytesWithOptions("s3://bucket/session.yml", sessionYAML, SessionLoadOptions{SessionID: "2026-05-03"}) + _, err := LoadSessionBytesWithOptions("s3://bucket/session.yml", sessionYAML, SessionLoadOptions{SessionID: "2026-05-03"}) + if err == nil { + t.Fatal("expected error, got nil") + } + assertConcreteSessionTemplateError(t, err, "session_id") +} + +func TestLoadSessionBytesWithOptionsStrictDecode(t *testing.T) { + cfg, err := LoadSessionBytesWithOptions("s3://bucket/session.yml", []byte("session_id: 2026-05-03\n"), SessionLoadOptions{}) if err != nil { t.Fatalf("LoadSessionBytesWithOptions() error = %v", err) } @@ -276,3 +204,18 @@ func TestLoadSessionBytesWithOptionsMismatchFails(t *testing.T) { t.Fatalf("error = %q, want mismatch context", err.Error()) } } + +func assertConcreteSessionTemplateError(t *testing.T, err error, vars ...string) { + t.Helper() + if !strings.Contains(err.Error(), "session.yml must be concrete") { + t.Fatalf("error = %q, want concrete session guidance", err.Error()) + } + if !strings.Contains(err.Error(), "run narratio session init") { + t.Fatalf("error = %q, want session init guidance", err.Error()) + } + for _, name := range vars { + if !strings.Contains(err.Error(), name) { + t.Fatalf("error = %q, want variable %q", err.Error(), name) + } + } +}