fix(codex): stop review-mode exec from exploring the repo; raise budget to 600s - #2806
Open
AngeloAdam wants to merge 1 commit into
Open
fix(codex): stop review-mode exec from exploring the repo; raise budget to 600s#2806AngeloAdam wants to merge 1 commit into
AngeloAdam wants to merge 1 commit into
Conversation
…et to 600s Trigger-2 style reviews on a large monorepo were failing as exit 124 and getting reported upstream as "codex is unavailable". Two causes, both here. 1. The custom-instructions path runs `codex exec`, which - unlike `codex review --base` - is NOT auto-scoped to the diff. It has a full read-only sandbox over the repo and spends the budget grepping (403KB of grep output, empty stdout, indistinguishable from a model stall). The prompt bounded the filesystem (~/.claude/, agents/) but never told the model to stay off the tree. Adds a SCOPE: line: no repo exploration, no grep/find/ls/rg, at most 5 targeted reads and only to confirm an already-named finding. 2. The 330s wrapper was too tight regardless. With exploration fully suppressed - zero tool calls in stderr - a 23-file / 1,649-insertion / 103KB diff still took 324s at model_reasoning_effort="high", six seconds under the old ceiling. Raises the review wrapper 330 -> 600s and the Bash gate 360000 -> 660000, matching the challenge/consult pair rather than inventing a third budget. Both review paths move together so the two are not documented as differing. Verified end to end on a private monorepo against a 23-file diff: exit 0, 324s, 54,231 tokens, clean verdict, no exploration. Templates are the source of truth; the rendered .md files are regenerated output committed alongside them. The live install at ~/.gstack/render/ needs `./setup` - `gen:skill-docs` alone leaves it stale. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
AngeloAdam
marked this pull request as ready for review
September 5, 2026 11:20
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Why (in your own words)
On a large monorepo,
/codex reviewwith custom instructions kept dying at exit 124. Downstream agents reported it to me as "codex is unavailable" — so I spent time checking the binary, the auth, the MCP connectors, all of which were fine. The actual failure is in this skill.The custom-instructions path runs
codex exec, which — unlikecodex review --base— is not auto-scoped to the diff. It gets a full read-only sandbox over the repo and goes exploring. The prompt bounds the filesystem (~/.claude/,agents/) but never tells the model to stay off the tree, so on a big repo it burns the whole 330s wrapper grepping and produces empty stdout. Empty stdout after a timeout is indistinguishable from a model stall, which is why it gets misreported as an availability problem rather than a scoping one.gstack's own operational-learning loop had already caught this and logged it a day before I found it —
codex exec on a large repo burns the whole 330s wrapper budget exploring (403KB of grep output, empty stdout, reads as a stall)— but the learning was never applied to the skill.Live evidence
Before. The skill has no instruction against exploration anywhere — the only boundary is the filesystem one:
And the logged learning that predicted the failure:
After. Ran the patched exec path end to end against a real 23-file / 1,649-insertion / 103KB diff, timed, with the exact prompt the skill now builds:
Output — a real verdict, not an empty stall:
Exploration is gone — zero tool invocations in stderr:
Why the timeout moves too. Note the 324s above: with exploration fully suppressed, that diff still lands only six seconds under the old 330s ceiling. The no-explore instruction alone does not make this path safe at that size — the remaining time is reasoning at
model_reasoning_effort="high". So the wrapper goes 330 → 600s and the Bash gate 360000 → 660000, which also collapses review onto the same budget challenge and consult already use instead of documenting a third one.Scope
codex/sections/review-mode.md.tmpl— added aSCOPE:line to thecodex execprompt (no repo exploration; no grep/find/ls/rg across the tree; at most 5 targeted file reads, and only to confirm an already-named finding), and raised both review paths from the 330s wrapper /timeout: 360000gate to 600s /660000.codex/SKILL.md.tmpl— updated the two places that documented the old 330/360000 pair. Rendered.mdfiles regenerated from the templates and committed alongside them.~/.gstack/render/claude/codex/carries both changes after./setup;bun test test/codex-hardening.test.ts test/codex-web-search-flag.test.ts test/skill-validation.test.ts.--xhighpath,--commit/--uncommittedscopes, or any host other thanclaude. I also did not test whether 600s is sufficient for diffs substantially larger than 103KB — I suspect it is not, and the better lever there is probably splitting the diff or dropping tomediumeffort rather than raising the ceiling again.On the test suite: it is flaky on my machine in a way unrelated to this change. Same suite, same commit, back to back:
52 pass / 2 failthen54 pass / 0 fail. Cleanmainat 0d1bd56 fails 6 of the same set before my change is applied. The failures are timing-sensitive (gstack-slug, broken-install detection, and twogrep -rlntests that exceed the 5000ms default while scanning the install dir) plus a tracked-binaries check. None referencereview-mode.mdor the timeout constants.Liveness proof (required)
Being straightforward about this: the branch was prepared by Claude (Claude Code) at the repo owner's direction, and the timed runs above were executed by it on the owner's machine. The liveness screenshot exists precisely to confirm a human is behind the PR, so it is not mine to produce — an agent generating it would defeat the check it is there to perform. @AngeloAdam will attach a real one. Until it lands, please treat this as not meeting the evidence bar, regardless of the ready-for-review state.
Checklist
gen:skill-docs, then deployed with./setup)🤖 Generated with Claude Code