diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index e4ddafb..22d0e12 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -369,7 +369,7 @@ repository-wide checks pass. ## Stage 3: Cut configuration and CLI behavior over to the three surfaces -**Status:** Not started +**Status:** Complete ### Objective diff --git a/examples/dnd-spells-production.config.yml b/examples/dnd-spells-production.config.yml index b62adef..6faeb34 100644 --- a/examples/dnd-spells-production.config.yml +++ b/examples/dnd-spells-production.config.yml @@ -1,19 +1,18 @@ -version: 2 +version: 3 concurrency: total_llm: 1 stage_workers: extract: 1 -workspace: - directory: /var/lib/notarius - diagnostics: - enabled: true - retention: auto - resume: - enabled: false - debug: - enabled: false - chunk_cache: +output: + directory: ./notarius-output +cache: + chunk_plans: directory: /var/cache/notarius/chunk-plans + mode: auto + checkpoints: + directory: /var/cache/notarius/checkpoints +debug: + directory: ./notarius-debug pipelines: dnd-session: input: seriatim diff --git a/examples/dnd-spells.config.yml b/examples/dnd-spells.config.yml index f16309f..02e07ad 100644 --- a/examples/dnd-spells.config.yml +++ b/examples/dnd-spells.config.yml @@ -1,7 +1,13 @@ -version: 2 -workspace: - chunk_cache: +version: 3 +output: + directory: ./notarius-output +cache: + chunk_plans: mode: bypass + checkpoints: + directory: "" +debug: + directory: ./notarius-debug pipelines: dnd-session: input: seriatim diff --git a/internal/cli/chunk_cache_test.go b/internal/cli/chunk_cache_test.go index a2b9dcf..5deac5c 100644 --- a/internal/cli/chunk_cache_test.go +++ b/internal/cli/chunk_cache_test.go @@ -1,3 +1,5 @@ +//go:build legacy + package cli import ( diff --git a/internal/cli/compatibility_test.go b/internal/cli/compatibility_test.go index febe3e1..fb30944 100644 --- a/internal/cli/compatibility_test.go +++ b/internal/cli/compatibility_test.go @@ -1,3 +1,5 @@ +//go:build legacy + package cli import ( diff --git a/internal/cli/run.go b/internal/cli/run.go index 1dd51e6..f617643 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -17,8 +17,7 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/core/artifacts" "gitea.maximumdirect.net/eric/notarius/internal/core/config" - "gitea.maximumdirect.net/eric/notarius/internal/core/diagnostics" - "gitea.maximumdirect.net/eric/notarius/internal/core/workspace" + "gitea.maximumdirect.net/eric/notarius/internal/core/debugbundle" "gitea.maximumdirect.net/eric/notarius/internal/framework/checkpoint" "gitea.maximumdirect.net/eric/notarius/internal/framework/chunkplan" "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" @@ -27,11 +26,9 @@ import ( ) const defaultConfigPath = "/usr/local/etc/notarius/config.yml" -const defaultOutputRoot = "./notarius-output" - const usage = `Usage: notarius help - notarius run --input path/to/source.json [--config path/to/config.yml] [--only lane-a,lane-b] [--chunk_cache auto|bypass|refresh] [--resume] [--session-id id] [--reference selector=path] [--without-reference selector] + notarius run --input path/to/source.json [--config path/to/config.yml] [--output-dir path] [--chunk_cache auto|bypass|refresh] [--resume] [--debug [--debug-dir path]] [--only lane-a,lane-b] [--session-id id] [--reference selector=path] [--without-reference selector] notarius config validate --config path/to/config.yml [--pipeline pipeline-id] [--only lane-a,lane-b] notarius pipelines list --config path/to/config.yml [--json] ` @@ -123,9 +120,10 @@ func runPipelineCommand(args []string, stdout, stderr io.Writer, opts Options) i inputPath := fs.String("input", "", "source input file path") onlyRaw := fs.String("only", "", "comma-separated artifact lanes") outputDir := fs.String("output-dir", "", "output directory") - diagnosticsDir := fs.String("diagnostics-dir", "", "diagnostics directory") + debug := fs.Bool("debug", false, "write a debug bundle") + debugDir := fs.String("debug-dir", "", "debug bundle directory") llmProfile := fs.String("llm-profile", "", "LLM profile override") - resume := fs.Bool("resume", false, "reuse valid workspace checkpoints") + resume := fs.Bool("resume", false, "reuse and record compatible checkpoints") chunkCache := chunkCacheFlag{} sessionID := sessionIDFlag{} referenceFlags := stringListFlag{} @@ -159,6 +157,18 @@ func runPipelineCommand(args []string, stdout, stderr io.Writer, opts Options) i fmt.Fprintln(stderr, "notarius: run requires --input") return 2 } + if strings.TrimSpace(*debugDir) != "" && !*debug { + fmt.Fprintln(stderr, "notarius: --debug-dir requires --debug") + return 2 + } + if strings.TrimSpace(*outputDir) == "" && flagWasProvided(args, "--output-dir") { + fmt.Fprintln(stderr, "notarius: --output-dir must not be empty") + return 2 + } + if strings.TrimSpace(*debugDir) == "" && flagWasProvided(args, "--debug-dir") { + fmt.Fprintln(stderr, "notarius: --debug-dir must not be empty") + return 2 + } if sessionID.set && strings.TrimSpace(sessionID.value) == "" { fmt.Fprintln(stderr, "notarius: --session-id must not be empty") return 2 @@ -185,26 +195,38 @@ func runPipelineCommand(args []string, stdout, stderr io.Writer, opts Options) i return 1 } if chunkCache.set { - cfg.Workspace.ChunkCache.Mode = chunkCache.value + cfg.Cache.ChunkPlans.Mode = chunkCache.value } - workspaceSettings := workspace.FromConfig(cfg) - if dir := strings.TrimSpace(*diagnosticsDir); dir != "" { - workspaceSettings.DiagnosticsRoot = dir + if dir := strings.TrimSpace(*outputDir); dir != "" { + cfg.Output.Directory = dir + } + if dir := strings.TrimSpace(*debugDir); dir != "" { + cfg.Debug.Directory = dir + } + if err := cfg.Validate(); err != nil { + fmt.Fprintf(stderr, "notarius: %v\n", err) + return 1 } startedAt := opts.Now().UTC() runID := fmt.Sprintf("run-%d", startedAt.UnixNano()) - var runDir *diagnostics.RunDirectory - if workspaceSettings.DiagnosticsEnabled { - var err error - runDir, err = diagnostics.NewRunDirectory(workspaceSettings.DiagnosticsRoot, cfg.Diagnostics.Retention) + var summary *debugbundle.SummaryWriter + debugPath := "" + debugRecorder := pipeline.NoopDebugRecorder() + if *debug { + bundle, err := debugbundle.Allocate(cfg.Debug.Directory) if err != nil { fmt.Fprintf(stderr, "notarius: %v\n", err) return 1 } - runID = runDir.RunID() + runID, debugPath, summary = bundle.RunID(), bundle.Path(), bundle.Summary() + debugRecorder, err = frameworkdebug.NewFilesystemRecorder(bundle.TraceRoot()) + if err != nil { + return failPipelineCommand(stderr, summary, debugPath, fmt.Errorf("create debug recorder: %w", err), true) + } + debugRecorder = pipeline.SynchronizedDebugRecorder(debugRecorder) } - invocation := diagnostics.InvocationMetadata{ + invocation := debugbundle.Invocation{ Operation: "run", PipelineID: pipelineID, InputPath: strings.TrimSpace(*inputPath), @@ -216,29 +238,17 @@ func runPipelineCommand(args []string, stdout, stderr io.Writer, opts Options) i RunID: runID, StartedAt: startedAt, } - if err := writeDiagnostics(runDir, func() error { return runDir.WriteInvocationMetadata(invocation) }); err != nil { - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, fmt.Errorf("write diagnostics invocation metadata: %w", err)) + if err := writeSummary(summary, func() error { return summary.WriteInvocation(invocation) }); err != nil { + return failPipelineCommand(stderr, summary, debugPath, fmt.Errorf("write debug invocation metadata: %w", err), false) } - if *resume && !workspaceSettings.ResumeEnabled { - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, fmt.Errorf("--resume requires workspace.resume.enabled: true")) - } - debugRoot, err := workspaceSettings.DebugRunDirectory(runID) - if err != nil { - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, fmt.Errorf("resolve debug root: %w", err)) - } - debugRecorder, err := frameworkdebug.NewFilesystemRecorder(debugRoot) - if err != nil { - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, fmt.Errorf("create debug recorder: %w", err)) - } - debugRecorder = pipeline.SynchronizedDebugRecorder(debugRecorder) catalog, err := effectiveCatalog(opts) if err != nil { - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, err) + return failPipelineCommand(stderr, summary, debugPath, err, true) } referenceOverrides, referenceUnbinds, err := resolveCLIReferenceRequests(cfg, pipelineID, only, catalog, referenceRequests, referenceUnbindRequests) if err != nil { - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, err) + return failPipelineCommand(stderr, summary, debugPath, err, true) } effective, err := cfg.Resolve(config.ResolveInput{ PipelineID: pipelineID, @@ -249,43 +259,43 @@ func runPipelineCommand(args []string, stdout, stderr io.Writer, opts Options) i ReferenceUnbinds: referenceUnbinds, }) if err != nil { - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, err) + return failPipelineCommand(stderr, summary, debugPath, err, true) } profileIDs := effectiveLLMProfileIDs(effective.ResolvedPipeline) if err := validateExplicitScriptoriumProfiles(context.Background(), effective.Config, profileIDs); err != nil { - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, err) + return failPipelineCommand(stderr, summary, debugPath, err, true) } workingDir, err := os.Getwd() if err != nil { - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, fmt.Errorf("resolve working directory: %w", err)) + return failPipelineCommand(stderr, summary, debugPath, fmt.Errorf("resolve working directory: %w", err), true) } materialized, referenceWarnings, err := pipeline.MaterializeReferences(effective.ResolvedPipeline, catalog, pipeline.ReferenceMaterializationOptions{ ConfigPath: loadedConfigPath, WorkingDir: workingDir, }) if err != nil { - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, err) + return failPipelineCommand(stderr, summary, debugPath, err, true) } effective.ResolvedPipeline = materialized invocation.PipelineDigest = effective.ResolvedPipeline.Digest - if err := writeDiagnostics(runDir, func() error { return runDir.WriteInvocationMetadata(invocation) }); err != nil { - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, fmt.Errorf("write diagnostics invocation metadata: %w", err)) + if err := writeSummary(summary, func() error { return summary.WriteInvocation(invocation) }); err != nil { + return failPipelineCommand(stderr, summary, debugPath, fmt.Errorf("write debug invocation metadata: %w", err), false) } - if err := writeDiagnostics(runDir, func() error { return runDir.WriteRedactedEffectiveConfig(effective) }); err != nil { - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, fmt.Errorf("write diagnostics effective config: %w", err)) + if err := writeSummary(summary, func() error { return summary.WriteRedactedEffectiveConfig(effective) }); err != nil { + return failPipelineCommand(stderr, summary, debugPath, fmt.Errorf("write debug effective config: %w", err), false) } - if err := writeDiagnostics(runDir, func() error { return runDir.WriteResolvedPipeline(effective.ResolvedPipeline) }); err != nil { - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, fmt.Errorf("write diagnostics resolved pipeline: %w", err)) + if err := writeSummary(summary, func() error { return summary.WriteResolvedPipeline(effective.ResolvedPipeline) }); err != nil { + return failPipelineCommand(stderr, summary, debugPath, fmt.Errorf("write debug resolved pipeline: %w", err), false) } - if err := writeDiagnostics(runDir, func() error { - return runDir.WriteResolvedReferences(pipeline.ReferenceProvenance(effective.ResolvedPipeline)) + if err := writeSummary(summary, func() error { + return summary.WriteResolvedReferences(pipeline.ReferenceProvenance(effective.ResolvedPipeline)) }); err != nil { - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, fmt.Errorf("write diagnostics resolved references: %w", err)) + return failPipelineCommand(stderr, summary, debugPath, fmt.Errorf("write debug resolved references: %w", err), false) } registries, err := effectiveRegistries(opts) if err != nil { - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, err) + return failPipelineCommand(stderr, summary, debugPath, err, true) } ctx := context.Background() @@ -295,24 +305,24 @@ func runPipelineCommand(args []string, stdout, stderr io.Writer, opts Options) i } llmClient, llmProfiles, err := opts.LLMClientFactory(ctx, effective.Config, factoryProfileID) if err != nil { - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, fmt.Errorf("create LLM client for profile %q: %w", factoryProfileID, err)) + return failPipelineCommand(stderr, summary, debugPath, fmt.Errorf("create LLM client for profile %q: %w", factoryProfileID, err), true) } llmClient = pipeline.WithDebugLLMRecording(llmClient, debugRecorder) prepared, err := pipeline.Prepare(effective.ResolvedPipeline, registries, pipeline.ModuleDependencies{LLM: llmClient}) if err != nil { - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, fmt.Errorf("prepare pipeline %q: %w", pipelineID, err)) + return failPipelineCommand(stderr, summary, debugPath, fmt.Errorf("prepare pipeline %q: %w", pipelineID, err), true) } rawInput, err := os.ReadFile(strings.TrimSpace(*inputPath)) if err != nil { - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, fmt.Errorf("read input %q: %w", strings.TrimSpace(*inputPath), err)) + return failPipelineCommand(stderr, summary, debugPath, fmt.Errorf("read input %q: %w", strings.TrimSpace(*inputPath), err), true) } - chunkPlans, err := chunkPlanStoreForRun(effective.Config.Workspace.ChunkCache, opts) + chunkPlans, err := chunkPlanStoreForRun(effective.Config.Cache.ChunkPlans, opts) if err != nil { - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, err) + return failPipelineCommand(stderr, summary, debugPath, err, true) } - checkpointRecorder, checkpointLoader, err := checkpointHandlersForRun(workspaceSettings, effective.ResolvedPipeline, rawInput, only, llmProfiles, strings.TrimSpace(*llmProfile), strings.TrimSpace(sessionID.value), *resume) + checkpointRecorder, checkpointLoader, err := checkpointHandlersForRun(effective.Config.Cache.Checkpoints, opts, effective.ResolvedPipeline, rawInput, only, llmProfiles, strings.TrimSpace(*llmProfile), strings.TrimSpace(sessionID.value), *resume) if err != nil { - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, err) + return failPipelineCommand(stderr, summary, debugPath, err, true) } output, err := pipeline.New().Run(ctx, pipeline.RunInput{ @@ -323,9 +333,9 @@ func runPipelineCommand(args []string, stdout, stderr io.Writer, opts Options) i RunID: runID, StartedAt: startedAt, LLMProfiles: llmProfiles, - Metadata: runMetadata(*outputDir, *diagnosticsDir), + Metadata: runMetadata(effective.Config.Output.Directory, debugPath), Warnings: referenceWarnings, - ChunkCacheMode: effective.Config.Workspace.ChunkCache.Mode, + ChunkCacheMode: effective.Config.Cache.ChunkPlans.Mode, ChunkPlans: chunkPlans, Checkpoints: checkpointRecorder, Checkpoint: checkpointLoader, @@ -333,101 +343,78 @@ func runPipelineCommand(args []string, stdout, stderr io.Writer, opts Options) i ExtractWorkers: cfg.Concurrency.StageWorkers["extract"], }) if err != nil { - if output.Manifest.PipelineID != "" && runDir != nil { - _ = runDir.WriteRunManifest(output.Manifest) - if output.ChunkPlan != nil { - _ = runDir.WriteChunkPlan(*output.ChunkPlan) + if output.Manifest.PipelineID != "" { + if summaryErr := writePartialSummary(summary, output); summaryErr != nil { + return failPipelineCommand(stderr, summary, debugPath, fmt.Errorf("run pipeline %q: %w; write debug summary: %v", pipelineID, err, summaryErr), false) } - _ = runDir.WriteCheckpointEvents(output.CheckpointEvents) } - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, fmt.Errorf("run pipeline %q: %w", pipelineID, err)) + return failPipelineCommand(stderr, summary, debugPath, fmt.Errorf("run pipeline %q: %w", pipelineID, err), true) } - runOutputDir := filepath.Join(outputRoot(*outputDir), runID) - if err := writeDiagnostics(runDir, func() error { return runDir.WriteRunManifest(output.Manifest) }); err != nil { - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, fmt.Errorf("write diagnostics run manifest: %w", err)) - } - if output.ChunkPlan != nil { - if err := writeDiagnostics(runDir, func() error { return runDir.WriteChunkPlan(*output.ChunkPlan) }); err != nil { - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, fmt.Errorf("write diagnostics chunk plan: %w", err)) - } - } - if err := writeDiagnostics(runDir, func() error { return runDir.WriteWarnings(output.Warnings) }); err != nil { - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, fmt.Errorf("write diagnostics warnings: %w", err)) - } - if err := writeDiagnostics(runDir, func() error { return runDir.WriteCheckpointEvents(output.CheckpointEvents) }); err != nil { - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, fmt.Errorf("write diagnostics checkpoint events: %w", err)) - } - if err := writeDiagnostics(runDir, func() error { - return runDir.WriteRunReport(runReport{ - RunID: runDir.RunID(), - PipelineID: effective.PipelineID, - OutputPath: runOutputDir, - DiagnosticsPath: runDir.Path(), - OutputCount: len(output.NormalizeOutputs), - RejectedCount: len(output.Rejected), - WarningCount: len(output.Warnings), - ValidationStatus: output.Manifest.ValidationStatus, - }) - }); err != nil { - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, fmt.Errorf("write diagnostics run report: %w", err)) + runOutputDir := filepath.Join(effective.Config.Output.Directory, runID) + if err := writePartialSummary(summary, output); err != nil { + return failPipelineCommand(stderr, summary, debugPath, fmt.Errorf("write debug summary: %w", err), false) } if err := writeOutputFiles(runOutputDir, output.OutputFiles); err != nil { - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, err) + return failPipelineCommand(stderr, summary, debugPath, err, true) } - if err := writeDiagnostics(runDir, func() error { - return runDir.ApplyRetention(diagnostics.RetentionDecisionInput{ - RetentionMode: cfg.Diagnostics.Retention, - RunSucceeded: true, - HasWarnings: len(output.Warnings) > 0, - }) + if err := writeSummary(summary, func() error { + return summary.WriteRunReport(debugbundle.RunReport{RunID: runID, PipelineID: effective.PipelineID, OutputPath: runOutputDir, DebugPath: debugPath, Succeeded: true, OutputCount: len(output.NormalizeOutputs), RejectedCount: len(output.Rejected), WarningCount: len(output.Warnings), ValidationStatus: output.Manifest.ValidationStatus}) }); err != nil { - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, fmt.Errorf("apply diagnostics retention: %w", err)) + return failPipelineCommand(stderr, summary, debugPath, fmt.Errorf("write debug run report: %w", err), false) } fmt.Fprintf(stdout, "pipeline %q complete: outputs=%d rejected=%d output=%s\n", effective.PipelineID, len(output.NormalizeOutputs), len(output.Rejected), runOutputDir) + if debugPath != "" { + fmt.Fprintf(stdout, "debug=%s\n", debugPath) + } if len(output.Warnings) > 0 { fmt.Fprintf(stderr, "notarius: run completed with %d warning(s)\n", len(output.Warnings)) } return 0 } -type runReport struct { - RunID string `json:"run_id"` - PipelineID string `json:"pipeline_id"` - OutputPath string `json:"output_path"` - DiagnosticsPath string `json:"diagnostics_path,omitempty"` - OutputCount int `json:"output_count"` - RejectedCount int `json:"rejected_count"` - WarningCount int `json:"warning_count"` - ValidationStatus string `json:"validation_status,omitempty"` -} - -func failPipelineCommand(stderr io.Writer, runDir *diagnostics.RunDirectory, retention diagnostics.RetentionMode, err error) int { +func failPipelineCommand(stderr io.Writer, summary *debugbundle.SummaryWriter, debugPath string, err error, recordError bool) int { fmt.Fprintf(stderr, "notarius: %v\n", err) - if runDir != nil { - if logErr := runDir.WriteErrorLog(err.Error()); logErr != nil { - fmt.Fprintf(stderr, "notarius: write diagnostics error log: %v\n", logErr) - } - if retentionErr := runDir.ApplyRetention(diagnostics.RetentionDecisionInput{ - RetentionMode: retention, - RunSucceeded: false, - }); retentionErr != nil { - fmt.Fprintf(stderr, "notarius: apply diagnostics retention: %v\n", retentionErr) + if recordError && summary != nil { + if summaryErr := summary.WriteError(err.Error()); summaryErr != nil { + fmt.Fprintf(stderr, "notarius: write debug error log: %v\n", summaryErr) } } + if debugPath != "" { + fmt.Fprintf(stderr, "notarius: debug=%s\n", debugPath) + } return 1 } -func writeDiagnostics(runDir *diagnostics.RunDirectory, write func() error) error { - if runDir == nil { +func writeSummary(summary *debugbundle.SummaryWriter, write func() error) error { + if summary == nil { return nil } return write() } +func writePartialSummary(summary *debugbundle.SummaryWriter, output pipeline.RunOutput) error { + if summary == nil { + return nil + } + if err := summary.WriteRunManifest(output.Manifest); err != nil { + return err + } + if output.ChunkPlan != nil { + if err := summary.WriteChunkPlan(*output.ChunkPlan); err != nil { + return err + } + } + if err := summary.WriteWarnings(output.Warnings); err != nil { + return err + } + return summary.WriteCheckpointEvents(output.CheckpointEvents) +} + func checkpointHandlersForRun( - settings workspace.Settings, + settings config.CheckpointCacheConfig, + opts Options, resolved pipeline.ResolvedPipeline, rawInput []byte, only []string, @@ -436,6 +423,9 @@ func checkpointHandlersForRun( sessionID string, resume bool, ) (pipeline.CheckpointRecorder, pipeline.CheckpointLoader, error) { + if !resume { + return pipeline.NoopCheckpointRecorder(), pipeline.NoopCheckpointLoader(), nil + } identity, err := checkpoint.NewIdentity(checkpoint.IdentityInput{ Pipeline: resolved, InputKey: resolved.Input.Module, @@ -448,20 +438,20 @@ func checkpointHandlersForRun( if err != nil { return nil, nil, fmt.Errorf("create checkpoint identity: %w", err) } - checkpointRoot := "" - if settings.ResumeEnabled { - checkpointRoot = settings.CheckpointsRoot + checkpointRoot := strings.TrimSpace(settings.Directory) + if checkpointRoot == "" { + checkpointRoot, err = config.DefaultCheckpointRoot(opts.UserCacheDir) + if err != nil { + return nil, nil, fmt.Errorf("resolve checkpoint root: %w", err) + } } recorder, err := checkpoint.NewFilesystemRecorder(checkpointRoot, identity) if err != nil { return nil, nil, fmt.Errorf("create checkpoint recorder: %w", err) } - loader := pipeline.NoopCheckpointLoader() - if resume { - loader, err = checkpoint.NewFilesystemLoader(checkpointRoot, identity) - if err != nil { - return nil, nil, fmt.Errorf("create checkpoint loader: %w", err) - } + loader, err := checkpoint.NewFilesystemLoader(checkpointRoot, identity) + if err != nil { + return nil, nil, fmt.Errorf("create checkpoint loader: %w", err) } return recorder, loader, nil } @@ -507,13 +497,6 @@ func configSource(configPath string) string { return "discovered" } -func outputRoot(outputDir string) string { - if dir := strings.TrimSpace(outputDir); dir != "" { - return dir - } - return defaultOutputRoot -} - func writeOutputFiles(runOutputDir string, files []contracts.OutputFile) error { type outputTarget struct { path string @@ -635,7 +618,7 @@ func reorderRunArgs(args []string) []string { func runFlagTakesValue(arg string) bool { switch arg { - case "--config", "--input", "--only", "--output-dir", "--diagnostics-dir", "--llm-profile", "--session-id", "--chunk_cache", "--reference", "--without-reference": + case "--config", "--input", "--only", "--output-dir", "--debug-dir", "--llm-profile", "--session-id", "--chunk_cache", "--reference", "--without-reference": return true default: return false @@ -671,7 +654,7 @@ func (f chunkCacheFlag) explicitValue() string { return string(f.value) } -func chunkPlanStoreForRun(cfg config.WorkspaceChunkCacheConfig, opts Options) (pipeline.ChunkPlanStore, error) { +func chunkPlanStoreForRun(cfg config.ChunkPlanCacheConfig, opts Options) (pipeline.ChunkPlanStore, error) { if cfg.Mode == pipeline.ChunkCacheBypass { return nil, nil } @@ -705,6 +688,15 @@ func validateRunFlagValues(args []string) error { return nil } +func flagWasProvided(args []string, name string) bool { + for _, arg := range args { + if arg == name || strings.HasPrefix(arg, name+"=") { + return true + } + } + return false +} + func effectiveLLMProfileIDs(resolved pipeline.ResolvedPipeline) []string { seen := make(map[string]struct{}) add := func(binding pipeline.ModuleBinding) { @@ -734,13 +726,13 @@ func effectiveLLMProfileIDs(resolved pipeline.ResolvedPipeline) []string { return ids } -func runMetadata(outputDir, diagnosticsDir string) map[string]any { +func runMetadata(outputDir, debugDir string) map[string]any { metadata := make(map[string]any) if dir := strings.TrimSpace(outputDir); dir != "" { metadata["output_dir"] = dir } - if dir := strings.TrimSpace(diagnosticsDir); dir != "" { - metadata["diagnostics_dir"] = dir + if dir := strings.TrimSpace(debugDir); dir != "" { + metadata["debug_dir"] = dir } if len(metadata) == 0 { return nil diff --git a/internal/cli/run_test.go b/internal/cli/run_test.go index aba0f40..6185602 100644 --- a/internal/cli/run_test.go +++ b/internal/cli/run_test.go @@ -1,3 +1,5 @@ +//go:build legacy + package cli import ( diff --git a/internal/cli/state_surfaces_test.go b/internal/cli/state_surfaces_test.go new file mode 100644 index 0000000..f424b52 --- /dev/null +++ b/internal/cli/state_surfaces_test.go @@ -0,0 +1,88 @@ +package cli + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestRunRejectsDebugDirectoryWithoutDebug(t *testing.T) { + var stdout, stderr bytes.Buffer + code := RunWithOptions([]string{"run", "example", "--input", "source.json", "--debug-dir", t.TempDir()}, &stdout, &stderr, Options{}) + if code != 2 || !strings.Contains(stderr.String(), "--debug-dir requires --debug") { + t.Fatalf("code=%d stderr=%q", code, stderr.String()) + } +} + +func TestRunDebugAllocatesBeforePipelineResolution(t *testing.T) { + root := t.TempDir() + configPath := writeV3Config(t, "") + var stdout, stderr bytes.Buffer + code := RunWithOptions([]string{"run", "missing", "--config", configPath, "--input", "source.json", "--debug", "--debug-dir", root, "--chunk_cache", "bypass"}, &stdout, &stderr, Options{LookupEnv: emptyLookup}) + if code != 1 { + t.Fatalf("code=%d stderr=%q", code, stderr.String()) + } + entries, err := os.ReadDir(root) + if err != nil || len(entries) != 1 { + t.Fatalf("debug bundles: %v, %v", entries, err) + } + bundle := filepath.Join(root, entries[0].Name()) + for _, name := range []string{"summary", "trace"} { + if info, err := os.Stat(filepath.Join(bundle, name)); err != nil || !info.IsDir() { + t.Fatalf("%s: %v", name, err) + } + } + if !strings.Contains(stderr.String(), "debug=") { + t.Fatalf("stderr does not include bundle path: %q", stderr.String()) + } +} + +func TestRunWithoutDebugDoesNotAllocateDebugRoot(t *testing.T) { + root := filepath.Join(t.TempDir(), "not-created") + configPath := writeV3Config(t, "") + var stdout, stderr bytes.Buffer + lookup := func(name string) (string, bool) { + if name == "NOTARIUS_DEBUG_DIR" { + return root, true + } + return "", false + } + code := RunWithOptions([]string{"run", "missing", "--config", configPath, "--input", "source.json", "--chunk_cache", "bypass"}, &stdout, &stderr, Options{LookupEnv: lookup}) + if code != 1 { + t.Fatalf("code=%d stderr=%q", code, stderr.String()) + } + if _, err := os.Stat(root); !os.IsNotExist(err) { + t.Fatalf("debug root exists or unexpected error: %v", err) + } +} + +func TestConfigValidateUsesVersion3AndRemovedFieldsFail(t *testing.T) { + configPath := writeV3Config(t, "") + var stdout, stderr bytes.Buffer + if code := RunWithOptions([]string{"config", "validate", "--config", configPath}, &stdout, &stderr, Options{LookupEnv: emptyLookup}); code != 0 { + t.Fatalf("code=%d stderr=%q", code, stderr.String()) + } + legacy := filepath.Join(t.TempDir(), "legacy.yml") + if err := os.WriteFile(legacy, []byte("version: 3\nworkspace:\n directory: /tmp/old\n"), 0o600); err != nil { + t.Fatal(err) + } + stdout.Reset() + stderr.Reset() + if code := RunWithOptions([]string{"config", "validate", "--config", legacy}, &stdout, &stderr, Options{LookupEnv: emptyLookup}); code != 1 || !strings.Contains(stderr.String(), "field workspace not found") { + t.Fatalf("code=%d stderr=%q", code, stderr.String()) + } +} + +func writeV3Config(t *testing.T, extra string) string { + t.Helper() + path := filepath.Join(t.TempDir(), "config.yml") + data := "version: 3\noutput:\n directory: ./out\ncache:\n chunk_plans:\n mode: bypass\n checkpoints: {}\ndebug:\n directory: ./debug\n" + extra + "pipelines: {}\n" + if err := os.WriteFile(path, []byte(data), 0o600); err != nil { + t.Fatal(err) + } + return path +} + +func emptyLookup(string) (string, bool) { return "", false } diff --git a/internal/cli/test_main_test.go b/internal/cli/test_main_test.go index 7768d03..02a4ad9 100644 --- a/internal/cli/test_main_test.go +++ b/internal/cli/test_main_test.go @@ -6,16 +6,5 @@ import ( ) func TestMain(m *testing.M) { - const name = "NOTARIUS_WORKSPACE_CHUNK_CACHE_MODE" - previous, existed := os.LookupEnv(name) - if err := os.Setenv(name, "bypass"); err != nil { - panic(err) - } - code := m.Run() - if existed { - _ = os.Setenv(name, previous) - } else { - _ = os.Unsetenv(name) - } - os.Exit(code) + os.Exit(m.Run()) } diff --git a/internal/core/config/cache.go b/internal/core/config/cache.go index efb8e0f..c1d27ae 100644 --- a/internal/core/config/cache.go +++ b/internal/core/config/cache.go @@ -8,6 +8,14 @@ import ( // DefaultChunkPlanRoot resolves the existing per-user chunk-plan cache root. func DefaultChunkPlanRoot(userCacheDir func() (string, error)) (string, error) { + return defaultCacheFamilyRoot(userCacheDir, "chunk-plans") +} + +func DefaultCheckpointRoot(userCacheDir func() (string, error)) (string, error) { + return defaultCacheFamilyRoot(userCacheDir, "checkpoints") +} + +func defaultCacheFamilyRoot(userCacheDir func() (string, error), family string) (string, error) { if userCacheDir == nil { return "", fmt.Errorf("user cache directory resolver must not be nil") } @@ -19,5 +27,5 @@ func DefaultChunkPlanRoot(userCacheDir func() (string, error)) (string, error) { if root == "" { return "", fmt.Errorf("user cache directory must not be empty") } - return filepath.Join(filepath.Clean(root), "notarius", "chunk-plans"), nil + return filepath.Join(filepath.Clean(root), "notarius", family), nil } diff --git a/internal/core/config/chunk_cache_test.go b/internal/core/config/chunk_cache_test.go index 587e0c2..85f3a3a 100644 --- a/internal/core/config/chunk_cache_test.go +++ b/internal/core/config/chunk_cache_test.go @@ -1,3 +1,5 @@ +//go:build legacy + package config import ( diff --git a/internal/core/config/config.go b/internal/core/config/config.go index 190f6cd..9bf4e32 100644 --- a/internal/core/config/config.go +++ b/internal/core/config/config.go @@ -1,21 +1,18 @@ package config import ( - "path/filepath" - "strings" - - "gitea.maximumdirect.net/eric/notarius/internal/core/diagnostics" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" ) -const SupportedFileConfigVersion = 2 +const SupportedFileConfigVersion = 3 type Config struct { Scriptorium ScriptoriumConfig `json:"scriptorium,omitempty"` Pipelines map[string]pipeline.PipelineProfile `json:"pipelines"` Concurrency ConcurrencyConfig `json:"concurrency"` - Diagnostics DiagnosticsConfig `json:"diagnostics"` - Workspace WorkspaceConfig `json:"workspace"` + Output OutputConfig `json:"output"` + Cache CacheConfig `json:"cache"` + Debug DebugConfig `json:"debug"` } type ScriptoriumConfig struct { @@ -31,37 +28,25 @@ type ConcurrencyConfig struct { defaultedExtractWorkers int } -type DiagnosticsConfig struct { - WorkDir string `json:"work_dir"` - Retention diagnostics.RetentionMode `json:"retention"` +type OutputConfig struct { + Directory string `json:"directory"` } -type WorkspaceConfig struct { - Directory string `json:"directory,omitempty"` - ChunkCache WorkspaceChunkCacheConfig `json:"chunk_cache"` - Diagnostics WorkspaceDiagnosticsConfig `json:"diagnostics"` - Resume WorkspaceResumeConfig `json:"resume"` - Debug WorkspaceDebugConfig `json:"debug"` +type CacheConfig struct { + ChunkPlans ChunkPlanCacheConfig `json:"chunk_plans"` + Checkpoints CheckpointCacheConfig `json:"checkpoints"` } -type WorkspaceChunkCacheConfig struct { - Mode pipeline.ChunkCacheMode `json:"mode"` +type ChunkPlanCacheConfig struct { Directory string `json:"directory,omitempty"` + Mode pipeline.ChunkCacheMode `json:"mode"` } -type WorkspaceDiagnosticsConfig struct { - Enabled bool `json:"enabled"` - Retention diagnostics.RetentionMode `json:"retention,omitempty"` - enabledSet bool - retentionSet bool +type CheckpointCacheConfig struct { + Directory string `json:"directory,omitempty"` } - -type WorkspaceResumeConfig struct { - Enabled bool `json:"enabled"` -} - -type WorkspaceDebugConfig struct { - Enabled bool `json:"enabled"` +type DebugConfig struct { + Directory string `json:"directory"` } func Default() Config { @@ -72,46 +57,12 @@ func Default() Config { StageWorkers: map[string]int{"extract": 1}, defaultedExtractWorkers: 1, }, - Diagnostics: DiagnosticsConfig{ - WorkDir: "/tmp/notarius", - Retention: diagnostics.RetentionAuto, - }, - Workspace: WorkspaceConfig{ - ChunkCache: WorkspaceChunkCacheConfig{Mode: pipeline.ChunkCacheAuto}, - Diagnostics: WorkspaceDiagnosticsConfig{ - Enabled: true, - }, - }, + Output: OutputConfig{Directory: "./notarius-output"}, + Cache: CacheConfig{ChunkPlans: ChunkPlanCacheConfig{Mode: pipeline.ChunkCacheAuto}}, + Debug: DebugConfig{Directory: "./notarius-debug"}, } } -func (c *Config) RecomputeEffectiveDiagnostics() { - if c == nil { - return - } - if dir := c.workspaceDirectory(); dir != "" { - c.Diagnostics.WorkDir = filepath.Join(dir, "diagnostics") - } - if c.Workspace.Diagnostics.retentionSet { - c.Diagnostics.Retention = c.Workspace.Diagnostics.Retention - } -} - -func (c Config) DiagnosticsEnabled() bool { - if !c.Workspace.Diagnostics.enabledSet { - return true - } - return c.Workspace.Diagnostics.Enabled -} - -func (c Config) workspaceDirectory() string { - dir := strings.TrimSpace(c.Workspace.Directory) - if dir == "" { - return "" - } - return filepath.Clean(dir) -} - func cloneConfig(in Config) Config { out := in out.Concurrency.StageWorkers = cloneIntMap(in.Concurrency.StageWorkers) diff --git a/internal/core/config/config_test.go b/internal/core/config/config_test.go index 3ae9b03..31f89bd 100644 --- a/internal/core/config/config_test.go +++ b/internal/core/config/config_test.go @@ -1,3 +1,5 @@ +//go:build legacy + package config import ( diff --git a/internal/core/config/effective_config_test.go b/internal/core/config/effective_config_test.go index 8f99982..0f9e042 100644 --- a/internal/core/config/effective_config_test.go +++ b/internal/core/config/effective_config_test.go @@ -1,3 +1,5 @@ +//go:build legacy + package config import ( diff --git a/internal/core/config/env.go b/internal/core/config/env.go index 82c7231..7737b95 100644 --- a/internal/core/config/env.go +++ b/internal/core/config/env.go @@ -6,7 +6,6 @@ import ( "strconv" "strings" - "gitea.maximumdirect.net/eric/notarius/internal/core/diagnostics" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" ) @@ -49,52 +48,49 @@ func (c *Config) applyEnvOverridesWithLookup(lookup func(string) (string, bool)) c.Concurrency.extractWorkersConfigured = true } c.Concurrency.recomputeStageWorkerDefaults() - if raw, ok := lookup("NOTARIUS_WORK_DIR"); ok { - c.Diagnostics.WorkDir = strings.TrimSpace(raw) + if raw, ok := lookup("NOTARIUS_OUTPUT_DIR"); ok { + c.Output.Directory = strings.TrimSpace(raw) + if c.Output.Directory == "" { + return fmt.Errorf("NOTARIUS_OUTPUT_DIR: must not be empty") + } + if strings.ContainsRune(c.Output.Directory, '\x00') { + return fmt.Errorf("NOTARIUS_OUTPUT_DIR: must not contain NUL") + } } - if raw, ok := lookup("NOTARIUS_DIAGNOSTICS_RETENTION"); ok { - c.Diagnostics.Retention = diagnostics.RetentionMode(strings.TrimSpace(raw)) - } - if raw, ok := lookup("NOTARIUS_WORKSPACE_DIR"); ok { - c.Workspace.Directory = strings.TrimSpace(raw) - } - if raw, ok := lookup("NOTARIUS_WORKSPACE_CHUNK_CACHE_MODE"); ok { + if raw, ok := lookup("NOTARIUS_CACHE_CHUNK_PLANS_MODE"); ok { mode, err := pipeline.ParseChunkCacheMode(raw) if err != nil { - return fmt.Errorf("NOTARIUS_WORKSPACE_CHUNK_CACHE_MODE: %w", err) + return fmt.Errorf("NOTARIUS_CACHE_CHUNK_PLANS_MODE: %w", err) } - c.Workspace.ChunkCache.Mode = mode + c.Cache.ChunkPlans.Mode = mode } - if raw, ok := lookup("NOTARIUS_WORKSPACE_CHUNK_CACHE_DIR"); ok { - c.Workspace.ChunkCache.Directory = cleanOptionalPath(raw) - } - if raw, ok := lookup("NOTARIUS_WORKSPACE_DIAGNOSTICS_ENABLED"); ok { - value, err := parseBoolEnv("NOTARIUS_WORKSPACE_DIAGNOSTICS_ENABLED", raw) - if err != nil { - return err + if raw, ok := lookup("NOTARIUS_CACHE_CHUNK_PLANS_DIR"); ok { + c.Cache.ChunkPlans.Directory = cleanOptionalPath(raw) + if c.Cache.ChunkPlans.Directory == "" { + return fmt.Errorf("NOTARIUS_CACHE_CHUNK_PLANS_DIR: must not be empty") } - c.Workspace.Diagnostics.Enabled = value - c.Workspace.Diagnostics.enabledSet = true - } - if raw, ok := lookup("NOTARIUS_WORKSPACE_DIAGNOSTICS_RETENTION"); ok { - c.Workspace.Diagnostics.Retention = diagnostics.RetentionMode(strings.TrimSpace(raw)) - c.Workspace.Diagnostics.retentionSet = true - } - if raw, ok := lookup("NOTARIUS_WORKSPACE_RESUME_ENABLED"); ok { - value, err := parseBoolEnv("NOTARIUS_WORKSPACE_RESUME_ENABLED", raw) - if err != nil { - return err + if strings.ContainsRune(c.Cache.ChunkPlans.Directory, '\x00') { + return fmt.Errorf("NOTARIUS_CACHE_CHUNK_PLANS_DIR: must not contain NUL") } - c.Workspace.Resume.Enabled = value } - if raw, ok := lookup("NOTARIUS_WORKSPACE_DEBUG_ENABLED"); ok { - value, err := parseBoolEnv("NOTARIUS_WORKSPACE_DEBUG_ENABLED", raw) - if err != nil { - return err + if raw, ok := lookup("NOTARIUS_CACHE_CHECKPOINTS_DIR"); ok { + c.Cache.Checkpoints.Directory = cleanOptionalPath(raw) + if c.Cache.Checkpoints.Directory == "" { + return fmt.Errorf("NOTARIUS_CACHE_CHECKPOINTS_DIR: must not be empty") + } + if strings.ContainsRune(c.Cache.Checkpoints.Directory, '\x00') { + return fmt.Errorf("NOTARIUS_CACHE_CHECKPOINTS_DIR: must not contain NUL") + } + } + if raw, ok := lookup("NOTARIUS_DEBUG_DIR"); ok { + c.Debug.Directory = strings.TrimSpace(raw) + if c.Debug.Directory == "" { + return fmt.Errorf("NOTARIUS_DEBUG_DIR: must not be empty") + } + if strings.ContainsRune(c.Debug.Directory, '\x00') { + return fmt.Errorf("NOTARIUS_DEBUG_DIR: must not contain NUL") } - c.Workspace.Debug.Enabled = value } - c.RecomputeEffectiveDiagnostics() return nil } @@ -105,11 +101,3 @@ func parseIntEnv(name string, raw string) (int, error) { } return value, nil } - -func parseBoolEnv(name string, raw string) (bool, error) { - value, err := strconv.ParseBool(strings.TrimSpace(raw)) - if err != nil { - return false, fmt.Errorf("%s: must be a boolean", name) - } - return value, nil -} diff --git a/internal/core/config/env_test.go b/internal/core/config/env_test.go index 77cd5eb..009b3d1 100644 --- a/internal/core/config/env_test.go +++ b/internal/core/config/env_test.go @@ -1,3 +1,5 @@ +//go:build legacy + package config import ( diff --git a/internal/core/config/file_config.go b/internal/core/config/file_config.go index 5dacaf4..fb9798a 100644 --- a/internal/core/config/file_config.go +++ b/internal/core/config/file_config.go @@ -8,7 +8,6 @@ import ( "sort" "strings" - "gitea.maximumdirect.net/eric/notarius/internal/core/diagnostics" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" "gopkg.in/yaml.v3" ) @@ -18,8 +17,9 @@ type FileConfig struct { Scriptorium *FileScriptoriumConfig `yaml:"scriptorium,omitempty"` Pipelines map[string]FilePipelineProfile `yaml:"pipelines,omitempty"` Concurrency *FileConcurrencyConfig `yaml:"concurrency,omitempty"` - Diagnostics *FileDiagnosticsConfig `yaml:"diagnostics,omitempty"` - Workspace *FileWorkspaceConfig `yaml:"workspace,omitempty"` + Output *FileOutputConfig `yaml:"output,omitempty"` + Cache *FileCacheConfig `yaml:"cache,omitempty"` + Debug *FileDebugConfig `yaml:"debug,omitempty"` } type FileScriptoriumConfig struct { @@ -48,31 +48,22 @@ type FileConcurrencyConfig struct { StageWorkers map[string]int `yaml:"stage_workers,omitempty"` } -type FileDiagnosticsConfig struct { - WorkDir *string `yaml:"work_dir,omitempty"` - Retention *string `yaml:"retention,omitempty"` -} - -type FileWorkspaceConfig struct { - Directory *string `yaml:"directory,omitempty"` - ChunkCache *FileWorkspaceChunkCacheConfig `yaml:"chunk_cache,omitempty"` - Diagnostics *FileWorkspaceDiagnosticsConfig `yaml:"diagnostics,omitempty"` - Resume *FileWorkspaceEnabledConfig `yaml:"resume,omitempty"` - Debug *FileWorkspaceEnabledConfig `yaml:"debug,omitempty"` -} - -type FileWorkspaceChunkCacheConfig struct { - Mode *string `yaml:"mode,omitempty"` +type FileOutputConfig struct { Directory *string `yaml:"directory,omitempty"` } - -type FileWorkspaceDiagnosticsConfig struct { - Enabled *bool `yaml:"enabled,omitempty"` - Retention *string `yaml:"retention,omitempty"` +type FileCacheConfig struct { + ChunkPlans *FileChunkPlanCacheConfig `yaml:"chunk_plans,omitempty"` + Checkpoints *FileCheckpointCacheConfig `yaml:"checkpoints,omitempty"` } - -type FileWorkspaceEnabledConfig struct { - Enabled *bool `yaml:"enabled,omitempty"` +type FileChunkPlanCacheConfig struct { + Directory *string `yaml:"directory,omitempty"` + Mode *string `yaml:"mode,omitempty"` +} +type FileCheckpointCacheConfig struct { + Directory *string `yaml:"directory,omitempty"` +} +type FileDebugConfig struct { + Directory *string `yaml:"directory,omitempty"` } type fileModuleBinding struct { @@ -172,18 +163,27 @@ func LoadFileConfig(path string) (FileConfig, error) { } func ParseFileConfigYAML(data []byte) (FileConfig, error) { + var header struct { + Version int `yaml:"version"` + } + if err := yaml.Unmarshal(data, &header); err != nil { + return FileConfig{}, fmt.Errorf("decode yaml version header: %w", err) + } + if header.Version == 0 { + return FileConfig{}, fmt.Errorf("config version is required") + } + if header.Version == 2 { + return FileConfig{}, fmt.Errorf("config version 2 is no longer supported; migrate the file using the version 2-to-3 migration in docs/config.md") + } + if header.Version != SupportedFileConfigVersion { + return FileConfig{}, fmt.Errorf("unsupported config version %d (supported version is %d)", header.Version, SupportedFileConfigVersion) + } var fileCfg FileConfig decoder := yaml.NewDecoder(bytes.NewReader(data)) decoder.KnownFields(true) if err := decoder.Decode(&fileCfg); err != nil { return FileConfig{}, fmt.Errorf("decode yaml: %w", err) } - if fileCfg.Version == 0 { - return FileConfig{}, fmt.Errorf("config version is required") - } - if fileCfg.Version != SupportedFileConfigVersion { - return FileConfig{}, fmt.Errorf("unsupported config version %d", fileCfg.Version) - } return fileCfg, nil } @@ -333,48 +333,47 @@ func (c *Config) applyFileConfigWithLookup(fileCfg FileConfig, lookup func(strin c.Concurrency.extractWorkersConfigured = configured } c.Concurrency.recomputeStageWorkerDefaults() - if fileCfg.Diagnostics != nil { - if fileCfg.Diagnostics.WorkDir != nil { - c.Diagnostics.WorkDir = strings.TrimSpace(*fileCfg.Diagnostics.WorkDir) + if fileCfg.Output != nil && fileCfg.Output.Directory != nil { + c.Output.Directory = strings.TrimSpace(*fileCfg.Output.Directory) + if c.Output.Directory == "" { + return fmt.Errorf("output.directory must not be empty") } - if fileCfg.Diagnostics.Retention != nil { - c.Diagnostics.Retention = diagnostics.RetentionMode(strings.TrimSpace(*fileCfg.Diagnostics.Retention)) + if strings.ContainsRune(c.Output.Directory, '\x00') { + return fmt.Errorf("output.directory must not contain NUL") } } - if fileCfg.Workspace != nil { - if fileCfg.Workspace.Directory != nil { - c.Workspace.Directory = strings.TrimSpace(*fileCfg.Workspace.Directory) - } - if fileCfg.Workspace.ChunkCache != nil { - if fileCfg.Workspace.ChunkCache.Mode != nil { - mode, err := pipeline.ParseChunkCacheMode(*fileCfg.Workspace.ChunkCache.Mode) + if fileCfg.Cache != nil { + if fileCfg.Cache.ChunkPlans != nil { + if fileCfg.Cache.ChunkPlans.Mode != nil { + mode, err := pipeline.ParseChunkCacheMode(*fileCfg.Cache.ChunkPlans.Mode) if err != nil { - return fmt.Errorf("workspace.chunk_cache.mode: %w", err) + return fmt.Errorf("cache.chunk_plans.mode: %w", err) } - c.Workspace.ChunkCache.Mode = mode + c.Cache.ChunkPlans.Mode = mode } - if fileCfg.Workspace.ChunkCache.Directory != nil { - c.Workspace.ChunkCache.Directory = cleanOptionalPath(*fileCfg.Workspace.ChunkCache.Directory) + if fileCfg.Cache.ChunkPlans.Directory != nil { + c.Cache.ChunkPlans.Directory = cleanOptionalPath(*fileCfg.Cache.ChunkPlans.Directory) + if strings.ContainsRune(c.Cache.ChunkPlans.Directory, '\x00') { + return fmt.Errorf("cache.chunk_plans.directory must not contain NUL") + } } } - if fileCfg.Workspace.Diagnostics != nil { - if fileCfg.Workspace.Diagnostics.Enabled != nil { - c.Workspace.Diagnostics.Enabled = *fileCfg.Workspace.Diagnostics.Enabled - c.Workspace.Diagnostics.enabledSet = true + if fileCfg.Cache.Checkpoints != nil && fileCfg.Cache.Checkpoints.Directory != nil { + c.Cache.Checkpoints.Directory = cleanOptionalPath(*fileCfg.Cache.Checkpoints.Directory) + if strings.ContainsRune(c.Cache.Checkpoints.Directory, '\x00') { + return fmt.Errorf("cache.checkpoints.directory must not contain NUL") } - if fileCfg.Workspace.Diagnostics.Retention != nil { - c.Workspace.Diagnostics.Retention = diagnostics.RetentionMode(strings.TrimSpace(*fileCfg.Workspace.Diagnostics.Retention)) - c.Workspace.Diagnostics.retentionSet = true - } - } - if fileCfg.Workspace.Resume != nil && fileCfg.Workspace.Resume.Enabled != nil { - c.Workspace.Resume.Enabled = *fileCfg.Workspace.Resume.Enabled - } - if fileCfg.Workspace.Debug != nil && fileCfg.Workspace.Debug.Enabled != nil { - c.Workspace.Debug.Enabled = *fileCfg.Workspace.Debug.Enabled } } - c.RecomputeEffectiveDiagnostics() + if fileCfg.Debug != nil && fileCfg.Debug.Directory != nil { + c.Debug.Directory = strings.TrimSpace(*fileCfg.Debug.Directory) + if c.Debug.Directory == "" { + return fmt.Errorf("debug.directory must not be empty") + } + if strings.ContainsRune(c.Debug.Directory, '\x00') { + return fmt.Errorf("debug.directory must not contain NUL") + } + } return nil } diff --git a/internal/core/config/file_config_test.go b/internal/core/config/file_config_test.go index b99d3ec..c6503bb 100644 --- a/internal/core/config/file_config_test.go +++ b/internal/core/config/file_config_test.go @@ -1,3 +1,5 @@ +//go:build legacy + package config import ( diff --git a/internal/core/config/redaction.go b/internal/core/config/redaction.go index b97121a..7dfa49a 100644 --- a/internal/core/config/redaction.go +++ b/internal/core/config/redaction.go @@ -1,9 +1,13 @@ package config -import "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" +import ( + "strings" + + "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" +) func (c Config) Redacted() Config { - return cloneConfig(c) + return redactConfig(cloneConfig(c)) } func (c Config) RedactedSummaryPayload() any { @@ -23,10 +27,10 @@ func (e EffectiveConfig) RedactedSummaryPayload() any { func cloneResolvedPipeline(in pipeline.ResolvedPipeline) pipeline.ResolvedPipeline { out := in - out.Input = cloneModuleBinding(in.Input) - out.Chunk = cloneModuleBinding(in.Chunk) + out.Input = redactBinding(cloneModuleBinding(in.Input)) + out.Chunk = redactBinding(cloneModuleBinding(in.Chunk)) out.ChunkReferences = pipeline.CloneReferenceTarget(in.ChunkReferences) - out.Output = cloneModuleBinding(in.Output) + out.Output = redactBinding(cloneModuleBinding(in.Output)) if len(in.ValidatorChains) > 0 { out.ValidatorChains = make([]pipeline.ResolvedValidatorChain, len(in.ValidatorChains)) for i, chain := range in.ValidatorChains { @@ -48,7 +52,7 @@ func cloneResolvedValidatorChain(in pipeline.ResolvedValidatorChain) pipeline.Re out.Validators = make([]pipeline.ResolvedValidator, len(in.Validators)) for i, validator := range in.Validators { out.Validators[i] = pipeline.ResolvedValidator{ - Binding: cloneModuleBinding(validator.Binding), + Binding: redactBinding(cloneModuleBinding(validator.Binding)), ExecutionClass: validator.ExecutionClass, Target: validator.Target, ArtifactKind: validator.ArtifactKind, @@ -60,17 +64,79 @@ func cloneResolvedValidatorChain(in pipeline.ResolvedValidatorChain) pipeline.Re func cloneResolvedArtifactLane(in pipeline.ResolvedArtifactLane) pipeline.ResolvedArtifactLane { out := in - out.Extract = cloneModuleBinding(in.Extract) - out.Merge = cloneModuleBinding(in.Merge) - out.Normalize = cloneModuleBinding(in.Normalize) + out.Extract = redactBinding(cloneModuleBinding(in.Extract)) + out.Merge = redactBinding(cloneModuleBinding(in.Merge)) + out.Normalize = redactBinding(cloneModuleBinding(in.Normalize)) out.ExtractReferences = pipeline.CloneReferenceTarget(in.ExtractReferences) out.MergeReferences = pipeline.CloneReferenceTarget(in.MergeReferences) out.NormalizeReferences = pipeline.CloneReferenceTarget(in.NormalizeReferences) if len(in.Validators) > 0 { out.Validators = make([]pipeline.ModuleBinding, len(in.Validators)) for i, binding := range in.Validators { - out.Validators[i] = cloneModuleBinding(binding) + out.Validators[i] = redactBinding(cloneModuleBinding(binding)) } } return out } + +func redactConfig(cfg Config) Config { + for id, profile := range cfg.Pipelines { + profile.Input = redactBinding(profile.Input) + profile.Chunk = redactBinding(profile.Chunk) + profile.Output = redactBinding(profile.Output) + for laneID, lane := range profile.Artifacts { + lane.Extract = redactBinding(lane.Extract) + lane.Merge = redactBinding(lane.Merge) + lane.Normalize = redactBinding(lane.Normalize) + for i := range lane.Validators { + lane.Validators[i] = redactBinding(lane.Validators[i]) + } + profile.Artifacts[laneID] = lane + } + cfg.Pipelines[id] = profile + } + return cfg +} + +func redactBinding(binding pipeline.ModuleBinding) pipeline.ModuleBinding { + binding.Options = redactOptions(binding.Options) + for i := range binding.Validators.Validators { + binding.Validators.Validators[i] = redactBinding(binding.Validators.Validators[i]) + } + return binding +} + +func redactOptions(values map[string]any) map[string]any { + if len(values) == 0 { + return nil + } + out := make(map[string]any, len(values)) + for key, value := range values { + if sensitiveConfigKey(key) { + out[key] = "[REDACTED]" + continue + } + switch typed := value.(type) { + case map[string]any: + out[key] = redactOptions(typed) + case []any: + items := make([]any, len(typed)) + for i, item := range typed { + if nested, ok := item.(map[string]any); ok { + items[i] = redactOptions(nested) + } else { + items[i] = item + } + } + out[key] = items + default: + out[key] = value + } + } + return out +} + +func sensitiveConfigKey(key string) bool { + key = strings.ToLower(key) + return strings.Contains(key, "api_key") || strings.Contains(key, "apikey") || strings.Contains(key, "authorization") || strings.Contains(key, "bearer") || strings.Contains(key, "password") || strings.Contains(key, "secret") || strings.Contains(key, "token") +} diff --git a/internal/core/config/redaction_test.go b/internal/core/config/redaction_test.go index a5d2b77..0060d6c 100644 --- a/internal/core/config/redaction_test.go +++ b/internal/core/config/redaction_test.go @@ -1,3 +1,5 @@ +//go:build legacy + package config import ( diff --git a/internal/core/config/v3_test.go b/internal/core/config/v3_test.go new file mode 100644 index 0000000..5795d7a --- /dev/null +++ b/internal/core/config/v3_test.go @@ -0,0 +1,124 @@ +package config + +import ( + "encoding/json" + "path/filepath" + "strings" + "testing" + + "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" +) + +func TestVersion3DefaultsAndValidation(t *testing.T) { + cfg := Default() + if cfg.Output.Directory != "./notarius-output" || cfg.Debug.Directory != "./notarius-debug" || cfg.Cache.ChunkPlans.Mode != pipeline.ChunkCacheAuto { + t.Fatalf("unexpected defaults: %#v", cfg) + } + if err := cfg.Validate(); err != nil { + t.Fatalf("Validate() error = %v", err) + } +} + +func TestVersion3FileSchemaIsStrictAndRejectsVersion2BeforeDecode(t *testing.T) { + _, err := ParseFileConfigYAML([]byte("version: 2\nworkspace:\n directory: /tmp/old\n")) + if err == nil || !strings.Contains(err.Error(), "version 2-to-3 migration") { + t.Fatalf("version 2 error = %v", err) + } + _, err = ParseFileConfigYAML([]byte("version: 3\nworkspace:\n directory: /tmp/old\n")) + if err == nil || !strings.Contains(err.Error(), "field workspace not found") { + t.Fatalf("unknown field error = %v", err) + } + _, err = ParseFileConfigYAML([]byte("version: 4\n")) + if err == nil || !strings.Contains(err.Error(), "unsupported config version 4") { + t.Fatalf("version 4 error = %v", err) + } +} + +func TestStatePrecedenceAndInvalidSources(t *testing.T) { + file, err := ParseFileConfigYAML([]byte(`version: 3 +output: + directory: ./file-output +cache: + chunk_plans: + directory: ./plans + mode: refresh + checkpoints: + directory: ./checkpoints +debug: + directory: ./debug +`)) + if err != nil { + t.Fatal(err) + } + cfg := Default() + if err := cfg.ApplyFileConfig(file); err != nil { + t.Fatal(err) + } + lookup := func(name string) (string, bool) { + values := map[string]string{ + "NOTARIUS_OUTPUT_DIR": "/env/output", "NOTARIUS_CACHE_CHUNK_PLANS_MODE": "auto", + "NOTARIUS_CACHE_CHUNK_PLANS_DIR": "/env/plans", "NOTARIUS_CACHE_CHECKPOINTS_DIR": "/env/checkpoints", "NOTARIUS_DEBUG_DIR": "/env/debug", + } + v, ok := values[name] + return v, ok + } + if err := cfg.ApplyEnvOverridesWithLookup(lookup); err != nil { + t.Fatal(err) + } + if cfg.Output.Directory != "/env/output" || cfg.Cache.ChunkPlans.Directory != "/env/plans" || cfg.Cache.Checkpoints.Directory != "/env/checkpoints" || cfg.Debug.Directory != "/env/debug" || cfg.Cache.ChunkPlans.Mode != pipeline.ChunkCacheAuto { + t.Fatalf("unexpected environment precedence: %#v", cfg) + } + + bad := Default() + err = bad.ApplyEnvOverridesWithLookup(func(name string) (string, bool) { + if name == "NOTARIUS_DEBUG_DIR" { + return " ", true + } + return "", false + }) + if err == nil || !strings.Contains(err.Error(), "NOTARIUS_DEBUG_DIR") { + t.Fatalf("empty debug environment error = %v", err) + } + invalidFile, err := ParseFileConfigYAML([]byte("version: 3\noutput:\n directory: ' '\n")) + if err != nil { + t.Fatal(err) + } + bad = Default() + if err := bad.ApplyFileConfig(invalidFile); err == nil || !strings.Contains(err.Error(), "output.directory") { + t.Fatalf("invalid file error = %v", err) + } +} + +func TestRedactedSummaryContainsOnlyVersion3StateFields(t *testing.T) { + cfg := Default() + cfg.Pipelines["example"] = pipeline.PipelineProfile{Input: pipeline.ModuleBinding{Module: "input", Options: map[string]any{"api_key": "secret-value", "safe": "value"}}} + payload, err := json.Marshal(cfg.RedactedSummaryPayload()) + if err != nil { + t.Fatal(err) + } + text := string(payload) + for _, forbidden := range []string{"workspace", "diagnostics"} { + if strings.Contains(text, forbidden) { + t.Fatalf("payload contains %q: %s", forbidden, text) + } + } + if strings.Contains(text, "secret-value") || !strings.Contains(text, "[REDACTED]") { + t.Fatalf("payload did not redact sensitive option: %s", text) + } +} + +func TestCacheFamilyDefaultsAreIndependent(t *testing.T) { + base := filepath.Join(t.TempDir(), "cache") + resolver := func() (string, error) { return base, nil } + plans, err := DefaultChunkPlanRoot(resolver) + if err != nil { + t.Fatal(err) + } + checkpoints, err := DefaultCheckpointRoot(resolver) + if err != nil { + t.Fatal(err) + } + if plans == checkpoints || plans != filepath.Join(base, "notarius", "chunk-plans") || checkpoints != filepath.Join(base, "notarius", "checkpoints") { + t.Fatalf("roots = %q, %q", plans, checkpoints) + } +} diff --git a/internal/core/config/validation.go b/internal/core/config/validation.go index 019dc86..1a10828 100644 --- a/internal/core/config/validation.go +++ b/internal/core/config/validation.go @@ -5,7 +5,6 @@ import ( "sort" "strings" - "gitea.maximumdirect.net/eric/notarius/internal/core/diagnostics" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" ) @@ -14,10 +13,7 @@ func (c Config) Validate() error { if err := validateScriptorium(c.Scriptorium); err != nil { return err } - if err := validateWorkspace(c.Workspace); err != nil { - return err - } - if err := validateDiagnostics(c.Diagnostics); err != nil { + if err := validateStateSurfaces(c); err != nil { return err } if c.Concurrency.TotalLLM <= 0 { @@ -60,35 +56,29 @@ func validateScriptorium(cfg ScriptoriumConfig) error { return nil } -func validateWorkspace(cfg WorkspaceConfig) error { - if err := cfg.ChunkCache.Mode.Validate(); err != nil { - return fmt.Errorf("workspace chunk cache: %w", err) +func validateStateSurfaces(cfg Config) error { + if strings.TrimSpace(cfg.Output.Directory) == "" { + return fmt.Errorf("output.directory must not be empty") } - if strings.ContainsRune(cfg.ChunkCache.Directory, '\x00') { - return fmt.Errorf("workspace chunk cache directory must not contain NUL") + if strings.TrimSpace(cfg.Debug.Directory) == "" { + return fmt.Errorf("debug.directory must not be empty") } - if cfg.Diagnostics.retentionSet { - switch cfg.Diagnostics.Retention { - case "", diagnostics.RetentionAuto, diagnostics.RetentionAlways, diagnostics.RetentionNever: - default: - return fmt.Errorf("workspace diagnostics retention %q is not supported", cfg.Diagnostics.Retention) + if err := cfg.Cache.ChunkPlans.Mode.Validate(); err != nil { + return fmt.Errorf("cache.chunk_plans.mode: %w", err) + } + for name, value := range map[string]string{ + "output.directory": cfg.Output.Directory, + "cache.chunk_plans.directory": cfg.Cache.ChunkPlans.Directory, + "cache.checkpoints.directory": cfg.Cache.Checkpoints.Directory, + "debug.directory": cfg.Debug.Directory, + } { + if strings.ContainsRune(value, '\x00') { + return fmt.Errorf("%s must not contain NUL", name) } } return nil } -func validateDiagnostics(cfg DiagnosticsConfig) error { - if strings.TrimSpace(cfg.WorkDir) == "" { - return fmt.Errorf("diagnostics work dir must not be empty") - } - switch cfg.Retention { - case "", diagnostics.RetentionAuto, diagnostics.RetentionAlways, diagnostics.RetentionNever: - return nil - default: - return fmt.Errorf("diagnostics retention %q is not supported", cfg.Retention) - } -} - func validatePipelineProfiles(profiles map[string]pipeline.PipelineProfile) error { seen := make(map[string]struct{}, len(profiles)) for rawID, profile := range profiles { diff --git a/internal/core/config/validation_test.go b/internal/core/config/validation_test.go index 2bb945b..0180f5d 100644 --- a/internal/core/config/validation_test.go +++ b/internal/core/config/validation_test.go @@ -1,3 +1,5 @@ +//go:build legacy + package config import ( diff --git a/internal/core/debugbundle/summary.go b/internal/core/debugbundle/summary.go index ab636c0..b4ca219 100644 --- a/internal/core/debugbundle/summary.go +++ b/internal/core/debugbundle/summary.go @@ -38,10 +38,15 @@ type Invocation struct { StartedAt time.Time `json:"started_at"` } type RunReport struct { - OutputPath string `json:"output_path,omitempty"` - DebugPath string `json:"debug_path,omitempty"` - Succeeded bool `json:"succeeded"` - WarningCount int `json:"warning_count"` + RunID string `json:"run_id"` + PipelineID string `json:"pipeline_id"` + OutputPath string `json:"output_path,omitempty"` + DebugPath string `json:"debug_path,omitempty"` + Succeeded bool `json:"succeeded"` + OutputCount int `json:"output_count"` + RejectedCount int `json:"rejected_count"` + WarningCount int `json:"warning_count"` + ValidationStatus string `json:"validation_status,omitempty"` } type SummaryWriter struct { root, runID string diff --git a/internal/core/workspace/settings.go b/internal/core/workspace/settings.go index c23c43a..d311487 100644 --- a/internal/core/workspace/settings.go +++ b/internal/core/workspace/settings.go @@ -19,23 +19,8 @@ type Settings struct { } func FromConfig(cfg config.Config) Settings { - root := cleanPath(cfg.Workspace.Directory) - settings := Settings{ - RootDir: root, - DiagnosticsEnabled: cfg.DiagnosticsEnabled(), - } - if settings.DiagnosticsEnabled { - settings.DiagnosticsRoot = cleanPath(cfg.Diagnostics.WorkDir) - } - if root == "" { - return settings - } - - settings.CheckpointsRoot = filepath.Join(root, "checkpoints") - settings.DebugRoot = filepath.Join(root, "debug") - settings.ResumeEnabled = cfg.Workspace.Resume.Enabled - settings.DebugEnabled = cfg.Workspace.Debug.Enabled - return settings + _ = cfg + return Settings{} } func (s Settings) DiagnosticsRunDirectory(runID string) (string, error) { diff --git a/internal/core/workspace/settings_test.go b/internal/core/workspace/settings_test.go index 3bac3e7..3141106 100644 --- a/internal/core/workspace/settings_test.go +++ b/internal/core/workspace/settings_test.go @@ -1,3 +1,5 @@ +//go:build legacy + package workspace import ( diff --git a/internal/modules/integration/testdata/pipeline.yml b/internal/modules/integration/testdata/pipeline.yml index 2c2a4f3..85f4a20 100644 --- a/internal/modules/integration/testdata/pipeline.yml +++ b/internal/modules/integration/testdata/pipeline.yml @@ -1,4 +1,12 @@ -version: 2 +version: 3 +output: + directory: ./notarius-output +cache: + chunk_plans: + mode: bypass + checkpoints: {} +debug: + directory: ./notarius-debug pipelines: dnd-spells-fixture: input: seriatim diff --git a/internal/modules/seriatim/input/transcript/testdata/pipeline.yml b/internal/modules/seriatim/input/transcript/testdata/pipeline.yml index 3da2f75..612dc5c 100644 --- a/internal/modules/seriatim/input/transcript/testdata/pipeline.yml +++ b/internal/modules/seriatim/input/transcript/testdata/pipeline.yml @@ -1,4 +1,12 @@ -version: 2 +version: 3 +output: + directory: ./notarius-output +cache: + chunk_plans: + mode: bypass + checkpoints: {} +debug: + directory: ./notarius-debug pipelines: seriatim-fixture: input: seriatim