feat(ports): return exit code and bounded stderr from shell runner
The runner discarded stderr, so a bar script failing with 'command not found' surfaced only as 'exit status 127' with no indication of what was missing. Return a CommandResult carrying bounded stdout, bounded stderr, and the exit code so callers can report actionable diagnostics.
- commit
- 456b0b1f84109bc45d7c18277c8e3ac6c7d3de52
- author
- brice · · 2026-07-20 08:01 UTC
- parent
- 8c3ac3a
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
6 files · +121 -35
M internal/adapters/shellcmd/runner.go +18 -7
| @@ -3,6 +3,7 @@ package shellcmd | |||
| 3 | 3 | import ( | |
| 4 | 4 | "bytes" | |
| 5 | 5 | "context" | |
| 6 | + | "errors" | |
| 6 | 7 | "io" | |
| 7 | 8 | "os/exec" | |
| 8 | 9 | "syscall" | |
| @@ -16,8 +17,8 @@ type Runner struct{} | |||
| 16 | 17 | // New returns a production shell command runner. | |
| 17 | 18 | func New() Runner { return Runner{} } | |
| 18 | 19 | ||
| 19 | - | // Run executes spec.Command through sh -c, capturing bounded stdout and discarding stderr. | |
| 20 | - | func (Runner) Run(ctx context.Context, spec ports.CommandSpec) ([]byte, error) { | |
| 20 | + | // Run executes spec.Command through sh -c, capturing bounded stdout and stderr. | |
| 21 | + | func (Runner) Run(ctx context.Context, spec ports.CommandSpec) (ports.CommandResult, error) { | |
| 21 | 22 | cmd := exec.CommandContext(ctx, "sh", "-c", spec.Command) | |
| 22 | 23 | cmd.Env = spec.Env | |
| 23 | 24 | cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} | |
| @@ -28,14 +29,24 @@ func (Runner) Run(ctx context.Context, spec ports.CommandSpec) ([]byte, error) { | |||
| 28 | 29 | } | |
| 29 | 30 | return syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) | |
| 30 | 31 | } | |
| 31 | - | var stdout boundedBuffer | |
| 32 | + | var stdout, stderr boundedBuffer | |
| 32 | 33 | stdout.limit = spec.StdoutLimit | |
| 34 | + | stderr.limit = spec.StderrLimit | |
| 33 | 35 | cmd.Stdout = &stdout | |
| 34 | - | cmd.Stderr = io.Discard | |
| 35 | - | if err := cmd.Run(); err != nil { | |
| 36 | - | return stdout.Bytes(), err | |
| 36 | + | cmd.Stderr = &stderr | |
| 37 | + | ||
| 38 | + | err := cmd.Run() | |
| 39 | + | res := ports.CommandResult{Stdout: stdout.Bytes(), Stderr: stderr.Bytes()} | |
| 40 | + | if err != nil { | |
| 41 | + | var exitErr *exec.ExitError | |
| 42 | + | if errors.As(err, &exitErr) { | |
| 43 | + | res.ExitCode = exitErr.ExitCode() | |
| 44 | + | } else { | |
| 45 | + | res.ExitCode = -1 | |
| 46 | + | } | |
| 47 | + | return res, err | |
| 37 | 48 | } | |
| 38 | - | return stdout.Bytes(), nil | |
| 49 | + | return res, nil | |
| 39 | 50 | } | |
| 40 | 51 | ||
| 41 | 52 | type boundedBuffer struct { | |
| @@ -3,6 +3,7 @@ package shellcmd | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "bytes" | 4 | "bytes" | ||
| 5 | "context" | 5 | "context" | ||
| 6 | + | "errors" | |||
| 6 | "io" | 7 | "io" | ||
| 7 | "os/exec" | 8 | "os/exec" | ||
| 8 | "syscall" | 9 | "syscall" | ||
| @@ -16,8 +17,8 @@ type Runner struct{} | |||||
| 16 | // New returns a production shell command runner. | 17 | // New returns a production shell command runner. | ||
| 17 | func New() Runner { return Runner{} } | 18 | func New() Runner { return Runner{} } | ||
| 18 | 19 | ||||
| 19 | - | // Run executes spec.Command through sh -c, capturing bounded stdout and discarding stderr. | 20 | + | // Run executes spec.Command through sh -c, capturing bounded stdout and stderr. |
| 20 | - | func (Runner) Run(ctx context.Context, spec ports.CommandSpec) ([]byte, error) { | 21 | + | func (Runner) Run(ctx context.Context, spec ports.CommandSpec) (ports.CommandResult, error) { |
| 21 | cmd := exec.CommandContext(ctx, "sh", "-c", spec.Command) | 22 | cmd := exec.CommandContext(ctx, "sh", "-c", spec.Command) | ||
| 22 | cmd.Env = spec.Env | 23 | cmd.Env = spec.Env | ||
| 23 | cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} | 24 | cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} | ||
| @@ -28,14 +29,24 @@ func (Runner) Run(ctx context.Context, spec ports.CommandSpec) ([]byte, error) { | |||||
| 28 | } | 29 | } | ||
| 29 | return syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) | 30 | return syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) | ||
| 30 | } | 31 | } | ||
| 31 | - | var stdout boundedBuffer | 32 | + | var stdout, stderr boundedBuffer |
| 32 | stdout.limit = spec.StdoutLimit | 33 | stdout.limit = spec.StdoutLimit | ||
| 34 | + | stderr.limit = spec.StderrLimit | |||
| 33 | cmd.Stdout = &stdout | 35 | cmd.Stdout = &stdout | ||
| 34 | - | cmd.Stderr = io.Discard | 36 | + | cmd.Stderr = &stderr |
| 35 | - | if err := cmd.Run(); err != nil { | 37 | + | |
| 36 | - | return stdout.Bytes(), err | 38 | + | err := cmd.Run() |
| 39 | + | res := ports.CommandResult{Stdout: stdout.Bytes(), Stderr: stderr.Bytes()} | |||
| 40 | + | if err != nil { | |||
| 41 | + | var exitErr *exec.ExitError | |||
| 42 | + | if errors.As(err, &exitErr) { | |||
| 43 | + | res.ExitCode = exitErr.ExitCode() | |||
| 44 | + | } else { | |||
| 45 | + | res.ExitCode = -1 | |||
| 46 | + | } | |||
| 47 | + | return res, err | |||
| 37 | } | 48 | } | ||
| 38 | - | return stdout.Bytes(), nil | 49 | + | return res, nil |
| 39 | } | 50 | } | ||
| 40 | 51 | ||||
| 41 | type boundedBuffer struct { | 52 | type boundedBuffer struct { | ||
M internal/adapters/shellcmd/runner_test.go +70 -7
| @@ -8,6 +8,7 @@ import ( | |||
| 8 | 8 | "time" | |
| 9 | 9 | ||
| 10 | 10 | "github.com/bnema/vev/internal/ports" | |
| 11 | + | "github.com/stretchr/testify/require" | |
| 11 | 12 | ) | |
| 12 | 13 | ||
| 13 | 14 | func TestRunnerRun(t *testing.T) { | |
| @@ -19,7 +20,7 @@ func TestRunnerRun(t *testing.T) { | |||
| 19 | 20 | }{ | |
| 20 | 21 | {name: "success", command: "printf 'ok'", limit: 1024, want: "ok"}, | |
| 21 | 22 | {name: "bounded stdout", command: "yes a | head -c 4096", limit: 8, want: "a\na\na\na\n"}, | |
| 22 | - | {name: "stderr discarded", command: "printf out; printf err >&2", limit: 1024, want: "out"}, | |
| 23 | + | {name: "stderr does not leak into stdout", command: "printf out; printf err >&2", limit: 1024, want: "out"}, | |
| 23 | 24 | {name: "zero limit captures nothing", command: "printf output", limit: 0, want: ""}, | |
| 24 | 25 | {name: "negative limit captures nothing", command: "printf output", limit: -1, want: ""}, | |
| 25 | 26 | } | |
| @@ -32,8 +33,8 @@ func TestRunnerRun(t *testing.T) { | |||
| 32 | 33 | if err != nil { | |
| 33 | 34 | t.Fatalf("Run() error = %v", err) | |
| 34 | 35 | } | |
| 35 | - | if string(got) != tt.want { | |
| 36 | - | t.Fatalf("Run() = %q, want %q", string(got), tt.want) | |
| 36 | + | if string(got.Stdout) != tt.want { | |
| 37 | + | t.Fatalf("Run() = %q, want %q", string(got.Stdout), tt.want) | |
| 37 | 38 | } | |
| 38 | 39 | }) | |
| 39 | 40 | } | |
| @@ -47,8 +48,8 @@ func TestRunnerRunReturnsPartialStdoutOnError(t *testing.T) { | |||
| 47 | 48 | if err == nil { | |
| 48 | 49 | t.Fatal("Run() error = nil, want non-zero exit error") | |
| 49 | 50 | } | |
| 50 | - | if string(got) != "abc" { | |
| 51 | - | t.Fatalf("Run() = %q, want bounded partial stdout %q", string(got), "abc") | |
| 51 | + | if string(got.Stdout) != "abc" { | |
| 52 | + | t.Fatalf("Run() = %q, want bounded partial stdout %q", string(got.Stdout), "abc") | |
| 52 | 53 | } | |
| 53 | 54 | } | |
| 54 | 55 | ||
| @@ -101,7 +102,69 @@ func TestRunnerRunUsesProvidedEnvironment(t *testing.T) { | |||
| 101 | 102 | if err != nil { | |
| 102 | 103 | t.Fatalf("Run() error = %v", err) | |
| 103 | 104 | } | |
| 104 | - | if strings.TrimSpace(string(got)) != "from-env" { | |
| 105 | - | t.Fatalf("Run() env output = %q, want from-env", string(got)) | |
| 105 | + | if strings.TrimSpace(string(got.Stdout)) != "from-env" { | |
| 106 | + | t.Fatalf("Run() env output = %q, want from-env", string(got.Stdout)) | |
| 106 | 107 | } | |
| 107 | 108 | } | |
| 109 | + | ||
| 110 | + | func TestRunnerCapturesStderrAndExitCode(t *testing.T) { | |
| 111 | + | tests := []struct { | |
| 112 | + | name string | |
| 113 | + | command string | |
| 114 | + | wantExitCode int | |
| 115 | + | wantStderr string | |
| 116 | + | wantStdout string | |
| 117 | + | }{ | |
| 118 | + | { | |
| 119 | + | name: "command not found reports 127 and stderr", | |
| 120 | + | command: "definitely-not-a-real-command-xyz", | |
| 121 | + | wantExitCode: 127, | |
| 122 | + | wantStderr: "not found", | |
| 123 | + | }, | |
| 124 | + | { | |
| 125 | + | name: "explicit exit code with stderr", | |
| 126 | + | command: "echo oops >&2; exit 3", | |
| 127 | + | wantExitCode: 3, | |
| 128 | + | wantStderr: "oops", | |
| 129 | + | }, | |
| 130 | + | { | |
| 131 | + | name: "success reports zero and stdout", | |
| 132 | + | command: "echo hello", | |
| 133 | + | wantExitCode: 0, | |
| 134 | + | wantStdout: "hello", | |
| 135 | + | }, | |
| 136 | + | } | |
| 137 | + | ||
| 138 | + | for _, tc := range tests { | |
| 139 | + | t.Run(tc.name, func(t *testing.T) { | |
| 140 | + | res, err := New().Run(context.Background(), ports.CommandSpec{ | |
| 141 | + | Command: tc.command, | |
| 142 | + | Timeout: 5 * time.Second, | |
| 143 | + | StdoutLimit: 1024, | |
| 144 | + | StderrLimit: 1024, | |
| 145 | + | }) | |
| 146 | + | if tc.wantExitCode == 0 { | |
| 147 | + | require.NoError(t, err) | |
| 148 | + | } else { | |
| 149 | + | require.Error(t, err) | |
| 150 | + | } | |
| 151 | + | require.Equal(t, tc.wantExitCode, res.ExitCode) | |
| 152 | + | if tc.wantStderr != "" { | |
| 153 | + | require.Contains(t, string(res.Stderr), tc.wantStderr) | |
| 154 | + | } | |
| 155 | + | if tc.wantStdout != "" { | |
| 156 | + | require.Contains(t, string(res.Stdout), tc.wantStdout) | |
| 157 | + | } | |
| 158 | + | }) | |
| 159 | + | } | |
| 160 | + | } | |
| 161 | + | ||
| 162 | + | func TestRunnerBoundsStderr(t *testing.T) { | |
| 163 | + | res, _ := New().Run(context.Background(), ports.CommandSpec{ | |
| 164 | + | Command: "yes badness 2>/dev/null | head -c 100000 >&2", | |
| 165 | + | Timeout: 5 * time.Second, | |
| 166 | + | StdoutLimit: 1024, | |
| 167 | + | StderrLimit: 64, | |
| 168 | + | }) | |
| 169 | + | require.LessOrEqual(t, len(res.Stderr), 64) | |
| 170 | + | } | |
| @@ -8,6 +8,7 @@ import ( | |||||
| 8 | "time" | 8 | "time" | ||
| 9 | 9 | ||||
| 10 | "github.com/bnema/vev/internal/ports" | 10 | "github.com/bnema/vev/internal/ports" | ||
| 11 | + | "github.com/stretchr/testify/require" | |||
| 11 | ) | 12 | ) | ||
| 12 | 13 | ||||
| 13 | func TestRunnerRun(t *testing.T) { | 14 | func TestRunnerRun(t *testing.T) { | ||
| @@ -19,7 +20,7 @@ func TestRunnerRun(t *testing.T) { | |||||
| 19 | }{ | 20 | }{ | ||
| 20 | {name: "success", command: "printf 'ok'", limit: 1024, want: "ok"}, | 21 | {name: "success", command: "printf 'ok'", limit: 1024, want: "ok"}, | ||
| 21 | {name: "bounded stdout", command: "yes a | head -c 4096", limit: 8, want: "a\na\na\na\n"}, | 22 | {name: "bounded stdout", command: "yes a | head -c 4096", limit: 8, want: "a\na\na\na\n"}, | ||
| 22 | - | {name: "stderr discarded", command: "printf out; printf err >&2", limit: 1024, want: "out"}, | 23 | + | {name: "stderr does not leak into stdout", command: "printf out; printf err >&2", limit: 1024, want: "out"}, |
| 23 | {name: "zero limit captures nothing", command: "printf output", limit: 0, want: ""}, | 24 | {name: "zero limit captures nothing", command: "printf output", limit: 0, want: ""}, | ||
| 24 | {name: "negative limit captures nothing", command: "printf output", limit: -1, want: ""}, | 25 | {name: "negative limit captures nothing", command: "printf output", limit: -1, want: ""}, | ||
| 25 | } | 26 | } | ||
| @@ -32,8 +33,8 @@ func TestRunnerRun(t *testing.T) { | |||||
| 32 | if err != nil { | 33 | if err != nil { | ||
| 33 | t.Fatalf("Run() error = %v", err) | 34 | t.Fatalf("Run() error = %v", err) | ||
| 34 | } | 35 | } | ||
| 35 | - | if string(got) != tt.want { | 36 | + | if string(got.Stdout) != tt.want { |
| 36 | - | t.Fatalf("Run() = %q, want %q", string(got), tt.want) | 37 | + | t.Fatalf("Run() = %q, want %q", string(got.Stdout), tt.want) |
| 37 | } | 38 | } | ||
| 38 | }) | 39 | }) | ||
| 39 | } | 40 | } | ||
| @@ -47,8 +48,8 @@ func TestRunnerRunReturnsPartialStdoutOnError(t *testing.T) { | |||||
| 47 | if err == nil { | 48 | if err == nil { | ||
| 48 | t.Fatal("Run() error = nil, want non-zero exit error") | 49 | t.Fatal("Run() error = nil, want non-zero exit error") | ||
| 49 | } | 50 | } | ||
| 50 | - | if string(got) != "abc" { | 51 | + | if string(got.Stdout) != "abc" { |
| 51 | - | t.Fatalf("Run() = %q, want bounded partial stdout %q", string(got), "abc") | 52 | + | t.Fatalf("Run() = %q, want bounded partial stdout %q", string(got.Stdout), "abc") |
| 52 | } | 53 | } | ||
| 53 | } | 54 | } | ||
| 54 | 55 | ||||
| @@ -101,7 +102,69 @@ func TestRunnerRunUsesProvidedEnvironment(t *testing.T) { | |||||
| 101 | if err != nil { | 102 | if err != nil { | ||
| 102 | t.Fatalf("Run() error = %v", err) | 103 | t.Fatalf("Run() error = %v", err) | ||
| 103 | } | 104 | } | ||
| 104 | - | if strings.TrimSpace(string(got)) != "from-env" { | 105 | + | if strings.TrimSpace(string(got.Stdout)) != "from-env" { |
| 105 | - | t.Fatalf("Run() env output = %q, want from-env", string(got)) | 106 | + | t.Fatalf("Run() env output = %q, want from-env", string(got.Stdout)) |
| 106 | } | 107 | } | ||
| 107 | } | 108 | } | ||
| 109 | + | ||||
| 110 | + | func TestRunnerCapturesStderrAndExitCode(t *testing.T) { | |||
| 111 | + | tests := []struct { | |||
| 112 | + | name string | |||
| 113 | + | command string | |||
| 114 | + | wantExitCode int | |||
| 115 | + | wantStderr string | |||
| 116 | + | wantStdout string | |||
| 117 | + | }{ | |||
| 118 | + | { | |||
| 119 | + | name: "command not found reports 127 and stderr", | |||
| 120 | + | command: "definitely-not-a-real-command-xyz", | |||
| 121 | + | wantExitCode: 127, | |||
| 122 | + | wantStderr: "not found", | |||
| 123 | + | }, | |||
| 124 | + | { | |||
| 125 | + | name: "explicit exit code with stderr", | |||
| 126 | + | command: "echo oops >&2; exit 3", | |||
| 127 | + | wantExitCode: 3, | |||
| 128 | + | wantStderr: "oops", | |||
| 129 | + | }, | |||
| 130 | + | { | |||
| 131 | + | name: "success reports zero and stdout", | |||
| 132 | + | command: "echo hello", | |||
| 133 | + | wantExitCode: 0, | |||
| 134 | + | wantStdout: "hello", | |||
| 135 | + | }, | |||
| 136 | + | } | |||
| 137 | + | ||||
| 138 | + | for _, tc := range tests { | |||
| 139 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 140 | + | res, err := New().Run(context.Background(), ports.CommandSpec{ | |||
| 141 | + | Command: tc.command, | |||
| 142 | + | Timeout: 5 * time.Second, | |||
| 143 | + | StdoutLimit: 1024, | |||
| 144 | + | StderrLimit: 1024, | |||
| 145 | + | }) | |||
| 146 | + | if tc.wantExitCode == 0 { | |||
| 147 | + | require.NoError(t, err) | |||
| 148 | + | } else { | |||
| 149 | + | require.Error(t, err) | |||
| 150 | + | } | |||
| 151 | + | require.Equal(t, tc.wantExitCode, res.ExitCode) | |||
| 152 | + | if tc.wantStderr != "" { | |||
| 153 | + | require.Contains(t, string(res.Stderr), tc.wantStderr) | |||
| 154 | + | } | |||
| 155 | + | if tc.wantStdout != "" { | |||
| 156 | + | require.Contains(t, string(res.Stdout), tc.wantStdout) | |||
| 157 | + | } | |||
| 158 | + | }) | |||
| 159 | + | } | |||
| 160 | + | } | |||
| 161 | + | ||||
| 162 | + | func TestRunnerBoundsStderr(t *testing.T) { | |||
| 163 | + | res, _ := New().Run(context.Background(), ports.CommandSpec{ | |||
| 164 | + | Command: "yes badness 2>/dev/null | head -c 100000 >&2", | |||
| 165 | + | Timeout: 5 * time.Second, | |||
| 166 | + | StdoutLimit: 1024, | |||
| 167 | + | StderrLimit: 64, | |||
| 168 | + | }) | |||
| 169 | + | require.LessOrEqual(t, len(res.Stderr), 64) | |||
| 170 | + | } | |||
M internal/ports/mocks/shell_mock.go +8 -10
| @@ -39,24 +39,22 @@ func (_m *MockShellCommandRunner) EXPECT() *MockShellCommandRunner_Expecter { | |||
| 39 | 39 | } | |
| 40 | 40 | ||
| 41 | 41 | // Run provides a mock function for the type MockShellCommandRunner | |
| 42 | - | func (_mock *MockShellCommandRunner) Run(ctx context.Context, spec ports.CommandSpec) ([]byte, error) { | |
| 42 | + | func (_mock *MockShellCommandRunner) Run(ctx context.Context, spec ports.CommandSpec) (ports.CommandResult, error) { | |
| 43 | 43 | ret := _mock.Called(ctx, spec) | |
| 44 | 44 | ||
| 45 | 45 | if len(ret) == 0 { | |
| 46 | 46 | panic("no return value specified for Run") | |
| 47 | 47 | } | |
| 48 | 48 | ||
| 49 | - | var r0 []byte | |
| 49 | + | var r0 ports.CommandResult | |
| 50 | 50 | var r1 error | |
| 51 | - | if returnFunc, ok := ret.Get(0).(func(context.Context, ports.CommandSpec) ([]byte, error)); ok { | |
| 51 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, ports.CommandSpec) (ports.CommandResult, error)); ok { | |
| 52 | 52 | return returnFunc(ctx, spec) | |
| 53 | 53 | } | |
| 54 | - | if returnFunc, ok := ret.Get(0).(func(context.Context, ports.CommandSpec) []byte); ok { | |
| 54 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, ports.CommandSpec) ports.CommandResult); ok { | |
| 55 | 55 | r0 = returnFunc(ctx, spec) | |
| 56 | 56 | } else { | |
| 57 | - | if ret.Get(0) != nil { | |
| 58 | - | r0 = ret.Get(0).([]byte) | |
| 59 | - | } | |
| 57 | + | r0 = ret.Get(0).(ports.CommandResult) | |
| 60 | 58 | } | |
| 61 | 59 | if returnFunc, ok := ret.Get(1).(func(context.Context, ports.CommandSpec) error); ok { | |
| 62 | 60 | r1 = returnFunc(ctx, spec) | |
| @@ -96,12 +94,12 @@ func (_c *MockShellCommandRunner_Run_Call) Run(run func(ctx context.Context, spe | |||
| 96 | 94 | return _c | |
| 97 | 95 | } | |
| 98 | 96 | ||
| 99 | - | func (_c *MockShellCommandRunner_Run_Call) Return(bytes []byte, err error) *MockShellCommandRunner_Run_Call { | |
| 100 | - | _c.Call.Return(bytes, err) | |
| 97 | + | func (_c *MockShellCommandRunner_Run_Call) Return(commandResult ports.CommandResult, err error) *MockShellCommandRunner_Run_Call { | |
| 98 | + | _c.Call.Return(commandResult, err) | |
| 101 | 99 | return _c | |
| 102 | 100 | } | |
| 103 | 101 | ||
| 104 | - | func (_c *MockShellCommandRunner_Run_Call) RunAndReturn(run func(ctx context.Context, spec ports.CommandSpec) ([]byte, error)) *MockShellCommandRunner_Run_Call { | |
| 102 | + | func (_c *MockShellCommandRunner_Run_Call) RunAndReturn(run func(ctx context.Context, spec ports.CommandSpec) (ports.CommandResult, error)) *MockShellCommandRunner_Run_Call { | |
| 105 | 103 | _c.Call.Return(run) | |
| 106 | 104 | return _c | |
| 107 | 105 | } | |
| @@ -39,24 +39,22 @@ func (_m *MockShellCommandRunner) EXPECT() *MockShellCommandRunner_Expecter { | |||||
| 39 | } | 39 | } | ||
| 40 | 40 | ||||
| 41 | // Run provides a mock function for the type MockShellCommandRunner | 41 | // Run provides a mock function for the type MockShellCommandRunner | ||
| 42 | - | func (_mock *MockShellCommandRunner) Run(ctx context.Context, spec ports.CommandSpec) ([]byte, error) { | 42 | + | func (_mock *MockShellCommandRunner) Run(ctx context.Context, spec ports.CommandSpec) (ports.CommandResult, error) { |
| 43 | ret := _mock.Called(ctx, spec) | 43 | ret := _mock.Called(ctx, spec) | ||
| 44 | 44 | ||||
| 45 | if len(ret) == 0 { | 45 | if len(ret) == 0 { | ||
| 46 | panic("no return value specified for Run") | 46 | panic("no return value specified for Run") | ||
| 47 | } | 47 | } | ||
| 48 | 48 | ||||
| 49 | - | var r0 []byte | 49 | + | var r0 ports.CommandResult |
| 50 | var r1 error | 50 | var r1 error | ||
| 51 | - | if returnFunc, ok := ret.Get(0).(func(context.Context, ports.CommandSpec) ([]byte, error)); ok { | 51 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, ports.CommandSpec) (ports.CommandResult, error)); ok { |
| 52 | return returnFunc(ctx, spec) | 52 | return returnFunc(ctx, spec) | ||
| 53 | } | 53 | } | ||
| 54 | - | if returnFunc, ok := ret.Get(0).(func(context.Context, ports.CommandSpec) []byte); ok { | 54 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, ports.CommandSpec) ports.CommandResult); ok { |
| 55 | r0 = returnFunc(ctx, spec) | 55 | r0 = returnFunc(ctx, spec) | ||
| 56 | } else { | 56 | } else { | ||
| 57 | - | if ret.Get(0) != nil { | 57 | + | r0 = ret.Get(0).(ports.CommandResult) |
| 58 | - | r0 = ret.Get(0).([]byte) | |||
| 59 | - | } | |||
| 60 | } | 58 | } | ||
| 61 | if returnFunc, ok := ret.Get(1).(func(context.Context, ports.CommandSpec) error); ok { | 59 | if returnFunc, ok := ret.Get(1).(func(context.Context, ports.CommandSpec) error); ok { | ||
| 62 | r1 = returnFunc(ctx, spec) | 60 | r1 = returnFunc(ctx, spec) | ||
| @@ -96,12 +94,12 @@ func (_c *MockShellCommandRunner_Run_Call) Run(run func(ctx context.Context, spe | |||||
| 96 | return _c | 94 | return _c | ||
| 97 | } | 95 | } | ||
| 98 | 96 | ||||
| 99 | - | func (_c *MockShellCommandRunner_Run_Call) Return(bytes []byte, err error) *MockShellCommandRunner_Run_Call { | 97 | + | func (_c *MockShellCommandRunner_Run_Call) Return(commandResult ports.CommandResult, err error) *MockShellCommandRunner_Run_Call { |
| 100 | - | _c.Call.Return(bytes, err) | 98 | + | _c.Call.Return(commandResult, err) |
| 101 | return _c | 99 | return _c | ||
| 102 | } | 100 | } | ||
| 103 | 101 | ||||
| 104 | - | func (_c *MockShellCommandRunner_Run_Call) RunAndReturn(run func(ctx context.Context, spec ports.CommandSpec) ([]byte, error)) *MockShellCommandRunner_Run_Call { | 102 | + | func (_c *MockShellCommandRunner_Run_Call) RunAndReturn(run func(ctx context.Context, spec ports.CommandSpec) (ports.CommandResult, error)) *MockShellCommandRunner_Run_Call { |
| 105 | _c.Call.Return(run) | 103 | _c.Call.Return(run) | ||
| 106 | return _c | 104 | return _c | ||
| 107 | } | 105 | } | ||
M internal/ports/shell.go +12 -2
| @@ -11,9 +11,19 @@ type CommandSpec struct { | |||
| 11 | 11 | Env []string | |
| 12 | 12 | Timeout time.Duration | |
| 13 | 13 | StdoutLimit int | |
| 14 | + | StderrLimit int | |
| 14 | 15 | } | |
| 15 | 16 | ||
| 16 | - | // ShellCommandRunner runs shell commands and returns bounded stdout bytes. | |
| 17 | + | // CommandResult carries bounded output and the process exit status. ExitCode is | |
| 18 | + | // 0 when the command ran to completion successfully, the process exit code when | |
| 19 | + | // it ran and failed, and -1 when the command could not be run at all. | |
| 20 | + | type CommandResult struct { | |
| 21 | + | Stdout []byte | |
| 22 | + | Stderr []byte | |
| 23 | + | ExitCode int | |
| 24 | + | } | |
| 25 | + | ||
| 26 | + | // ShellCommandRunner runs shell commands and returns bounded output. | |
| 17 | 27 | type ShellCommandRunner interface { | |
| 18 | - | Run(ctx context.Context, spec CommandSpec) ([]byte, error) | |
| 28 | + | Run(ctx context.Context, spec CommandSpec) (CommandResult, error) | |
| 19 | 29 | } | |
| @@ -11,9 +11,19 @@ type CommandSpec struct { | |||||
| 11 | Env []string | 11 | Env []string | ||
| 12 | Timeout time.Duration | 12 | Timeout time.Duration | ||
| 13 | StdoutLimit int | 13 | StdoutLimit int | ||
| 14 | + | StderrLimit int | |||
| 14 | } | 15 | } | ||
| 15 | 16 | ||||
| 16 | - | // ShellCommandRunner runs shell commands and returns bounded stdout bytes. | 17 | + | // CommandResult carries bounded output and the process exit status. ExitCode is |
| 18 | + | // 0 when the command ran to completion successfully, the process exit code when | |||
| 19 | + | // it ran and failed, and -1 when the command could not be run at all. | |||
| 20 | + | type CommandResult struct { | |||
| 21 | + | Stdout []byte | |||
| 22 | + | Stderr []byte | |||
| 23 | + | ExitCode int | |||
| 24 | + | } | |||
| 25 | + | ||||
| 26 | + | // ShellCommandRunner runs shell commands and returns bounded output. | |||
| 17 | type ShellCommandRunner interface { | 27 | type ShellCommandRunner interface { | ||
| 18 | - | Run(ctx context.Context, spec CommandSpec) ([]byte, error) | 28 | + | Run(ctx context.Context, spec CommandSpec) (CommandResult, error) |
| 19 | } | 29 | } | ||
M internal/usecase/daemon/bar_script.go +4 -2
| @@ -14,6 +14,7 @@ import ( | |||
| 14 | 14 | ||
| 15 | 15 | const ( | |
| 16 | 16 | barScriptOutputLimit = 1024 | |
| 17 | + | barScriptStderrLimit = 512 | |
| 17 | 18 | barScriptDisplayLimit = 256 | |
| 18 | 19 | barScriptTimeout = time.Second | |
| 19 | 20 | ) | |
| @@ -71,16 +72,17 @@ func (r barScriptRunner) run(ctx context.Context, command string, env []string, | |||
| 71 | 72 | ctx, cancel := context.WithTimeout(ctx, timeout) | |
| 72 | 73 | defer cancel() | |
| 73 | 74 | ||
| 74 | - | stdout, err := r.runner.Run(ctx, ports.CommandSpec{ | |
| 75 | + | res, err := r.runner.Run(ctx, ports.CommandSpec{ | |
| 75 | 76 | Command: command, | |
| 76 | 77 | Env: scriptCtx.env(env), | |
| 77 | 78 | Timeout: timeout, | |
| 78 | 79 | StdoutLimit: barScriptOutputLimit, | |
| 80 | + | StderrLimit: barScriptStderrLimit, | |
| 79 | 81 | }) | |
| 80 | 82 | if errors.Is(ctx.Err(), context.DeadlineExceeded) { | |
| 81 | 83 | return "", context.DeadlineExceeded | |
| 82 | 84 | } | |
| 83 | - | text := sanitizeBarScriptOutput(stdout, barScriptOutputLimit) | |
| 85 | + | text := sanitizeBarScriptOutput(res.Stdout, barScriptOutputLimit) | |
| 84 | 86 | if err != nil { | |
| 85 | 87 | return text, err | |
| 86 | 88 | } | |
| @@ -14,6 +14,7 @@ import ( | |||||
| 14 | 14 | ||||
| 15 | const ( | 15 | const ( | ||
| 16 | barScriptOutputLimit = 1024 | 16 | barScriptOutputLimit = 1024 | ||
| 17 | + | barScriptStderrLimit = 512 | |||
| 17 | barScriptDisplayLimit = 256 | 18 | barScriptDisplayLimit = 256 | ||
| 18 | barScriptTimeout = time.Second | 19 | barScriptTimeout = time.Second | ||
| 19 | ) | 20 | ) | ||
| @@ -71,16 +72,17 @@ func (r barScriptRunner) run(ctx context.Context, command string, env []string, | |||||
| 71 | ctx, cancel := context.WithTimeout(ctx, timeout) | 72 | ctx, cancel := context.WithTimeout(ctx, timeout) | ||
| 72 | defer cancel() | 73 | defer cancel() | ||
| 73 | 74 | ||||
| 74 | - | stdout, err := r.runner.Run(ctx, ports.CommandSpec{ | 75 | + | res, err := r.runner.Run(ctx, ports.CommandSpec{ |
| 75 | Command: command, | 76 | Command: command, | ||
| 76 | Env: scriptCtx.env(env), | 77 | Env: scriptCtx.env(env), | ||
| 77 | Timeout: timeout, | 78 | Timeout: timeout, | ||
| 78 | StdoutLimit: barScriptOutputLimit, | 79 | StdoutLimit: barScriptOutputLimit, | ||
| 80 | + | StderrLimit: barScriptStderrLimit, | |||
| 79 | }) | 81 | }) | ||
| 80 | if errors.Is(ctx.Err(), context.DeadlineExceeded) { | 82 | if errors.Is(ctx.Err(), context.DeadlineExceeded) { | ||
| 81 | return "", context.DeadlineExceeded | 83 | return "", context.DeadlineExceeded | ||
| 82 | } | 84 | } | ||
| 83 | - | text := sanitizeBarScriptOutput(stdout, barScriptOutputLimit) | 85 | + | text := sanitizeBarScriptOutput(res.Stdout, barScriptOutputLimit) |
| 84 | if err != nil { | 86 | if err != nil { | ||
| 85 | return text, err | 87 | return text, err | ||
| 86 | } | 88 | } | ||
M internal/usecase/daemon/bar_script_test.go +9 -7
| @@ -119,7 +119,7 @@ func TestBarScriptRunner(t *testing.T) { | |||
| 119 | 119 | containsEnv(spec.Env, "VEV_PANE_CWD=/repo") && | |
| 120 | 120 | containsEnv(spec.Env, "VEV_COLS=120") && | |
| 121 | 121 | !containsEnv(spec.Env, "VEV_ANCHOR=old") | |
| 122 | - | })).Return([]byte("\x1b[31mok\x1b[0m\nignored"), nil) | |
| 122 | + | })).Return(ports.CommandResult{Stdout: []byte("\x1b[31mok\x1b[0m\nignored")}, nil) | |
| 123 | 123 | runner := barScriptRunner{runner: portRunner, timeout: 50 * time.Millisecond} | |
| 124 | 124 | ||
| 125 | 125 | got, err := runner.run(context.Background(), "printf 'ok\\nignored'", | |
| @@ -136,7 +136,8 @@ func TestBarScriptRunner(t *testing.T) { | |||
| 136 | 136 | t.Run("port failure returns sanitized partial stdout and error", func(t *testing.T) { | |
| 137 | 137 | wantErr := errors.New("exit 1") | |
| 138 | 138 | portRunner := portsmocks.NewMockShellCommandRunner(t) | |
| 139 | - | portRunner.EXPECT().Run(mock.Anything, mock.Anything).Return([]byte("partial\x00 output\nignored"), wantErr) | |
| 139 | + | portRunner.EXPECT().Run(mock.Anything, mock.Anything). | |
| 140 | + | Return(ports.CommandResult{Stdout: []byte("partial\x00 output\nignored"), ExitCode: 1}, wantErr) | |
| 140 | 141 | runner := barScriptRunner{runner: portRunner, timeout: time.Second} | |
| 141 | 142 | ||
| 142 | 143 | got, err := runner.run(context.Background(), "false", nil, barScriptContext{}) | |
| @@ -150,10 +151,11 @@ func TestBarScriptRunner(t *testing.T) { | |||
| 150 | 151 | ||
| 151 | 152 | t.Run("timeout propagates deadline", func(t *testing.T) { | |
| 152 | 153 | portRunner := portsmocks.NewMockShellCommandRunner(t) | |
| 153 | - | portRunner.EXPECT().Run(mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, _ ports.CommandSpec) ([]byte, error) { | |
| 154 | - | <-ctx.Done() | |
| 155 | - | return nil, ctx.Err() | |
| 156 | - | }) | |
| 154 | + | portRunner.EXPECT().Run(mock.Anything, mock.Anything).RunAndReturn( | |
| 155 | + | func(ctx context.Context, _ ports.CommandSpec) (ports.CommandResult, error) { | |
| 156 | + | <-ctx.Done() | |
| 157 | + | return ports.CommandResult{ExitCode: -1}, ctx.Err() | |
| 158 | + | }) | |
| 157 | 159 | runner := barScriptRunner{runner: portRunner, timeout: time.Nanosecond} | |
| 158 | 160 | ||
| 159 | 161 | _, err := runner.run(context.Background(), "sleep 1", nil, barScriptContext{}) | |
| @@ -167,7 +169,7 @@ func TestBarScriptRunnerBoundsStdoutBeforeSanitize(t *testing.T) { | |||
| 167 | 169 | portRunner := portsmocks.NewMockShellCommandRunner(t) | |
| 168 | 170 | portRunner.EXPECT().Run(mock.Anything, mock.MatchedBy(func(spec ports.CommandSpec) bool { | |
| 169 | 171 | return spec.StdoutLimit == barScriptOutputLimit | |
| 170 | - | })).Return([]byte(strings.Repeat("a", 4096)), nil) | |
| 172 | + | })).Return(ports.CommandResult{Stdout: []byte(strings.Repeat("a", 4096))}, nil) | |
| 171 | 173 | runner := barScriptRunner{runner: portRunner, timeout: time.Second} | |
| 172 | 174 | ||
| 173 | 175 | got, err := runner.run(context.Background(), "long-output", nil, barScriptContext{}) | |
| @@ -119,7 +119,7 @@ func TestBarScriptRunner(t *testing.T) { | |||||
| 119 | containsEnv(spec.Env, "VEV_PANE_CWD=/repo") && | 119 | containsEnv(spec.Env, "VEV_PANE_CWD=/repo") && | ||
| 120 | containsEnv(spec.Env, "VEV_COLS=120") && | 120 | containsEnv(spec.Env, "VEV_COLS=120") && | ||
| 121 | !containsEnv(spec.Env, "VEV_ANCHOR=old") | 121 | !containsEnv(spec.Env, "VEV_ANCHOR=old") | ||
| 122 | - | })).Return([]byte("\x1b[31mok\x1b[0m\nignored"), nil) | 122 | + | })).Return(ports.CommandResult{Stdout: []byte("\x1b[31mok\x1b[0m\nignored")}, nil) |
| 123 | runner := barScriptRunner{runner: portRunner, timeout: 50 * time.Millisecond} | 123 | runner := barScriptRunner{runner: portRunner, timeout: 50 * time.Millisecond} | ||
| 124 | 124 | ||||
| 125 | got, err := runner.run(context.Background(), "printf 'ok\\nignored'", | 125 | got, err := runner.run(context.Background(), "printf 'ok\\nignored'", | ||
| @@ -136,7 +136,8 @@ func TestBarScriptRunner(t *testing.T) { | |||||
| 136 | t.Run("port failure returns sanitized partial stdout and error", func(t *testing.T) { | 136 | t.Run("port failure returns sanitized partial stdout and error", func(t *testing.T) { | ||
| 137 | wantErr := errors.New("exit 1") | 137 | wantErr := errors.New("exit 1") | ||
| 138 | portRunner := portsmocks.NewMockShellCommandRunner(t) | 138 | portRunner := portsmocks.NewMockShellCommandRunner(t) | ||
| 139 | - | portRunner.EXPECT().Run(mock.Anything, mock.Anything).Return([]byte("partial\x00 output\nignored"), wantErr) | 139 | + | portRunner.EXPECT().Run(mock.Anything, mock.Anything). |
| 140 | + | Return(ports.CommandResult{Stdout: []byte("partial\x00 output\nignored"), ExitCode: 1}, wantErr) | |||
| 140 | runner := barScriptRunner{runner: portRunner, timeout: time.Second} | 141 | runner := barScriptRunner{runner: portRunner, timeout: time.Second} | ||
| 141 | 142 | ||||
| 142 | got, err := runner.run(context.Background(), "false", nil, barScriptContext{}) | 143 | got, err := runner.run(context.Background(), "false", nil, barScriptContext{}) | ||
| @@ -150,10 +151,11 @@ func TestBarScriptRunner(t *testing.T) { | |||||
| 150 | 151 | ||||
| 151 | t.Run("timeout propagates deadline", func(t *testing.T) { | 152 | t.Run("timeout propagates deadline", func(t *testing.T) { | ||
| 152 | portRunner := portsmocks.NewMockShellCommandRunner(t) | 153 | portRunner := portsmocks.NewMockShellCommandRunner(t) | ||
| 153 | - | portRunner.EXPECT().Run(mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, _ ports.CommandSpec) ([]byte, error) { | 154 | + | portRunner.EXPECT().Run(mock.Anything, mock.Anything).RunAndReturn( |
| 154 | - | <-ctx.Done() | 155 | + | func(ctx context.Context, _ ports.CommandSpec) (ports.CommandResult, error) { |
| 155 | - | return nil, ctx.Err() | 156 | + | <-ctx.Done() |
| 156 | - | }) | 157 | + | return ports.CommandResult{ExitCode: -1}, ctx.Err() |
| 158 | + | }) | |||
| 157 | runner := barScriptRunner{runner: portRunner, timeout: time.Nanosecond} | 159 | runner := barScriptRunner{runner: portRunner, timeout: time.Nanosecond} | ||
| 158 | 160 | ||||
| 159 | _, err := runner.run(context.Background(), "sleep 1", nil, barScriptContext{}) | 161 | _, err := runner.run(context.Background(), "sleep 1", nil, barScriptContext{}) | ||
| @@ -167,7 +169,7 @@ func TestBarScriptRunnerBoundsStdoutBeforeSanitize(t *testing.T) { | |||||
| 167 | portRunner := portsmocks.NewMockShellCommandRunner(t) | 169 | portRunner := portsmocks.NewMockShellCommandRunner(t) | ||
| 168 | portRunner.EXPECT().Run(mock.Anything, mock.MatchedBy(func(spec ports.CommandSpec) bool { | 170 | portRunner.EXPECT().Run(mock.Anything, mock.MatchedBy(func(spec ports.CommandSpec) bool { | ||
| 169 | return spec.StdoutLimit == barScriptOutputLimit | 171 | return spec.StdoutLimit == barScriptOutputLimit | ||
| 170 | - | })).Return([]byte(strings.Repeat("a", 4096)), nil) | 172 | + | })).Return(ports.CommandResult{Stdout: []byte(strings.Repeat("a", 4096))}, nil) |
| 171 | runner := barScriptRunner{runner: portRunner, timeout: time.Second} | 173 | runner := barScriptRunner{runner: portRunner, timeout: time.Second} | ||
| 172 | 174 | ||||
| 173 | got, err := runner.run(context.Background(), "long-output", nil, barScriptContext{}) | 175 | got, err := runner.run(context.Background(), "long-output", nil, barScriptContext{}) | ||