21725698798

Committed 05 Feb 2026 07:35PM UTC coverage: 80.789% (+0.4%) from 80.435%

Build # 21725698798

Build Type

Pull #67

github

Committed by

umputun

Commit Message

fix: strengthen codex eval prompt to prevent premature signal

claude was emitting CODEX_REVIEW_DONE after fixing issues instead of
stopping to let codex verify fixes. add explicit instructions that
the signal must only be emitted when codex reports no findings.

Pull Request Pull Request #67: feat: custom external review support

Run Details

222 of 252 new or added lines in 9 files covered. (88.1%)

2 existing lines in 1 file now uncovered.

4525 of 5601 relevant lines covered (80.79%)

152.02 hits per line

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

84.16

/pkg/executor/custom.go

package executor

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

// CustomRunner abstracts command execution for custom review scripts.
// Returns stdout reader and a wait function for completion.
type CustomRunner interface {
        Run(ctx context.Context, script, promptFile string) (stdout io.Reader, wait func() error, err error)
}

// execCustomRunner is the default command runner using os/exec.
type execCustomRunner struct{}

func (r *execCustomRunner) Run(ctx context.Context, script, promptFile string) (io.Reader, func() error, error) {
        // check context before starting to avoid spawning a process that will be immediately killed
        if err := ctx.Err(); err != nil {
                return nil, 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(script, promptFile) //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)

        stdout, err := cmd.StdoutPipe()
        if err != nil {
                return nil, nil, fmt.Errorf("stdout pipe: %w", err)
        }
        // merge stderr into stdout
        cmd.Stderr = cmd.Stdout

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

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

        return stdout, cleanup.Wait, nil
}

// CustomExecutor runs custom review scripts and streams output.
type CustomExecutor struct {
        Script        string            // path to the custom review script
        OutputHandler func(text string) // called for each output line, can be nil
        ErrorPatterns []string          // patterns to detect in output (e.g., rate limit messages)
        runner        CustomRunner      // for testing, nil uses default
}

// SetRunner sets the custom runner for testing purposes.
func (e *CustomExecutor) SetRunner(r CustomRunner) {
        e.runner = r
}

// Run executes the custom review script with the prompt content written to a temp file.
// The script receives the path to the prompt file as its single argument.
// Output is streamed line-by-line to OutputHandler.
func (e *CustomExecutor) Run(ctx context.Context, promptContent string) Result {
        if e.Script == "" {
                return Result{Error: errors.New("custom review script not configured")}
        }

        // write prompt to temp file
        promptFile, err := os.CreateTemp("", "ralphex-custom-prompt-*.txt")
        if err != nil {
                return Result{Error: fmt.Errorf("create prompt file: %w", err)}
        }
        promptPath := promptFile.Name()
        defer os.Remove(promptPath) //nolint:errcheck // cleanup temp file

        if _, writeErr := promptFile.WriteString(promptContent); writeErr != nil {
                promptFile.Close()
                return Result{Error: fmt.Errorf("write prompt file: %w", writeErr)}
        }
        if closeErr := promptFile.Close(); closeErr != nil {
                return Result{Error: fmt.Errorf("close prompt file: %w", closeErr)}
        }

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

        stdout, wait, err := runner.Run(ctx, e.Script, promptPath)
        if err != nil {
                return Result{Error: fmt.Errorf("start custom script: %w", err)}
        }

        // process stdout for output and signal detection
        output, signal, streamErr := e.processOutput(ctx, stdout)

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

        // determine final error
        var finalErr error
        switch {
        case streamErr != nil:
                finalErr = streamErr
        case waitErr != nil:
                if ctx.Err() != nil {
                        finalErr = fmt.Errorf("context error: %w", ctx.Err())
                } else {
                        finalErr = fmt.Errorf("custom script exited with error: %w", waitErr)
                }
        }

        // check for error patterns in output
        if pattern := checkErrorPatterns(output, e.ErrorPatterns); pattern != "" {
                return Result{
                        Output: output,
                        Signal: signal,
                        Error:  &PatternMatchError{Pattern: pattern, HelpCmd: e.Script + " --help"},
                }
        }

        return Result{Output: output, Signal: signal, Error: finalErr}
}

// processOutput reads stdout line-by-line, streams to OutputHandler, and detects signals.
func (e *CustomExecutor) processOutput(ctx context.Context, r io.Reader) (output, signal string, err error) {
        var outputBuf []byte
        scanner := bufio.NewScanner(r)
        // increase buffer size for large output lines
        buf := make([]byte, 0, 64*1024)
        scanner.Buffer(buf, maxScannerBuffer)

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

                line := scanner.Text()
                outputBuf = append(outputBuf, line...)
                outputBuf = append(outputBuf, '\n')

                if e.OutputHandler != nil {
                        e.OutputHandler(line + "\n")
                }

                // check for signals in each line
                if sig := detectSignal(line); sig != "" {
                        signal = sig
                }
        }

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

1	package executor
2
3	import (
4	"bufio"
5	"context"
6	"errors"
7	"fmt"
8	"io"
9	"os"
10	"os/exec"
11	)
12
13	// CustomRunner abstracts command execution for custom review scripts.
14	// Returns stdout reader and a wait function for completion.
15	type CustomRunner interface {
16	Run(ctx context.Context, script, promptFile string) (stdout io.Reader, wait func() error, err error)
17	}
18
19	// execCustomRunner is the default command runner using os/exec.
20	type execCustomRunner struct{}
21
22	func (r *execCustomRunner) Run(ctx context.Context, script, promptFile string) (io.Reader, func() error, error) {	3✔
23	// check context before starting to avoid spawning a process that will be immediately killed	3✔
24	if err := ctx.Err(); err != nil {	4✔
25	return nil, nil, fmt.Errorf("context already canceled: %w", err)	1✔
26	}	1✔
27
28	// use exec.Command (not CommandContext) because we handle cancellation ourselves
29	// to ensure the entire process group is killed, not just the direct child
30	cmd := exec.Command(script, promptFile) //nolint:noctx // intentional: we handle context cancellation via process group kill	2✔
31		2✔
32	// create new process group so we can kill all descendants on cleanup	2✔
33	setupProcessGroup(cmd)	2✔
34		2✔
35	stdout, err := cmd.StdoutPipe()	2✔
36	if err != nil {	2✔
NEW 37	return nil, nil, fmt.Errorf("stdout pipe: %w", err)	×
NEW 38	}	×
39	// merge stderr into stdout
40	cmd.Stderr = cmd.Stdout	2✔
41		2✔
42	if err := cmd.Start(); err != nil {	3✔
43	return nil, nil, fmt.Errorf("start command: %w", err)	1✔
44	}	1✔
45
46	// setup process group cleanup with graceful shutdown on context cancellation
47	cleanup := newProcessGroupCleanup(cmd, ctx.Done())	1✔
48		1✔
49	return stdout, cleanup.Wait, nil	1✔
50	}
51
52	// CustomExecutor runs custom review scripts and streams output.
53	type CustomExecutor struct {
54	Script string // path to the custom review script
55	OutputHandler func(text string) // called for each output line, can be nil
56	ErrorPatterns []string // patterns to detect in output (e.g., rate limit messages)
57	runner CustomRunner // for testing, nil uses default
58	}
59
60	// SetRunner sets the custom runner for testing purposes.
NEW 61	func (e *CustomExecutor) SetRunner(r CustomRunner) {	×
NEW 62	e.runner = r	×
NEW 63	}	×
64
65	// Run executes the custom review script with the prompt content written to a temp file.
66	// The script receives the path to the prompt file as its single argument.
67	// Output is streamed line-by-line to OutputHandler.
68	func (e *CustomExecutor) Run(ctx context.Context, promptContent string) Result {	19✔
69	if e.Script == "" {	20✔
70	return Result{Error: errors.New("custom review script not configured")}	1✔
71	}	1✔
72
73	// write prompt to temp file
74	promptFile, err := os.CreateTemp("", "ralphex-custom-prompt-*.txt")	18✔
75	if err != nil {	18✔
NEW 76	return Result{Error: fmt.Errorf("create prompt file: %w", err)}	×
NEW 77	}	×
78	promptPath := promptFile.Name()	18✔
79	defer os.Remove(promptPath) //nolint:errcheck // cleanup temp file	18✔
80		18✔
81	if _, writeErr := promptFile.WriteString(promptContent); writeErr != nil {	18✔
NEW 82	promptFile.Close()	×
NEW 83	return Result{Error: fmt.Errorf("write prompt file: %w", writeErr)}	×
NEW 84	}	×
85	if closeErr := promptFile.Close(); closeErr != nil {	18✔
NEW 86	return Result{Error: fmt.Errorf("close prompt file: %w", closeErr)}	×
NEW 87	}	×
88
89	runner := e.runner	18✔
90	if runner == nil {	18✔
NEW 91	runner = &execCustomRunner{}	×
NEW 92	}	×
93
94	stdout, wait, err := runner.Run(ctx, e.Script, promptPath)	18✔
95	if err != nil {	19✔
96	return Result{Error: fmt.Errorf("start custom script: %w", err)}	1✔
97	}	1✔
98
99	// process stdout for output and signal detection
100	output, signal, streamErr := e.processOutput(ctx, stdout)	17✔
101		17✔
102	// wait for command completion	17✔
103	waitErr := wait()	17✔
104		17✔
105	// determine final error	17✔
106	var finalErr error	17✔
107	switch {	17✔
NEW 108	case streamErr != nil:	×
NEW 109	finalErr = streamErr	×
110	case waitErr != nil:	2✔
111	if ctx.Err() != nil {	3✔
112	finalErr = fmt.Errorf("context error: %w", ctx.Err())	1✔
113	} else {	2✔
114	finalErr = fmt.Errorf("custom script exited with error: %w", waitErr)	1✔
115	}	1✔
116	}
117
118	// check for error patterns in output
119	if pattern := checkErrorPatterns(output, e.ErrorPatterns); pattern != "" {	19✔
120	return Result{	2✔
121	Output: output,	2✔
122	Signal: signal,	2✔
123	Error: &PatternMatchError{Pattern: pattern, HelpCmd: e.Script + " --help"},	2✔
124	}	2✔
125	}	2✔
126
127	return Result{Output: output, Signal: signal, Error: finalErr}	15✔
128	}
129
130	// processOutput reads stdout line-by-line, streams to OutputHandler, and detects signals.
131	func (e *CustomExecutor) processOutput(ctx context.Context, r io.Reader) (output, signal string, err error) {	19✔
132	var outputBuf []byte	19✔
133	scanner := bufio.NewScanner(r)	19✔
134	// increase buffer size for large output lines	19✔
135	buf := make([]byte, 0, 64*1024)	19✔
136	scanner.Buffer(buf, maxScannerBuffer)	19✔
137		19✔
138	for scanner.Scan() {	50✔
139	select {	31✔
140	case <-ctx.Done():	1✔
141	return string(outputBuf), signal, fmt.Errorf("context done: %w", ctx.Err())	1✔
142	default:	30✔
143	}
144
145	line := scanner.Text()	30✔
146	outputBuf = append(outputBuf, line...)	30✔
147	outputBuf = append(outputBuf, '\n')	30✔
148		30✔
149	if e.OutputHandler != nil {	36✔
150	e.OutputHandler(line + "\n")	6✔
151	}	6✔
152
153	// check for signals in each line
154	if sig := detectSignal(line); sig != "" {	39✔
155	signal = sig	9✔
156	}	9✔
157	}
158
159	if err := scanner.Err(); err != nil {	19✔
160	return string(outputBuf), signal, fmt.Errorf("read output: %w", err)	1✔
161	}	1✔
162	return string(outputBuf), signal, nil	17✔
163	}

umputun / ralphex / 21725698798

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