21342539068

Committed 26 Jan 2026 12:40AM UTC coverage: 78.564% (+0.3%) from 78.234%

Build # 21342539068

Build Type

Pull #21

github

Committed by

melonamin

Commit Message

fix: address codex review findings

Add pre-start context check to both Claude and Codex runners to avoid
spawning processes when the context is already canceled. This addresses
the valid finding that switching from exec.CommandContext to exec.Command
lost the pre-start cancellation behavior.

Note: The "Windows build break risk" finding was rejected as Windows
support is explicitly excluded in CLAUDE.md - the project only targets
Linux/macOS due to syscall.Flock usage for progress file locking.

Pull Request Pull Request #21: Fix: kill entire process group on context cancellation

Coverage Stats

58 of 71 new or added lines in 3 files covered. (81.69%)

2998 of 3816 relevant lines covered (78.56%)

67.12 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

94.01

/pkg/executor/codex.go

package executor

import (
        "bufio"
        "context"
        "errors"
        "fmt"
        "io"
        "os/exec"
        "strings"
)

// CodexStreams holds both stderr and stdout from codex command.
type CodexStreams struct {
        Stderr io.Reader
        Stdout io.Reader
}

// CodexRunner abstracts command execution for codex.
// Returns both stderr (streaming progress) and stdout (final response).
type CodexRunner interface {
        Run(ctx context.Context, name string, args ...string) (streams CodexStreams, wait func() error, err error)
}

// execCodexRunner is the default command runner using os/exec for codex.
// codex outputs streaming progress to stderr, final response to stdout.
type execCodexRunner struct{}

func (r *execCodexRunner) Run(ctx context.Context, name string, args ...string) (CodexStreams, func() error, error) {
        // check context before starting to avoid spawning a process that will be immediately killed
        if err := ctx.Err(); err != nil {
                return CodexStreams{}, nil, fmt.Errorf("context already canceled: %w", err)
        }

        // use exec.Command (not CommandContext) because we handle cancellation ourselves
        // to ensure the entire process group is killed, not just the direct child
        cmd := exec.Command(name, args...) //nolint:noctx // intentional: we handle context cancellation via process group kill

        // create new process group so we can kill all descendants on cleanup
        setupProcessGroup(cmd)

        stderr, err := cmd.StderrPipe()
        if err != nil {
                return CodexStreams{}, nil, fmt.Errorf("stderr pipe: %w", err)
        }

        stdout, err := cmd.StdoutPipe()
        if err != nil {
                return CodexStreams{}, nil, fmt.Errorf("stdout pipe: %w", err)
        }

        if err := cmd.Start(); err != nil {
                return CodexStreams{}, nil, fmt.Errorf("start command: %w", err)
        }

        // setup process group cleanup with graceful shutdown on context cancellation
        cleanup := newProcessGroupCleanup(cmd, ctx.Done())

        return CodexStreams{Stderr: stderr, Stdout: stdout}, cleanup.Wait, nil
}

// CodexExecutor runs codex CLI commands and filters output.
type CodexExecutor struct {
        Command         string            // command to execute, defaults to "codex"
        Model           string            // model to use, defaults to gpt-5.2-codex
        ReasoningEffort string            // reasoning effort level, defaults to "xhigh"
        TimeoutMs       int               // stream idle timeout in ms, defaults to 3600000
        Sandbox         string            // sandbox mode, defaults to "read-only"
        ProjectDoc      string            // path to project documentation file
        OutputHandler   func(text string) // called for each filtered output line in real-time
        Debug           bool              // enable debug output
        runner          CodexRunner       // for testing, nil uses default
}

// codexFilterState tracks header separator count for filtering.
type codexFilterState struct {
        headerCount int             // tracks "--------" separators seen (show content between first two)
        seen        map[string]bool // track all shown lines for deduplication
}

// Run executes codex CLI with the given prompt and returns filtered output.
// stderr is streamed line-by-line to OutputHandler for progress indication.
// stdout is captured entirely as the final response (returned in Result.Output).
func (e *CodexExecutor) Run(ctx context.Context, prompt string) Result {
        cmd := e.Command
        if cmd == "" {
                cmd = "codex"
        }

        model := e.Model
        if model == "" {
                model = "gpt-5.2-codex"
        }

        reasoningEffort := e.ReasoningEffort
        if reasoningEffort == "" {
                reasoningEffort = "xhigh"
        }

        timeoutMs := e.TimeoutMs
        if timeoutMs <= 0 {
                timeoutMs = 3600000
        }

        sandbox := e.Sandbox
        if sandbox == "" {
                sandbox = "read-only"
        }

        args := []string{
                "exec",
                "--sandbox", sandbox,
                "-c", fmt.Sprintf("model=%q", model),
                "-c", "model_reasoning_effort=" + reasoningEffort,
                "-c", fmt.Sprintf("stream_idle_timeout_ms=%d", timeoutMs),
        }

        if e.ProjectDoc != "" {
                args = append(args, "-c", fmt.Sprintf("project_doc=%q", e.ProjectDoc))
        }

        args = append(args, prompt)

        runner := e.runner
        if runner == nil {
                runner = &execCodexRunner{}
        }

        streams, wait, err := runner.Run(ctx, cmd, args...)
        if err != nil {
                return Result{Error: fmt.Errorf("start codex: %w", err)}
        }

        // process stderr for progress display (header block + bold summaries)
        stderrDone := make(chan error, 1)
        go func() {
                stderrDone <- e.processStderr(ctx, streams.Stderr)
        }()

        // read stdout entirely as final response
        stdoutContent, stdoutErr := e.readStdout(streams.Stdout)

        // wait for stderr processing to complete
        stderrErr := <-stderrDone

        // wait for command completion
        waitErr := wait()

        // determine final error (prefer stderr/stdout errors over wait error)
        var finalErr error
        switch {
        case stderrErr != nil && !errors.Is(stderrErr, context.Canceled):
                finalErr = stderrErr
        case stdoutErr != nil:
                finalErr = stdoutErr
        case waitErr != nil:
                if ctx.Err() != nil {
                        finalErr = fmt.Errorf("context error: %w", ctx.Err())
                } else {
                        finalErr = fmt.Errorf("codex exited with error: %w", waitErr)
                }
        }

        // detect signal in stdout (the actual response)
        signal := detectSignal(stdoutContent)

        // return stdout content as the result (the actual answer from codex)
        return Result{Output: stdoutContent, Signal: signal, Error: finalErr}
}

// processStderr reads stderr line-by-line, filters for progress display.
// shows header block (between first two "--------" separators) and bold summaries.
func (e *CodexExecutor) processStderr(ctx context.Context, r io.Reader) error {
        state := &codexFilterState{}
        scanner := bufio.NewScanner(r)
        // increase buffer size for large output lines (16MB max)
        buf := make([]byte, 0, 64*1024)
        scanner.Buffer(buf, 16*1024*1024)

        for scanner.Scan() {
                select {
                case <-ctx.Done():
                        return fmt.Errorf("context done: %w", ctx.Err())
                default:
                }

                line := scanner.Text()
                if show, filtered := e.shouldDisplay(line, state); show {
                        if e.OutputHandler != nil {
                                e.OutputHandler(filtered + "\n")
                        }
                }
        }

        if err := scanner.Err(); err != nil {
                return fmt.Errorf("read stderr: %w", err)
        }
        return nil
}

// readStdout reads the entire stdout content as the final response.
func (e *CodexExecutor) readStdout(r io.Reader) (string, error) {
        data, err := io.ReadAll(r)
        if err != nil {
                return "", fmt.Errorf("read stdout: %w", err)
        }
        return string(data), nil
}

// shouldDisplay implements a simple filter for codex stderr output.
// shows: header block (between first two "--------" separators) and bold summaries.
// also deduplicates lines to avoid non-consecutive repeats.
func (e *CodexExecutor) shouldDisplay(line string, state *codexFilterState) (bool, string) {
        s := strings.TrimSpace(line)
        if s == "" {
                return false, ""
        }

        var show bool
        var filtered string
        var skipDedup bool // separators are not deduplicated

        switch {
        case strings.HasPrefix(s, "--------"):
                // track "--------" separators for header block
                state.headerCount++
                show = state.headerCount <= 2 // show first two separators
                filtered = line
                skipDedup = true // don't deduplicate separators
        case state.headerCount == 1:
                // show everything between first two separators (header block)
                show = true
                filtered = line
        case strings.HasPrefix(s, "**"):
                // show bold summaries after header (progress indication)
                show = true
                filtered = e.stripBold(s)
        }

        // check for duplicates before returning (except separators)
        if show && !skipDedup {
                if state.seen == nil {
                        state.seen = make(map[string]bool)
                }
                if state.seen[filtered] {
                        return false, "" // skip duplicate
                }
                state.seen[filtered] = true
        }

        return show, filtered
}

// stripBold removes markdown bold markers (**text**) from text.
func (e *CodexExecutor) stripBold(s string) string {
        // replace **text** with text
        result := s
        for {
                start := strings.Index(result, "**")
                if start == -1 {
                        break
                }
                end := strings.Index(result[start+2:], "**")
                if end == -1 {
                        break
                }
                // remove both markers
                result = result[:start] + result[start+2:start+2+end] + result[start+2+end+2:]
        }
        return result
}

1	package executor
2
3	import (
4	"bufio"
5	"context"
6	"errors"
7	"fmt"
8	"io"
9	"os/exec"
10	"strings"
11	)
12
13	// CodexStreams holds both stderr and stdout from codex command.
14	type CodexStreams struct {
15	Stderr io.Reader
16	Stdout io.Reader
17	}
18
19	// CodexRunner abstracts command execution for codex.
20	// Returns both stderr (streaming progress) and stdout (final response).
21	type CodexRunner interface {
22	Run(ctx context.Context, name string, args ...string) (streams CodexStreams, wait func() error, err error)
23	}
24
25	// execCodexRunner is the default command runner using os/exec for codex.
26	// codex outputs streaming progress to stderr, final response to stdout.
27	type execCodexRunner struct{}
28
29	func (r *execCodexRunner) Run(ctx context.Context, name string, args ...string) (CodexStreams, func() error, error) {	2✔
30	// check context before starting to avoid spawning a process that will be immediately killed	2✔
31	if err := ctx.Err(); err != nil {	2✔
NEW 32	return CodexStreams{}, nil, fmt.Errorf("context already canceled: %w", err)	×
NEW 33	}	×
34
35	// use exec.Command (not CommandContext) because we handle cancellation ourselves
36	// to ensure the entire process group is killed, not just the direct child
37	cmd := exec.Command(name, args...) //nolint:noctx // intentional: we handle context cancellation via process group kill	2✔
38		2✔
39	// create new process group so we can kill all descendants on cleanup	2✔
40	setupProcessGroup(cmd)	2✔
41		2✔
42	stderr, err := cmd.StderrPipe()	2✔
43	if err != nil {	2✔
44	return CodexStreams{}, nil, fmt.Errorf("stderr pipe: %w", err)	×
45	}	×
46
47	stdout, err := cmd.StdoutPipe()	2✔
48	if err != nil {	2✔
49	return CodexStreams{}, nil, fmt.Errorf("stdout pipe: %w", err)	×
50	}	×
51
52	if err := cmd.Start(); err != nil {	3✔
53	return CodexStreams{}, nil, fmt.Errorf("start command: %w", err)	1✔
54	}	1✔
55
56	// setup process group cleanup with graceful shutdown on context cancellation
57	cleanup := newProcessGroupCleanup(cmd, ctx.Done())	1✔
58		1✔
59	return CodexStreams{Stderr: stderr, Stdout: stdout}, cleanup.Wait, nil	1✔
60	}
61
62	// CodexExecutor runs codex CLI commands and filters output.
63	type CodexExecutor struct {
64	Command string // command to execute, defaults to "codex"
65	Model string // model to use, defaults to gpt-5.2-codex
66	ReasoningEffort string // reasoning effort level, defaults to "xhigh"
67	TimeoutMs int // stream idle timeout in ms, defaults to 3600000
68	Sandbox string // sandbox mode, defaults to "read-only"
69	ProjectDoc string // path to project documentation file
70	OutputHandler func(text string) // called for each filtered output line in real-time
71	Debug bool // enable debug output
72	runner CodexRunner // for testing, nil uses default
73	}
74
75	// codexFilterState tracks header separator count for filtering.
76	type codexFilterState struct {
77	headerCount int // tracks "--------" separators seen (show content between first two)
78	seen map[string]bool // track all shown lines for deduplication
79	}
80
81	// Run executes codex CLI with the given prompt and returns filtered output.
82	// stderr is streamed line-by-line to OutputHandler for progress indication.
83	// stdout is captured entirely as the final response (returned in Result.Output).
84	func (e *CodexExecutor) Run(ctx context.Context, prompt string) Result {	11✔
85	cmd := e.Command	11✔
86	if cmd == "" {	21✔
87	cmd = "codex"	10✔
88	}	10✔
89
90	model := e.Model	11✔
91	if model == "" {	21✔
92	model = "gpt-5.2-codex"	10✔
93	}	10✔
94
95	reasoningEffort := e.ReasoningEffort	11✔
96	if reasoningEffort == "" {	21✔
97	reasoningEffort = "xhigh"	10✔
98	}	10✔
99
100	timeoutMs := e.TimeoutMs	11✔
101	if timeoutMs <= 0 {	21✔
102	timeoutMs = 3600000	10✔
103	}	10✔
104
105	sandbox := e.Sandbox	11✔
106	if sandbox == "" {	21✔
107	sandbox = "read-only"	10✔
108	}	10✔
109
110	args := []string{	11✔
111	"exec",	11✔
112	"--sandbox", sandbox,	11✔
113	"-c", fmt.Sprintf("model=%q", model),	11✔
114	"-c", "model_reasoning_effort=" + reasoningEffort,	11✔
115	"-c", fmt.Sprintf("stream_idle_timeout_ms=%d", timeoutMs),	11✔
116	}	11✔
117		11✔
118	if e.ProjectDoc != "" {	12✔
119	args = append(args, "-c", fmt.Sprintf("project_doc=%q", e.ProjectDoc))	1✔
120	}	1✔
121
122	args = append(args, prompt)	11✔
123		11✔
124	runner := e.runner	11✔
125	if runner == nil {	11✔
126	runner = &execCodexRunner{}	×
127	}	×
128
129	streams, wait, err := runner.Run(ctx, cmd, args...)	11✔
130	if err != nil {	12✔
131	return Result{Error: fmt.Errorf("start codex: %w", err)}	1✔
132	}	1✔
133
134	// process stderr for progress display (header block + bold summaries)
135	stderrDone := make(chan error, 1)	10✔
136	go func() {	20✔
137	stderrDone <- e.processStderr(ctx, streams.Stderr)	10✔
138	}()	10✔
139
140	// read stdout entirely as final response
141	stdoutContent, stdoutErr := e.readStdout(streams.Stdout)	10✔
142		10✔
143	// wait for stderr processing to complete	10✔
144	stderrErr := <-stderrDone	10✔
145		10✔
146	// wait for command completion	10✔
147	waitErr := wait()	10✔
148		10✔
149	// determine final error (prefer stderr/stdout errors over wait error)	10✔
150	var finalErr error	10✔
151	switch {	10✔
152	case stderrErr != nil && !errors.Is(stderrErr, context.Canceled):	1✔
153	finalErr = stderrErr	1✔
154	case stdoutErr != nil:	×
155	finalErr = stdoutErr	×
156	case waitErr != nil:	2✔
157	if ctx.Err() != nil {	3✔
158	finalErr = fmt.Errorf("context error: %w", ctx.Err())	1✔
159	} else {	2✔
160	finalErr = fmt.Errorf("codex exited with error: %w", waitErr)	1✔
161	}	1✔
162	}
163
164	// detect signal in stdout (the actual response)
165	signal := detectSignal(stdoutContent)	10✔
166		10✔
167	// return stdout content as the result (the actual answer from codex)	10✔
168	return Result{Output: stdoutContent, Signal: signal, Error: finalErr}	10✔
169	}
170
171	// processStderr reads stderr line-by-line, filters for progress display.
172	// shows header block (between first two "--------" separators) and bold summaries.
173	func (e *CodexExecutor) processStderr(ctx context.Context, r io.Reader) error {	15✔
174	state := &codexFilterState{}	15✔
175	scanner := bufio.NewScanner(r)	15✔
176	// increase buffer size for large output lines (16MB max)	15✔
177	buf := make([]byte, 0, 64*1024)	15✔
178	scanner.Buffer(buf, 1610241024)	15✔
179		15✔
180	for scanner.Scan() {	50✔
181	select {	35✔
182	case <-ctx.Done():	1✔
183	return fmt.Errorf("context done: %w", ctx.Err())	1✔
184	default:	34✔
185	}
186
187	line := scanner.Text()	34✔
188	if show, filtered := e.shouldDisplay(line, state); show {	63✔
189	if e.OutputHandler != nil {	49✔
190	e.OutputHandler(filtered + "\n")	20✔
191	}	20✔
192	}
193	}
194
195	if err := scanner.Err(); err != nil {	16✔
196	return fmt.Errorf("read stderr: %w", err)	2✔
197	}	2✔
198	return nil	12✔
199	}
200
201	// readStdout reads the entire stdout content as the final response.
202	func (e *CodexExecutor) readStdout(r io.Reader) (string, error) {	12✔
203	data, err := io.ReadAll(r)	12✔
204	if err != nil {	13✔
205	return "", fmt.Errorf("read stdout: %w", err)	1✔
206	}	1✔
207	return string(data), nil	11✔
208	}
209
210	// shouldDisplay implements a simple filter for codex stderr output.
211	// shows: header block (between first two "--------" separators) and bold summaries.
212	// also deduplicates lines to avoid non-consecutive repeats.
213	func (e CodexExecutor) shouldDisplay(line string, state codexFilterState) (bool, string) {	71✔
214	s := strings.TrimSpace(line)	71✔
215	if s == "" {	75✔
216	return false, ""	4✔
217	}	4✔
218
219	var show bool	67✔
220	var filtered string	67✔
221	var skipDedup bool // separators are not deduplicated	67✔
222		67✔
223	switch {	67✔
224	case strings.HasPrefix(s, "--------"):	26✔
225	// track "--------" separators for header block	26✔
226	state.headerCount++	26✔
227	show = state.headerCount <= 2 // show first two separators	26✔
228	filtered = line	26✔
229	skipDedup = true // don't deduplicate separators	26✔
230	case state.headerCount == 1:	17✔
231	// show everything between first two separators (header block)	17✔
232	show = true	17✔
233	filtered = line	17✔
234	case strings.HasPrefix(s, "**"):	16✔
235	// show bold summaries after header (progress indication)	16✔
236	show = true	16✔
237	filtered = e.stripBold(s)	16✔
238	}
239
240	// check for duplicates before returning (except separators)
241	if show && !skipDedup {	100✔
242	if state.seen == nil {	48✔
243	state.seen = make(map[string]bool)	15✔
244	}	15✔
245	if state.seen[filtered] {	37✔
246	return false, "" // skip duplicate	4✔
247	}	4✔
248	state.seen[filtered] = true	29✔
249	}
250
251	return show, filtered	63✔
252	}
253
254	// stripBold removes markdown bold markers (text) from text.
255	func (e *CodexExecutor) stripBold(s string) string {	22✔
256	// replace text with text	22✔
257	result := s	22✔
258	for {	65✔
259	start := strings.Index(result, "**")	43✔
260	if start == -1 {	64✔
261	break	21✔
262	}
263	end := strings.Index(result[start+2:], "**")	22✔
264	if end == -1 {	23✔
265	break	1✔
266	}
267	// remove both markers
268	result = result[:start] + result[start+2:start+2+end] + result[start+2+end+2:]	21✔
269	}
270	return result	22✔
271	}

umputun / ralphex / 21342539068

Source File Press 'n' to go to next uncovered line, 'b' for previous

Source File
Press 'n' to go to next uncovered line, 'b' for previous