From dc8e1040f2110fa088a4f8c3b2c581b961c68ef9 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Thu, 14 May 2026 21:02:06 -0500 Subject: [PATCH] Rationalize config file locations and update documentation --- README.md | 11 +++- architecture.md | 7 +++ internal/app/commands_test.go | 36 +++++++++++-- internal/app/pipeline_config_path.go | 49 +++++++++++++++++ internal/app/pipeline_config_path_test.go | 65 +++++++++++++++++++++++ internal/app/plan.go | 13 +++-- internal/app/resume.go | 13 +++-- internal/app/run.go | 13 +++-- internal/app/run_stage.go | 13 +++-- internal/config/defaults.go | 18 +++++++ 10 files changed, 217 insertions(+), 21 deletions(-) create mode 100644 internal/app/pipeline_config_path.go create mode 100644 internal/app/pipeline_config_path_test.go create mode 100644 internal/config/defaults.go diff --git a/README.md b/README.md index 5bb8d7d..2534a30 100644 --- a/README.md +++ b/README.md @@ -28,6 +28,13 @@ Narratio expects two YAML files: - `pipeline.yml`: pipeline/workspace settings - `session.yml`: per-session settings +Pipeline config lookup for CLI commands: + +- if `--config ` is provided, Narratio uses that path +- if `--config` is omitted, Narratio searches in this order: + - `/usr/local/etc/narratio/pipeline.yml` + - `/etc/narratio/pipeline.yml` + YAML decoding is strict (`KnownFields(true)`), so unknown fields fail fast. ## Canonical Stage Order @@ -242,9 +249,11 @@ go test ./... Plan a run: ```bash -go run ./cmd/narratio plan --config examples/pipeline.minimal.yml --session examples/session.minimal.yml +go run ./cmd/narratio plan --session examples/session.minimal.yml ``` +Use `--config ` to override default pipeline lookup when needed. + Run full pipeline: ```bash diff --git a/architecture.md b/architecture.md index 8f191bc..22ebc8a 100644 --- a/architecture.md +++ b/architecture.md @@ -94,6 +94,13 @@ Adapter behavior: ## 5. Configuration Contract +CLI pipeline config path resolution: + +- when `--config ` is provided, that path is used +- when `--config` is omitted, Narratio searches defaults in order: + - `/usr/local/etc/narratio/pipeline.yml` + - `/etc/narratio/pipeline.yml` + `pipeline.scriptorium` is optional. Existing pipelines without Scriptorium continue to work. `pipeline.trim` is optional. Existing pipelines without trim config continue to work. diff --git a/internal/app/commands_test.go b/internal/app/commands_test.go index b839237..4cba931 100644 --- a/internal/app/commands_test.go +++ b/internal/app/commands_test.go @@ -12,6 +12,7 @@ import ( "testing" "time" + "gitea.maximumdirect.net/eric/narratio/internal/config" "gitea.maximumdirect.net/eric/narratio/internal/manifest" ) @@ -63,12 +64,13 @@ func TestExecuteMissingRequiredFlags(t *testing.T) { args []string want string }{ - {name: "run missing flags", args: []string{"run"}, want: "run: --config and --session are required"}, - {name: "plan missing flags", args: []string{"plan"}, want: "plan: --config and --session are required"}, + {name: "run missing flags", args: []string{"run"}, want: "run: --session is required"}, + {name: "plan missing flags", args: []string{"plan"}, want: "plan: --session is required"}, {name: "status missing flags", args: []string{"status"}, want: "status: --manifest is required"}, - {name: "resume missing flags", args: []string{"resume"}, want: "resume: --config and --session are required"}, + {name: "resume missing flags", args: []string{"resume"}, want: "resume: --session is required"}, {name: "run-stage missing name", args: []string{"run-stage", "--config", "a", "--session", "b"}, want: "run-stage: expected exactly one stage name"}, - {name: "run-stage missing config flags", args: []string{"run-stage", "polish"}, want: "run-stage: --config and --session are required"}, + {name: "run-stage missing config flags", args: []string{"run-stage", "polish"}, want: "run-stage: --session is required"}, + {name: "run missing config uses defaults", args: []string{"run", "--session", "session.yml"}, want: "run: no pipeline config path provided and no default pipeline config found; searched:"}, } for _, tc := range cases { @@ -168,6 +170,32 @@ func TestExecuteRunStageTranscribeUsesConfiguredWhisperXServer(t *testing.T) { } } +func TestExecuteUsesDefaultPipelineConfigPathWhenConfigFlagOmitted(t *testing.T) { + workspaceRoot := t.TempDir() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"source":"default-config-test","segments":[{"speaker":"alice"}]}`)) + })) + defer srv.Close() + + pipelinePath, sessionPath := writeValidConfigFiles(t, workspaceRoot, srv.URL) + originalDefaults := append([]string(nil), config.DefaultPipelineConfigSearchPaths...) + config.DefaultPipelineConfigSearchPaths = []string{pipelinePath} + defer func() { + config.DefaultPipelineConfigSearchPaths = originalDefaults + }() + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := Execute([]string{"run", "--session", sessionPath}, &stdout, &stderr) + if code != 0 { + t.Fatalf("exit code = %d, want 0; stderr=%q", code, stderr.String()) + } + if !strings.Contains(stdout.String(), "narratio run: session 2026-05-03; executed=9 skipped=0; manifest=") { + t.Fatalf("stdout = %q, want successful run output", stdout.String()) + } +} + func TestExecuteInvalidCommand(t *testing.T) { var stdout bytes.Buffer var stderr bytes.Buffer diff --git a/internal/app/pipeline_config_path.go b/internal/app/pipeline_config_path.go new file mode 100644 index 0000000..457a55c --- /dev/null +++ b/internal/app/pipeline_config_path.go @@ -0,0 +1,49 @@ +package app + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "strings" + + "gitea.maximumdirect.net/eric/narratio/internal/config" +) + +func resolvePipelineConfigPath(flagValue string) (string, error) { + return resolvePipelineConfigPathWithCandidates(flagValue, config.DefaultPipelineConfigSearchPaths) +} + +func resolvePipelineConfigPathWithCandidates(flagValue string, candidates []string) (string, error) { + if explicit := strings.TrimSpace(flagValue); explicit != "" { + return explicit, nil + } + + ordered := make([]string, 0, len(candidates)) + for _, raw := range candidates { + path := strings.TrimSpace(raw) + if path == "" { + continue + } + ordered = append(ordered, path) + info, err := os.Stat(path) + if err == nil { + if info.IsDir() { + continue + } + return filepath.Clean(path), nil + } + if errors.Is(err, os.ErrNotExist) { + continue + } + return "", fmt.Errorf("check default pipeline config %q: %w", path, err) + } + + if len(ordered) == 0 { + return "", fmt.Errorf("no pipeline config path provided and no default locations configured") + } + return "", fmt.Errorf( + "no pipeline config path provided and no default pipeline config found; searched: %s", + strings.Join(ordered, ", "), + ) +} diff --git a/internal/app/pipeline_config_path_test.go b/internal/app/pipeline_config_path_test.go new file mode 100644 index 0000000..b8c4918 --- /dev/null +++ b/internal/app/pipeline_config_path_test.go @@ -0,0 +1,65 @@ +package app + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestResolvePipelineConfigPathWithCandidatesExplicitWins(t *testing.T) { + got, err := resolvePipelineConfigPathWithCandidates(" ./custom/pipeline.yml ", []string{"/a", "/b"}) + if err != nil { + t.Fatalf("resolvePipelineConfigPathWithCandidates() error = %v", err) + } + if got != "./custom/pipeline.yml" { + t.Fatalf("resolved path = %q, want explicit path", got) + } +} + +func TestResolvePipelineConfigPathWithCandidatesUsesFirstExisting(t *testing.T) { + dir := t.TempDir() + first := filepath.Join(dir, "first.yml") + second := filepath.Join(dir, "second.yml") + if err := os.WriteFile(second, []byte("workspace:\n root: ./tmp\n"), 0o644); err != nil { + t.Fatalf("write second default: %v", err) + } + + got, err := resolvePipelineConfigPathWithCandidates("", []string{first, second}) + if err != nil { + t.Fatalf("resolvePipelineConfigPathWithCandidates() error = %v", err) + } + if got != filepath.Clean(second) { + t.Fatalf("resolved path = %q, want %q", got, filepath.Clean(second)) + } +} + +func TestResolvePipelineConfigPathWithCandidatesPrecedence(t *testing.T) { + dir := t.TempDir() + first := filepath.Join(dir, "first.yml") + second := filepath.Join(dir, "second.yml") + if err := os.WriteFile(first, []byte("workspace:\n root: ./tmp\n"), 0o644); err != nil { + t.Fatalf("write first default: %v", err) + } + if err := os.WriteFile(second, []byte("workspace:\n root: ./tmp\n"), 0o644); err != nil { + t.Fatalf("write second default: %v", err) + } + + got, err := resolvePipelineConfigPathWithCandidates("", []string{first, second}) + if err != nil { + t.Fatalf("resolvePipelineConfigPathWithCandidates() error = %v", err) + } + if got != filepath.Clean(first) { + t.Fatalf("resolved path = %q, want first candidate %q", got, filepath.Clean(first)) + } +} + +func TestResolvePipelineConfigPathWithCandidatesMissing(t *testing.T) { + _, err := resolvePipelineConfigPathWithCandidates("", []string{"/does/not/exist/one.yml", "/does/not/exist/two.yml"}) + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), "no default pipeline config found") { + t.Fatalf("error = %q, want missing-defaults context", err.Error()) + } +} diff --git a/internal/app/plan.go b/internal/app/plan.go index 6db5d0c..cdccb07 100644 --- a/internal/app/plan.go +++ b/internal/app/plan.go @@ -19,7 +19,7 @@ func Plan(ctx context.Context, args []string, out io.Writer) error { var pipelinePath string var sessionPath string var force bool - fs.StringVar(&pipelinePath, "config", "", "path to pipeline.yml") + fs.StringVar(&pipelinePath, "config", "", "path to pipeline.yml (optional; defaults searched)") fs.StringVar(&sessionPath, "session", "", "path to session.yml") fs.BoolVar(&force, "force", false, "force stage execution (reserved for future behavior)") @@ -29,11 +29,16 @@ func Plan(ctx context.Context, args []string, out io.Writer) error { if fs.NArg() != 0 { return fmt.Errorf("plan: unexpected positional arguments") } - if pipelinePath == "" || sessionPath == "" { - return fmt.Errorf("plan: --config and --session are required") + if sessionPath == "" { + return fmt.Errorf("plan: --session is required") } - cfg, err := config.Load(pipelinePath, sessionPath) + resolvedPipelinePath, err := resolvePipelineConfigPath(pipelinePath) + if err != nil { + return fmt.Errorf("plan: %w", err) + } + + cfg, err := config.Load(resolvedPipelinePath, sessionPath) if err != nil { return fmt.Errorf("plan: %w", err) } diff --git a/internal/app/resume.go b/internal/app/resume.go index e9997a5..9a22019 100644 --- a/internal/app/resume.go +++ b/internal/app/resume.go @@ -18,7 +18,7 @@ func Resume(ctx context.Context, args []string, out io.Writer) error { var pipelinePath string var sessionPath string var force bool - fs.StringVar(&pipelinePath, "config", "", "path to pipeline.yml") + fs.StringVar(&pipelinePath, "config", "", "path to pipeline.yml (optional; defaults searched)") fs.StringVar(&sessionPath, "session", "", "path to session.yml") fs.BoolVar(&force, "force", false, "force stage execution") @@ -28,11 +28,16 @@ func Resume(ctx context.Context, args []string, out io.Writer) error { if fs.NArg() != 0 { return fmt.Errorf("resume: unexpected positional arguments") } - if pipelinePath == "" || sessionPath == "" { - return fmt.Errorf("resume: --config and --session are required") + if sessionPath == "" { + return fmt.Errorf("resume: --session is required") } - cfg, err := config.Load(pipelinePath, sessionPath) + resolvedPipelinePath, err := resolvePipelineConfigPath(pipelinePath) + if err != nil { + return fmt.Errorf("resume: %w", err) + } + + cfg, err := config.Load(resolvedPipelinePath, sessionPath) if err != nil { return fmt.Errorf("resume: %w", err) } diff --git a/internal/app/run.go b/internal/app/run.go index a82faf9..f020cac 100644 --- a/internal/app/run.go +++ b/internal/app/run.go @@ -17,7 +17,7 @@ func Run(ctx context.Context, args []string, out io.Writer) error { var pipelinePath string var sessionPath string var force bool - fs.StringVar(&pipelinePath, "config", "", "path to pipeline.yml") + fs.StringVar(&pipelinePath, "config", "", "path to pipeline.yml (optional; defaults searched)") fs.StringVar(&sessionPath, "session", "", "path to session.yml") fs.BoolVar(&force, "force", false, "force stage execution (reserved for future behavior)") @@ -27,11 +27,16 @@ func Run(ctx context.Context, args []string, out io.Writer) error { if fs.NArg() != 0 { return fmt.Errorf("run: unexpected positional arguments") } - if pipelinePath == "" || sessionPath == "" { - return fmt.Errorf("run: --config and --session are required") + if sessionPath == "" { + return fmt.Errorf("run: --session is required") } - cfg, err := config.Load(pipelinePath, sessionPath) + resolvedPipelinePath, err := resolvePipelineConfigPath(pipelinePath) + if err != nil { + return fmt.Errorf("run: %w", err) + } + + cfg, err := config.Load(resolvedPipelinePath, sessionPath) if err != nil { return fmt.Errorf("run: %w", err) } diff --git a/internal/app/run_stage.go b/internal/app/run_stage.go index 38b2f6a..cc1b081 100644 --- a/internal/app/run_stage.go +++ b/internal/app/run_stage.go @@ -17,7 +17,7 @@ func RunStage(ctx context.Context, args []string, out io.Writer) error { var pipelinePath string var sessionPath string var force bool - fs.StringVar(&pipelinePath, "config", "", "path to pipeline.yml") + fs.StringVar(&pipelinePath, "config", "", "path to pipeline.yml (optional; defaults searched)") fs.StringVar(&sessionPath, "session", "", "path to session.yml") fs.BoolVar(&force, "force", false, "force stage execution (reserved for future behavior)") @@ -27,8 +27,8 @@ func RunStage(ctx context.Context, args []string, out io.Writer) error { if fs.NArg() != 1 { return fmt.Errorf("run-stage: expected exactly one stage name") } - if pipelinePath == "" || sessionPath == "" { - return fmt.Errorf("run-stage: --config and --session are required") + if sessionPath == "" { + return fmt.Errorf("run-stage: --session is required") } stageName := fs.Arg(0) @@ -37,7 +37,12 @@ func RunStage(ctx context.Context, args []string, out io.Writer) error { return fmt.Errorf("run-stage: %w", err) } - cfg, err := config.Load(pipelinePath, sessionPath) + resolvedPipelinePath, err := resolvePipelineConfigPath(pipelinePath) + if err != nil { + return fmt.Errorf("run-stage: %w", err) + } + + cfg, err := config.Load(resolvedPipelinePath, sessionPath) if err != nil { return fmt.Errorf("run-stage: %w", err) } diff --git a/internal/config/defaults.go b/internal/config/defaults.go new file mode 100644 index 0000000..765c438 --- /dev/null +++ b/internal/config/defaults.go @@ -0,0 +1,18 @@ +package config + +// Default filesystem locations for pipeline configuration lookup when --config +// is omitted. Order is highest to lowest precedence. +const ( + DefaultPipelineConfigPathUsrLocal = "/usr/local/etc/narratio/pipeline.yml" + DefaultPipelineConfigPathEtc = "/etc/narratio/pipeline.yml" +) + +// DefaultPipelineConfigSearchPaths defines the default search order for +// pipeline.yml when callers do not provide an explicit path. +// +// Keep this in a variable so future defaults can be extended without changing +// call sites. +var DefaultPipelineConfigSearchPaths = []string{ + DefaultPipelineConfigPathUsrLocal, + DefaultPipelineConfigPathEtc, +}