diff --git a/internal/app/analyze_artifacts.go b/internal/app/analyze_artifacts.go new file mode 100644 index 0000000..500b8ba --- /dev/null +++ b/internal/app/analyze_artifacts.go @@ -0,0 +1,66 @@ +package app + +import ( + "fmt" + "sort" + "strings" + + "gitea.maximumdirect.net/eric/narratio/internal/config" +) + +type artifactSelectionFlag struct { + values []string +} + +func (f *artifactSelectionFlag) String() string { + return strings.Join(f.values, ",") +} + +func (f *artifactSelectionFlag) Set(value string) error { + f.values = append(f.values, value) + return nil +} + +func (f *artifactSelectionFlag) Normalize() ([]string, error) { + if len(f.values) == 0 { + return nil, nil + } + + seen := map[string]struct{}{} + out := make([]string, 0, len(f.values)) + for _, raw := range f.values { + for _, part := range strings.Split(raw, ",") { + name := strings.TrimSpace(part) + if name == "" { + return nil, fmt.Errorf("artifact names must be non-empty") + } + if _, ok := seen[name]; ok { + continue + } + seen[name] = struct{}{} + out = append(out, name) + } + } + + sort.Strings(out) + return out, nil +} + +func validateSelectedAnalyzeArtifacts(cfg *config.Config, selected []string) error { + if len(selected) == 0 { + return nil + } + if cfg == nil || cfg.Pipeline == nil || cfg.Pipeline.Scriptorium == nil { + return fmt.Errorf("--artifacts requires pipeline.scriptorium.artifacts to be configured") + } + configured := cfg.Pipeline.Scriptorium.Artifacts + if len(configured) == 0 { + return fmt.Errorf("--artifacts requires at least one configured artifact in pipeline.scriptorium.artifacts") + } + for _, name := range selected { + if _, ok := configured[name]; !ok { + return fmt.Errorf("--artifacts includes unknown artifact %q", name) + } + } + return nil +} diff --git a/internal/app/analyze_artifacts_commands_test.go b/internal/app/analyze_artifacts_commands_test.go new file mode 100644 index 0000000..9fbd17c --- /dev/null +++ b/internal/app/analyze_artifacts_commands_test.go @@ -0,0 +1,112 @@ +package app + +import ( + "bytes" + "context" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "gitea.maximumdirect.net/eric/narratio/internal/manifest" +) + +func TestExecuteRunStageArtifactsNonAnalyzeFails(t *testing.T) { + workspaceRoot := t.TempDir() + pipelinePath, sessionPath := writeValidConfigFilesWithScriptoriumArtifacts(t, workspaceRoot) + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := Execute( + []string{"run-stage", "--config", pipelinePath, "--session", sessionPath, "--artifacts", "session_recap", "polish"}, + &stdout, + &stderr, + ) + 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"`) { + t.Fatalf("stderr = %q, want stage-gating error", stderr.String()) + } +} + +func TestExecuteUnknownArtifactsFailValidation(t *testing.T) { + workspaceRoot := t.TempDir() + pipelinePath, sessionPath := writeValidConfigFilesWithScriptoriumArtifacts(t, workspaceRoot) + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := Execute( + []string{"run", "--config", pipelinePath, "--session", sessionPath, "--artifacts", "unknown_artifact"}, + &stdout, + &stderr, + ) + if code == 0 { + t.Fatal("exit code = 0, want non-zero") + } + if !strings.Contains(stderr.String(), `run: --artifacts includes unknown artifact "unknown_artifact"`) { + t.Fatalf("stderr = %q, want unknown-artifact validation error", stderr.String()) + } +} + +func TestRunStageArtifactsDoesNotImplyForce(t *testing.T) { + workspaceRoot := t.TempDir() + pipelinePath, sessionPath := writeValidConfigFilesWithScriptoriumArtifacts(t, workspaceRoot) + manifestPath := filepath.Join(workspaceRoot, "work", "sample-campaign", "2026-05-03", "manifest.json") + + store := &manifest.LocalStore{} + seed := manifest.New("2026-05-03", time.Date(2026, 5, 3, 10, 0, 0, 0, time.UTC)) + seed.MarkStageSucceeded("analyze", time.Date(2026, 5, 3, 10, 1, 0, 0, time.UTC), nil) + if err := store.Save(context.Background(), manifestPath, seed); err != nil { + t.Fatalf("save manifest: %v", err) + } + + var out bytes.Buffer + err := RunStage( + context.Background(), + []string{"--config", pipelinePath, "--session", sessionPath, "--artifacts", "session_recap,session_recap", "analyze"}, + &out, + ) + if err != nil { + t.Fatalf("RunStage() error = %v", err) + } + if !strings.Contains(out.String(), "stage=analyze executed=0 skipped=1 force=false") { + t.Fatalf("output = %q, want analyze skip without force", out.String()) + } +} + +func writeValidConfigFilesWithScriptoriumArtifacts(t *testing.T, workspaceRoot string) (string, string) { + t.Helper() + + pipelinePath, sessionPath := writeValidConfigFiles(t, workspaceRoot) + f, err := os.OpenFile(pipelinePath, os.O_APPEND|os.O_WRONLY, 0) + if err != nil { + t.Fatalf("open pipeline config for append: %v", err) + } + defer f.Close() + + extra := ` +scriptorium: + binary: scriptorium + artifacts: + session_recap: + enabled: true + prompt_id: dnd.session_recap + output_path: artifacts/session_recap.md + player_handout: + enabled: true + prompt_id: dnd.player_handout + output_path: artifacts/player_handout.md + depends_on: + - session_recap + inputs: + recap: + source: narratio.artifact.session_recap + required: true +` + if _, err := f.WriteString(extra); err != nil { + t.Fatalf("append scriptorium config: %v", err) + } + return pipelinePath, sessionPath +} diff --git a/internal/app/analyze_artifacts_test.go b/internal/app/analyze_artifacts_test.go new file mode 100644 index 0000000..1883097 --- /dev/null +++ b/internal/app/analyze_artifacts_test.go @@ -0,0 +1,132 @@ +package app + +import ( + "testing" + + "gitea.maximumdirect.net/eric/narratio/internal/config" +) + +func TestArtifactSelectionFlagNormalize(t *testing.T) { + tests := []struct { + name string + inputs []string + want []string + wantErr string + }{ + { + name: "single value", + inputs: []string{"session_recap"}, + want: []string{"session_recap"}, + }, + { + name: "repeatable and comma separated values are deduped and sorted", + inputs: []string{"session_recap,player_handout", "session_recap"}, + want: []string{"player_handout", "session_recap"}, + }, + { + name: "empty token fails", + inputs: []string{"session_recap,"}, + wantErr: "artifact names must be non-empty", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var flag artifactSelectionFlag + for _, in := range tt.inputs { + if err := flag.Set(in); err != nil { + t.Fatalf("Set(%q) error = %v", in, err) + } + } + + got, err := flag.Normalize() + if tt.wantErr != "" { + if err == nil { + t.Fatalf("Normalize() error = nil, want %q", tt.wantErr) + } + if err.Error() != tt.wantErr { + t.Fatalf("Normalize() error = %q, want %q", err.Error(), tt.wantErr) + } + return + } + if err != nil { + t.Fatalf("Normalize() error = %v", err) + } + if len(got) != len(tt.want) { + t.Fatalf("Normalize() len = %d, want %d; got=%v", len(got), len(tt.want), got) + } + for i := range got { + if got[i] != tt.want[i] { + t.Fatalf("Normalize()[%d] = %q, want %q", i, got[i], tt.want[i]) + } + } + }) + } +} + +func TestValidateSelectedAnalyzeArtifacts(t *testing.T) { + tests := []struct { + name string + cfg *config.Config + selected []string + wantErr string + }{ + { + name: "empty selection is accepted", + cfg: &config.Config{}, + selected: nil, + }, + { + name: "scriptorium required when selected artifacts present", + cfg: &config.Config{Pipeline: &config.PipelineConfig{}}, + selected: []string{"session_recap"}, + wantErr: "--artifacts requires pipeline.scriptorium.artifacts to be configured", + }, + { + name: "unknown selected artifact fails", + cfg: &config.Config{ + Pipeline: &config.PipelineConfig{ + Scriptorium: &config.ScriptoriumConfig{ + Artifacts: map[string]config.ScriptoriumArtifactConfig{ + "session_recap": {Enabled: true, PromptID: "dnd.session_recap", OutputPath: "artifacts/session_recap.md"}, + }, + }, + }, + }, + selected: []string{"player_handout"}, + wantErr: `--artifacts includes unknown artifact "player_handout"`, + }, + { + name: "known selected artifacts are accepted", + cfg: &config.Config{ + Pipeline: &config.PipelineConfig{ + Scriptorium: &config.ScriptoriumConfig{ + Artifacts: map[string]config.ScriptoriumArtifactConfig{ + "session_recap": {Enabled: true, PromptID: "dnd.session_recap", OutputPath: "artifacts/session_recap.md"}, + "player_handout": {Enabled: true, PromptID: "dnd.player_handout", OutputPath: "artifacts/player_handout.md"}, + }, + }, + }, + }, + selected: []string{"player_handout", "session_recap"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateSelectedAnalyzeArtifacts(tt.cfg, tt.selected) + if tt.wantErr != "" { + if err == nil { + t.Fatalf("error = nil, want %q", tt.wantErr) + } + if err.Error() != tt.wantErr { + t.Fatalf("error = %q, want %q", err.Error(), tt.wantErr) + } + return + } + if err != nil { + t.Fatalf("error = %v, want nil", err) + } + }) + } +} diff --git a/internal/app/resume.go b/internal/app/resume.go index a66a6bb..fec720d 100644 --- a/internal/app/resume.go +++ b/internal/app/resume.go @@ -20,10 +20,12 @@ func Resume(ctx context.Context, args []string, out io.Writer) error { var sessionPath string var sessionID string var force bool + var selectedArtifacts artifactSelectionFlag fs.StringVar(&pipelinePath, "config", "", "path to pipeline.yml (optional; defaults searched)") fs.StringVar(&sessionPath, "session", "", "path to session.yml") fs.StringVar(&sessionID, "session-id", "", "session identifier for session.yml templates") fs.BoolVar(&force, "force", false, "force stage execution") + fs.Var(&selectedArtifacts, "artifacts", "artifact names to execute during analyze (comma-separated or repeatable)") if err := fs.Parse(args); err != nil { return fmt.Errorf("resume: invalid flags: %w", err) @@ -49,6 +51,13 @@ func Resume(ctx context.Context, args []string, out io.Writer) error { if err := config.Validate(cfg); err != nil { return fmt.Errorf("resume: %w", err) } + normalizedArtifacts, err := selectedArtifacts.Normalize() + if err != nil { + return fmt.Errorf("resume: invalid --artifacts: %w", err) + } + if err := validateSelectedAnalyzeArtifacts(cfg, normalizedArtifacts); err != nil { + return fmt.Errorf("resume: %w", err) + } full := BuildFullPlan() selected := full @@ -67,7 +76,10 @@ func Resume(ctx context.Context, args []string, out io.Writer) error { } } - summary, err := executeStages(ctx, cfg, selected, RunOptions{Force: force}) + summary, err := executeStages(ctx, cfg, selected, RunOptions{ + Force: force, + SelectedArtifacts: normalizedArtifacts, + }) if err != nil { return fmt.Errorf("resume: %w", err) } diff --git a/internal/app/run.go b/internal/app/run.go index b9fbb45..9401556 100644 --- a/internal/app/run.go +++ b/internal/app/run.go @@ -18,10 +18,12 @@ func Run(ctx context.Context, args []string, out io.Writer) error { var sessionPath string var sessionID string var force bool + var selectedArtifacts artifactSelectionFlag fs.StringVar(&pipelinePath, "config", "", "path to pipeline.yml (optional; defaults searched)") fs.StringVar(&sessionPath, "session", "", "path to session.yml") fs.StringVar(&sessionID, "session-id", "", "session identifier for session.yml templates") 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)") if err := fs.Parse(args); err != nil { return fmt.Errorf("run: invalid flags: %w", err) @@ -47,9 +49,19 @@ func Run(ctx context.Context, args []string, out io.Writer) error { if err := config.Validate(cfg); err != nil { return fmt.Errorf("run: %w", err) } + normalizedArtifacts, err := selectedArtifacts.Normalize() + if err != nil { + return fmt.Errorf("run: invalid --artifacts: %w", err) + } + if err := validateSelectedAnalyzeArtifacts(cfg, normalizedArtifacts); err != nil { + return fmt.Errorf("run: %w", err) + } stages := BuildFullPlan() - summary, err := executeStages(ctx, cfg, stages, RunOptions{Force: force}) + summary, err := executeStages(ctx, cfg, stages, RunOptions{ + Force: force, + SelectedArtifacts: normalizedArtifacts, + }) if err != nil { return fmt.Errorf("run: %w", err) } diff --git a/internal/app/run_stage.go b/internal/app/run_stage.go index fd457d3..32ccc7f 100644 --- a/internal/app/run_stage.go +++ b/internal/app/run_stage.go @@ -18,10 +18,12 @@ func RunStage(ctx context.Context, args []string, out io.Writer) error { var sessionPath string var sessionID string var force bool + var selectedArtifacts artifactSelectionFlag fs.StringVar(&pipelinePath, "config", "", "path to pipeline.yml (optional; defaults searched)") fs.StringVar(&sessionPath, "session", "", "path to session.yml") fs.StringVar(&sessionID, "session-id", "", "session identifier for session.yml templates") 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)") if err := fs.Parse(args); err != nil { return fmt.Errorf("run-stage: invalid flags: %w", err) @@ -30,6 +32,13 @@ func RunStage(ctx context.Context, args []string, out io.Writer) error { return fmt.Errorf("run-stage: expected exactly one stage name") } stageName := fs.Arg(0) + normalizedArtifacts, err := selectedArtifacts.Normalize() + if err != nil { + return fmt.Errorf("run-stage: invalid --artifacts: %w", err) + } + if len(normalizedArtifacts) > 0 && stageName != "analyze" { + return fmt.Errorf("run-stage: --artifacts is only supported for stage \"analyze\"") + } stages, err := BuildSingleStagePlan(stageName) if err != nil { return fmt.Errorf("run-stage: %w", err) @@ -53,8 +62,14 @@ func RunStage(ctx context.Context, args []string, out io.Writer) error { if err := config.Validate(cfg); err != nil { return fmt.Errorf("run-stage: %w", err) } + if err := validateSelectedAnalyzeArtifacts(cfg, normalizedArtifacts); err != nil { + return fmt.Errorf("run-stage: %w", err) + } - summary, err := executeStages(ctx, cfg, stages, RunOptions{Force: force}) + summary, err := executeStages(ctx, cfg, stages, RunOptions{ + Force: force, + SelectedArtifacts: normalizedArtifacts, + }) if err != nil { return fmt.Errorf("run-stage: %w", err) } diff --git a/internal/app/runner.go b/internal/app/runner.go index b494588..1b3ca32 100644 --- a/internal/app/runner.go +++ b/internal/app/runner.go @@ -21,8 +21,9 @@ import ( ) type RunOptions struct { - Force bool - Env *Env + Force bool + SelectedArtifacts []string + Env *Env } type RunSummary struct { @@ -43,6 +44,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...) 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 693226a..6bba179 100644 --- a/internal/app/runner_test.go +++ b/internal/app/runner_test.go @@ -44,6 +44,40 @@ func (s countingStage) Run(_ context.Context, _ *stage.Env, _ *manifest.Manifest return &stage.StageResult{Metadata: map[string]any{"counting": true}}, nil } +type captureSelectedArtifactsStage struct { + name string + captured *[]string +} + +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...) + } + return &stage.StageResult{Metadata: map[string]any{"captured": true}}, nil +} + +func TestExecuteStagesPropagatesSelectedArtifactsToEnv(t *testing.T) { + cfg := testConfig(t) + + captured := []string{} + stageToRun := captureSelectedArtifactsStage{name: "analyze", captured: &captured} + _, err := executeStages(context.Background(), cfg, []stage.Stage{stageToRun}, RunOptions{ + SelectedArtifacts: []string{"player_handout", "session_recap"}, + }) + if err != nil { + t.Fatalf("executeStages() error = %v", err) + } + + if len(captured) != 2 { + t.Fatalf("captured len = %d, want 2 (%v)", len(captured), captured) + } + if captured[0] != "player_handout" || captured[1] != "session_recap" { + t.Fatalf("captured = %v, want [player_handout session_recap]", captured) + } +} + func TestExecuteStagesPlaceholderSuccessUpdatesManifest(t *testing.T) { cfg := testConfig(t) diff --git a/internal/config/config.go b/internal/config/config.go index fefb268..7400f5e 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -176,6 +176,7 @@ type ScriptoriumConfig struct { // ScriptoriumArtifactConfig configures one named output artifact workflow. type ScriptoriumArtifactConfig struct { Enabled bool `yaml:"enabled"` + DependsOn []string `yaml:"depends_on"` RenderDebug *bool `yaml:"render_debug"` PromptID string `yaml:"prompt_id"` ProfileID string `yaml:"profile_id"` diff --git a/internal/config/defaults.go b/internal/config/defaults.go index 3993f6e..ee05de2 100644 --- a/internal/config/defaults.go +++ b/internal/config/defaults.go @@ -30,8 +30,9 @@ const ( DefaultAuditaTimeout = "3h" DefaultAuditaReport = true - DefaultScriptoriumBinary = "scriptorium" - DefaultScriptoriumTimeout = "10m" + DefaultScriptoriumBinary = "scriptorium" + DefaultScriptoriumTimeout = "10m" + DefaultScriptoriumArtifactOutputRoot = "artifacts" DefaultTrimBoundsTimeout = "10m" DefaultTrimSeriatimReport = false diff --git a/internal/config/scriptorium_test.go b/internal/config/scriptorium_test.go index 1df7262..229f15c 100644 --- a/internal/config/scriptorium_test.go +++ b/internal/config/scriptorium_test.go @@ -205,6 +205,195 @@ func TestScriptoriumLoadAndValidate(t *testing.T) { } }, }, + { + name: "valid artifact dependency is accepted", + scriptoriumYAML: `scriptorium: + binary: scriptorium + artifacts: + session_recap: + enabled: true + prompt_id: dnd.session_recap + output_path: artifacts/session_recap.md + inputs: + transcript: + source: narratio.transcript.trimmed + required: true + player_handout: + enabled: true + depends_on: + - session_recap + prompt_id: dnd.player_handout + output_path: artifacts/player_handout.md + inputs: + recap: + source: narratio.artifact.session_recap + required: true +`, + }, + { + name: "valid dependency on disabled artifact with output path is accepted", + scriptoriumYAML: `scriptorium: + binary: scriptorium + artifacts: + session_recap: + enabled: false + output_path: artifacts/session_recap.md + player_handout: + enabled: true + depends_on: + - session_recap + prompt_id: dnd.player_handout + output_path: artifacts/player_handout.md + inputs: + recap: + source: narratio.artifact.session_recap + required: true +`, + }, + { + name: "invalid artifact name fails validation", + scriptoriumYAML: `scriptorium: + binary: scriptorium + artifacts: + SessionRecap: + enabled: true + prompt_id: dnd.session_recap + output_path: artifacts/session_recap.md +`, + wantValidateErr: "pipeline.scriptorium.artifacts keys must match ^[a-z][a-z0-9_]*$", + }, + { + name: "artifact output path outside artifacts root fails validation", + scriptoriumYAML: `scriptorium: + binary: scriptorium + artifacts: + session_recap: + enabled: true + prompt_id: dnd.session_recap + output_path: transcripts/session_recap.md +`, + wantValidateErr: "pipeline.scriptorium.artifacts.session_recap.output_path must be under artifacts/", + }, + { + name: "missing depends_on for artifact source fails validation", + scriptoriumYAML: `scriptorium: + binary: scriptorium + artifacts: + session_recap: + enabled: true + prompt_id: dnd.session_recap + output_path: artifacts/session_recap.md + inputs: + transcript: + source: narratio.transcript.polished + required: true + player_handout: + enabled: true + prompt_id: dnd.player_handout + output_path: artifacts/player_handout.md + inputs: + recap: + source: narratio.artifact.session_recap + required: true +`, + wantValidateErr: `pipeline.scriptorium.artifacts.player_handout.inputs.recap.source "narratio.artifact.session_recap" requires depends_on entry "session_recap"`, + }, + { + name: "dependency on unknown artifact fails validation", + scriptoriumYAML: `scriptorium: + binary: scriptorium + artifacts: + player_handout: + enabled: true + depends_on: + - session_recap + prompt_id: dnd.player_handout + output_path: artifacts/player_handout.md + inputs: + recap: + source: narratio.artifact.session_recap + required: true +`, + wantValidateErr: `pipeline.scriptorium.artifacts.player_handout.depends_on[0] "session_recap" is not a configured artifact key`, + }, + { + name: "self dependency fails validation", + scriptoriumYAML: `scriptorium: + binary: scriptorium + artifacts: + session_recap: + enabled: true + depends_on: + - session_recap + prompt_id: dnd.session_recap + output_path: artifacts/session_recap.md +`, + wantValidateErr: "pipeline.scriptorium.artifacts.session_recap.depends_on must not include itself", + }, + { + name: "enabled dependency cycle fails validation", + scriptoriumYAML: `scriptorium: + binary: scriptorium + artifacts: + artifact_a: + enabled: true + depends_on: + - artifact_b + prompt_id: dnd.a + output_path: artifacts/a.md + inputs: + b: + source: narratio.artifact.artifact_b + required: true + artifact_b: + enabled: true + depends_on: + - artifact_a + prompt_id: dnd.b + output_path: artifacts/b.md + inputs: + a: + source: narratio.artifact.artifact_a + required: true +`, + wantValidateErr: "pipeline.scriptorium.artifacts enabled dependencies must not contain cycles", + }, + { + name: "artifact source typo fails validation", + scriptoriumYAML: `scriptorium: + binary: scriptorium + artifacts: + player_handout: + enabled: true + prompt_id: dnd.player_handout + output_path: artifacts/player_handout.md + inputs: + recap: + source: narratio.artifact.session-recap + required: true +`, + wantValidateErr: `pipeline.scriptorium.artifacts.player_handout.inputs.recap.source "narratio.artifact.session-recap" is unsupported`, + }, + { + name: "referenced disabled artifact missing output path fails validation", + scriptoriumYAML: `scriptorium: + binary: scriptorium + artifacts: + session_recap: + enabled: false + player_handout: + enabled: true + depends_on: + - session_recap + prompt_id: dnd.player_handout + output_path: artifacts/player_handout.md + inputs: + recap: + source: narratio.artifact.session_recap + required: true +`, + wantValidateErr: "pipeline.scriptorium.artifacts.session_recap.output_path is required when artifact is referenced", + }, } for _, tt := range tests { @@ -258,6 +447,7 @@ audita: ` const testSessionBaseYAML = `session_id: 2026-05-03 +campaign: test-campaign inputs: audio_dir: ./audio speakers_file: ./speakers.yml diff --git a/internal/config/validate.go b/internal/config/validate.go index c716622..307f4d1 100644 --- a/internal/config/validate.go +++ b/internal/config/validate.go @@ -324,20 +324,52 @@ func validateScriptorium(cfg *ScriptoriumConfig) error { return err } - for artifactName, artifactCfg := range cfg.Artifacts { - trimmedArtifactName := strings.TrimSpace(artifactName) - if trimmedArtifactName == "" { - return fmt.Errorf("pipeline.scriptorium.artifacts keys must be non-empty") + configuredArtifacts := make(map[string]struct{}, len(cfg.Artifacts)) + referencedArtifacts := make(map[string]struct{}) + for artifactName := range cfg.Artifacts { + if !scriptoriumArtifactKeyRE.MatchString(strings.TrimSpace(artifactName)) { + return fmt.Errorf("pipeline.scriptorium.artifacts keys must match ^[a-z][a-z0-9_]*$") } + configuredArtifacts[artifactName] = struct{}{} + } + + for artifactName, artifactCfg := range cfg.Artifacts { if artifactCfg.Enabled && strings.TrimSpace(artifactCfg.PromptID) == "" { return fmt.Errorf("pipeline.scriptorium.artifacts.%s.prompt_id is required when enabled", artifactName) } if artifactCfg.Enabled && strings.TrimSpace(artifactCfg.OutputPath) == "" { return fmt.Errorf("pipeline.scriptorium.artifacts.%s.output_path is required when enabled", artifactName) } + if strings.TrimSpace(artifactCfg.OutputPath) != "" { + pathField := "pipeline.scriptorium.artifacts." + artifactName + ".output_path" + if err := validateRelativeSafePath(pathField, artifactCfg.OutputPath); err != nil { + return err + } + if err := validatePathWithinRoot(pathField, artifactCfg.OutputPath, DefaultScriptoriumArtifactOutputRoot); err != nil { + return err + } + } if err := validateDuration("pipeline.scriptorium.artifacts."+artifactName+".timeout", artifactCfg.Timeout); err != nil { return err } + + depSet := make(map[string]struct{}, len(artifactCfg.DependsOn)) + for i, depName := range artifactCfg.DependsOn { + trimmedDep := strings.TrimSpace(depName) + field := fmt.Sprintf("pipeline.scriptorium.artifacts.%s.depends_on[%d]", artifactName, i) + if trimmedDep == "" { + return fmt.Errorf("%s must be non-empty", field) + } + if _, ok := configuredArtifacts[trimmedDep]; !ok { + return fmt.Errorf("%s %q is not a configured artifact key", field, depName) + } + if trimmedDep == artifactName { + return fmt.Errorf("pipeline.scriptorium.artifacts.%s.depends_on must not include itself", artifactName) + } + depSet[trimmedDep] = struct{}{} + referencedArtifacts[trimmedDep] = struct{}{} + } + for inputName, inputCfg := range artifactCfg.Inputs { trimmedInputName := strings.TrimSpace(inputName) if trimmedInputName == "" { @@ -347,8 +379,22 @@ func validateScriptorium(cfg *ScriptoriumConfig) error { if source == "" { return fmt.Errorf("pipeline.scriptorium.artifacts.%s.inputs.%s.source is required", artifactName, inputName) } - if !isSupportedScriptoriumInputSource(source) { - return fmt.Errorf("pipeline.scriptorium.artifacts.%s.inputs.%s.source %q is unsupported", artifactName, inputName, inputCfg.Source) + + referencedArtifact, err := validateScriptoriumInputSource(artifactName, inputName, source, configuredArtifacts) + if err != nil { + return err + } + if referencedArtifact != "" { + if _, ok := depSet[referencedArtifact]; !ok { + return fmt.Errorf( + "pipeline.scriptorium.artifacts.%s.inputs.%s.source %q requires depends_on entry %q", + artifactName, + inputName, + source, + referencedArtifact, + ) + } + referencedArtifacts[referencedArtifact] = struct{}{} } } for varName, varValue := range artifactCfg.Vars { @@ -363,6 +409,17 @@ func validateScriptorium(cfg *ScriptoriumConfig) error { } } + for artifactName := range referencedArtifacts { + artifactCfg := cfg.Artifacts[artifactName] + if strings.TrimSpace(artifactCfg.OutputPath) == "" { + return fmt.Errorf("pipeline.scriptorium.artifacts.%s.output_path is required when artifact is referenced", artifactName) + } + } + + if err := validateEnabledArtifactDependencyCycles(cfg.Artifacts); err != nil { + return err + } + return nil } @@ -439,7 +496,38 @@ func archiveUploadConfiguredForS3(pipeline *PipelineConfig) bool { return enabled && upload } -func isSupportedScriptoriumInputSource(source string) bool { +var windowsAbsPathRE = regexp.MustCompile(`^[A-Za-z]:[\\/].*`) +var envVarNameRE = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`) +var scriptoriumArtifactKeyRE = regexp.MustCompile(`^[a-z][a-z0-9_]*$`) +var narratioArtifactSourceRE = regexp.MustCompile(`^narratio\.artifact\.([a-z][a-z0-9_]*)$`) + +func validateScriptoriumInputSource(artifactName, inputName, source string, configuredArtifacts map[string]struct{}) (string, error) { + if isStaticSupportedScriptoriumInputSource(source) { + return "", nil + } + matches := narratioArtifactSourceRE.FindStringSubmatch(source) + if len(matches) != 2 { + return "", fmt.Errorf( + "pipeline.scriptorium.artifacts.%s.inputs.%s.source %q is unsupported", + artifactName, + inputName, + source, + ) + } + referenced := matches[1] + if _, ok := configuredArtifacts[referenced]; !ok { + return "", fmt.Errorf( + "pipeline.scriptorium.artifacts.%s.inputs.%s.source %q references unknown artifact %q", + artifactName, + inputName, + source, + referenced, + ) + } + return referenced, nil +} + +func isStaticSupportedScriptoriumInputSource(source string) bool { switch strings.TrimSpace(source) { case "previous_session_artifact": return true @@ -453,16 +541,11 @@ func isSupportedScriptoriumInputSource(source string) bool { return true case "narratio.bounds.session": return true - case "narratio.artifact.session_recap": - return true default: return false } } -var windowsAbsPathRE = regexp.MustCompile(`^[A-Za-z]:[\\/].*`) -var envVarNameRE = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`) - func validateEnvVarNameField(fieldName, value string) error { trimmed := strings.TrimSpace(value) if trimmed == "" { @@ -492,6 +575,73 @@ func validateRelativeSafePath(fieldName, value string) error { return nil } +func validatePathWithinRoot(fieldName, value, root string) error { + normalizedValue := filepath.ToSlash(filepath.Clean(strings.TrimSpace(value))) + normalizedRoot := filepath.ToSlash(filepath.Clean(strings.TrimSpace(root))) + if normalizedValue == normalizedRoot { + return nil + } + if strings.HasPrefix(normalizedValue, normalizedRoot+"/") { + return nil + } + return fmt.Errorf("%s must be under %s/", fieldName, normalizedRoot) +} + +func validateEnabledArtifactDependencyCycles(artifacts map[string]ScriptoriumArtifactConfig) error { + if len(artifacts) == 0 { + return nil + } + + enabled := make(map[string]struct{}, len(artifacts)) + graph := make(map[string][]string, len(artifacts)) + for name, cfg := range artifacts { + if !cfg.Enabled { + continue + } + enabled[name] = struct{}{} + } + for name, cfg := range artifacts { + if !cfg.Enabled { + continue + } + for _, dep := range cfg.DependsOn { + trimmedDep := strings.TrimSpace(dep) + if _, ok := enabled[trimmedDep]; ok { + graph[name] = append(graph[name], trimmedDep) + } + } + } + + visiting := make(map[string]bool, len(enabled)) + visited := make(map[string]bool, len(enabled)) + + var visit func(node string) error + visit = func(node string) error { + if visiting[node] { + return fmt.Errorf("pipeline.scriptorium.artifacts enabled dependencies must not contain cycles") + } + if visited[node] { + return nil + } + visiting[node] = true + for _, dep := range graph[node] { + if err := visit(dep); err != nil { + return err + } + } + visiting[node] = false + visited[node] = true + return nil + } + + for node := range enabled { + if err := visit(node); err != nil { + return err + } + } + return nil +} + func validateDuration(fieldName, value string) error { trimmed := strings.TrimSpace(value) if trimmed == "" { diff --git a/internal/stage/stage.go b/internal/stage/stage.go index 55ad32b..5174ccc 100644 --- a/internal/stage/stage.go +++ b/internal/stage/stage.go @@ -18,10 +18,11 @@ import ( // Env is the shared dependency container visible to stages. type Env struct { - Config *config.Config - ArtifactStore artifacts.Store - ManifestStore manifest.Store - Logger *slog.Logger + Config *config.Config + SelectedAnalyzeArtifacts []string + ArtifactStore artifacts.Store + ManifestStore manifest.Store + Logger *slog.Logger WhisperX whisperx.Client Seriatim seriatim.Runner