diff --git a/internal/adapters/scriptorium/runner.go b/internal/adapters/scriptorium/runner.go index b197a8c..3c6017f 100644 --- a/internal/adapters/scriptorium/runner.go +++ b/internal/adapters/scriptorium/runner.go @@ -140,61 +140,57 @@ func (r Runner) Render(ctx context.Context, req RenderRequest) (*RenderResult, e } func (r Runner) Run(ctx context.Context, req RunRequest) (*RunResult, error) { - if req.PromptID == "" { - return nil, fmt.Errorf("prompt id is required") - } - if req.DataPackagePath == "" { - return nil, fmt.Errorf("data package path is required") - } - if req.OutputPath == "" { - return nil, fmt.Errorf("output path is required") - } - execution, err := r.execute(ctx, r.runArgs(req)) - if err != nil { - return nil, fmt.Errorf("run scriptorium: %w", err) - } - result := &RunResult{ - Command: execution.argv(), - Stdout: string(execution.result.Stdout), - Stderr: string(execution.result.Stderr), - StdoutTruncated: execution.result.StdoutTruncated, - StderrTruncated: execution.result.StderrTruncated, - ExitCode: execution.result.ExitCode, + result, err := r.executeRun(ctx, outputRunRequest{ + PromptID: req.PromptID, + DataPackagePath: req.DataPackagePath, OutputPath: req.OutputPath, + }, "run scriptorium", "scriptorium run") + if err != nil { + if result == nil { + return nil, err + } + return result.runResult(), err } - if execution.result.ExitCode != 0 { - return result, fmt.Errorf("scriptorium run exited with code %d: %s", execution.result.ExitCode, result.Stderr) - } - return result, nil + return result.runResult(), nil } func (r Runner) StructuredRun(ctx context.Context, req StructuredRunRequest) (*StructuredRunResult, error) { - if req.PromptID == "" { - return nil, fmt.Errorf("prompt id is required") - } - if req.DataPackagePath == "" { - return nil, fmt.Errorf("data package path is required") - } - if req.OutputPath == "" { - return nil, fmt.Errorf("output path is required") - } - execution, err := r.execute(ctx, r.structuredRunArgs(req)) - if err != nil { - return nil, fmt.Errorf("run scriptorium structured output: %w", err) - } - result := &StructuredRunResult{ - Command: execution.argv(), - Stdout: string(execution.result.Stdout), - Stderr: string(execution.result.Stderr), - StdoutTruncated: execution.result.StdoutTruncated, - StderrTruncated: execution.result.StderrTruncated, - ExitCode: execution.result.ExitCode, + result, err := r.executeRun(ctx, outputRunRequest{ + PromptID: req.PromptID, + DataPackagePath: req.DataPackagePath, OutputPath: req.OutputPath, + }, "run scriptorium structured output", "scriptorium structured run") + if err != nil { + if result == nil { + return nil, err + } + return result.structuredRunResult(), err } - if execution.result.ExitCode != 0 { - return result, fmt.Errorf("scriptorium structured run exited with code %d: %s", execution.result.ExitCode, result.Stderr) + return result.structuredRunResult(), nil +} + +func (result outputRunResult) runResult() *RunResult { + return &RunResult{ + Command: result.Command, + Stdout: result.Stdout, + Stderr: result.Stderr, + StdoutTruncated: result.StdoutTruncated, + StderrTruncated: result.StderrTruncated, + ExitCode: result.ExitCode, + OutputPath: result.OutputPath, + } +} + +func (result outputRunResult) structuredRunResult() *StructuredRunResult { + return &StructuredRunResult{ + Command: result.Command, + Stdout: result.Stdout, + Stderr: result.Stderr, + StdoutTruncated: result.StdoutTruncated, + StderrTruncated: result.StderrTruncated, + ExitCode: result.ExitCode, + OutputPath: result.OutputPath, } - return result, nil } type execution struct { @@ -203,6 +199,55 @@ type execution struct { result CommandResult } +type outputRunRequest struct { + PromptID string + DataPackagePath string + OutputPath string +} + +type outputRunResult struct { + Command []string + Stdout string + Stderr string + StdoutTruncated bool + StderrTruncated bool + ExitCode int + OutputPath string +} + +func (r Runner) executeRun(ctx context.Context, req outputRunRequest, executeContext string, exitContext string) (*outputRunResult, error) { + if req.PromptID == "" { + return nil, fmt.Errorf("prompt id is required") + } + if req.DataPackagePath == "" { + return nil, fmt.Errorf("data package path is required") + } + if req.OutputPath == "" { + return nil, fmt.Errorf("output path is required") + } + execution, err := r.execute(ctx, r.runArgs(RunRequest{ + PromptID: req.PromptID, + DataPackagePath: req.DataPackagePath, + OutputPath: req.OutputPath, + })) + if err != nil { + return nil, fmt.Errorf("%s: %w", executeContext, err) + } + result := &outputRunResult{ + Command: execution.argv(), + Stdout: string(execution.result.Stdout), + Stderr: string(execution.result.Stderr), + StdoutTruncated: execution.result.StdoutTruncated, + StderrTruncated: execution.result.StderrTruncated, + ExitCode: execution.result.ExitCode, + OutputPath: req.OutputPath, + } + if execution.result.ExitCode != 0 { + return result, fmt.Errorf("%s exited with code %d: %s", exitContext, execution.result.ExitCode, result.Stderr) + } + return result, nil +} + func (r Runner) execute(ctx context.Context, args []string) (execution, error) { binary := r.Binary if binary == "" { @@ -257,14 +302,6 @@ func (r Runner) runArgs(req RunRequest) []string { return args } -func (r Runner) structuredRunArgs(req StructuredRunRequest) []string { - return r.runArgs(RunRequest{ - PromptID: req.PromptID, - DataPackagePath: req.DataPackagePath, - OutputPath: req.OutputPath, - }) -} - type limitedBuffer struct { data []byte limit int diff --git a/internal/adapters/scriptorium/runner_test.go b/internal/adapters/scriptorium/runner_test.go index 7448e78..8ee4907 100644 --- a/internal/adapters/scriptorium/runner_test.go +++ b/internal/adapters/scriptorium/runner_test.go @@ -2,6 +2,7 @@ package scriptorium import ( "context" + "fmt" "reflect" "strings" "testing" @@ -236,53 +237,281 @@ func TestStructuredRunReturnsResultForNonzeroExit(t *testing.T) { } } -func TestStructuredRunValidatesRequiredFieldsBeforeExecution(t *testing.T) { +func TestOutputRunsPreserveCapturedResultFields(t *testing.T) { + type commonResult struct { + Command []string + Stdout string + Stderr string + StdoutTruncated bool + StderrTruncated bool + ExitCode int + OutputPath string + } tests := []struct { name string - req StructuredRunRequest - want string + run func(Runner) (*commonResult, error) }{ { - name: "prompt id", - req: StructuredRunRequest{ - DataPackagePath: "/tmp/hourly.data_package.yaml", - OutputPath: "/tmp/hourly.generated_text.raw.json", + name: "Run", + run: func(runner Runner) (*commonResult, error) { + result, err := runner.Run(context.Background(), RunRequest{ + PromptID: "weather.markdown_report", + DataPackagePath: "/tmp/data_package.yaml", + OutputPath: "/tmp/report.md", + }) + if result == nil { + return nil, err + } + return &commonResult{ + Command: result.Command, + Stdout: result.Stdout, + Stderr: result.Stderr, + StdoutTruncated: result.StdoutTruncated, + StderrTruncated: result.StderrTruncated, + ExitCode: result.ExitCode, + OutputPath: result.OutputPath, + }, err }, - want: "prompt id is required", }, { - name: "data package path", - req: StructuredRunRequest{ - PromptID: "weather.hourly_generated_text", - OutputPath: "/tmp/hourly.generated_text.raw.json", + name: "StructuredRun", + run: func(runner Runner) (*commonResult, error) { + result, err := runner.StructuredRun(context.Background(), StructuredRunRequest{ + PromptID: "weather.markdown_report", + DataPackagePath: "/tmp/data_package.yaml", + OutputPath: "/tmp/report.md", + }) + if result == nil { + return nil, err + } + return &commonResult{ + Command: result.Command, + Stdout: result.Stdout, + Stderr: result.Stderr, + StdoutTruncated: result.StdoutTruncated, + StderrTruncated: result.StderrTruncated, + ExitCode: result.ExitCode, + OutputPath: result.OutputPath, + }, err }, - want: "data package path is required", - }, - { - name: "output path", - req: StructuredRunRequest{ - PromptID: "weather.hourly_generated_text", - DataPackagePath: "/tmp/hourly.data_package.yaml", - }, - want: "output path is required", }, } + for _, test := range tests { t.Run(test.name, func(t *testing.T) { - commands := &fakeCommands{} - runner := Runner{Commands: commands} - result, err := runner.StructuredRun(context.Background(), test.req) + commands := &fakeCommands{result: CommandResult{ + Stdout: []byte("captured stdout"), + Stderr: []byte("captured stderr"), + StdoutTruncated: true, + StderrTruncated: true, + }} + runner := Runner{ + Binary: "/usr/local/bin/scriptorium", + ConfigPath: "/etc/scriptorium.yml", + Profile: "weather", + Timeout: 15 * time.Second, + Commands: commands, + } + + result, err := test.run(runner) + if err != nil { + t.Fatalf("%s error = %v", test.name, err) + } + wantArgs := []string{ + "run", + "--config", "/etc/scriptorium.yml", + "--profile", "weather", + "--prompt", "weather.markdown_report", + "--input", "data_package=/tmp/data_package.yaml", + "--out", "/tmp/report.md", + } + if !reflect.DeepEqual(commands.args, wantArgs) { + t.Fatalf("args = %#v, want %#v", commands.args, wantArgs) + } + if commands.timeout != 15*time.Second { + t.Fatalf("timeout = %s, want 15s", commands.timeout) + } + if !reflect.DeepEqual(result.Command, append([]string{"/usr/local/bin/scriptorium"}, wantArgs...)) { + t.Fatalf("Command = %#v, want full argv", result.Command) + } + if result.Stdout != "captured stdout" || result.Stderr != "captured stderr" { + t.Fatalf("captured output = %q/%q, want stdout/stderr", result.Stdout, result.Stderr) + } + if !result.StdoutTruncated || !result.StderrTruncated { + t.Fatalf("truncation flags = %t/%t, want both true", result.StdoutTruncated, result.StderrTruncated) + } + if result.ExitCode != 0 || result.OutputPath != "/tmp/report.md" { + t.Fatalf("result = %#v, want exit 0 and output path", result) + } + }) + } +} + +func TestOutputRunsReturnCapturedResultForNonzeroExit(t *testing.T) { + type commonResult struct { + Stdout string + Stderr string + StderrTruncated bool + ExitCode int + OutputPath string + } + tests := []struct { + name string + run func(Runner) (*commonResult, error) + wantErr string + }{ + { + name: "Run", + run: func(runner Runner) (*commonResult, error) { + result, err := runner.Run(context.Background(), RunRequest{ + PromptID: "weather.markdown_report", + DataPackagePath: "/tmp/data_package.yaml", + OutputPath: "/tmp/report.md", + }) + if result == nil { + return nil, err + } + return &commonResult{ + Stdout: result.Stdout, + Stderr: result.Stderr, + StderrTruncated: result.StderrTruncated, + ExitCode: result.ExitCode, + OutputPath: result.OutputPath, + }, err + }, + wantErr: "scriptorium run exited with code 7: captured stderr", + }, + { + name: "StructuredRun", + run: func(runner Runner) (*commonResult, error) { + result, err := runner.StructuredRun(context.Background(), StructuredRunRequest{ + PromptID: "weather.markdown_report", + DataPackagePath: "/tmp/data_package.yaml", + OutputPath: "/tmp/report.md", + }) + if result == nil { + return nil, err + } + return &commonResult{ + Stdout: result.Stdout, + Stderr: result.Stderr, + StderrTruncated: result.StderrTruncated, + ExitCode: result.ExitCode, + OutputPath: result.OutputPath, + }, err + }, + wantErr: "scriptorium structured run exited with code 7: captured stderr", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + runner := Runner{ + Commands: &fakeCommands{result: CommandResult{ + Stdout: []byte("captured stdout"), + Stderr: []byte("captured stderr"), + StderrTruncated: true, + ExitCode: 7, + }}, + } + + result, err := test.run(runner) if err == nil { - t.Fatal("StructuredRun() error = nil, want validation error") + t.Fatalf("%s error = nil, want nonzero exit error", test.name) } - if result != nil { - t.Fatalf("StructuredRun() result = %#v, want nil", result) + if result == nil { + t.Fatalf("%s result = nil, want captured result", test.name) } - if !strings.Contains(err.Error(), test.want) { - t.Fatalf("StructuredRun() error = %v, want %q", err, test.want) + if err.Error() != test.wantErr { + t.Fatalf("%s error = %q, want %q", test.name, err.Error(), test.wantErr) } - if commands.calls != 0 { - t.Fatalf("commands calls = %d, want no subprocess execution", commands.calls) + if result.Stdout != "captured stdout" || result.Stderr != "captured stderr" || !result.StderrTruncated { + t.Fatalf("captured result = %#v, want stdout/stderr/truncation", result) + } + if result.ExitCode != 7 || result.OutputPath != "/tmp/report.md" { + t.Fatalf("result = %#v, want exit 7 and output path", result) + } + }) + } +} + +func TestOutputRunsValidateRequiredFieldsBeforeExecution(t *testing.T) { + tests := []struct { + name string + run func(Runner, string, string, string) error + }{ + { + name: "Run", + run: func(runner Runner, promptID string, dataPackagePath string, outputPath string) error { + result, err := runner.Run(context.Background(), RunRequest{ + PromptID: promptID, + DataPackagePath: dataPackagePath, + OutputPath: outputPath, + }) + if result != nil { + return fmt.Errorf("result = %#v, want nil", result) + } + return err + }, + }, + { + name: "StructuredRun", + run: func(runner Runner, promptID string, dataPackagePath string, outputPath string) error { + result, err := runner.StructuredRun(context.Background(), StructuredRunRequest{ + PromptID: promptID, + DataPackagePath: dataPackagePath, + OutputPath: outputPath, + }) + if result != nil { + return fmt.Errorf("result = %#v, want nil", result) + } + return err + }, + }, + } + cases := []struct { + name string + promptID string + dataPackagePath string + outputPath string + want string + }{ + { + name: "prompt id", + dataPackagePath: "/tmp/data_package.yaml", + outputPath: "/tmp/report.md", + want: "prompt id is required", + }, + { + name: "data package path", + promptID: "weather.markdown_report", + outputPath: "/tmp/report.md", + want: "data package path is required", + }, + { + name: "output path", + promptID: "weather.markdown_report", + dataPackagePath: "/tmp/data_package.yaml", + want: "output path is required", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + commands := &fakeCommands{} + err := test.run(Runner{Commands: commands}, tc.promptID, tc.dataPackagePath, tc.outputPath) + if err == nil { + t.Fatalf("%s error = nil, want validation error", test.name) + } + if !strings.Contains(err.Error(), tc.want) { + t.Fatalf("%s error = %v, want %q", test.name, err, tc.want) + } + if commands.calls != 0 { + t.Fatalf("commands calls = %d, want no subprocess execution", commands.calls) + } + }) } }) }