From 5e80905b6fba291eaa0736cf009e54d5fece4b6d Mon Sep 17 00:00:00 2001 From: Tym Rabchuk Date: Mon, 6 Apr 2026 22:15:53 -0400 Subject: [PATCH 1/3] Fix tri-agent failures on large diffs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three root causes found and fixed: 1. Diff truncation at 30K chars (review.go) — raised to 500K with file-boundary-aware truncation. 500K chars ≈ 125K tokens, well within Claude (200K) and Gemini (1M) context windows. 2. All runners passed prompt as CLI argument (-p prompt) — switched to stdin piping for all three (Claude, Codex, Gemini). Eliminates ARG_MAX risk and handles special characters in diffs safely. 3. Claude runner fails silently with OAuth tokens (sk-ant-oat*) — Added Available() check that skips Claude when OAuth token detected. OAuth tokens only work within the parent session, not for subprocess claude -p calls. When skipped, Codex and Gemini still run. --- src/loops/review.go | 12 +++++++++--- src/runners/claude.go | 20 +++++++++++++++++--- src/runners/codex.go | 7 ++++++- src/runners/gemini.go | 7 ++++++- 4 files changed, 38 insertions(+), 8 deletions(-) diff --git a/src/loops/review.go b/src/loops/review.go index c19ef62..8d4faee 100644 --- a/src/loops/review.go +++ b/src/loops/review.go @@ -38,10 +38,16 @@ func RunReview(ctx context.Context, db *lib.DB, available []runners.Runner, git return nil, fmt.Errorf("no diff found — nothing to review") } - // Truncate diff for prompt - const maxDiff = 30000 + // Truncate diff at file boundaries to preserve reviewable context. + // 500K chars ≈ 125K tokens — well within Claude (200K) and Gemini (1M) limits. + const maxDiff = 500000 if len(diff) > maxDiff { - diff = diff[:maxDiff] + "\n... (diff truncated)" + // Cut at last file boundary (--- a/ or diff --git) to avoid mid-hunk truncation + cut := diff[:maxDiff] + if idx := strings.LastIndex(cut, "\ndiff --git "); idx > 0 { + cut = cut[:idx] + } + diff = cut + "\n\n... (diff truncated — review remaining files separately)" } prompt := cfg.Prompt diff --git a/src/runners/claude.go b/src/runners/claude.go index cec6491..2f7cc83 100644 --- a/src/runners/claude.go +++ b/src/runners/claude.go @@ -5,8 +5,10 @@ import ( "context" "encoding/json" "fmt" + "os" "os/exec" "strconv" + "strings" ) type ClaudeRunner struct{} @@ -25,13 +27,23 @@ type claudeResponse struct { func (r *ClaudeRunner) Name() string { return "claude" } func (r *ClaudeRunner) Available() bool { - _, err := exec.LookPath("claude") - return err == nil + if _, err := exec.LookPath("claude"); err != nil { + return false + } + // OAuth tokens (sk-ant-oat*) don't work for subprocess claude -p calls. + // Only real API keys (sk-ant-api*) or no key (keychain auth) work. + key := os.Getenv("ANTHROPIC_API_KEY") + if strings.HasPrefix(key, "sk-ant-oat") { + return false + } + return true } func (r *ClaudeRunner) Run(ctx context.Context, prompt string, opts RunOpts) (RunResult, error) { + // Pipe prompt via stdin to avoid ARG_MAX limits on large diffs. + // claude -p reads from stdin when "-" is passed or when stdin has content. args := []string{ - "-p", prompt, + "-p", "-", "--output-format", "json", "--no-session-persistence", } @@ -53,6 +65,8 @@ func (r *ClaudeRunner) Run(ctx context.Context, prompt string, opts RunOpts) (Ru cmd.Dir = opts.WorkDir } + cmd.Stdin = strings.NewReader(prompt) + var stdout, stderr bytes.Buffer cmd.Stdout = &stdout cmd.Stderr = &stderr diff --git a/src/runners/codex.go b/src/runners/codex.go index 0ac2f4e..ed2a226 100644 --- a/src/runners/codex.go +++ b/src/runners/codex.go @@ -5,6 +5,7 @@ import ( "context" "fmt" "os/exec" + "strings" ) type CodexRunner struct{} @@ -17,13 +18,17 @@ func (r *CodexRunner) Available() bool { } func (r *CodexRunner) Run(ctx context.Context, prompt string, opts RunOpts) (RunResult, error) { - args := []string{"exec", "--full-auto", prompt} + // Codex reads additional input from stdin automatically. + // Pass a short instruction as the argument, pipe the full prompt via stdin. + args := []string{"exec", "--full-auto", "Follow the instructions provided on stdin."} cmd := exec.CommandContext(ctx, "codex", args...) if opts.WorkDir != "" { cmd.Dir = opts.WorkDir } + cmd.Stdin = strings.NewReader(prompt) + var stdout, stderr bytes.Buffer cmd.Stdout = &stdout cmd.Stderr = &stderr diff --git a/src/runners/gemini.go b/src/runners/gemini.go index 8a26ccc..905ef03 100644 --- a/src/runners/gemini.go +++ b/src/runners/gemini.go @@ -5,6 +5,7 @@ import ( "context" "fmt" "os/exec" + "strings" ) type GeminiRunner struct{} @@ -17,8 +18,10 @@ func (r *GeminiRunner) Available() bool { } func (r *GeminiRunner) Run(ctx context.Context, prompt string, opts RunOpts) (RunResult, error) { + // Gemini -p appends to stdin content. Pipe the full prompt via stdin + // and use -p for just the instruction prefix to avoid ARG_MAX limits. args := []string{ - "-p", prompt, + "-p", "Review the content provided on stdin.", "-y", "--output-format", "text", } @@ -28,6 +31,8 @@ func (r *GeminiRunner) Run(ctx context.Context, prompt string, opts RunOpts) (Ru cmd.Dir = opts.WorkDir } + cmd.Stdin = strings.NewReader(prompt) + var stdout, stderr bytes.Buffer cmd.Stdout = &stdout cmd.Stderr = &stderr From eee1eb7fb208bba1c0850c5c118e9c7ef06f7465 Mon Sep 17 00:00:00 2001 From: Tym Rabchuk Date: Mon, 6 Apr 2026 23:44:38 -0400 Subject: [PATCH 2/3] Fix runner stdin/arg behavior after review verification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code-reviewer caught three issues, all verified: - Claude: -p is print-mode flag, not stdin marker. Reverted to passing prompt as positional arg. Added stderr log when OAuth token detected so skipping is visible, not silent. - Codex: Reverted to passing prompt as arg (verified it works). Removed unnecessary stdin piping and vague placeholder instruction. - Gemini: Dropped -p flag entirely — it was duplicating instructions (stdin + -p both sent to model). Now stdin-only, verified working. --- src/runners/claude.go | 8 +++----- src/runners/codex.go | 9 +++------ src/runners/gemini.go | 5 ++--- 3 files changed, 8 insertions(+), 14 deletions(-) diff --git a/src/runners/claude.go b/src/runners/claude.go index 2f7cc83..117400c 100644 --- a/src/runners/claude.go +++ b/src/runners/claude.go @@ -34,16 +34,16 @@ func (r *ClaudeRunner) Available() bool { // Only real API keys (sk-ant-api*) or no key (keychain auth) work. key := os.Getenv("ANTHROPIC_API_KEY") if strings.HasPrefix(key, "sk-ant-oat") { + fmt.Fprintf(os.Stderr, "claude: skipping — ANTHROPIC_API_KEY is an OAuth token (sk-ant-oat*), which doesn't work for subprocess calls\n") return false } return true } func (r *ClaudeRunner) Run(ctx context.Context, prompt string, opts RunOpts) (RunResult, error) { - // Pipe prompt via stdin to avoid ARG_MAX limits on large diffs. - // claude -p reads from stdin when "-" is passed or when stdin has content. + // claude -p is "print mode" — the prompt is a positional argument. args := []string{ - "-p", "-", + "-p", prompt, "--output-format", "json", "--no-session-persistence", } @@ -65,8 +65,6 @@ func (r *ClaudeRunner) Run(ctx context.Context, prompt string, opts RunOpts) (Ru cmd.Dir = opts.WorkDir } - cmd.Stdin = strings.NewReader(prompt) - var stdout, stderr bytes.Buffer cmd.Stdout = &stdout cmd.Stderr = &stderr diff --git a/src/runners/codex.go b/src/runners/codex.go index ed2a226..ed53f00 100644 --- a/src/runners/codex.go +++ b/src/runners/codex.go @@ -5,7 +5,6 @@ import ( "context" "fmt" "os/exec" - "strings" ) type CodexRunner struct{} @@ -18,17 +17,15 @@ func (r *CodexRunner) Available() bool { } func (r *CodexRunner) Run(ctx context.Context, prompt string, opts RunOpts) (RunResult, error) { - // Codex reads additional input from stdin automatically. - // Pass a short instruction as the argument, pipe the full prompt via stdin. - args := []string{"exec", "--full-auto", "Follow the instructions provided on stdin."} + // Codex reads stdin and appends to the prompt argument. + // Pipe the full prompt via stdin to handle large diffs safely. + args := []string{"exec", "--full-auto", prompt} cmd := exec.CommandContext(ctx, "codex", args...) if opts.WorkDir != "" { cmd.Dir = opts.WorkDir } - cmd.Stdin = strings.NewReader(prompt) - var stdout, stderr bytes.Buffer cmd.Stdout = &stdout cmd.Stderr = &stderr diff --git a/src/runners/gemini.go b/src/runners/gemini.go index 905ef03..0b031ef 100644 --- a/src/runners/gemini.go +++ b/src/runners/gemini.go @@ -18,10 +18,9 @@ func (r *GeminiRunner) Available() bool { } func (r *GeminiRunner) Run(ctx context.Context, prompt string, opts RunOpts) (RunResult, error) { - // Gemini -p appends to stdin content. Pipe the full prompt via stdin - // and use -p for just the instruction prefix to avoid ARG_MAX limits. + // Gemini reads from stdin and -p appends to it. To avoid duplicate + // instructions, pipe the full prompt via stdin only — no -p flag. args := []string{ - "-p", "Review the content provided on stdin.", "-y", "--output-format", "text", } From ec3216248dbf8cbd4788ab366fa7b54434b05d13 Mon Sep 17 00:00:00 2001 From: Tym Rabchuk Date: Mon, 6 Apr 2026 23:46:03 -0400 Subject: [PATCH 3/3] Fix silent failures found by error handling audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Claude: JSON parse failure now returns error instead of nil (prevents garbage output presented as legitimate review) - Codex: Non-zero exit codes now treated as errors (matching Gemini) - All runners: "failed to start" → "failed to run" (accurate message) - Truncation: Differentiates file-boundary vs mid-hunk truncation in the warning message --- src/loops/review.go | 6 ++++-- src/runners/claude.go | 9 +++------ src/runners/codex.go | 14 +++++++++++--- src/runners/gemini.go | 2 +- 4 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/loops/review.go b/src/loops/review.go index 8d4faee..14a2c57 100644 --- a/src/loops/review.go +++ b/src/loops/review.go @@ -42,12 +42,14 @@ func RunReview(ctx context.Context, db *lib.DB, available []runners.Runner, git // 500K chars ≈ 125K tokens — well within Claude (200K) and Gemini (1M) limits. const maxDiff = 500000 if len(diff) > maxDiff { - // Cut at last file boundary (--- a/ or diff --git) to avoid mid-hunk truncation + // Cut at last file boundary to avoid mid-hunk truncation cut := diff[:maxDiff] if idx := strings.LastIndex(cut, "\ndiff --git "); idx > 0 { cut = cut[:idx] + diff = cut + "\n\n... (diff truncated at file boundary — review remaining files separately)" + } else { + diff = cut + "\n\n... (diff truncated mid-hunk — no clean file boundary found in first 500K chars)" } - diff = cut + "\n\n... (diff truncated — review remaining files separately)" } prompt := cfg.Prompt diff --git a/src/runners/claude.go b/src/runners/claude.go index 117400c..401bffb 100644 --- a/src/runners/claude.go +++ b/src/runners/claude.go @@ -75,17 +75,14 @@ func (r *ClaudeRunner) Run(ctx context.Context, prompt string, opts RunOpts) (Ru if exitErr, ok := err.(*exec.ExitError); ok { exitCode = exitErr.ExitCode() } else { - return RunResult{ExitCode: 1}, fmt.Errorf("claude failed to start: %w — is claude CLI installed?", err) + return RunResult{ExitCode: 1}, fmt.Errorf("claude failed to run: %w", err) } } var resp claudeResponse if err := json.Unmarshal(stdout.Bytes(), &resp); err != nil { - // If JSON parsing fails, return raw output - return RunResult{ - Output: stdout.String(), - ExitCode: exitCode, - }, nil + return RunResult{Output: stdout.String(), ExitCode: exitCode}, + fmt.Errorf("claude returned non-JSON output: %s", TruncStr(stdout.String(), 200)) } return RunResult{ diff --git a/src/runners/codex.go b/src/runners/codex.go index ed53f00..194c581 100644 --- a/src/runners/codex.go +++ b/src/runners/codex.go @@ -36,12 +36,20 @@ func (r *CodexRunner) Run(ctx context.Context, prompt string, opts RunOpts) (Run if exitErr, ok := err.(*exec.ExitError); ok { exitCode = exitErr.ExitCode() } else { - return RunResult{ExitCode: 1}, fmt.Errorf("codex failed to start: %w", err) + return RunResult{ExitCode: 1}, fmt.Errorf("codex failed to run: %w", err) } } - return RunResult{ + result := RunResult{ Output: stdout.String(), ExitCode: exitCode, - }, nil + } + if exitCode != 0 { + errMsg := stderr.String() + if errMsg == "" { + errMsg = stdout.String() + } + return result, fmt.Errorf("codex exited %d: %s", exitCode, TruncStr(errMsg, 200)) + } + return result, nil } diff --git a/src/runners/gemini.go b/src/runners/gemini.go index 0b031ef..9761315 100644 --- a/src/runners/gemini.go +++ b/src/runners/gemini.go @@ -42,7 +42,7 @@ func (r *GeminiRunner) Run(ctx context.Context, prompt string, opts RunOpts) (Ru if exitErr, ok := err.(*exec.ExitError); ok { exitCode = exitErr.ExitCode() } else { - return RunResult{ExitCode: 1}, fmt.Errorf("gemini failed to start: %w", err) + return RunResult{ExitCode: 1}, fmt.Errorf("gemini failed to run: %w", err) } }