From 591c529a0960352772ca110f5774a30625c4c0d9 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Fri, 22 May 2026 18:01:05 -0500 Subject: [PATCH] Updated the analyze stage to accept --artifacts as a CLI flag --- docs/cli.md | 23 +-- docs/internal/stage-archive.md | 2 + docs/operations.md | 9 +- internal/app/analyze_artifacts.go | 2 +- .../app/analyze_artifacts_commands_test.go | 73 +++++++++- internal/app/analyze_artifacts_test.go | 4 +- internal/app/resume.go | 4 +- internal/app/run.go | 4 +- internal/app/run_stage.go | 17 ++- internal/app/runner.go | 2 +- internal/app/runner_test.go | 39 +++-- internal/artifacts/artifact_resolver.go | 11 +- internal/artifacts/artifact_resolver_test.go | 52 +++++++ internal/stage/analyze.go | 2 +- internal/stage/analyze_test.go | 2 +- internal/stage/archive.go | 133 ++++++++++++------ internal/stage/archive_test.go | 102 ++++++++++++++ internal/stage/stage.go | 10 +- 18 files changed, 399 insertions(+), 92 deletions(-) diff --git a/docs/cli.md b/docs/cli.md index c1de003..5e4be6e 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -44,7 +44,7 @@ For config semantics, see [docs/config.md](./config.md). For operator lifecycle - `--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). +- `--artifacts `: configured artifact keys to execute and publish (repeatable or comma-separated). ### `plan` @@ -63,7 +63,7 @@ For config semantics, see [docs/config.md](./config.md). For operator lifecycle - `--session-id ` - `--previous-session-id ` - `--force` -- `--artifacts `: analyze artifact keys to execute (repeatable or comma-separated). +- `--artifacts `: configured artifact keys to execute and publish (repeatable or comma-separated). ### `run-stage` @@ -73,7 +73,7 @@ For config semantics, see [docs/config.md](./config.md). For operator lifecycle - `--session-id ` - `--previous-session-id ` - `--force` -- `--artifacts `: analyze artifact keys to execute (repeatable or comma-separated). +- `--artifacts `: configured artifact keys to execute or publish (repeatable or comma-separated). - positional ``: required stage name. ### `analyze` @@ -83,7 +83,7 @@ For config semantics, see [docs/config.md](./config.md). For operator lifecycle - `--session ` - `--session-id ` - `--previous-session-id ` -- `--artifacts `: analyze artifact keys to execute (repeatable or comma-separated). +- `--artifacts `: configured artifact keys to execute (repeatable or comma-separated). `analyze` is force-by-design and does not accept `--force`. @@ -94,8 +94,9 @@ For config semantics, see [docs/config.md](./config.md). For operator lifecycle - `--session ` - `--session-id ` - `--previous-session-id ` +- `--artifacts `: configured artifact keys to publish (repeatable or comma-separated). -`publish` is force-by-design and does not accept `--force`, `--artifacts`, or a stage positional argument. +`publish` is force-by-design and does not accept `--force` or a stage positional argument. Valid stage names: @@ -379,14 +380,15 @@ Success output: - `narratio run-stage: stage= executed= skipped= force=; manifest=` `--artifacts` behavior: -- accepted only when `` is `analyze`. +- accepted only when `` is `analyze` or `archive`. - names are normalized (trimmed, deduplicated, sorted). - unknown configured artifact keys fail. +- for `archive`, unselected configured artifact promotions are skipped; built-in transcript and bounds promotions still run. Common failure cases: - missing stage positional arg. - unknown stage name. -- using `--artifacts` with any non-`analyze` stage. +- using `--artifacts` with any stage other than `analyze` or `archive`. ### `analyze` @@ -417,7 +419,7 @@ Purpose: Syntax: ```bash -narratio publish [--config ] [--campaign ] [--session ] [--session-id ] [--previous-session-id ] +narratio publish [--config ] [--campaign ] [--session ] [--session-id ] [--previous-session-id ] [--artifacts ] ``` Success output: @@ -426,7 +428,7 @@ Success output: Common failure cases: - positional arguments. - `--force`, because force is implicit. -- `--artifacts`, because artifact selection only applies to analyze. +- unknown configured artifact keys. - archive-stage failures such as missing required promotion sources or locked storage errors. ### `restore` @@ -580,6 +582,7 @@ Get manifest path from previous output: ## `--artifacts` and `--force` -- `--artifacts` filters which configured artifacts are executable when analyze runs. +- `--artifacts` filters which configured artifacts are executable when analyze runs and which configured artifact promotions archive publishes. - `--artifacts` does not imply `--force`. - if analyze is already `succeeded` and `--force` is not set, runner-level skip still applies. +- `--artifacts` does not suppress built-in transcript or bounds promotions. diff --git a/docs/internal/stage-archive.md b/docs/internal/stage-archive.md index b4425fd..923cd6e 100644 --- a/docs/internal/stage-archive.md +++ b/docs/internal/stage-archive.md @@ -52,10 +52,12 @@ Does not own: - Resolves bucket/prefix from manifest identity first, then config fallback. - Uploads session `previous/**` files as durable session state when the local `previous/` directory exists. - Skips top-level promotion uploads for effective locked sources; run-local uploads still publish. +- When selected configured artifact keys are supplied, skips promotion rules for unselected `narratio.artifact.` sources; built-in transcript and bounds promotions still publish. - Effective locks are the union of `pipeline.archive.locks` and remote `{session_prefix}/locks.yml`; static pipeline locks win on duplicate sources. - Writes metadata including: - upload counts/paths - `previous_files_uploaded` and `previous_uploaded_paths` + - `skipped_unselected_promotions` - `locked_promotion_count` and `locked_promotions` - `current_manifest_key` - `current_run_id_key` diff --git a/docs/operations.md b/docs/operations.md index fb2ab93..9b8fbc9 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -126,10 +126,11 @@ Configured artifact source reuse: - reused configured artifact provenance is `filesystem.disabled_artifact_output`. `--artifacts` behavior: -- accepted on `run`, `resume`, `run-stage analyze`, and `analyze`. -- filters analyze execution only. +- accepted on `run`, `resume`, `run-stage analyze`, `run-stage archive`, `analyze`, and `publish`. +- filters analyze execution and configured artifact promotions. +- built-in transcript and bounds promotions are not filtered. - does not imply force on `run`, `resume`, or `run-stage`; `narratio analyze` is force-by-design. -- `publish` does not accept `--artifacts`; it is a force-by-design archive rerun. +- `publish` is force-by-design and accepts `--artifacts` for configured artifact promotions. Canonical previous-session input behavior: - canonical sources use `narratio.previous_session.artifact.`. @@ -287,5 +288,5 @@ Dry-run does not write restore report files. - `status --session-id ` includes the same promoted remote output availability view as `artifacts list --remote` when storage is configured. - local and S3 audio input modes are mutually exclusive. - archive publish requires upstream stages through `analyze` to be `succeeded`. -- required promotion rules can fail when selected analyze artifacts did not generate a required file path. +- required configured artifact promotions for unselected `--artifacts` keys are skipped intentionally; selected required promotions still fail if their files are missing. - restore requires configured remote object storage and committed remote current state. diff --git a/internal/app/analyze_artifacts.go b/internal/app/analyze_artifacts.go index 500b8ba..37f4986 100644 --- a/internal/app/analyze_artifacts.go +++ b/internal/app/analyze_artifacts.go @@ -46,7 +46,7 @@ func (f *artifactSelectionFlag) Normalize() ([]string, error) { return out, nil } -func validateSelectedAnalyzeArtifacts(cfg *config.Config, selected []string) error { +func validateSelectedArtifacts(cfg *config.Config, selected []string) error { if len(selected) == 0 { return nil } diff --git a/internal/app/analyze_artifacts_commands_test.go b/internal/app/analyze_artifacts_commands_test.go index ce516af..91547c6 100644 --- a/internal/app/analyze_artifacts_commands_test.go +++ b/internal/app/analyze_artifacts_commands_test.go @@ -14,7 +14,7 @@ import ( "gitea.maximumdirect.net/eric/narratio/internal/stage" ) -func TestExecuteRunStageArtifactsNonAnalyzeFails(t *testing.T) { +func TestExecuteRunStageArtifactsUnsupportedStageFails(t *testing.T) { workspaceRoot := t.TempDir() pipelinePath, campaignPath, sessionPath := writeValidConfigFilesWithScriptoriumArtifacts(t, workspaceRoot) @@ -28,11 +28,54 @@ func TestExecuteRunStageArtifactsNonAnalyzeFails(t *testing.T) { if code == 0 { t.Fatal("exit code = 0, want non-zero") } - if !strings.Contains(stderr.String(), `run-stage: --artifacts is only supported for stage "analyze"`) { + if !strings.Contains(stderr.String(), `run-stage: --artifacts is only supported for stages "analyze" and "archive"`) { t.Fatalf("stderr = %q, want stage-gating error", stderr.String()) } } +func TestExecuteRunStageArchivePropagatesSelectedArtifacts(t *testing.T) { + workspaceRoot := t.TempDir() + pipelinePath, campaignPath, sessionPath := writeValidConfigFilesWithScriptoriumArtifacts(t, workspaceRoot) + + var capturedStages []string + var capturedArtifacts []string + origExecuteStagesFn := executeStagesFn + t.Cleanup(func() { + executeStagesFn = origExecuteStagesFn + }) + executeStagesFn = func(_ context.Context, _ *config.Config, stages []stage.Stage, opts RunOptions) (*RunSummary, error) { + for _, s := range stages { + capturedStages = append(capturedStages, s.Name()) + } + capturedArtifacts = append([]string(nil), opts.SelectedArtifacts...) + return &RunSummary{ManifestPath: filepath.Join(workspaceRoot, "manifest.json"), Executed: []string{"archive"}}, nil + } + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := Execute( + []string{ + "run-stage", + "--config", pipelinePath, + "--campaign", campaignPath, + "--session", sessionPath, + "--artifacts", "session_recap", + "archive", + }, + &stdout, + &stderr, + ) + if code != 0 { + t.Fatalf("exit code = %d, want 0; stderr=%q", code, stderr.String()) + } + if len(capturedStages) != 1 || capturedStages[0] != "archive" { + t.Fatalf("captured stages = %#v, want [archive]", capturedStages) + } + if strings.Join(capturedArtifacts, ",") != "session_recap" { + t.Fatalf("captured artifacts = %#v, want [session_recap]", capturedArtifacts) + } +} + func TestExecuteUnknownArtifactsFailValidation(t *testing.T) { workspaceRoot := t.TempDir() pipelinePath, campaignPath, sessionPath := writeValidConfigFilesWithScriptoriumArtifacts(t, workspaceRoot) @@ -264,7 +307,7 @@ func TestExecutePublishForceRunsArchive(t *testing.T) { var stdout bytes.Buffer var stderr bytes.Buffer code := Execute( - []string{"publish", "--config", pipelinePath, "--campaign", campaignPath, "--session", sessionPath}, + []string{"publish", "--config", pipelinePath, "--campaign", campaignPath, "--session", sessionPath, "--artifacts", "session_recap"}, &stdout, &stderr, ) @@ -277,8 +320,8 @@ func TestExecutePublishForceRunsArchive(t *testing.T) { if !capturedForce { t.Fatal("captured force = false, want true") } - if len(capturedArtifacts) != 0 { - t.Fatalf("captured artifacts = %#v, want empty", capturedArtifacts) + if strings.Join(capturedArtifacts, ",") != "session_recap" { + t.Fatalf("captured artifacts = %#v, want [session_recap]", capturedArtifacts) } if !strings.Contains(stdout.String(), "narratio publish: executed=1 skipped=0 force=true; manifest=") { t.Fatalf("stdout = %q, want publish summary", stdout.String()) @@ -293,7 +336,6 @@ func TestExecutePublishRejectsUnsupportedArgsAndFlags(t *testing.T) { }{ {name: "positional", args: []string{"publish", "archive"}, want: "publish: unexpected positional arguments"}, {name: "force flag", args: []string{"publish", "--force"}, want: "publish: invalid flags: flag provided but not defined: -force"}, - {name: "artifacts flag", args: []string{"publish", "--artifacts", "session_recap"}, want: "publish: invalid flags: flag provided but not defined: -artifacts"}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -310,6 +352,25 @@ func TestExecutePublishRejectsUnsupportedArgsAndFlags(t *testing.T) { } } +func TestExecutePublishUnknownArtifactFailsValidation(t *testing.T) { + workspaceRoot := t.TempDir() + pipelinePath, campaignPath, sessionPath := writeValidConfigFilesWithScriptoriumArtifacts(t, workspaceRoot) + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := Execute( + []string{"publish", "--config", pipelinePath, "--campaign", campaignPath, "--session", sessionPath, "--artifacts", "unknown_artifact"}, + &stdout, + &stderr, + ) + if code == 0 { + t.Fatal("exit code = 0, want non-zero") + } + if !strings.Contains(stderr.String(), `publish: --artifacts includes unknown artifact "unknown_artifact"`) { + t.Fatalf("stderr = %q, want unknown-artifact validation error", stderr.String()) + } +} + func TestExecutePublishMissingConfigUsesRunStageLoadingPath(t *testing.T) { var stdout bytes.Buffer var stderr bytes.Buffer diff --git a/internal/app/analyze_artifacts_test.go b/internal/app/analyze_artifacts_test.go index 1883097..fa4d01a 100644 --- a/internal/app/analyze_artifacts_test.go +++ b/internal/app/analyze_artifacts_test.go @@ -64,7 +64,7 @@ func TestArtifactSelectionFlagNormalize(t *testing.T) { } } -func TestValidateSelectedAnalyzeArtifacts(t *testing.T) { +func TestValidateSelectedArtifacts(t *testing.T) { tests := []struct { name string cfg *config.Config @@ -114,7 +114,7 @@ func TestValidateSelectedAnalyzeArtifacts(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := validateSelectedAnalyzeArtifacts(tt.cfg, tt.selected) + err := validateSelectedArtifacts(tt.cfg, tt.selected) if tt.wantErr != "" { if err == nil { t.Fatalf("error = nil, want %q", tt.wantErr) diff --git a/internal/app/resume.go b/internal/app/resume.go index 20d0233..2a5deb3 100644 --- a/internal/app/resume.go +++ b/internal/app/resume.go @@ -29,7 +29,7 @@ func Resume(ctx context.Context, args []string, out io.Writer) error { 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)") + fs.Var(&selectedArtifacts, "artifacts", "configured artifact names to execute and publish (comma-separated or repeatable)") if err := fs.Parse(args); err != nil { return fmt.Errorf("resume: invalid flags: %w", err) @@ -51,7 +51,7 @@ func Resume(ctx context.Context, args []string, out io.Writer) error { if err != nil { return fmt.Errorf("resume: invalid --artifacts: %w", err) } - if err := validateSelectedAnalyzeArtifacts(cfg, normalizedArtifacts); err != nil { + if err := validateSelectedArtifacts(cfg, normalizedArtifacts); err != nil { return fmt.Errorf("resume: %w", err) } diff --git a/internal/app/run.go b/internal/app/run.go index b87d7d5..c06ae62 100644 --- a/internal/app/run.go +++ b/internal/app/run.go @@ -27,7 +27,7 @@ func Run(ctx context.Context, args []string, out io.Writer) error { 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)") + fs.Var(&selectedArtifacts, "artifacts", "configured artifact names to execute and publish (comma-separated or repeatable)") if err := fs.Parse(args); err != nil { return fmt.Errorf("run: invalid flags: %w", err) @@ -49,7 +49,7 @@ func Run(ctx context.Context, args []string, out io.Writer) error { if err != nil { return fmt.Errorf("run: invalid --artifacts: %w", err) } - if err := validateSelectedAnalyzeArtifacts(cfg, normalizedArtifacts); err != nil { + if err := validateSelectedArtifacts(cfg, normalizedArtifacts); err != nil { return fmt.Errorf("run: %w", err) } diff --git a/internal/app/run_stage.go b/internal/app/run_stage.go index ca64d5f..8793c44 100644 --- a/internal/app/run_stage.go +++ b/internal/app/run_stage.go @@ -27,7 +27,7 @@ func RunStage(ctx context.Context, args []string, out io.Writer) error { 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)") + fs.Var(&selectedArtifacts, "artifacts", "configured artifact names to execute or publish (comma-separated or repeatable)") if err := fs.Parse(args); err != nil { return fmt.Errorf("run-stage: invalid flags: %w", err) @@ -40,8 +40,8 @@ func RunStage(ctx context.Context, args []string, out io.Writer) error { return fmt.Errorf("run-stage: invalid --artifacts: %w", err) } stageName := fs.Arg(0) - if len(normalizedArtifacts) > 0 && stageName != "analyze" { - return fmt.Errorf("run-stage: --artifacts is only supported for stage \"analyze\"") + if len(normalizedArtifacts) > 0 && stageName != "analyze" && stageName != "archive" { + return fmt.Errorf("run-stage: --artifacts is only supported for stages \"analyze\" and \"archive\"") } summary, err := runSingleStageCommand(ctx, singleStageCommand{ @@ -87,7 +87,7 @@ func Analyze(ctx context.Context, args []string, out io.Writer) error { fs.StringVar(&sessionPath, "session", "", "path to session.yml") 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)") + fs.Var(&selectedArtifacts, "artifacts", "configured artifact names to execute during analyze (comma-separated or repeatable)") if err := fs.Parse(args); err != nil { return fmt.Errorf("analyze: invalid flags: %w", err) @@ -135,11 +135,13 @@ func Publish(ctx context.Context, args []string, out io.Writer) error { var sessionPath string var sessionID string var previousSessionID string + var selectedArtifacts artifactSelectionFlag 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", "", "expected session identifier and remote session lookup value") fs.StringVar(&previousSessionID, "previous-session-id", "", "expected previous session identifier") + fs.Var(&selectedArtifacts, "artifacts", "configured artifact names to publish (comma-separated or repeatable)") if err := fs.Parse(args); err != nil { return fmt.Errorf("publish: invalid flags: %w", err) @@ -147,6 +149,10 @@ func Publish(ctx context.Context, args []string, out io.Writer) error { if fs.NArg() != 0 { return fmt.Errorf("publish: unexpected positional arguments") } + normalizedArtifacts, err := selectedArtifacts.Normalize() + if err != nil { + return fmt.Errorf("publish: invalid --artifacts: %w", err) + } summary, err := runSingleStageCommand(ctx, singleStageCommand{ CommandName: "publish", @@ -157,6 +163,7 @@ func Publish(ctx context.Context, args []string, out io.Writer) error { SessionID: sessionID, PreviousSessionID: previousSessionID, Force: true, + SelectedArtifacts: normalizedArtifacts, }) if err != nil { return err @@ -200,7 +207,7 @@ func runSingleStageCommand(ctx context.Context, req singleStageCommand) (*RunSum if err := config.Validate(cfg); err != nil { return nil, fmt.Errorf("%s: %w", req.CommandName, err) } - if err := validateSelectedAnalyzeArtifacts(cfg, req.SelectedArtifacts); err != nil { + if err := validateSelectedArtifacts(cfg, req.SelectedArtifacts); err != nil { return nil, fmt.Errorf("%s: %w", req.CommandName, err) } diff --git a/internal/app/runner.go b/internal/app/runner.go index bbd270f..58b02ac 100644 --- a/internal/app/runner.go +++ b/internal/app/runner.go @@ -46,7 +46,7 @@ func executeStages(ctx context.Context, cfg *config.Config, stages []stage.Stage if env.Config == nil { env.Config = cfg } - env.SelectedAnalyzeArtifacts = append([]string(nil), opts.SelectedArtifacts...) + env.SelectedArtifactKeys = append([]string(nil), opts.SelectedArtifacts...) if env.ArtifactStore == nil { env.ArtifactStore = artifacts.NewLocalStore(cfg.Pipeline.Workspace.Root) } diff --git a/internal/app/runner_test.go b/internal/app/runner_test.go index b36b6da..0d1a66f 100644 --- a/internal/app/runner_test.go +++ b/internal/app/runner_test.go @@ -54,7 +54,7 @@ func (s captureSelectedArtifactsStage) Name() string { return s.name } func (s captureSelectedArtifactsStage) Declares() stage.IODecl { return stage.IODecl{} } func (s captureSelectedArtifactsStage) Run(_ context.Context, env *stage.Env, _ *manifest.Manifest) (*stage.StageResult, error) { if s.captured != nil { - *s.captured = append((*s.captured)[:0], env.SelectedAnalyzeArtifacts...) + *s.captured = append((*s.captured)[:0], env.SelectedArtifactKeys...) } return &stage.StageResult{Metadata: map[string]any{"captured": true}}, nil } @@ -78,12 +78,12 @@ type selectedAnalyzeArtifactStage struct { func (s selectedAnalyzeArtifactStage) Name() string { return "analyze" } func (s selectedAnalyzeArtifactStage) Declares() stage.IODecl { return stage.IODecl{} } func (s selectedAnalyzeArtifactStage) Run(_ context.Context, env *stage.Env, m *manifest.Manifest) (*stage.StageResult, error) { - if len(env.SelectedAnalyzeArtifacts) != len(s.expected) { - return nil, fmt.Errorf("selected artifacts len = %d, want %d", len(env.SelectedAnalyzeArtifacts), len(s.expected)) + if len(env.SelectedArtifactKeys) != len(s.expected) { + return nil, fmt.Errorf("selected artifacts len = %d, want %d", len(env.SelectedArtifactKeys), len(s.expected)) } for i := range s.expected { - if env.SelectedAnalyzeArtifacts[i] != s.expected[i] { - return nil, fmt.Errorf("selected artifacts[%d] = %q, want %q", i, env.SelectedAnalyzeArtifacts[i], s.expected[i]) + if env.SelectedArtifactKeys[i] != s.expected[i] { + return nil, fmt.Errorf("selected artifacts[%d] = %q, want %q", i, env.SelectedArtifactKeys[i], s.expected[i]) } } @@ -248,7 +248,7 @@ func TestNeedsObjectStoreForRunPrepareWithPreviousRequirements(t *testing.T) { } } -func TestExecuteStagesArchiveFailsWhenRequiredRecapPromotionMissingForSelectedArtifacts(t *testing.T) { +func TestExecuteStagesArchiveSkipsRequiredUnselectedConfiguredPromotion(t *testing.T) { cfg := testConfig(t) cfg.Pipeline.Storage.S3 = &config.StorageS3Config{ Bucket: "my-dnd-archive", @@ -288,7 +288,7 @@ func TestExecuteStagesArchiveFailsWhenRequiredRecapPromotionMissingForSelectedAr t.Fatalf("Select(archive) error = %v", err) } - _, err = executeStages( + summary, err := executeStages( context.Background(), cfg, []stage.Stage{ @@ -300,11 +300,28 @@ func TestExecuteStagesArchiveFailsWhenRequiredRecapPromotionMissingForSelectedAr Env: &Env{ObjectStore: &storage.FakeBackend{}}, }, ) - if err == nil { - t.Fatal("expected archive promotion failure, got nil") + if err != nil { + t.Fatalf("executeStages() error = %v", err) } - if !strings.Contains(err.Error(), "required promotion source unavailable") { - t.Fatalf("error = %q, want required promotion source unavailable", err.Error()) + if len(summary.Executed) != 2 || summary.Executed[0] != "analyze" || summary.Executed[1] != "archive" { + t.Fatalf("executed = %#v, want analyze and archive", summary.Executed) + } + + loadedManifest, err := store.Load(context.Background(), summary.ManifestPath) + if err != nil { + t.Fatalf("Load manifest error = %v", err) + } + meta := loadedManifest.Stages["archive"].Metadata + skipped, ok := meta["skipped_unselected_promotions"].([]any) + if !ok || len(skipped) != 1 { + t.Fatalf("skipped_unselected_promotions = %#v, want one item", meta["skipped_unselected_promotions"]) + } + item, ok := skipped[0].(map[string]any) + if !ok { + t.Fatalf("skipped item = %#v, want object", skipped[0]) + } + if item["source"] != "narratio.artifact.session_recap" || item["dest"] != "artifacts/session_recap.md" || item["required"] != true { + t.Fatalf("skipped item = %#v, want required session_recap promotion", item) } } diff --git a/internal/artifacts/artifact_resolver.go b/internal/artifacts/artifact_resolver.go index 4e67165..c22b2dd 100644 --- a/internal/artifacts/artifact_resolver.go +++ b/internal/artifacts/artifact_resolver.go @@ -25,7 +25,7 @@ const ( // ErrSessionArtifactNotFound is returned when no readable artifact exists for a known ID. var ErrSessionArtifactNotFound = errors.New("session artifact not found") -var configuredArtifactSourceRE = regexp.MustCompile(`^narratio\.artifact\.[a-z][a-z0-9_]*$`) +var configuredArtifactSourceRE = regexp.MustCompile(`^narratio\.artifact\.([a-z][a-z0-9_]*)$`) var previousSessionArtifactSourceRE = regexp.MustCompile(`^narratio\.previous_session\.artifact\.([a-z][a-z0-9_]*)$`) type artifactContentKind string @@ -122,6 +122,15 @@ func IsConfiguredArtifactSource(source string) bool { return configuredArtifactSourceRE.MatchString(strings.TrimSpace(source)) } +// ConfiguredArtifactName extracts from narratio.artifact.. +func ConfiguredArtifactName(source string) (string, bool) { + matches := configuredArtifactSourceRE.FindStringSubmatch(strings.TrimSpace(source)) + if len(matches) != 2 { + return "", false + } + return matches[1], true +} + // IsPreviousSessionArtifactSource returns true when source is narratio.previous_session.artifact.. func IsPreviousSessionArtifactSource(source string) bool { _, ok := PreviousSessionArtifactName(source) diff --git a/internal/artifacts/artifact_resolver_test.go b/internal/artifacts/artifact_resolver_test.go index 156d50a..e939e6a 100644 --- a/internal/artifacts/artifact_resolver_test.go +++ b/internal/artifacts/artifact_resolver_test.go @@ -46,6 +46,58 @@ func TestNormalizeSessionArtifactSource(t *testing.T) { } } +func TestConfiguredArtifactSourceHelpers(t *testing.T) { + tests := []struct { + name string + source string + wantName string + wantMatch bool + }{ + { + name: "valid", + source: "narratio.artifact.session_recap", + wantName: "session_recap", + wantMatch: true, + }, + { + name: "valid with surrounding whitespace", + source: " narratio.artifact.player_handout ", + wantName: "player_handout", + wantMatch: true, + }, + { + name: "missing name", + source: "narratio.artifact.", + wantMatch: false, + }, + { + name: "invalid hyphen", + source: "narratio.artifact.session-recap", + wantMatch: false, + }, + { + name: "built-in", + source: ArtifactTranscriptMerged, + wantMatch: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := IsConfiguredArtifactSource(tt.source); got != tt.wantMatch { + t.Fatalf("IsConfiguredArtifactSource(%q) = %t, want %t", tt.source, got, tt.wantMatch) + } + gotName, gotOK := ConfiguredArtifactName(tt.source) + if gotOK != tt.wantMatch { + t.Fatalf("ConfiguredArtifactName(%q) ok = %t, want %t", tt.source, gotOK, tt.wantMatch) + } + if gotName != tt.wantName { + t.Fatalf("ConfiguredArtifactName(%q) name = %q, want %q", tt.source, gotName, tt.wantName) + } + }) + } +} + func TestPreviousSessionArtifactSourceHelpers(t *testing.T) { tests := []struct { name string diff --git a/internal/stage/analyze.go b/internal/stage/analyze.go index e0322dd..71e3c9e 100644 --- a/internal/stage/analyze.go +++ b/internal/stage/analyze.go @@ -83,7 +83,7 @@ func (analyzeStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*S }}, nil } - runtimeCatalog, err := buildAnalyzeRuntimeArtifactCatalog(paths, env.Config.Pipeline.Scriptorium, env.SelectedAnalyzeArtifacts) + runtimeCatalog, err := buildAnalyzeRuntimeArtifactCatalog(paths, env.Config.Pipeline.Scriptorium, env.SelectedArtifactKeys) if err != nil { return nil, fmt.Errorf("analyze: build runtime artifact catalog: %w", err) } diff --git a/internal/stage/analyze_test.go b/internal/stage/analyze_test.go index 1cee1b5..d1de0a3 100644 --- a/internal/stage/analyze_test.go +++ b/internal/stage/analyze_test.go @@ -613,7 +613,7 @@ func TestAnalyzeAppliesSelectedArtifactsFilter(t *testing.T) { }, }, } - env.SelectedAnalyzeArtifacts = []string{"player_handout"} + env.SelectedArtifactKeys = []string{"player_handout"} result, err := (analyzeStage{}).Run(context.Background(), env, m) if err != nil { diff --git a/internal/stage/archive.go b/internal/stage/archive.go index ce3d596..e482471 100644 --- a/internal/stage/archive.go +++ b/internal/stage/archive.go @@ -126,12 +126,13 @@ func (archiveStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*S if err != nil { return nil, fmt.Errorf("archive: build runtime artifact catalog: %w", err) } - promotions, skippedOptional, lockedPromotions, err := resolveArchivePromotions( + promotions, skippedOptional, skippedUnselected, lockedPromotions, err := resolveArchivePromotions( sessionPaths, m, runtimeCatalog, env.Config.Pipeline.Archive.PromoteArtifacts, env.Config.Pipeline.Archive.Locks, + env.SelectedArtifactKeys, sessionPrefix, ) if err != nil { @@ -173,6 +174,7 @@ func (archiveStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*S promotedUploaded, previousUploaded, skippedOptional, + skippedUnselected, lockedPromotions, currentManifestKey, )) @@ -201,23 +203,24 @@ func (archiveStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*S return &StageResult{ Metadata: map[string]any{ - "stage": "archive", - "uploaded": true, - "s3_bucket": bucket, - "s3_run_prefix": runPrefix, - "run_files_uploaded": len(runUploaded), - "run_uploaded_paths": runUploaded, - "promoted_files_uploaded": len(promotedUploaded), - "promoted_paths": promotedUploaded, - "previous_files_uploaded": len(previousUploaded), - "previous_uploaded_paths": previousUploaded, - "skipped_optional_promotions": skippedOptional, - "locked_promotion_count": len(lockedPromotions), - "locked_promotions": lockedPromotionMetadata(lockedPromotions), - "current_manifest_key": currentManifestKey, - "current_run_id_key": currentRunPointerKey, - "current_pointer_written": true, - "audio_upload_skipped": true, + "stage": "archive", + "uploaded": true, + "s3_bucket": bucket, + "s3_run_prefix": runPrefix, + "run_files_uploaded": len(runUploaded), + "run_uploaded_paths": runUploaded, + "promoted_files_uploaded": len(promotedUploaded), + "promoted_paths": promotedUploaded, + "previous_files_uploaded": len(previousUploaded), + "previous_uploaded_paths": previousUploaded, + "skipped_optional_promotions": skippedOptional, + "skipped_unselected_promotions": skippedUnselectedPromotionMetadata(skippedUnselected), + "locked_promotion_count": len(lockedPromotions), + "locked_promotions": lockedPromotionMetadata(lockedPromotions), + "current_manifest_key": currentManifestKey, + "current_run_id_key": currentRunPointerKey, + "current_pointer_written": true, + "audio_upload_skipped": true, }, }, nil } @@ -240,6 +243,12 @@ type archiveLockedPromotion struct { Provenance string } +type archiveSkippedUnselectedPromotion struct { + Source string + Dest string + Required bool +} + func archiveDisabled(env *Env) bool { cfg := env.Config.Pipeline.Archive if cfg == nil { @@ -340,18 +349,33 @@ func resolveArchivePromotions( catalog *artifacts.ArtifactCatalog, rules []config.ArchivePromotionRule, locks []config.ArchiveLockRule, + selectedArtifactKeys []string, sessionPrefix string, -) ([]archivePromotion, []string, []archiveLockedPromotion, error) { +) ([]archivePromotion, []string, []archiveSkippedUnselectedPromotion, []archiveLockedPromotion, error) { out := make([]archivePromotion, 0, len(rules)) skippedOptional := make([]string, 0) + skippedUnselected := make([]archiveSkippedUnselectedPromotion, 0) lockedPromotions := make([]archiveLockedPromotion, 0) lockSet := archiveLockSet(locks) + selectedSet := archiveSelectedArtifactSet(selectedArtifactKeys) for _, rule := range rules { source := strings.TrimSpace(rule.Source) required := rule.Required == nil || *rule.Required dest, err := resolveArchivePromotionDest(rule, catalog) if err != nil { - return nil, nil, nil, fmt.Errorf("source %q: %w", source, err) + return nil, nil, nil, nil, fmt.Errorf("source %q: %w", source, err) + } + if len(selectedSet) > 0 { + if key, ok := artifacts.ConfiguredArtifactName(source); ok { + if _, selected := selectedSet[key]; !selected { + skippedUnselected = append(skippedUnselected, archiveSkippedUnselectedPromotion{ + Source: source, + Dest: dest, + Required: required, + }) + continue + } + } } lock, locked := lockSet[source] resolved, err := artifacts.ResolveSessionArtifactWithCatalog(paths, m, source, catalog) @@ -371,9 +395,9 @@ func resolveArchivePromotions( continue } if errors.Is(err, artifacts.ErrSessionArtifactNotFound) { - return nil, nil, nil, fmt.Errorf("required promotion source unavailable: %q", source) + return nil, nil, nil, nil, fmt.Errorf("required promotion source unavailable: %q", source) } - return nil, nil, nil, fmt.Errorf("resolve source %q: %w", source, err) + return nil, nil, nil, nil, fmt.Errorf("resolve source %q: %w", source, err) } if locked { lockedPromotions = append(lockedPromotions, archiveLockedPromotion{ @@ -395,7 +419,22 @@ func resolveArchivePromotions( Provenance: resolved.Provenance, }) } - return out, skippedOptional, lockedPromotions, nil + return out, skippedOptional, skippedUnselected, lockedPromotions, nil +} + +func archiveSelectedArtifactSet(selected []string) map[string]struct{} { + if len(selected) == 0 { + return nil + } + out := make(map[string]struct{}, len(selected)) + for _, key := range selected { + trimmed := strings.TrimSpace(key) + if trimmed == "" { + continue + } + out[trimmed] = struct{}{} + } + return out } func archiveLockSet(locks []config.ArchiveLockRule) map[string]config.ArchiveLockRule { @@ -724,30 +763,44 @@ func archiveMetadataPreview( promotedUploaded []string, previousUploaded []string, skippedOptional []string, + skippedUnselected []archiveSkippedUnselectedPromotion, lockedPromotions []archiveLockedPromotion, currentManifestKey string, ) map[string]any { return map[string]any{ - "stage": "archive", - "uploaded": true, - "s3_bucket": bucket, - "s3_run_prefix": runPrefix, - "run_files_uploaded": len(runUploaded), - "run_uploaded_paths": append([]string(nil), runUploaded...), - "promoted_files_uploaded": len(promotedUploaded), - "promoted_paths": append([]string(nil), promotedUploaded...), - "previous_files_uploaded": len(previousUploaded), - "previous_uploaded_paths": append([]string(nil), previousUploaded...), - "skipped_optional_promotions": append([]string(nil), skippedOptional...), - "locked_promotion_count": len(lockedPromotions), - "locked_promotions": lockedPromotionMetadata(lockedPromotions), - "current_manifest_key": currentManifestKey, - "current_run_id_key": artifacts.S3CurrentRunPointerKey(sessionPrefix), - "current_pointer_written": false, - "audio_upload_skipped": true, + "stage": "archive", + "uploaded": true, + "s3_bucket": bucket, + "s3_run_prefix": runPrefix, + "run_files_uploaded": len(runUploaded), + "run_uploaded_paths": append([]string(nil), runUploaded...), + "promoted_files_uploaded": len(promotedUploaded), + "promoted_paths": append([]string(nil), promotedUploaded...), + "previous_files_uploaded": len(previousUploaded), + "previous_uploaded_paths": append([]string(nil), previousUploaded...), + "skipped_optional_promotions": append([]string(nil), skippedOptional...), + "skipped_unselected_promotions": skippedUnselectedPromotionMetadata(skippedUnselected), + "locked_promotion_count": len(lockedPromotions), + "locked_promotions": lockedPromotionMetadata(lockedPromotions), + "current_manifest_key": currentManifestKey, + "current_run_id_key": artifacts.S3CurrentRunPointerKey(sessionPrefix), + "current_pointer_written": false, + "audio_upload_skipped": true, } } +func skippedUnselectedPromotionMetadata(skipped []archiveSkippedUnselectedPromotion) []map[string]any { + out := make([]map[string]any, 0, len(skipped)) + for _, item := range skipped { + out = append(out, map[string]any{ + "source": item.Source, + "dest": item.Dest, + "required": item.Required, + }) + } + return out +} + func lockedPromotionMetadata(locked []archiveLockedPromotion) []map[string]any { out := make([]map[string]any, 0, len(locked)) for _, item := range locked { diff --git a/internal/stage/archive_test.go b/internal/stage/archive_test.go index 03a4a7e..612d89b 100644 --- a/internal/stage/archive_test.go +++ b/internal/stage/archive_test.go @@ -216,6 +216,108 @@ func TestArchiveSkipsOptionalMissingPromotion(t *testing.T) { } } +func TestArchiveSkipsRequiredUnselectedConfiguredPromotion(t *testing.T) { + env, m, _ := archiveFixture(t) + env.SelectedArtifactKeys = []string{"player_handout"} + env.Config.Pipeline.Scriptorium.Artifacts["player_handout"] = config.ScriptoriumArtifactConfig{ + Enabled: true, + PromptID: "dnd.player_handout", + OutputPath: "artifacts/player_handout.md", + } + fake := env.ObjectStore.(*storage.FakeBackend) + + result, err := archiveStage{}.Run(context.Background(), env, m) + if err != nil { + t.Fatalf("Run() error = %v", err) + } + if _, ok := fake.Objects[m.S3SessionPrefix+"transcripts/trimmed.json"]; !ok { + t.Fatalf("missing built-in promoted trimmed key") + } + if _, ok := fake.Objects[m.S3SessionPrefix+"artifacts/session_recap.md"]; ok { + t.Fatalf("unexpected unselected recap promotion upload") + } + skipped := result.Metadata["skipped_unselected_promotions"].([]map[string]any) + if len(skipped) != 1 { + t.Fatalf("skipped_unselected_promotions = %#v, want one item", skipped) + } + if skipped[0]["source"] != "narratio.artifact.session_recap" || skipped[0]["dest"] != "artifacts/session_recap.md" || skipped[0]["required"] != true { + t.Fatalf("skipped_unselected_promotions[0] = %#v, want session recap", skipped[0]) + } + if result.Metadata["locked_promotion_count"] != 0 { + t.Fatalf("locked_promotion_count = %#v, want 0", result.Metadata["locked_promotion_count"]) + } + if _, ok := fake.Objects[m.S3SessionPrefix+"current/run_id.txt"]; !ok { + t.Fatalf("missing current pointer") + } +} + +func TestArchiveSelectedConfiguredPromotionStillFailsWhenMissing(t *testing.T) { + env, m, _ := archiveFixture(t) + env.SelectedArtifactKeys = []string{"session_recap"} + sessionRoot := artifacts.SessionWorkDirForCampaign( + env.Config.Pipeline.Workspace.Root, + env.Config.Session.Campaign, + env.Config.Session.SessionID, + ) + if err := os.Remove(filepath.Join(sessionRoot, "artifacts", "session_recap.md")); err != nil { + t.Fatalf("remove recap: %v", err) + } + + _, err := archiveStage{}.Run(context.Background(), env, m) + if err == nil || !strings.Contains(err.Error(), `required promotion source unavailable: "narratio.artifact.session_recap"`) { + t.Fatalf("Run() error = %v, want required selected promotion failure", err) + } +} + +func TestArchiveLockedSelectedPromotionSkipsAsLocked(t *testing.T) { + env, m, _ := archiveFixture(t) + env.SelectedArtifactKeys = []string{"session_recap"} + env.Config.Pipeline.Archive.Locks = []config.ArchiveLockRule{ + {Source: "narratio.artifact.session_recap", Reason: "reviewed"}, + } + fake := env.ObjectStore.(*storage.FakeBackend) + + result, err := archiveStage{}.Run(context.Background(), env, m) + if err != nil { + t.Fatalf("Run() error = %v", err) + } + if _, ok := fake.Objects[m.S3SessionPrefix+"artifacts/session_recap.md"]; ok { + t.Fatalf("unexpected locked recap promotion upload") + } + if result.Metadata["locked_promotion_count"] != 1 { + t.Fatalf("locked_promotion_count = %#v, want 1", result.Metadata["locked_promotion_count"]) + } + skipped := result.Metadata["skipped_unselected_promotions"].([]map[string]any) + if len(skipped) != 0 { + t.Fatalf("skipped_unselected_promotions = %#v, want empty", skipped) + } +} + +func TestArchiveLockedUnselectedConfiguredPromotionSkipsAsUnselected(t *testing.T) { + env, m, _ := archiveFixture(t) + env.SelectedArtifactKeys = []string{"player_handout"} + env.Config.Pipeline.Scriptorium.Artifacts["player_handout"] = config.ScriptoriumArtifactConfig{ + Enabled: true, + PromptID: "dnd.player_handout", + OutputPath: "artifacts/player_handout.md", + } + env.Config.Pipeline.Archive.Locks = []config.ArchiveLockRule{ + {Source: "narratio.artifact.session_recap", Reason: "reviewed"}, + } + + result, err := archiveStage{}.Run(context.Background(), env, m) + if err != nil { + t.Fatalf("Run() error = %v", err) + } + if result.Metadata["locked_promotion_count"] != 0 { + t.Fatalf("locked_promotion_count = %#v, want 0", result.Metadata["locked_promotion_count"]) + } + skipped := result.Metadata["skipped_unselected_promotions"].([]map[string]any) + if len(skipped) != 1 || skipped[0]["source"] != "narratio.artifact.session_recap" { + t.Fatalf("skipped_unselected_promotions = %#v, want unselected recap", skipped) + } +} + func TestArchiveSkipsLockedRequiredPromotionAndCommits(t *testing.T) { env, m, _ := archiveFixture(t) env.Config.Pipeline.Archive.Locks = []config.ArchiveLockRule{ diff --git a/internal/stage/stage.go b/internal/stage/stage.go index 5174ccc..74fa1d0 100644 --- a/internal/stage/stage.go +++ b/internal/stage/stage.go @@ -18,11 +18,11 @@ import ( // Env is the shared dependency container visible to stages. type Env struct { - Config *config.Config - SelectedAnalyzeArtifacts []string - ArtifactStore artifacts.Store - ManifestStore manifest.Store - Logger *slog.Logger + Config *config.Config + SelectedArtifactKeys []string + ArtifactStore artifacts.Store + ManifestStore manifest.Store + Logger *slog.Logger WhisperX whisperx.Client Seriatim seriatim.Runner