fix(runner,fixer): pipe claude prompt via stdin to dodge ARG_MAX#25
fix(runner,fixer): pipe claude prompt via stdin to dodge ARG_MAX#25openlawbot merged 1 commit intomainfrom
Conversation
Three call sites assemble a prompt and shell out to `claude --print`: StructuredRunner.run (triage / verdict / fix-rereview / fix-plan / fix-polish), FixPipeline._resume_fix_session, and the polish-resume helper in fixer_polish.py. All three appended the prompt to argv. PR openlawteam/adin-chat#261 (554 files changed, 5.5 MB diff) blew past macOS ARG_MAX (~1 MB) and crashed Gate's triage stage with `OSError: [Errno 7] Argument list too long: 'claude'` raised inside subprocess._execute_child — claude never started, so the rate-limit / transient-error stderr handling never saw it and the review silently fell into the fallback path. Fix: pass the prompt via subprocess.run(input=...) instead of argv. `claude --print` reads its prompt from stdin when no positional prompt arg is supplied (verified: `echo "..." | claude --print --max-turns 1` works), so command shape is otherwise unchanged. Side benefit: the two fixer resume sites previously set `stdin=subprocess.DEVNULL` to keep the child off the parent's TTY (TUI-deadlock guard). `input=` auto-allocates `stdin=PIPE`, which is also not a TTY, so that guarantee still holds — the existing test_resume_detaches_stdin regression test is updated to pin the new contract instead of the old DEVNULL identity check. Tests: - test_run_passes_prompt_via_stdin_not_argv: feeds a 5.5 MB prompt (same scale as PR #261) and asserts it is in `input=` and NOT in `cmd`. Direct regression test for the ARG_MAX overflow. - test_run_does_not_set_stdin_kwarg: pins that nobody re-adds an explicit `stdin=` (which would conflict with `input=` and ValueError at runtime). - test_resume_detaches_stdin: rewritten to assert the new pipe-based contract (stdin not set explicitly + input is populated) while preserving the original TTY-detachment intent. Full suite: 1091 passed. Made-with: Cursor
Gate Review ✅Approved — Clean bugfix PR that resolves OSError ARG_MAX overflow on large PR diffs by moving assembled Claude prompts from argv to stdin (subprocess.run input= parameter) at three call sites: StructuredRunner.run, FixPipeline._resume_fix_session, and fixer_polish._resume_fix_senior. All build checks pass (1085 tests, ruff lint, typecheck). Architecture and security stages found no issues; security noted the change is a mild positive (prompt content no longer exposed via ps/procfs). The only finding across all stages is an info-level test coverage gap: the mechanically identical fix in fixer_polish._resume_fix_senior lacks a regression test analogous to the one added for fixer.py. This is informational — risk is low given the fix is a verbatim pattern — and does not block approval. The stdin/input= encoding asymmetry flagged during triage (str in runner.py vs bytes in fixer*.py) is correct-by-context and confirmed consistent with each call site's subprocess mode. Notes
Build Results
1 note across 6 stages (555s, confidence: high) |
1 participant
Summary
Three Gate code paths assemble a prompt and shell out to `claude --print`:
All three appended the prompt to argv.
The bug
openlawteam/adin-chat#261 (554 files changed, 5.5 MB diff) blew past macOS `ARG_MAX` (~1 MB) and crashed Gate's triage stage with:
```
OSError: [Errno 7] Argument list too long: 'claude'
```
raised inside `subprocess._execute_child` — `claude` never started, so the rate-limit / transient-error stderr handling never got a chance to see it. The review silently fell through to the fallback path, leaving `gate-review` red on the PR with no actionable signal.
Fix
Pass the prompt via `subprocess.run(input=...)` instead of argv. `claude --print` reads its prompt from stdin when no positional prompt arg is supplied, so command shape is otherwise unchanged.
Verified:
```bash
echo "say hi in 2 words" | claude --print --max-turns 1 --tools "" --model haiku
Hey there!
```
Side benefit
The two fixer-resume sites previously set `stdin=subprocess.DEVNULL` to keep the child off the parent's TTY (TUI-deadlock guard from when Gate runs under `gate up`). `input=` auto-allocates `stdin=PIPE`, which is also not a TTY, so that guarantee still holds. The existing `test_resume_detaches_stdin` regression test is updated to pin the new contract (no explicit `stdin` kwarg + `input` is populated) instead of the old `is DEVNULL` identity check.
Tests
New regression coverage in `tests/test_runner.py`:
Plus the rewritten `test_resume_detaches_stdin` in `tests/test_fixer.py`.
Full suite: 1091 passed in 113.62s.
Test plan
Out of scope
`make ci` runs `ruff format` which wants to reformat 56 files repo-wide (pre-existing drift). Not bundled into this PR — separate "format the world" pass would be cleaner.
Made with Cursor