README install fixes harvested from upstream + plan-codex command hardening - #4
Open
hellosurbhi wants to merge 15 commits into
Open
README install fixes harvested from upstream + plan-codex command hardening#4hellosurbhi wants to merge 15 commits into
hellosurbhi wants to merge 15 commits into
Conversation
Pre-existing uncommitted change dated 2026-07-28: a Codex call that dies mid-audit is not an approval; retry once, then stop and declare the plan UNREVIEWED out loud. Committed alone before the iteration changes so each lands separately. Verified: read the paragraph, matches the cross-review rule already recorded in global CLAUDE.md Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The audit loop ran up to 3 xhigh Codex rounds and only exited early on APPROVED, so a harmless WARNING verdict bought another 10-15 minute round. Measured 2026-08-02: three rounds, zero BLOCKED findings, ~45 minutes before any code. Now WARNING means incorporate the accepted findings and proceed with no further call; only BLOCKED (CRITICAL findings) triggers a re-audit, still capped at 3 iterations. The critical-evaluation and discussion-reply protocol is unchanged. Verified: read the revised Phase 2 end to end; verdict grammar matches codex-analyzer-role.md (VERDICT: APPROVED | WARNING | BLOCKED) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
claude-codex, execute-codex and both tdd variants re-reviewed on any non-APPROVED verdict, so a WARNING (HIGH-only) verdict bought another full xhigh Codex round. Session forensics showed a 6:1 blocked-to- approved ratio with most rounds churning on non-critical findings. Now: WARNING means fix the accepted HIGHs in one batch and proceed with no further call (the pre-push gate and CodeRabbit still review the final code); only BLOCKED/CRITICAL verdicts trigger a re-review; iteration caps are unchanged. Critical-evaluation and discussion-reply protocols untouched. Verified: grepped all four skills for stale 'without APPROVED' / 'WARNING -> re-review' phrasing, none remain Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The justification for skipping WARNING re-reviews cited a pre-push gate and CodeRabbit, which are operator-environment infrastructure this repo cannot assume; the justification now stands on its own (HIGHs are fixed and verified, only the paid re-verification round is skipped). WARNING paths now run post-fix verification (tests/lint, GREEN suite) and route MEDIUM/LOW handling instead of jumping straight to delivery. Route B HIGH-only branches name their path-specific fix mechanisms (subagents for the 3+ task path, codex-reply for the single-session path) so the no-Edit/Write rule cannot be violated. plan-codex WARNING description aligned to the analyzer-role grammar (at least one HIGH). Verified: grepped skills/ for 'pre-push gate and CodeRabbit', zero remain; re-read all four After-evaluation blocks for mechanism coverage Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…LOCKED-only re-review The commands/*.md compatibility entrypoints and both eval contracts still mandated re-review on WARNING and re-audit until APPROVED, so the policy shipped in the skills did not apply across all supported review loops and the repo's own evals would enforce the removed behavior. All entrypoints now match the skills: WARNING fixes accepted HIGHs, verifies, and proceeds with no further call; only BLOCKED iterates. BUGS.md drops the four HIGH findings fixed by this commit and the previous one; the two MEDIUMs stay queued. Verified: grep across commands/ and evals for 'without APPROVED' and WARNING-re-review phrasing returns only BLOCKED-scoped lines Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review loops: re-review only on BLOCKED, single plan audit
Verified: read the BUGS.md diff, auto-routed findings only, no code touched Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The 3+ task re-review branches in commands/execute-codex.md and commands/tdd-execute-codex.md now trigger on CRITICAL only, matching the BLOCKED-only policy, and their iteration stops key on CRITICALs instead of approval. The obsolete until-APPROVED contract is removed from the plan-codex skill description, command description, eval expectation and all three README references. HIGH-only branches now carry MEDIUM/LOW findings forward the same way their approved branches do (delivery report for execute-codex, user prompt for the TDD variants). BUGS.md drops the fixed HIGH entries and duplicates; the blank-line LOW is fixed by the rewrite and its entry removed with it. Verified: grep across skills, commands and README for 'until approved' and CRITICAL/HIGH re-review phrasing returns only BLOCKED or CRITICAL scoped lines Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Skills backlog: all three queued HIGH findings fixed
Verified: read the BUGS.md diff, auto-routed MEDIUM entries only, no code touched Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adapted from ching-kuo#1 by Stephen-Kimoi, taken into this fork so the upstream PR can be left alone: codex login as the primary auth path with the API key as alternative, the corrected plugin install command (claude plugin install feature-dev@claude-plugins-official; the old claude plugin add syntax no longer exists), mkdir -p for the target dirs before the cp lines, the TOML note that hide_agent_reasoning is a top-level key that must sit above any table header, and the MCP model example updated to this setup's actual pin (gpt-5.6-sol at xhigh; the bare gpt-5.6 alias is API-only and rejected on ChatGPT-account auth). Verified: read the full rendered README install section top to bottom; every command matches this machine's working configuration Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…aced, BLOCKED covers HIGHs The command branched on WARNING and BLOCKED while its injected analyzer prompt (~/.claude/prompts/codex/analyzer.md) defined no verdicts at all and did not even exist in this install; non-approved audits could fall through unparsed. The command now injects the skill's codex-analyzer-role.md, which defines VERDICT: APPROVED | WARNING | BLOCKED, both audit prompts request the structured verdict, and the BLOCKED branch addresses accompanying HIGH findings alongside CRITICALs. ~/.claude/prompts is now symlinked to this repo's prompts/ so the architect.md references in the execute command variants resolve as well. BUGS.md drops the fixed HIGH. Verified: grep confirms codex-analyzer-role.md defines the verdict grammar while prompts/codex/analyzer.md contains zero VERDICT mentions; ls ~/.claude/prompts/codex resolves through the new symlink Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…indings preserved The parse step matched the word APPROVED anywhere in the response, so a finding merely containing it could trigger delivery, and missing or multiple verdict lines fell through unparsed. Parsing is now anchored to exactly one VERDICT: line with an explicit malformed path (retry once, else declare the plan UNREVIEWED). MEDIUM/LOW findings that accompany APPROVED are folded into the plan notes per the analyzer contract instead of being discarded. The analyzer-path note no longer claims the legacy prompt file does not exist; it exists but predates the verdict grammar, which is the actual reason it must not be used. BUGS.md consolidates three overlapping MEDIUM composites into the one canonical copy and drops the entries fixed here. Verified: read the revised parse block against codex-analyzer-role.md's grammar; BUGS.md reread, each open item appears once Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
hellosurbhi
marked this pull request as ready for review
August 3, 2026 04:51
📝 WalkthroughWalkthroughThe PR updates Codex plan audits, review verdict handling, severity-specific remediation, iteration limits, installation guidance, MCP configuration, and repository review tracking. ChangesCodex workflow handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CodexWorkflow
participant Codex
participant Fixer
User->>CodexWorkflow: Start plan or code review
CodexWorkflow->>Codex: Submit audit
Codex-->>CodexWorkflow: Return APPROVED, WARNING, or BLOCKED
alt WARNING or HIGH-only finding
CodexWorkflow->>Fixer: Apply accepted fixes
Fixer-->>CodexWorkflow: Verify fixes and tests
else BLOCKED or CRITICAL finding
CodexWorkflow->>Fixer: Apply required fixes
Fixer-->>CodexWorkflow: Return updated state
CodexWorkflow->>Codex: Request limited re-review
end
CodexWorkflow-->>User: Proceed or stop at the iteration limit
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…n commands, working CLI fallback Resolves the CodeRabbit threads on the mainline-adoption PR. Every capped review route now states explicitly that surviving accepted CRITICALs mean STOP and report, never deliver: a capped-out review is not an approval. Both compatibility commands define the critical-evaluation gate (accuracy, context, applicability; contested findings go through codex-reply and do not count as iterations) that their skill counterparts already had. The plan-codex CLI retry carries the analyzer role and audit prompt inline, since a bare codex exec reads stdin and hangs. The verdict state machine thread was already resolved by the earlier line-anchored VERDICT parsing commit on this branch. reviews/ is gitignored: review-pipeline artifacts must never be tracked (an earlier attempt of this commit swept the cache in via git add -A and was reset before pushing). BUGS.md drops the two queued items these fixes close; ENHANCEMENTS.md carries the routed LOWs. Verified: git show --stat contains only intended files; grep confirms every capped route has a fail-closed stop clause Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (2)
.gitignore (1)
24-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAnchor
reviews/to the repository root.
reviews/ignores nestedreviewsdirectories as well. If only root-level review artifacts are generated, use/reviews/.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.gitignore at line 24, Update the reviews/ entry in .gitignore to /reviews/ so only the repository-root reviews directory is ignored, while nested reviews directories remain trackable.skills/plan-codex/evals/evals.json (1)
7-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd eval coverage for the strict parser and failure paths.
The changed expectations cover audit iteration counts, but they do not assert exactly one line-anchored
VERDICT, retry on malformed or failed output, explicitUNREVIEWEDstatus, or MEDIUM/LOW preservation onAPPROVED.Add focused expectations for these paths so the hardening contract cannot regress silently.
Also applies to: 23-31
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/plan-codex/evals/evals.json` at line 7, Expand the eval expectations in the plan-agent audit scenarios to cover strict parsing and failure handling: require exactly one line-anchored VERDICT, retry malformed or failed Codex output, emit explicit UNREVIEWED status when auditing cannot complete, and preserve MEDIUM/LOW findings when the final verdict is APPROVED. Add focused assertions for these paths without changing the existing audit iteration expectations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@BUGS.md`:
- Line 12: Update the repeated Codex headings in BUGS.md so each heading is
unique, either by merging duplicate entries or adding sequence labels such as
“(continued)” to the later headings; preserve the review-batch entries and
ensure markdownlint MD024 passes.
In `@commands/claude-codex.md`:
- Around line 84-87: Route every no-CRITICAL/HIGH outcome through the MEDIUM/LOW
user-decision gate before delivery: update the verdict flow in
commands/claude-codex.md lines 84-87, skills/claude-codex/SKILL.md lines 94-97,
commands/tdd-claude-codex.md lines 161-164, and skills/tdd-claude-codex/SKILL.md
lines 194-197 so APPROVED proceeds through Step R3 before the respective
delivery phase; update commands/execute-codex.md lines 132-136 to collect
MEDIUM/LOW findings and ask whether to fix or proceed before delivery.
In `@commands/execute-codex.md`:
- Line 135: Update the single-session CRITICAL path in the execute-codex
workflow to explicitly invoke the outer feature-dev:code-reviewer again after
mcp__codex__codex-reply completes and the iteration count is incremented.
Preserve the existing verbatim feedback and maximum three-iteration behavior,
ensuring the fix is re-reviewed before the workflow proceeds.
- Line 72: Require every BLOCKED execution path to address accepted CRITICAL and
HIGH findings together before re-review: update Route A at
commands/execute-codex.md:72, both Route B CRITICAL branches at
commands/execute-codex.md:134-136, and the BLOCKED assertion at
skills/claude-codex/evals/evals.json:28 to enforce this behavior.
In `@commands/plan-codex.md`:
- Around line 40-50: The Codex audit flow in commands/plan-codex.md must mirror
the fail-closed handling defined by skills/plan-codex/SKILL.md: perform the
mandatory availability check, retry once for call errors, timeouts, or aborts,
and stop with UNREVIEWED if the retry fails. Preserve the existing single retry
specifically for malformed VERDICT output, and ensure every unrecoverable audit
failure prevents the plan from being treated as reviewed.
In `@commands/tdd-execute-codex.md`:
- Around line 104-107: Update the remediation branching in the TDD execution
instructions to apply the documented critical-evaluation gate before every
WARNING/BLOCKED test-audit branch and both code-review branches. Use the
correctness, context, and applicability checks from the referenced SKILL.md
guidance, and route only accepted CRITICAL/HIGH findings into remediation while
preserving the existing iteration limits and outcomes.
In `@ENHANCEMENTS.md`:
- Line 2: Update the “Low priority enhancements” heading in ENHANCEMENTS.md to
“Low-priority enhancements,” preserving the heading level and remaining content.
In `@README.md`:
- Around line 101-104: Update the README Codex setup example to use a tested
ChatGPT-account-safe model by default, such as gpt-5.6-terra or gpt-5.6-luna,
instead of gpt-5.6-sol. Document gpt-5.6-sol only as an optional fallback where
supported, and retain the existing model and reasoning-effort customization
guidance.
In `@skills/plan-codex/SKILL.md`:
- Around line 63-65: Propagate non-blocking MEDIUM/LOW findings on every
non-blocking path. In skills/plan-codex/SKILL.md lines 63-65, require WARNING
handling to fold accompanying MEDIUM/LOW findings into the plan before Phase 3,
matching commands/plan-codex.md Line 52. In skills/execute-codex/SKILL.md lines
87-89, require the Phase 3 delivery handoff to report remaining MEDIUM/LOW
findings, matching the Route B behavior.
---
Nitpick comments:
In @.gitignore:
- Line 24: Update the reviews/ entry in .gitignore to /reviews/ so only the
repository-root reviews directory is ignored, while nested reviews directories
remain trackable.
In `@skills/plan-codex/evals/evals.json`:
- Line 7: Expand the eval expectations in the plan-agent audit scenarios to
cover strict parsing and failure handling: require exactly one line-anchored
VERDICT, retry malformed or failed Codex output, emit explicit UNREVIEWED status
when auditing cannot complete, and preserve MEDIUM/LOW findings when the final
verdict is APPROVED. Add focused assertions for these paths without changing the
existing audit iteration expectations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 01098708-b795-4f78-b08c-414b05c6b8e2
📒 Files selected for processing (16)
.gitignoreBUGS.mdENHANCEMENTS.mdREADME.mdcommands/claude-codex.mdcommands/execute-codex.mdcommands/plan-codex.mdcommands/tdd-claude-codex.mdcommands/tdd-execute-codex.mdskills/claude-codex/SKILL.mdskills/claude-codex/evals/evals.jsonskills/execute-codex/SKILL.mdskills/plan-codex/SKILL.mdskills/plan-codex/evals/evals.jsonskills/tdd-claude-codex/SKILL.mdskills/tdd-execute-codex/SKILL.md
Addressed CodeRabbit/CodeQL findings (9 thread(s) in this pass): - Files changed: BUGS.md ENHANCEMENTS.md commands/claude-codex.md commands/execute-codex.md commands/plan-codex.md commands/tdd-claude-codex.md commands/tdd-execute-codex.md skills/claude-codex/SKILL.md skills/claude-codex/evals/evals.json skills/execute-codex/SKILL.md skills/plan-codex/SKILL.md skills/tdd-claude-codex/SKILL.md skills/tdd-execute-codex/SKILL.md Verified: Claude read each flagged file and made targeted fixes per the review comments. No unrelated changes staged.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@commands/tdd-execute-codex.md`:
- Line 149: Update the critical evaluation gate instructions so that when
accepted CRITICAL and HIGH findings coexist, the Edit/Write remediation fixes
both severities before re-review. Preserve the existing re-review limit and stop
behavior for unresolved CRITICAL issues, while ensuring accepted HIGH findings
cannot remain unresolved in this branch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 71aa1634-417a-4ed9-970d-64ee519eea24
📒 Files selected for processing (13)
BUGS.mdENHANCEMENTS.mdcommands/claude-codex.mdcommands/execute-codex.mdcommands/plan-codex.mdcommands/tdd-claude-codex.mdcommands/tdd-execute-codex.mdskills/claude-codex/SKILL.mdskills/claude-codex/evals/evals.jsonskills/execute-codex/SKILL.mdskills/plan-codex/SKILL.mdskills/tdd-claude-codex/SKILL.mdskills/tdd-execute-codex/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (7)
- skills/claude-codex/evals/evals.json
- skills/claude-codex/SKILL.md
- ENHANCEMENTS.md
- skills/tdd-claude-codex/SKILL.md
- skills/execute-codex/SKILL.md
- commands/execute-codex.md
- commands/tdd-claude-codex.md
| - `git diff $START_SHA` output | ||
| - Original task requirements | ||
| 8. If CRITICAL/HIGH issues: fix with Edit/Write, re-review (max 2 rounds) | ||
| 8. **Critical evaluation gate** (before acting on ANY finding): assess whether it is technically accurate against the actual code and project conventions, whether the reviewer had full context for intentional decisions and whether it improves correctness/security or is a preference / false positive. If a finding seems incorrect, launch a new code-reviewer agent with the original diff, the contested findings and your technical reasoning; accept confirmed findings, drop withdrawn ones. Discussion rounds do NOT count toward the iteration cap. Then: accepted CRITICAL issues → fix with Edit/Write, re-review (max 2 rounds). Accepted HIGH-only issues → fix with Edit/Write, re-run the test suite (must stay GREEN), proceed with no re-review call. If accepted CRITICAL issues remain after the final round, STOP and report them; do not deliver. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Remediate accepted HIGH findings with CRITICAL findings.
If a review returns both CRITICAL and HIGH findings, this branch only instructs Claude to fix accepted CRITICAL issues. The HIGH-only branch does not apply, so accepted HIGH findings can remain unresolved.
Change the CRITICAL branch to fix all accepted CRITICAL and HIGH findings before re-review.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@commands/tdd-execute-codex.md` at line 149, Update the critical evaluation
gate instructions so that when accepted CRITICAL and HIGH findings coexist, the
Edit/Write remediation fixes both severities before re-review. Preserve the
existing re-review limit and stop behavior for unresolved CRITICAL issues, while
ensuring accepted HIGH findings cannot remain unresolved in this branch.
Owner
Author
|
🤖 Auto-resolve pass complete (2026-08-03T07:05Z) Threads fixed and resolved: 8 · dismissed with reasoning: 1 · left open for retry: 0 Dismissals carry a reply on their thread explaining the reasoning. Re-request a CodeRabbit review if anything looks off. |
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
2 participants
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.
TL;DR (plain language)
Everything useful from the stray upstream PRs, pulled into YOUR repo. The install docs get the fixes from ching-kuo#1 (credited to Stephen-Kimoi, adapted to this setup: codex login auth, the corrected claude plugin install command, mkdir before copy, the TOML placement note, and the model example updated to the real gpt-5.6-sol pin). Alongside it, the plan-codex command entrypoint got real hardening the reviews demanded: it now injects the analyzer role that actually defines the verdict grammar (the old prompt path had none), validates exactly one line-anchored VERDICT line with a malformed-means-UNREVIEWED path, and preserves MEDIUM/LOW findings that accompany APPROVED.
Merge order
Merge PR #3 (Adopt main as the only mainline) FIRST. Once it lands, this PR's diff collapses to just these changes.
Review trail
Three push reviews on this branch: WARNING each, zero CRITICALs; remaining MEDIUMs queued in BUGS.md per policy.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores