Judge a WIP commit before and after it separately (0.13.3) - #31
Merged
Merged
Conversation
The hook decided a `-m "wip…"` commit with one check used twice: the WIP note before the commit and the kept counters after it. So a repository hook that rewrote the message into a real one, or a chain such as `git commit -m "real" && git commit -m "WIP"`, still kept the Gate-B cycle. The two decisions are now separate. Before: only an allow-listed one-line form counts - `git commit`, then only -a/--all, -q/--quiet, -n/--no-verify, --amend or --allow-empty, and exactly one -m whose quoted or bare value starts with wip; no editor, --fixup, -F, second -m, chain, cd, git -C, shell metacharacter or backslash. Where a hook or setting could rewrite the message, the ordinary Gate-B reminder is shown instead of the WIP note (a reminder, not a reset). After: the counters stay only if the resulting commit starts with wip and is attributable to the command. PreToolUse records HEAD, its parent, the HEAD reflog length and whether --amend is an option in .context/codex-gate.wipBase; afterwards HEAD and the reflog are unchanged (the commit failed), or the reflog grew by exactly one and HEAD sits on the recorded HEAD, or on its parent for --amend. A hook that commits, amends or resets in between, a missing or corrupt record, an empty HEAD reflog, or a Bash call sent to the background resets. The 0.13.2 --amend --no-edit rules are unchanged. CLAUDE.md §5 Mechanics and its workflow-init copy (identical) now recommend `git commit -m 'WIP: …'`, state what else is accepted and the after-commit condition; docs/architecture.md and docs/getting-started.md point to those rules. The WIP note no longer promises the counters are kept. Tests run real commits between PreToolUse and PostToolUse under sh and dash for every row: plain, harmless hook, rewriting hook, failed commit, chains, extra -m/-F/--fixup/-e, hook-made extra commits, amend-back, --amend inside the message, no reflog, corrupt record, background call; each repair's new cases fail on the previous hook. dev-workflow 0.13.2 -> 0.13.3. Gate B: six logical passes, each two sequential calls (spec, then quality) against the same base and head ids; closed on pass 6 with no findings. A small persistent record was added by the user's decision after pass 2. Pass reports: one tell at pass 3 (finding count rose, 5 -> 9), none reaching the two-tell stop. No story cited, so no evidence entry is owed. cycle hpg0kyk2nz; floor 3 per none; hook reminder threshold absent cycle hpg0kyk2nz; Gate B (passes 1-6, codex): Findings 6,5,9,7,1,0. Blockers 0,0,0,0,0,0. Majors 3,2,4,4,1,0. Human exceptions: none
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (11)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe hook now records eligible WIP commit calls and checks the resulting subject and repository history before retaining Gate-B counters. Tests cover commit attribution and reset cases. The changelog and workflow guidance describe the updated conditions. ChangesWIP commit flow
Priority: ⬇️ Low Estimated code review effort: 4 (Complex) | ~45 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant PreToolUse
participant CodexGate as codex-gate.sh
participant Git
participant PostToolUse
PreToolUse->>CodexGate: Classify command and record commit state
CodexGate->>Git: Execute eligible commit command
Git-->>PostToolUse: Return command result and repository state
PostToolUse->>CodexGate: Verify subject, HEAD, and reflog
CodexGate-->>PostToolUse: Retain or reset counters and remove call record
Merge Risk: ⚪ Minimal · up to WIP commits reset Gate-B counters when jq is unavailable, as documented. No actionable merge-blocking issue remains after normal checks. Architecture SummaryArchitecture risk: 🟡 Medium · up to The change affects 4 systems. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
Reliability and maintainability
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 2 files. (9 skipped: 9 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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. A rabbit checks the commit trail, Comment |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Greptile on PR #31: the attribution record was one shared file, so overlapping tool calls could take each other's record. Both directions were real, reproduced as regressions on the PR head: two valid WIP calls lost their counters (Greptile's order), and a call whose commit a hook had rewritten into a real one kept the counters on the next call's record (the reviewer's counter-case). Each call now has its own record, .context/codex-gate.wipBase.<tool_use_id>. The id is read with jq as a top-level string, checked there (1-100 chars of [A-Za-z0-9_-]) before any shell capture, and encoded injectively into lower case (`_` -> `__`, X -> `_x`) so ids differing only in case stay apart on a case-insensitive filesystem. A missing, malformed or nested-only id, and every call without jq (whose fallback reader cannot tell top level from nested), gets no record, so the WIP commit resets - the safe direction. Mechanics copies (identical), CHANGELOG and the hook comment say so. New tests: the two interleavings, malformed ids, jq-free nested id, and ids differing only in case; each fails on the hook it fixes. Gate B: three logical passes, each two sequential calls (spec, then quality) against the same base and head ids; closed on pass 3 with no findings. No story cited, so no evidence entry is owed. cycle p0nhw0wb2d; floor 3 per none; hook reminder threshold absent cycle p0nhw0wb2d; Gate B (passes 1-3, codex): Findings 4,2,0. Blockers 0,0,0. Majors 4,2,0. Human exceptions: none
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.
The hook decided a
-m "wip…"commit with one check used twice: the WIP note before thecommit and the kept counters after it. So a repository hook that rewrote the message into a
real one, or a chain such as
git commit -m "real" && git commit -m "WIP", still kept theGate-B cycle. The two decisions are now separate.
Before: only an allow-listed one-line form counts -
git commit, then only -a/--all,-q/--quiet, -n/--no-verify, --amend or --allow-empty, and exactly one -m whose quoted or bare
value starts with wip; no editor, --fixup, -F, second -m, chain, cd, git -C, shell
metacharacter or backslash. Where a hook or setting could rewrite the message, the ordinary
Gate-B reminder is shown instead of the WIP note (a reminder, not a reset).
After: the counters stay only if the resulting commit starts with wip and is attributable to
the command. PreToolUse records HEAD, its parent, the HEAD reflog length and whether --amend
is an option in .context/codex-gate.wipBase; afterwards HEAD and the reflog are unchanged
(the commit failed), or the reflog grew by exactly one and HEAD sits on the recorded HEAD, or
on its parent for --amend. A hook that commits, amends or resets in between, a missing or
corrupt record, an empty HEAD reflog, or a Bash call sent to the background resets. The
0.13.2 --amend --no-edit rules are unchanged.
CLAUDE.md §5 Mechanics and its workflow-init copy (identical) now recommend
git commit -m 'WIP: …', state what else is accepted and the after-commit condition;docs/architecture.md and docs/getting-started.md point to those rules. The WIP note no longer
promises the counters are kept. Tests run real commits between PreToolUse and PostToolUse
under sh and dash for every row: plain, harmless hook, rewriting hook, failed commit, chains,
extra -m/-F/--fixup/-e, hook-made extra commits, amend-back, --amend inside the message, no
reflog, corrupt record, background call; each repair's new cases fail on the previous hook.
dev-workflow 0.13.2 -> 0.13.3.
Gate B: six logical passes, each two sequential calls (spec, then quality) against the same
base and head ids; closed on pass 6 with no findings. A small persistent record was added by
the user's decision after pass 2. Pass reports: one tell at pass 3 (finding count rose,
5 -> 9), none reaching the two-tell stop. No story cited, so no evidence entry is owed.
cycle hpg0kyk2nz; floor 3 per none; hook reminder threshold absent
cycle hpg0kyk2nz; Gate B (passes 1-6, codex): Findings 6,5,9,7,1,0. Blockers 0,0,0,0,0,0. Majors 3,2,4,4,1,0.
Human exceptions: none
Summary by CodeRabbit
Bug Fixes
Documentation