diff --git a/docs/operations.md b/docs/operations.md index 4e76053..89326bd 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -57,7 +57,8 @@ The default diagnostics work directory is `/tmp/notarius`. It can be set with `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//`. +`/diagnostics//` unless `--diagnostics-dir` +overrides the diagnostics work directory for that invocation. Set `workspace.diagnostics.enabled: false` or `NOTARIUS_WORKSPACE_DIAGNOSTICS_ENABLED=false` to skip diagnostics directory diff --git a/internal/cli/run.go b/internal/cli/run.go index afb60de..4f3a690 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -16,6 +16,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/framework/contracts" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" ) @@ -151,16 +152,17 @@ func runPipelineCommand(args []string, stdout, stderr io.Writer, opts Options) i fmt.Fprintf(stderr, "notarius: %v\n", err) return 1 } + workspaceSettings := workspace.FromConfig(cfg) if dir := strings.TrimSpace(*diagnosticsDir); dir != "" { - cfg.Diagnostics.WorkDir = dir + workspaceSettings.DiagnosticsRoot = dir } startedAt := opts.Now().UTC() runID := fmt.Sprintf("run-%d", startedAt.UnixNano()) var runDir *diagnostics.RunDirectory - if cfg.DiagnosticsEnabled() { + if workspaceSettings.DiagnosticsEnabled { var err error - runDir, err = diagnostics.NewRunDirectory(cfg.Diagnostics.WorkDir, cfg.Diagnostics.Retention) + runDir, err = diagnostics.NewRunDirectory(workspaceSettings.DiagnosticsRoot, cfg.Diagnostics.Retention) if err != nil { fmt.Fprintf(stderr, "notarius: %v\n", err) return 1 diff --git a/internal/cli/run_test.go b/internal/cli/run_test.go index c606d58..afd6aee 100644 --- a/internal/cli/run_test.go +++ b/internal/cli/run_test.go @@ -2148,6 +2148,38 @@ func TestRunPipelineWritesDiagnosticsArtifactsOnSuccess(t *testing.T) { } } +func TestRunPipelineWritesWorkspaceDiagnosticsArtifactsOnSuccess(t *testing.T) { + workspaceDir := filepath.Join(t.TempDir(), "workspace") + outputDir := t.TempDir() + configPath := writeTestConfig(t, mvpConfigYAMLWithWorkspaceDiagnostics("dnd-session", workspaceDir, "always")) + 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()) + } + runDir := onlyChildDir(t, filepath.Join(workspaceDir, "diagnostics")) + for _, name := range []string{ + diagnostics.ArtifactInvocationMetadata, + diagnostics.ArtifactEffectiveConfig, + diagnostics.ArtifactResolvedPipeline, + diagnostics.ArtifactRunManifest, + diagnostics.ArtifactRunReport, + diagnostics.ArtifactWarnings, + } { + if _, err := os.Stat(filepath.Join(runDir, name)); err != nil { + t.Fatalf("expected workspace diagnostics artifact %q: %v", name, err) + } + } + assertPathNotExist(t, filepath.Join(workspaceDir, "checkpoints")) + assertPathNotExist(t, filepath.Join(workspaceDir, "debug")) +} + func TestRunPipelineSkipsDiagnosticsWhenWorkspaceDiagnosticsDisabled(t *testing.T) { workspaceDir := filepath.Join(t.TempDir(), "workspace") outputDir := t.TempDir() @@ -2171,6 +2203,27 @@ func TestRunPipelineSkipsDiagnosticsWhenWorkspaceDiagnosticsDisabled(t *testing. } } +func TestRunPipelineLegacyDiagnosticsConfigStillWritesDiagnostics(t *testing.T) { + diagnosticsDir := t.TempDir() + outputDir := t.TempDir() + configPath := writeTestConfig(t, mvpConfigYAMLWithDiagnostics("dnd-session", diagnosticsDir, "always")) + 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()) + } + runDir := onlyChildDir(t, diagnosticsDir) + if _, err := os.Stat(filepath.Join(runDir, diagnostics.ArtifactRunReport)); err != nil { + t.Fatalf("expected legacy diagnostics run report: %v", err) + } +} + func TestRunPipelineWritesErrorLogAfterDiagnosticsCreation(t *testing.T) { diagnosticsDir := t.TempDir() configPath := writeTestConfig(t, mvpConfigYAMLWithDiagnostics("dnd-session", diagnosticsDir, "always")) @@ -2262,6 +2315,30 @@ func TestRunPipelineDiagnosticsDirFlagOverridesConfig(t *testing.T) { } } +func TestRunPipelineDiagnosticsDirFlagOverridesWorkspaceDiagnosticsOnly(t *testing.T) { + workspaceDir := filepath.Join(t.TempDir(), "workspace") + overrideDiagnosticsDir := t.TempDir() + outputDir := t.TempDir() + configPath := writeTestConfig(t, mvpConfigYAMLWithWorkspaceDiagnosticsAndStateEnabled("dnd-session", workspaceDir, "always")) + inputPath := writeSeriatimInput(t) + var stdout bytes.Buffer + var stderr bytes.Buffer + + code := RunWithOptions([]string{"run", "dnd-session", "--config", configPath, "--input", inputPath, "--output-dir", outputDir, "--diagnostics-dir", overrideDiagnosticsDir}, &stdout, &stderr, Options{ + LLMClientFactory: fakeLLMFactory(newFakeRunLLMClient(false), nil), + }) + + if code != 0 { + t.Fatalf("RunWithOptions() code = %d, stderr=%q", code, stderr.String()) + } + if entries := childDirs(t, overrideDiagnosticsDir); len(entries) != 1 { + t.Fatalf("override diagnostics dir entries = %v, want one run dir", entries) + } + assertPathNotExist(t, filepath.Join(workspaceDir, "diagnostics")) + assertPathNotExist(t, filepath.Join(workspaceDir, "checkpoints")) + assertPathNotExist(t, filepath.Join(workspaceDir, "debug")) +} + func TestExampleFixtureConfigValidateAndPipelinesList(t *testing.T) { configPath := fixturePath(t, "examples/dnd-spells.config.yml") @@ -2755,6 +2832,42 @@ pipelines: ` } +func mvpConfigYAMLWithWorkspaceDiagnostics(pipelineID, workspaceDir, retention string) string { + return `version: 2 +workspace: + directory: ` + workspaceDir + ` + diagnostics: + enabled: true + retention: ` + retention + ` +pipelines: + ` + pipelineID + `: + input: seriatim + artifacts: + spells: + extract: dnd/spells +` +} + +func mvpConfigYAMLWithWorkspaceDiagnosticsAndStateEnabled(pipelineID, workspaceDir, retention string) string { + return `version: 2 +workspace: + directory: ` + workspaceDir + ` + diagnostics: + enabled: true + retention: ` + retention + ` + resume: + enabled: true + debug: + enabled: true +pipelines: + ` + pipelineID + `: + input: seriatim + artifacts: + spells: + extract: dnd/spells +` +} + func mvpConfigYAMLWithWorkspaceDiagnosticsDisabled(pipelineID, workspaceDir string) string { return `version: 2 workspace: @@ -3136,6 +3249,13 @@ func childDirs(t *testing.T, root string) []string { return dirs } +func assertPathNotExist(t *testing.T, path string) { + t.Helper() + if _, err := os.Stat(path); !os.IsNotExist(err) { + t.Fatalf("path %q stat err = %v, want not exist", path, err) + } +} + func readFile(t *testing.T, path string) []byte { t.Helper() data, err := os.ReadFile(path)