v1.39.2.0 fix(review): drop prompt arg from codex review --base in /review and /ship resolvers (closes #1479)#1527
Open
mvanhorn wants to merge 2 commits into
Conversation
…d /ship resolvers Closes garrytan#1479. codex-cli 0.130.0 made [PROMPT] and --base mutually exclusive. v1.34.2.0 fixed /codex review's structured-review path with the same shape this PR uses, but the shared resolver at scripts/resolvers/review.ts:535 (which gen-skill-docs feeds into review/SKILL.md and ship/SKILL.md) still emitted the buggy 'codex review "prompt" --base <base>' invocation. This drops the prompt arg from the resolver's bare-default invocation. /codex's existing dual-path pattern stays - bare codex review for default reviews (Codex internally diff-scopes), codex exec with file-based prompt for custom-instructions paths. Skill files under .claude/ and agents/ are public, so the dropped prompt is a token-efficiency concession, not a safety one (matches the v1.34.2.0 reasoning). Regenerated SKILL.md outputs via 'bun run gen:skill-docs' so review/SKILL.md and ship/SKILL.md (plus the three ship golden fixtures) drop the prompt arg in their structured Codex review block. test/codex-hardening.test.ts: 35 pass / 0 fail. New regression suite asserts no 'codex review' line in any of the 6 affected files combines a quoted/expanded prompt with --base, so this gap can't return silently. Reported by @Stashub via garrytan#1428 (the v1.34.2.0 fix); this PR completes the same pattern across /review and /ship.
Required by gstack's per-PR version-bump convention. CI's 'Check VERSION is not stale vs queue' job rejected v1.39.1.0 because main is already at v1.39.1.0; v1.39.2.0 is the next slot.
mvanhorn
changed the title
codex review --base in /review and /ship resolvers (closes #1479)codex review --base in /review and /ship resolvers (closes #1479)
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
1 participant
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.
Summary
/reviewand/ship's structured Codex review block now uses barecodex review --base <base>instead ofcodex review "$prompt" --base <base>. Codex CLI 0.130.0 made the prompt arg and--basemutually exclusive at argv level, so the prior call errored before the model ran.Why this matters
That's @Stashub on #1479. The same reporter drove the v1.34.2.0 fix for
/codex reviewvia #1428; this PR completes that pattern by applying the same shape to the shared resolver that feeds/reviewand/ship.The v1.34.2.0 changelog entry on master called out
codex/SKILL.md.tmplStep 2A as the fix site -/reviewand/shipwere the gap. They route throughscripts/resolvers/review.ts(per CLAUDE.md "Merge conflicts on SKILL.md files: NEVER resolve conflicts on generated SKILL.md files"), and that resolver still emitted the buggy prompt-plus-base invocation at line 535. Anyone on codex-cli >= 0.130.0 who hits the structured Codex review path in either skill (DIFF_TOTAL >= 200 line threshold) gets the sameerror: the argument '[PROMPT]' cannot be used with '--base <BRANCH>'failure Stashub reported.Changes
scripts/resolvers/review.ts:535drops the prompt arg from the bare-default Codex invocation. Codex internally scopes the diff via--base, so the filesystem-boundary prompt was a token-efficiency concession to begin with. Dropping it on the structured-review path matches the v1.34.2.0 reasoning for/codex review: skill files under.claude/andagents/are public, so a few extra tokens reading them is the cost, not a safety hole.bun run gen:skill-docsregeneratesreview/SKILL.mdandship/SKILL.md. Three ship golden fixtures (test/fixtures/golden-ship-claude.md,test/fixtures/golden/claude-ship-SKILL.md,test/fixtures/golden/factory-ship-SKILL.md) are regenerated alongside since they pin the codex-review snippet structure.test/codex-hardening.test.tsadds a regression suite over all 6 affected files (the resolver, both generated SKILL.md, and the three goldens) asserting nocodex reviewline combines a quoted/expanded prompt with--base. Closes the gap structurally.The
/codexSKILL is unchanged - it already had the right shape.Testing
bun test test/codex-hardening.test.ts: 35 pass / 0 fail. The 6 new regression cases (one per affected file) catch any future drift back to theprompt + --baseshape.bun run gen:skill-docsproduces no additional drift after the resolver edit, confirming the regenerated outputs are consistent.Fixes #1479. Completes the v1.34.2.0 / #1428 fix pattern across
/reviewand/ship.AI was used for assistance.