diff --git a/internal/cli/run.go b/internal/cli/run.go index 5aee8df..f25ec8c 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -214,9 +214,6 @@ func runPipelineCommand(args []string, stdout, stderr io.Writer, opts Options) i } runOutputDir := filepath.Join(outputRoot(*outputDir), runDir.RunID()) - if err := writeOutputFiles(runOutputDir, output.OutputFiles); err != nil { - return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, err) - } if err := runDir.WriteRunManifest(output.Manifest); err != nil { return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, fmt.Errorf("write diagnostics run manifest: %w", err)) } @@ -235,6 +232,9 @@ func runPipelineCommand(args []string, stdout, stderr io.Writer, opts Options) i }); 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, diff --git a/internal/cli/run_test.go b/internal/cli/run_test.go index f80b14c..b05cf0f 100644 --- a/internal/cli/run_test.go +++ b/internal/cli/run_test.go @@ -773,6 +773,40 @@ func TestRunPipelineRejectsUnsafeOutputFileName(t *testing.T) { if got := string(readFile(t, filepath.Join(runDir, diagnostics.ArtifactErrorLog))); !strings.Contains(got, "output file name") { t.Fatalf("error log = %q, want unsafe output file error", got) } + if _, err := os.Stat(filepath.Join(runDir, diagnostics.ArtifactRunManifest)); err != nil { + t.Fatalf("expected diagnostics manifest after unsafe output file failure: %v", err) + } +} + +func TestRunPipelineWritesDiagnosticsArtifactsWhenDurableOutputWriteFails(t *testing.T) { + diagnosticsDir := t.TempDir() + configPath := writeTestConfig(t, mvpConfigYAMLWithDiagnostics("dnd-session", diagnosticsDir, "always")) + inputPath := writeSeriatimInput(t) + outputRootFile := writeFile(t, "not-a-directory", "occupied") + var stdout bytes.Buffer + var stderr bytes.Buffer + + code := RunWithOptions([]string{"run", "dnd-session", "--config", configPath, "--input", inputPath, "--output-dir", outputRootFile}, &stdout, &stderr, Options{ + LLMClientFactory: fakeLLMFactory(newFakeRunLLMClient(false), nil), + }) + + if code != 1 { + t.Fatalf("RunWithOptions() code = %d, want 1", code) + } + if !strings.Contains(stderr.String(), "create output directory") { + t.Fatalf("stderr = %q, want output directory error", stderr.String()) + } + runDir := onlyChildDir(t, diagnosticsDir) + for _, name := range []string{ + diagnostics.ArtifactRunManifest, + diagnostics.ArtifactWarnings, + diagnostics.ArtifactRunReport, + diagnostics.ArtifactErrorLog, + } { + if _, err := os.Stat(filepath.Join(runDir, name)); err != nil { + t.Fatalf("expected diagnostics artifact %q after durable output write failure: %v", name, err) + } + } } func TestRunPipelineWritesDiagnosticsArtifactsOnSuccess(t *testing.T) { diff --git a/internal/framework/contracts/contracts.go b/internal/framework/contracts/contracts.go index 6ab8d6d..a4b8ecc 100644 --- a/internal/framework/contracts/contracts.go +++ b/internal/framework/contracts/contracts.go @@ -189,12 +189,8 @@ type OutputFile struct { } type OutputResult struct { - Files []OutputFile `json:"files,omitempty"` - // Bytes is the legacy single-output payload. New encoders should return Files. - Bytes []byte `json:"-"` - // ContentType is the legacy single-output content type. New encoders should return Files. - ContentType string `json:"content_type,omitempty"` - Warnings []Warning `json:"warnings,omitempty"` + Files []OutputFile `json:"files,omitempty"` + Warnings []Warning `json:"warnings,omitempty"` } type OutputEncoder interface { diff --git a/internal/framework/pipeline/registry_integration_test.go b/internal/framework/pipeline/registry_integration_test.go index 86e25e1..5b27306 100644 --- a/internal/framework/pipeline/registry_integration_test.go +++ b/internal/framework/pipeline/registry_integration_test.go @@ -193,7 +193,11 @@ func (output integrationOutput) Key() string { } func (output integrationOutput) Encode(ctx context.Context, req contracts.OutputRequest) (contracts.OutputResult, error) { - return contracts.OutputResult{Bytes: []byte(`{}`), ContentType: "application/json"}, nil + return contracts.OutputResult{ + Files: []contracts.OutputFile{ + {Name: "output.json", ContentType: "application/json", Bytes: []byte(`{}`)}, + }, + }, nil } type integrationValidator struct { diff --git a/internal/framework/pipeline/runner.go b/internal/framework/pipeline/runner.go index db2d7de..c65f72a 100644 --- a/internal/framework/pipeline/runner.go +++ b/internal/framework/pipeline/runner.go @@ -49,10 +49,6 @@ type RunOutput struct { Rejected []artifacts.RejectedArtifact `json:"rejected,omitempty"` Warnings []contracts.Warning `json:"warnings,omitempty"` OutputFiles []contracts.OutputFile `json:"-"` - // EncodedOutput is the legacy single-output payload. New callers should use OutputFiles. - EncodedOutput []byte `json:"-"` - // ContentType is the legacy single-output content type. New callers should use OutputFiles. - ContentType string `json:"content_type,omitempty"` } func (r *Runner) Run(ctx context.Context, input RunInput) (RunOutput, error) { @@ -143,10 +139,6 @@ func (r *Runner) Run(ctx context.Context, input RunInput) (RunOutput, error) { return failOutput(output), fmt.Errorf("validate output files from encoder %q: %w", encoder.Key(), err) } output.OutputFiles = files - if len(encoded.Files) == 0 { - output.EncodedOutput = append([]byte(nil), encoded.Bytes...) - output.ContentType = encoded.ContentType - } return output, nil } @@ -425,19 +417,8 @@ func manifestMetadataKey(module any) string { } func outputFilesFromResult(result contracts.OutputResult) ([]contracts.OutputFile, error) { - files := result.Files - if len(files) == 0 && len(result.Bytes) > 0 { - files = []contracts.OutputFile{ - { - Name: "output", - ContentType: result.ContentType, - Bytes: result.Bytes, - }, - } - } - - out := make([]contracts.OutputFile, 0, len(files)) - for _, file := range files { + out := make([]contracts.OutputFile, 0, len(result.Files)) + for _, file := range result.Files { if err := validateOutputFileName(file.Name); err != nil { return nil, err } diff --git a/internal/framework/pipeline/runner_test.go b/internal/framework/pipeline/runner_test.go index 9f056b2..7fc4e48 100644 --- a/internal/framework/pipeline/runner_test.go +++ b/internal/framework/pipeline/runner_test.go @@ -29,13 +29,11 @@ func TestNewAndDataTypes(t *testing.T) { Metadata: map[string]any{"request": "test"}, } output := RunOutput{ - Manifest: artifacts.RunManifest{PipelineID: "pipeline-1"}, - Approved: []artifacts.Artifact{{ExtractorKey: "extract-alpha"}}, - Rejected: []artifacts.RejectedArtifact{{ValidatorName: "validator"}}, - Warnings: []contracts.Warning{{ReasonCode: "note", Message: "message"}}, - OutputFiles: []contracts.OutputFile{{Name: "artifacts/generic.json", ContentType: "application/json", Bytes: []byte(`{}`)}}, - EncodedOutput: []byte(`{}`), - ContentType: "application/json", + Manifest: artifacts.RunManifest{PipelineID: "pipeline-1"}, + Approved: []artifacts.Artifact{{ExtractorKey: "extract-alpha"}}, + Rejected: []artifacts.RejectedArtifact{{ValidatorName: "validator"}}, + Warnings: []contracts.Warning{{ReasonCode: "note", Message: "message"}}, + OutputFiles: []contracts.OutputFile{{Name: "artifacts/generic.json", ContentType: "application/json", Bytes: []byte(`{}`)}}, } if input.Pipeline.ID != "pipeline-1" || input.SourceID != "source-1" { @@ -1140,13 +1138,11 @@ func (validator *runnerValidator) Validate(ctx context.Context, req contracts.Va } type runnerOutputEncoder struct { - key string - files []contracts.OutputFile - bytes []byte - contentType string - warnings []contracts.Warning - err error - requests []contracts.OutputRequest + key string + files []contracts.OutputFile + warnings []contracts.Warning + err error + requests []contracts.OutputRequest } func (encoder *runnerOutputEncoder) Key() string { @@ -1156,10 +1152,8 @@ func (encoder *runnerOutputEncoder) Key() string { func (encoder *runnerOutputEncoder) Encode(ctx context.Context, req contracts.OutputRequest) (contracts.OutputResult, error) { encoder.requests = append(encoder.requests, req) return contracts.OutputResult{ - Files: encoder.files, - Bytes: encoder.bytes, - ContentType: encoder.contentType, - Warnings: encoder.warnings, + Files: encoder.files, + Warnings: encoder.warnings, }, encoder.err } diff --git a/internal/framework/pipeline/walking_skeleton_test.go b/internal/framework/pipeline/walking_skeleton_test.go index fe9af0b..6ce05ff 100644 --- a/internal/framework/pipeline/walking_skeleton_test.go +++ b/internal/framework/pipeline/walking_skeleton_test.go @@ -36,10 +36,13 @@ func TestWalkingSkeletonFixture(t *testing.T) { if err != nil { t.Fatalf("Run() error = %v, want nil", err) } - if output.ContentType != "application/json" { - t.Fatalf("ContentType = %q, want application/json", output.ContentType) + if len(output.OutputFiles) != 1 { + t.Fatalf("len(OutputFiles) = %d, want 1", len(output.OutputFiles)) } - assertStructuralJSONEqual(t, output.EncodedOutput, expectedBytes) + if output.OutputFiles[0].ContentType != "application/json" { + t.Fatalf("ContentType = %q, want application/json", output.OutputFiles[0].ContentType) + } + assertStructuralJSONEqual(t, output.OutputFiles[0].Bytes, expectedBytes) if llmClient.calls != 2 { t.Fatalf("LLM calls = %d, want chunk count 2", llmClient.calls) } @@ -356,8 +359,9 @@ func (output walkingSkeletonOutput) Encode(ctx context.Context, req contracts.Ou return contracts.OutputResult{}, err } return contracts.OutputResult{ - Bytes: encoded, - ContentType: "application/json", + Files: []contracts.OutputFile{ + {Name: "output.json", ContentType: "application/json", Bytes: encoded}, + }, }, nil } diff --git a/internal/modules/extract/dnd/spells/config_test.go b/internal/modules/extract/dnd/spells/config_test.go index c43e276..14c7df8 100644 --- a/internal/modules/extract/dnd/spells/config_test.go +++ b/internal/modules/extract/dnd/spells/config_test.go @@ -257,8 +257,9 @@ func (dndSpellsOutput) Key() string { func (dndSpellsOutput) Encode(ctx context.Context, req contracts.OutputRequest) (contracts.OutputResult, error) { return contracts.OutputResult{ - Bytes: []byte(`{"encoded":true}`), - ContentType: "application/json", + Files: []contracts.OutputFile{ + {Name: "output.json", ContentType: "application/json", Bytes: []byte(`{"encoded":true}`)}, + }, }, nil } diff --git a/internal/modules/extract/dnd/spells/runner_test.go b/internal/modules/extract/dnd/spells/runner_test.go index ace6678..803a3a1 100644 --- a/internal/modules/extract/dnd/spells/runner_test.go +++ b/internal/modules/extract/dnd/spells/runner_test.go @@ -102,8 +102,11 @@ func TestRunnerProcessesSeriatimInputWithDNDSpellsExtractor(t *testing.T) { extractorMetadata["response_schema_name"] != ResponseSchemaName { t.Fatalf("extractor metadata = %#v, want prompt/schema identifiers", extractorMetadata) } - if output.ContentType != "application/json" { - t.Fatalf("ContentType = %q, want application/json", output.ContentType) + if len(output.OutputFiles) != 1 { + t.Fatalf("len(OutputFiles) = %d, want 1", len(output.OutputFiles)) + } + if output.OutputFiles[0].ContentType != "application/json" { + t.Fatalf("ContentType = %q, want application/json", output.OutputFiles[0].ContentType) } } diff --git a/internal/modules/input/seriatim/runner_test.go b/internal/modules/input/seriatim/runner_test.go index a4be64e..358d29f 100644 --- a/internal/modules/input/seriatim/runner_test.go +++ b/internal/modules/input/seriatim/runner_test.go @@ -63,8 +63,11 @@ func TestRunnerProcessesSeriatimInputWithFakeModules(t *testing.T) { if extractor.calls != 1 { t.Fatalf("extractor calls = %d, want 1", extractor.calls) } - if output.ContentType != "application/json" { - t.Fatalf("ContentType = %q, want application/json", output.ContentType) + if len(output.OutputFiles) != 1 { + t.Fatalf("len(OutputFiles) = %d, want 1", len(output.OutputFiles)) + } + if output.OutputFiles[0].ContentType != "application/json" { + t.Fatalf("ContentType = %q, want application/json", output.OutputFiles[0].ContentType) } } @@ -237,8 +240,9 @@ func (runnerSeriatimOutput) Key() string { func (runnerSeriatimOutput) Encode(ctx context.Context, req contracts.OutputRequest) (contracts.OutputResult, error) { return contracts.OutputResult{ - Bytes: []byte(`{"encoded":true}`), - ContentType: "application/json", + Files: []contracts.OutputFile{ + {Name: "output.json", ContentType: "application/json", Bytes: []byte(`{"encoded":true}`)}, + }, }, nil }