From 25a33f027a50e976945e8885b481544af8cd6f2f Mon Sep 17 00:00:00 2001 From: Felix Martin Date: Sun, 31 May 2026 14:14:04 -0400 Subject: [PATCH] M2: surface codex empty-output and clean CLI error messages Addresses code-review follow-ups: codex now returns a specific error when its -o file is empty rather than degrading to ErrEmptyResponse downstream; CLI errors omit a dangling stderr suffix when stderr is empty. Co-Authored-By: Claude Opus 4.8 --- internal/ai/backend.go | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/internal/ai/backend.go b/internal/ai/backend.go index c83376e..46b41a5 100644 --- a/internal/ai/backend.go +++ b/internal/ai/backend.go @@ -32,11 +32,20 @@ func (b claudeBackend) Run(ctx context.Context, prompt string) (string, error) { cmd.Stdout = &out cmd.Stderr = &errb if err := cmd.Run(); err != nil { - return "", fmt.Errorf("claude: %w: %s", err, strings.TrimSpace(errb.String())) + return "", cmdError("claude", err, errb.String()) } return out.String(), nil } +// cmdError wraps a CLI failure, appending stderr only when it is non-empty so +// the message does not end in a dangling ": ". +func cmdError(name string, err error, stderr string) error { + if s := strings.TrimSpace(stderr); s != "" { + return fmt.Errorf("%s: %w: %s", name, err, s) + } + return fmt.Errorf("%s: %w", name, err) +} + // codexBackend shells out to `codex exec` headlessly. codex's stdout includes // session preamble, so the final answer is written to a temp file via -o and // read back. The --ignore-* and read-only flags stop it from running shell @@ -79,12 +88,18 @@ func (b codexBackend) Run(ctx context.Context, prompt string) (string, error) { var errb bytes.Buffer cmd.Stderr = &errb // stdout is noisy and intentionally discarded if err := cmd.Run(); err != nil { - return "", fmt.Errorf("codex: %w: %s", err, strings.TrimSpace(errb.String())) + return "", cmdError("codex", err, errb.String()) } data, err := os.ReadFile(path) if err != nil { return "", fmt.Errorf("codex: read output: %w", err) } + // A zero-exit run that wrote nothing means the -o file was never populated + // (e.g. flag/version skew); surface that as a codex error instead of letting + // it masquerade downstream as an empty model answer. + if len(bytes.TrimSpace(data)) == 0 { + return "", fmt.Errorf("codex: empty output file: %s", strings.TrimSpace(errb.String())) + } return string(data), nil }