diff --git a/docs/cli.md b/docs/cli.md index a4d95c9..906ea00 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -44,7 +44,7 @@ Flags: - `--output-dir path`: output root. The run writes to `//`. Defaults to `./notarius-output`. - `--diagnostics-dir path`: diagnostics work directory override for this - invocation. + invocation. It does not change the workspace directory. - `--llm-profile id`: override every effective LLM-capable pipeline module binding to use one Scriptorium profile ID. Validator-specific profiles are not overridden. Configured LLM-backed validators with explicit profiles are diff --git a/docs/config.md b/docs/config.md index 4efacf2..cb6ac35 100644 --- a/docs/config.md +++ b/docs/config.md @@ -42,6 +42,7 @@ The maintained fixture is [examples/dnd-spells.config.yml](../examples/dnd-spell - `scriptorium`: optional Scriptorium profile source settings. - `pipelines`: optional map of pipeline IDs to pipeline definitions. - `concurrency`: optional global concurrency settings. +- `workspace`: optional workspace settings for Notarius-owned local state. - `diagnostics`: optional diagnostics settings. Unknown YAML fields are rejected. The removed top-level `llm_profiles` field is @@ -57,6 +58,13 @@ concurrency: diagnostics: work_dir: /tmp/notarius retention: auto +workspace: + diagnostics: + enabled: true + resume: + enabled: false + debug: + enabled: false ``` No pipelines are built in. A run requires a configured pipeline. @@ -98,10 +106,20 @@ These environment variables are applied after the config file: - `NOTARIUS_CONFIG`: config discovery path. - `NOTARIUS_TOTAL_LLM_CONCURRENCY`: integer global LLM concurrency. -- `NOTARIUS_WORK_DIR`: diagnostics work directory. -- `NOTARIUS_DIAGNOSTICS_RETENTION`: diagnostics retention mode. +- `NOTARIUS_WORKSPACE_DIR`: workspace directory. +- `NOTARIUS_WORKSPACE_DIAGNOSTICS_ENABLED`: boolean diagnostics enablement. +- `NOTARIUS_WORKSPACE_DIAGNOSTICS_RETENTION`: workspace diagnostics retention + mode. +- `NOTARIUS_WORKSPACE_RESUME_ENABLED`: boolean resume checkpointing + enablement. +- `NOTARIUS_WORKSPACE_DEBUG_ENABLED`: boolean debug artifact enablement. +- `NOTARIUS_WORK_DIR`: deprecated diagnostics work directory compatibility + override. +- `NOTARIUS_DIAGNOSTICS_RETENTION`: deprecated diagnostics retention + compatibility override. -Integer environment values must parse as base-10 integers. +Integer environment values must parse as base-10 integers. Boolean environment +values must parse as Go booleans such as `true`, `false`, `1`, or `0`. The removed `NOTARIUS_LLM_DEFAULT_*` variables are not read. Configure provider endpoint, model, and credential environment variable names through Scriptorium @@ -323,19 +341,48 @@ Both modules accept UTF-8 plain text, Markdown, YAML, or JSON reference files. The extractor uses references only as supporting disambiguation material; spell casts still must be present in the source transcript. +## Workspace + +`workspace` fields: + +- `directory`: optional workspace root for Notarius-owned local state. +- `diagnostics.enabled`: set to `false` to skip diagnostics run directories and + diagnostics artifact writes. Default: `true`. +- `diagnostics.retention`: `auto`, `always`, or `never`. +- `resume.enabled`: boolean resume checkpointing setting. Default: `false`. +- `debug.enabled`: boolean debug artifact setting. Default: `false`. + +The current run workflow uses workspace settings for diagnostics configuration. +Checkpoint and debug artifact writers are not part of the current workflow. + ## Diagnostics +Preferred workspace diagnostics fields: + +- `workspace.directory`: workspace root for Notarius-owned local state. +- `workspace.diagnostics.enabled`: set to `false` to skip creating diagnostics + run directories and diagnostics artifacts. Default: `true`. +- `workspace.diagnostics.retention`: `auto`, `always`, or `never`. + +When `workspace.directory` is set, diagnostics use +`/diagnostics` as their work directory. +`workspace.diagnostics.retention` overrides legacy diagnostics retention when +set. + `diagnostics` fields: -- `work_dir`: directory for per-run diagnostics. Default: `/tmp/notarius`. -- `retention`: `auto`, `always`, or `never`. Empty uses `auto`. +- `work_dir`: deprecated compatibility directory for per-run diagnostics. + Default: `/tmp/notarius`. +- `retention`: deprecated compatibility retention mode. `auto`, `always`, or + `never`. Empty uses `auto`. `auto` retains diagnostics for failed runs and successful runs with warnings. `always` retains diagnostics for every run. `never` removes diagnostics for successful runs without regard to warnings; failed runs are retained. -The `--diagnostics-dir` run flag overrides `diagnostics.work_dir` for that -invocation. +The `--diagnostics-dir` run flag overrides the effective diagnostics work +directory for that invocation. It affects diagnostics only and does not change +the workspace directory. ## Validation diff --git a/docs/internal/diagnostics.md b/docs/internal/diagnostics.md index 476b889..cfdc026 100644 --- a/docs/internal/diagnostics.md +++ b/docs/internal/diagnostics.md @@ -69,8 +69,13 @@ Retention is decided by `ShouldRetainRunDirectory`. ## CLI Failure Behavior -The CLI creates the diagnostics run directory after config loading and before -pipeline resolution. Failures before that point do not have diagnostics. +When diagnostics are enabled, the CLI creates the diagnostics run directory +after config loading and before pipeline resolution. Failures before that point +do not have diagnostics. + +When workspace diagnostics are explicitly disabled, the CLI does not create a +diagnostics run directory and skips diagnostics artifact writes. Failures are +still printed to stderr. After diagnostics creation, run failures call `WriteErrorLog` and apply retention with `RunSucceeded: false`, so the run directory remains available. diff --git a/docs/operations.md b/docs/operations.md index 1641ec2..4e76053 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -54,7 +54,15 @@ Diagnostics are written under: ``` The default diagnostics work directory is `/tmp/notarius`. It can be set with -`diagnostics.work_dir`, `NOTARIUS_WORK_DIR`, or `--diagnostics-dir`. +`workspace.directory`, `NOTARIUS_WORKSPACE_DIR`, legacy +`diagnostics.work_dir`, legacy `NOTARIUS_WORK_DIR`, or `--diagnostics-dir`. +When a workspace directory is set, diagnostics are written under +`/diagnostics//`. + +Set `workspace.diagnostics.enabled: false` or +`NOTARIUS_WORKSPACE_DIAGNOSTICS_ENABLED=false` to skip diagnostics directory +creation and diagnostics artifact writes. Concise failures are still printed to +stderr. Implemented diagnostics artifacts: @@ -77,8 +85,9 @@ by the current CLI run workflow. ## Retention -Diagnostics retention is configured with `diagnostics.retention`, -`NOTARIUS_DIAGNOSTICS_RETENTION`, or the default `auto`. +Diagnostics retention is configured with `workspace.diagnostics.retention`, +`NOTARIUS_WORKSPACE_DIAGNOSTICS_RETENTION`, legacy `diagnostics.retention`, +legacy `NOTARIUS_DIAGNOSTICS_RETENTION`, or the default `auto`. - `auto`: keep failed runs and successful runs with warnings; remove successful warning-free runs. diff --git a/internal/cli/run.go b/internal/cli/run.go index cd19a27..afb60de 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -156,10 +156,16 @@ func runPipelineCommand(args []string, stdout, stderr io.Writer, opts Options) i } startedAt := opts.Now().UTC() - runDir, err := diagnostics.NewRunDirectory(cfg.Diagnostics.WorkDir, cfg.Diagnostics.Retention) - if err != nil { - fmt.Fprintf(stderr, "notarius: %v\n", err) - return 1 + runID := fmt.Sprintf("run-%d", startedAt.UnixNano()) + var runDir *diagnostics.RunDirectory + if cfg.DiagnosticsEnabled() { + var err error + runDir, err = diagnostics.NewRunDirectory(cfg.Diagnostics.WorkDir, cfg.Diagnostics.Retention) + if err != nil { + fmt.Fprintf(stderr, "notarius: %v\n", err) + return 1 + } + runID = runDir.RunID() } invocation := diagnostics.InvocationMetadata{ Operation: "run", @@ -168,10 +174,10 @@ func runPipelineCommand(args []string, stdout, stderr io.Writer, opts Options) i ConfigPath: loadedConfigPath, ConfigSource: configSource(*configPath), OnlyLanes: append([]string(nil), only...), - RunID: runDir.RunID(), + RunID: runID, StartedAt: startedAt, } - if err := runDir.WriteInvocationMetadata(invocation); err != nil { + 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)) } @@ -211,16 +217,18 @@ func runPipelineCommand(args []string, stdout, stderr io.Writer, opts Options) i } effective.ResolvedPipeline = materialized invocation.PipelineDigest = effective.ResolvedPipeline.Digest - if err := runDir.WriteInvocationMetadata(invocation); err != nil { + 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 := runDir.WriteRedactedEffectiveConfig(effective); err != nil { + 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 := runDir.WriteResolvedPipeline(effective.ResolvedPipeline); err != nil { + 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 := runDir.WriteResolvedReferences(pipeline.ReferenceProvenance(effective.ResolvedPipeline)); err != nil { + if err := writeDiagnostics(runDir, func() error { + return runDir.WriteResolvedReferences(pipeline.ReferenceProvenance(effective.ResolvedPipeline)) + }); err != nil { return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, fmt.Errorf("write diagnostics resolved references: %w", err)) } @@ -250,45 +258,49 @@ func runPipelineCommand(args []string, stdout, stderr io.Writer, opts Options) i RawInput: rawInput, LLMClient: llmClient, SessionID: strings.TrimSpace(sessionID.value), - RunID: runDir.RunID(), + RunID: runID, StartedAt: startedAt, LLMProfiles: llmProfiles, Metadata: runMetadata(*outputDir, *diagnosticsDir), Warnings: referenceWarnings, }) if err != nil { - if output.Manifest.PipelineID != "" { + if output.Manifest.PipelineID != "" && runDir != nil { _ = runDir.WriteRunManifest(output.Manifest) } return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, fmt.Errorf("run pipeline %q: %w", pipelineID, err)) } - runOutputDir := filepath.Join(outputRoot(*outputDir), runDir.RunID()) - if err := runDir.WriteRunManifest(output.Manifest); err != nil { + 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 err := runDir.WriteWarnings(output.Warnings); err != nil { + 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 := 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, + 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)) } if err := writeOutputFiles(runOutputDir, output.OutputFiles); err != nil { return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, err) } - if err := runDir.ApplyRetention(diagnostics.RetentionDecisionInput{ - RetentionMode: cfg.Diagnostics.Retention, - RunSucceeded: true, - HasWarnings: len(output.Warnings) > 0, + if err := writeDiagnostics(runDir, func() error { + return runDir.ApplyRetention(diagnostics.RetentionDecisionInput{ + RetentionMode: cfg.Diagnostics.Retention, + RunSucceeded: true, + HasWarnings: len(output.Warnings) > 0, + }) }); err != nil { return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, fmt.Errorf("apply diagnostics retention: %w", err)) } @@ -327,6 +339,13 @@ func failPipelineCommand(stderr io.Writer, runDir *diagnostics.RunDirectory, ret return 1 } +func writeDiagnostics(runDir *diagnostics.RunDirectory, write func() error) error { + if runDir == nil { + return nil + } + return write() +} + func configSource(configPath string) string { if strings.TrimSpace(configPath) != "" { return "flag" diff --git a/internal/cli/run_test.go b/internal/cli/run_test.go index e5fda5c..c606d58 100644 --- a/internal/cli/run_test.go +++ b/internal/cli/run_test.go @@ -2148,6 +2148,29 @@ func TestRunPipelineWritesDiagnosticsArtifactsOnSuccess(t *testing.T) { } } +func TestRunPipelineSkipsDiagnosticsWhenWorkspaceDiagnosticsDisabled(t *testing.T) { + workspaceDir := filepath.Join(t.TempDir(), "workspace") + outputDir := t.TempDir() + configPath := writeTestConfig(t, mvpConfigYAMLWithWorkspaceDiagnosticsDisabled("dnd-session", workspaceDir)) + inputPath := writeSeriatimInput(t) + var stdout bytes.Buffer + var stderr bytes.Buffer + + code := RunWithOptions([]string{"run", "dnd-session", "--config", configPath, "--input", inputPath, "--output-dir", outputDir}, &stdout, &stderr, Options{ + LLMClientFactory: fakeLLMFactory(newFakeRunLLMClient(false), nil), + }) + + if code != 0 { + t.Fatalf("RunWithOptions() code = %d, stderr=%q", code, stderr.String()) + } + if _, err := os.Stat(filepath.Join(workspaceDir, "diagnostics")); !os.IsNotExist(err) { + t.Fatalf("workspace diagnostics dir stat err = %v, want not exist", err) + } + if entries := childDirs(t, outputDir); len(entries) != 1 { + t.Fatalf("output run dirs = %v, want one run dir", entries) + } +} + func TestRunPipelineWritesErrorLogAfterDiagnosticsCreation(t *testing.T) { diagnosticsDir := t.TempDir() configPath := writeTestConfig(t, mvpConfigYAMLWithDiagnostics("dnd-session", diagnosticsDir, "always")) @@ -2732,6 +2755,21 @@ pipelines: ` } +func mvpConfigYAMLWithWorkspaceDiagnosticsDisabled(pipelineID, workspaceDir string) string { + return `version: 2 +workspace: + directory: ` + workspaceDir + ` + diagnostics: + enabled: false +pipelines: + ` + pipelineID + `: + input: seriatim + artifacts: + spells: + extract: dnd/spells +` +} + func writeSeriatimInput(t *testing.T) string { t.Helper() return writeFile(t, "source.json", `{ diff --git a/internal/core/config/config.go b/internal/core/config/config.go index 9487ccb..9dd99ac 100644 --- a/internal/core/config/config.go +++ b/internal/core/config/config.go @@ -1,6 +1,9 @@ package config import ( + "path/filepath" + "strings" + "gitea.maximumdirect.net/eric/notarius/internal/core/diagnostics" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" ) @@ -12,6 +15,7 @@ type Config struct { Pipelines map[string]pipeline.PipelineProfile `json:"pipelines"` Concurrency ConcurrencyConfig `json:"concurrency"` Diagnostics DiagnosticsConfig `json:"diagnostics"` + Workspace WorkspaceConfig `json:"workspace"` } type ScriptoriumConfig struct { @@ -28,6 +32,28 @@ type DiagnosticsConfig struct { Retention diagnostics.RetentionMode `json:"retention"` } +type WorkspaceConfig struct { + Directory string `json:"directory,omitempty"` + Diagnostics WorkspaceDiagnosticsConfig `json:"diagnostics"` + Resume WorkspaceResumeConfig `json:"resume"` + Debug WorkspaceDebugConfig `json:"debug"` +} + +type WorkspaceDiagnosticsConfig struct { + Enabled bool `json:"enabled"` + Retention diagnostics.RetentionMode `json:"retention,omitempty"` + enabledSet bool + retentionSet bool +} + +type WorkspaceResumeConfig struct { + Enabled bool `json:"enabled"` +} + +type WorkspaceDebugConfig struct { + Enabled bool `json:"enabled"` +} + func Default() Config { return Config{ Pipelines: map[string]pipeline.PipelineProfile{}, @@ -38,9 +64,41 @@ func Default() Config { WorkDir: "/tmp/notarius", Retention: diagnostics.RetentionAuto, }, + Workspace: WorkspaceConfig{ + Diagnostics: WorkspaceDiagnosticsConfig{ + Enabled: true, + }, + }, } } +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.Pipelines = make(map[string]pipeline.PipelineProfile, len(in.Pipelines)) diff --git a/internal/core/config/config_test.go b/internal/core/config/config_test.go index 687fb97..f97968a 100644 --- a/internal/core/config/config_test.go +++ b/internal/core/config/config_test.go @@ -24,6 +24,21 @@ func TestDefaultValues(t *testing.T) { if cfg.Diagnostics.Retention != diagnostics.RetentionAuto { t.Fatalf("unexpected diagnostics retention: %q", cfg.Diagnostics.Retention) } + if cfg.Workspace.Directory != "" { + t.Fatalf("unexpected workspace directory: %q", cfg.Workspace.Directory) + } + if !cfg.Workspace.Diagnostics.Enabled || !cfg.DiagnosticsEnabled() { + t.Fatalf("expected workspace diagnostics enabled by default: %+v", cfg.Workspace.Diagnostics) + } + if cfg.Workspace.Diagnostics.Retention != "" { + t.Fatalf("unexpected workspace diagnostics retention: %q", cfg.Workspace.Diagnostics.Retention) + } + if cfg.Workspace.Resume.Enabled { + t.Fatalf("workspace resume should be disabled by default") + } + if cfg.Workspace.Debug.Enabled { + t.Fatalf("workspace debug should be disabled by default") + } } func TestApplyFileConfigMergesWithDefaults(t *testing.T) { diff --git a/internal/core/config/env.go b/internal/core/config/env.go index da873f6..89caa58 100644 --- a/internal/core/config/env.go +++ b/internal/core/config/env.go @@ -42,6 +42,36 @@ func (c *Config) applyEnvOverridesWithLookup(lookup func(string) (string, bool)) 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_DIAGNOSTICS_ENABLED"); ok { + value, err := parseBoolEnv("NOTARIUS_WORKSPACE_DIAGNOSTICS_ENABLED", raw) + if err != nil { + return err + } + 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 + } + 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 + } + c.Workspace.Debug.Enabled = value + } + c.RecomputeEffectiveDiagnostics() return nil } @@ -52,3 +82,11 @@ 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 7cde78c..3618676 100644 --- a/internal/core/config/env_test.go +++ b/internal/core/config/env_test.go @@ -13,10 +13,15 @@ func TestApplyEnvOverridesOperationalValues(t *testing.T) { cfg.Pipelines["example"] = pipeline.PipelineProfile{ID: "example", Input: pipeline.Binding("before")} err := cfg.applyEnvOverridesWithLookup(mapLookup(map[string]string{ - "NOTARIUS_TOTAL_LLM_CONCURRENCY": "3", - "NOTARIUS_WORK_DIR": "/tmp/notarius-env", - "NOTARIUS_DIAGNOSTICS_RETENTION": "never", - "NOTARIUS_PIPELINE_INPUT": "after", + "NOTARIUS_TOTAL_LLM_CONCURRENCY": "3", + "NOTARIUS_WORK_DIR": "/tmp/notarius-env", + "NOTARIUS_DIAGNOSTICS_RETENTION": "never", + "NOTARIUS_WORKSPACE_DIR": "/var/lib/notarius-env", + "NOTARIUS_WORKSPACE_DIAGNOSTICS_ENABLED": "false", + "NOTARIUS_WORKSPACE_DIAGNOSTICS_RETENTION": "always", + "NOTARIUS_WORKSPACE_RESUME_ENABLED": "true", + "NOTARIUS_WORKSPACE_DEBUG_ENABLED": "true", + "NOTARIUS_PIPELINE_INPUT": "after", })) if err != nil { t.Fatalf("ApplyEnvOverrides: %v", err) @@ -28,9 +33,21 @@ func TestApplyEnvOverridesOperationalValues(t *testing.T) { if cfg.Concurrency.TotalLLM != 3 { t.Fatalf("unexpected total concurrency: %d", cfg.Concurrency.TotalLLM) } - if cfg.Diagnostics.WorkDir != "/tmp/notarius-env" || cfg.Diagnostics.Retention != diagnostics.RetentionNever { + if cfg.Workspace.Directory != "/var/lib/notarius-env" { + t.Fatalf("unexpected workspace directory: %q", cfg.Workspace.Directory) + } + if cfg.DiagnosticsEnabled() { + t.Fatalf("expected workspace diagnostics disabled") + } + if cfg.Diagnostics.WorkDir != "/var/lib/notarius-env/diagnostics" || cfg.Diagnostics.Retention != diagnostics.RetentionAlways { t.Fatalf("unexpected diagnostics config: %+v", cfg.Diagnostics) } + if !cfg.Workspace.Resume.Enabled { + t.Fatalf("expected workspace resume enabled") + } + if !cfg.Workspace.Debug.Enabled { + t.Fatalf("expected workspace debug enabled") + } if cfg.Pipelines["example"].Input.Module != "before" { t.Fatalf("environment overrides must not change pipeline wiring: %+v", cfg.Pipelines["example"]) } @@ -46,6 +63,41 @@ func TestApplyEnvOverridesRejectsInvalidIntegers(t *testing.T) { } } +func TestApplyEnvOverridesRejectsInvalidBooleans(t *testing.T) { + for _, name := range []string{ + "NOTARIUS_WORKSPACE_DIAGNOSTICS_ENABLED", + "NOTARIUS_WORKSPACE_RESUME_ENABLED", + "NOTARIUS_WORKSPACE_DEBUG_ENABLED", + } { + t.Run(name, func(t *testing.T) { + cfg := Default() + err := cfg.applyEnvOverridesWithLookup(mapLookup(map[string]string{name: "maybe"})) + if err == nil || !strings.Contains(err.Error(), name) { + t.Fatalf("expected named boolean error, got %v", err) + } + }) + } +} + +func TestApplyEnvOverridesLegacyDiagnosticsRemainCompatibleWithoutWorkspace(t *testing.T) { + cfg := Default() + + err := cfg.applyEnvOverridesWithLookup(mapLookup(map[string]string{ + "NOTARIUS_WORK_DIR": "/tmp/notarius-env", + "NOTARIUS_DIAGNOSTICS_RETENTION": "never", + })) + if err != nil { + t.Fatalf("ApplyEnvOverrides: %v", err) + } + + if cfg.Diagnostics.WorkDir != "/tmp/notarius-env" { + t.Fatalf("diagnostics work dir = %q, want legacy env", cfg.Diagnostics.WorkDir) + } + if cfg.Diagnostics.Retention != diagnostics.RetentionNever { + t.Fatalf("diagnostics retention = %q, want legacy env", cfg.Diagnostics.Retention) + } +} + func TestLoadFromEnvUsesDefaultConfig(t *testing.T) { t.Setenv("NOTARIUS_TOTAL_LLM_CONCURRENCY", "2") diff --git a/internal/core/config/file_config.go b/internal/core/config/file_config.go index 12899ff..398940b 100644 --- a/internal/core/config/file_config.go +++ b/internal/core/config/file_config.go @@ -18,6 +18,7 @@ type FileConfig struct { Pipelines map[string]FilePipelineProfile `yaml:"pipelines,omitempty"` Concurrency *FileConcurrencyConfig `yaml:"concurrency,omitempty"` Diagnostics *FileDiagnosticsConfig `yaml:"diagnostics,omitempty"` + Workspace *FileWorkspaceConfig `yaml:"workspace,omitempty"` } type FileScriptoriumConfig struct { @@ -50,6 +51,22 @@ type FileDiagnosticsConfig struct { Retention *string `yaml:"retention,omitempty"` } +type FileWorkspaceConfig struct { + Directory *string `yaml:"directory,omitempty"` + Diagnostics *FileWorkspaceDiagnosticsConfig `yaml:"diagnostics,omitempty"` + Resume *FileWorkspaceEnabledConfig `yaml:"resume,omitempty"` + Debug *FileWorkspaceEnabledConfig `yaml:"debug,omitempty"` +} + +type FileWorkspaceDiagnosticsConfig struct { + Enabled *bool `yaml:"enabled,omitempty"` + Retention *string `yaml:"retention,omitempty"` +} + +type FileWorkspaceEnabledConfig struct { + Enabled *bool `yaml:"enabled,omitempty"` +} + type fileModuleBinding struct { Module string LLMProfile string @@ -307,6 +324,28 @@ func (c *Config) applyFileConfigWithLookup(fileCfg FileConfig, lookup func(strin c.Diagnostics.Retention = diagnostics.RetentionMode(strings.TrimSpace(*fileCfg.Diagnostics.Retention)) } } + if fileCfg.Workspace != nil { + if fileCfg.Workspace.Directory != nil { + c.Workspace.Directory = strings.TrimSpace(*fileCfg.Workspace.Directory) + } + 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.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() return nil } diff --git a/internal/core/config/file_config_test.go b/internal/core/config/file_config_test.go index 4e24691..b5de6b2 100644 --- a/internal/core/config/file_config_test.go +++ b/internal/core/config/file_config_test.go @@ -554,6 +554,95 @@ diagnostics: } } +func TestApplyFileConfigWorkspaceSection(t *testing.T) { + cfg := parseAndApplyConfig(t, ` +version: 2 +workspace: + directory: /var/lib/notarius + diagnostics: + enabled: false + retention: never + resume: + enabled: true + debug: + enabled: true +diagnostics: + work_dir: /tmp/legacy + retention: always +`) + + if cfg.Workspace.Directory != "/var/lib/notarius" { + t.Fatalf("workspace directory = %q, want /var/lib/notarius", cfg.Workspace.Directory) + } + if cfg.DiagnosticsEnabled() { + t.Fatalf("expected diagnostics disabled") + } + if cfg.Diagnostics.WorkDir != "/var/lib/notarius/diagnostics" { + t.Fatalf("effective diagnostics work dir = %q, want workspace diagnostics root", cfg.Diagnostics.WorkDir) + } + if cfg.Diagnostics.Retention != diagnostics.RetentionNever { + t.Fatalf("effective diagnostics retention = %q, want workspace override", cfg.Diagnostics.Retention) + } + if !cfg.Workspace.Resume.Enabled { + t.Fatalf("expected resume enabled") + } + if !cfg.Workspace.Debug.Enabled { + t.Fatalf("expected debug enabled") + } +} + +func TestApplyFileConfigLegacyDiagnosticsRemainCompatible(t *testing.T) { + cfg := parseAndApplyConfig(t, ` +version: 2 +diagnostics: + work_dir: /tmp/legacy + retention: always +`) + + if cfg.Workspace.Directory != "" { + t.Fatalf("workspace directory = %q, want unset", cfg.Workspace.Directory) + } + if !cfg.DiagnosticsEnabled() { + t.Fatalf("expected diagnostics enabled") + } + if cfg.Diagnostics.WorkDir != "/tmp/legacy" { + t.Fatalf("effective diagnostics work dir = %q, want legacy", cfg.Diagnostics.WorkDir) + } + if cfg.Diagnostics.Retention != diagnostics.RetentionAlways { + t.Fatalf("effective diagnostics retention = %q, want legacy", cfg.Diagnostics.Retention) + } +} + +func TestApplyFileConfigWorkspaceRetentionOverridesLegacyRetentionOnlyWhenSet(t *testing.T) { + t.Run("legacy retained", func(t *testing.T) { + cfg := parseAndApplyConfig(t, ` +version: 2 +workspace: + directory: /var/lib/notarius +diagnostics: + retention: never +`) + if cfg.Diagnostics.Retention != diagnostics.RetentionNever { + t.Fatalf("effective diagnostics retention = %q, want legacy", cfg.Diagnostics.Retention) + } + }) + + t.Run("workspace overrides", func(t *testing.T) { + cfg := parseAndApplyConfig(t, ` +version: 2 +workspace: + directory: /var/lib/notarius + diagnostics: + retention: always +diagnostics: + retention: never +`) + if cfg.Diagnostics.Retention != diagnostics.RetentionAlways { + t.Fatalf("effective diagnostics retention = %q, want workspace", cfg.Diagnostics.Retention) + } + }) +} + func parseAndApplyConfig(t *testing.T, raw string) Config { t.Helper() fileCfg, err := ParseFileConfigYAML([]byte(raw)) diff --git a/internal/core/config/redaction_test.go b/internal/core/config/redaction_test.go index 6e1d8cb..70ee21b 100644 --- a/internal/core/config/redaction_test.go +++ b/internal/core/config/redaction_test.go @@ -10,6 +10,8 @@ import ( func TestRedactedConfigCopiesScriptoriumConfig(t *testing.T) { cfg := Default() cfg.Scriptorium.ProfileDir = "./profiles" + cfg.Workspace.Directory = "/var/lib/notarius" + cfg.Workspace.Resume.Enabled = true redacted := cfg.Redacted() @@ -20,6 +22,13 @@ func TestRedactedConfigCopiesScriptoriumConfig(t *testing.T) { if cfg.Scriptorium.ProfileDir != "./profiles" { t.Fatalf("redaction mutated original config") } + if redacted.Workspace.Directory != "/var/lib/notarius" || !redacted.Workspace.Resume.Enabled { + t.Fatalf("expected workspace config preserved, got %+v", redacted.Workspace) + } + redacted.Workspace.Directory = "/changed" + if cfg.Workspace.Directory != "/var/lib/notarius" { + t.Fatalf("redaction mutated original workspace config") + } } func TestConfigRedactedDiagnosticsPayloadCopiesConfig(t *testing.T) { diff --git a/internal/core/config/validation.go b/internal/core/config/validation.go index f76a120..59b7a88 100644 --- a/internal/core/config/validation.go +++ b/internal/core/config/validation.go @@ -12,6 +12,9 @@ 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 { return err } @@ -28,6 +31,17 @@ func validateScriptorium(cfg ScriptoriumConfig) error { return nil } +func validateWorkspace(cfg WorkspaceConfig) error { + 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) + } + } + return nil +} + func validateDiagnostics(cfg DiagnosticsConfig) error { if strings.TrimSpace(cfg.WorkDir) == "" { return fmt.Errorf("diagnostics work dir must not be empty") diff --git a/internal/core/config/validation_test.go b/internal/core/config/validation_test.go index 01b6143..0ae855b 100644 --- a/internal/core/config/validation_test.go +++ b/internal/core/config/validation_test.go @@ -99,6 +99,17 @@ func TestValidateRejectsInvalidDiagnosticsRetention(t *testing.T) { } } +func TestValidateRejectsInvalidWorkspaceDiagnosticsRetention(t *testing.T) { + cfg := validConfig() + cfg.Workspace.Diagnostics.Retention = diagnostics.RetentionMode("sometimes") + cfg.Workspace.Diagnostics.retentionSet = true + + err := cfg.Validate() + if err == nil || !strings.Contains(err.Error(), "workspace diagnostics retention") { + t.Fatalf("expected workspace retention error, got %v", err) + } +} + func TestValidateRejectsInvalidReferenceMaps(t *testing.T) { tests := []struct { name string