From b02c16a2a597a2c325a786a08e6143f766fa844e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20S=C3=A4nger?= <20968534+dsnger@users.noreply.github.com> Date: Mon, 28 Sep 2026 18:41:40 +0200 Subject: [PATCH 1/2] Judge a WIP commit before and after it separately (0.13.3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../gate-b-quality-hpg0kyk2nz-pass-1.md | 3 + .../gate-b-quality-hpg0kyk2nz-pass-2.md | 3 + .../gate-b-quality-hpg0kyk2nz-pass-3.md | 6 + .../gate-b-quality-hpg0kyk2nz-pass-4.md | 5 + .../gate-b-quality-hpg0kyk2nz-pass-5.md | 2 + .../gate-b-quality-hpg0kyk2nz-pass-6.md | 2 + .../gate-b-spec-hpg0kyk2nz-pass-1.md | 5 + .../gate-b-spec-hpg0kyk2nz-pass-2.md | 4 + .../gate-b-spec-hpg0kyk2nz-pass-3.md | 5 + .../gate-b-spec-hpg0kyk2nz-pass-4.md | 4 + .../gate-b-spec-hpg0kyk2nz-pass-5.md | 2 + .../gate-b-spec-hpg0kyk2nz-pass-6.md | 2 + CLAUDE.md | 19 +- docs/architecture.md | 13 +- docs/getting-started.md | 5 +- .../dev-workflow/.claude-plugin/plugin.json | 2 +- plugins/dev-workflow/CHANGELOG.md | 30 ++++ .../dev-workflow/commands/workflow-init.md | 19 +- plugins/dev-workflow/hooks/codex-gate.sh | 163 +++++++++++++----- plugins/dev-workflow/hooks/codex-gate.test.sh | 148 ++++++++++++++-- todos.md | 15 +- 21 files changed, 379 insertions(+), 78 deletions(-) create mode 100644 .context/codex-reviews/gate-b-quality-hpg0kyk2nz-pass-1.md create mode 100644 .context/codex-reviews/gate-b-quality-hpg0kyk2nz-pass-2.md create mode 100644 .context/codex-reviews/gate-b-quality-hpg0kyk2nz-pass-3.md create mode 100644 .context/codex-reviews/gate-b-quality-hpg0kyk2nz-pass-4.md create mode 100644 .context/codex-reviews/gate-b-quality-hpg0kyk2nz-pass-5.md create mode 100644 .context/codex-reviews/gate-b-quality-hpg0kyk2nz-pass-6.md create mode 100644 .context/codex-reviews/gate-b-spec-hpg0kyk2nz-pass-1.md create mode 100644 .context/codex-reviews/gate-b-spec-hpg0kyk2nz-pass-2.md create mode 100644 .context/codex-reviews/gate-b-spec-hpg0kyk2nz-pass-3.md create mode 100644 .context/codex-reviews/gate-b-spec-hpg0kyk2nz-pass-4.md create mode 100644 .context/codex-reviews/gate-b-spec-hpg0kyk2nz-pass-5.md create mode 100644 .context/codex-reviews/gate-b-spec-hpg0kyk2nz-pass-6.md diff --git a/.context/codex-reviews/gate-b-quality-hpg0kyk2nz-pass-1.md b/.context/codex-reviews/gate-b-quality-hpg0kyk2nz-pass-1.md new file mode 100644 index 0000000..5a2d58e --- /dev/null +++ b/.context/codex-reviews/gate-b-quality-hpg0kyk2nz-pass-1.md @@ -0,0 +1,3 @@ +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:818-831 | The WIP matcher accepts arbitrary flags before -m and an unchecked suffix after its WIP prefix, including message-changing options: real sh and dash runs of git commit --allow-empty -m 'WIP: snapshot' --fixup=HEAD emit the WIP exemption but create subject 'fixup! real baseline'; -e can likewise let an editor replace the subject. The generic prefix also accepts git commit --allow-empty -m -m -m WIP, whose actual subject is '-m'. | These successful non-WIP commits suppress the ordinary Gate-B reminder, violating the requested conservative PreToolUse behavior and AGENTS.md invariant 2; the later PostToolUse reset cannot restore the missed reminder. | Match a complete bounded command shape with explicitly supported flags and an unambiguous first message argument; reject editor and subject-changing options anywhere, and add real-run regression cases under sh and dash. +MINOR | high | plugins/dev-workflow/hooks/codex-gate.sh:982 | The existing WIP note still promises 'your pass counters are preserved' and 'Codex cycle preserved', although the new PostToolUse check may clear them. Reproduced under sh and dash with a non-WIP HEAD, count 2, index.lock, and plain git commit -m 'WIP: snapshot': PreToolUse prints the promise, Git exits 128, and PostToolUse deletes the counters. | The shipped prompt now falsely assures the agent of counter preservation, contrary to prompt-standard 11 and the requested audit of existing WIP descriptions. | Describe an attempted WIP snapshot and make preservation conditional on the subsequent HEAD check; cover this failed-commit lifecycle in the note assertions. +END OF FINDINGS (2 total) diff --git a/.context/codex-reviews/gate-b-quality-hpg0kyk2nz-pass-2.md b/.context/codex-reviews/gate-b-quality-hpg0kyk2nz-pass-2.md new file mode 100644 index 0000000..ff9f8a8 --- /dev/null +++ b/.context/codex-reviews/gate-b-quality-hpg0kyk2nz-pass-2.md @@ -0,0 +1,3 @@ +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:839 | PostToolUse treats final WIP HEAD as sufficient attribution even when repository hooks create additional commits. Reproduced under sh and dash: an executable post-commit hook creates an empty 'real boundary' commit followed by an empty 'WIP: hook snapshot' commit, using git -c core.hooksPath=/dev/null to prevent recursion; the outer command is the allowed git commit -q --allow-empty -m 'WIP: requested'. | All three Gate-B state files survive despite crossing a real boundary; this leaves the requested reset-on-undecidable-history behavior incomplete and violates invariant 2. | Conservatively reset when active hooks make the resulting history unattributable, or establish sufficient attribution before preserving counters; add this real-hook lifecycle regression under both shells without changing the no-edit rules. +MINOR | high | CLAUDE.md:1327-1329; plugins/dev-workflow/commands/workflow-init.md:1516-1518; plugins/dev-workflow/CHANGELOG.md:34-35; plugins/dev-workflow/hooks/codex-gate.sh:776 | The new prose says commits made any other way reset after prescribing single quotes and short flags, but the matcher also preserves bare messages, double-quoted messages with jq, --all and --quiet. The changelog and hook comment also incorrectly say only single-quoted messages qualify without jq; bare -m WIP qualifies too. | Readers receive incorrect counter-reset expectations, contrary to the task's documentation requirement and prompt-standards item 11; identical prompt copies propagate the same error downstream. | Label the single-quoted command as the recommended portable form, describe resets using the actual allow-list boundary, and acknowledge bare messages in the jq-free description while keeping both Mechanics copies identical. +END OF FINDINGS (2 total) diff --git a/.context/codex-reviews/gate-b-quality-hpg0kyk2nz-pass-3.md b/.context/codex-reviews/gate-b-quality-hpg0kyk2nz-pass-3.md new file mode 100644 index 0000000..3978626 --- /dev/null +++ b/.context/codex-reviews/gate-b-quality-hpg0kyk2nz-pass-3.md @@ -0,0 +1,6 @@ +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:853-855 | Final parent equality does not detect an intervening real commit followed by an amendment. Reproduced under sh and dash: prepare-commit-msg rewrites git commit --allow-empty -m 'WIP: next' to 'real boundary'; post-commit runs git -c core.hooksPath=/dev/null commit -q --allow-empty --amend -m 'WIP: amended by hook'. The reflog records both commits, but final HEAD is still a direct child of the saved HEAD | All Gate-B state survives a real boundary, violating the required conservative reset and invariant 2 | Record enough history to detect intervening ref updates, or reset when hook execution leaves the result unattributable; add this real-run regression under both shells +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:856 | The --amend search includes the quoted message. Reproduced under sh and dash with git commit --allow-empty -m 'WIP: --amend is documentation' and a post-commit hook that runs git reset --soft HEAD~2 followed by a hook-disabled WIP commit: the resulting sibling passes the amendment exception even though the command contains no amendment option | Counters survive an otherwise rejected, unattributable history because message text is treated as a command option | Determine amendment from the allow-listed option positions outside the message, record that classification before execution, and add a quoted-message regression +MINOR | high | plugins/dev-workflow/hooks/codex-gate.sh:1013 | The new WIP note and its title still promise counters are kept if the result is WIP, omitting the required readable record and ancestry checks. Under both sh and dash, an initial WIP commit gets this note but resets because the unborn HEAD could not be recorded | The shipped prompt predicts preservation when the implemented attribution rule requires a reset; CHANGELOG.md:43 repeats the incomplete condition | Use neutral wording about the pending decision or qualify preservation by successful attribution and record availability, updating the title and changelog consistently +MINOR | high | CLAUDE.md:1320-1323; plugins/dev-workflow/commands/workflow-init.md:1509-1512 | The purported accepted command grammar omits the blanket shell-metacharacter and backslash veto, including inside quotes. git commit --allow-empty -m 'WIP: fix(api)' satisfies the documented form but is rejected and resets after a successful WIP commit, reproduced under sh and dash | Users following the scaffolded instructions can unexpectedly lose counters for ordinary snapshot messages | State the character restriction in both identical Mechanics copies, or describe only a known accepted example without claiming the complete grammar +MINOR | high | docs/architecture.md:94-98; todos.md:512-514 | The updated summaries describe direct descent from recorded HEAD as the preservation condition and say other detected commits reset, omitting the implemented shared-parent --amend -m path and unchanged-HEAD failed-command path | These summaries contradict Mechanics and the m10/m4 regression cases, leaving the requested behavior documentation inaccurate | Include both alternatives or reference the authoritative Mechanics description instead of restating the decision procedure +END OF FINDINGS (5 total) diff --git a/.context/codex-reviews/gate-b-quality-hpg0kyk2nz-pass-4.md b/.context/codex-reviews/gate-b-quality-hpg0kyk2nz-pass-4.md new file mode 100644 index 0000000..2bc5918 --- /dev/null +++ b/.context/codex-reviews/gate-b-quality-hpg0kyk2nz-pass-4.md @@ -0,0 +1,5 @@ +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:864 | An absent HEAD reflog becomes length 0, and unchanged final HEAD with recorded/current lengths 0 passes as a failed commit. Reproduced under sh and dash with core.logAllRefUpdates=false, no reflog, a prepare-commit-msg hook rewriting the commit to a real subject, and a post-commit hook resetting to the original WIP HEAD. | Counters survive a real boundary whose intermediate history is unavailable, violating invariant 2 and the documented no-reflog reset rule. | Require an available, nonempty HEAD reflog before either preservation branch; reset on unavailable history and add this real lifecycle regression under both shells. +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:861-865 | The digit-only record validation accepts 08 or 09, but arithmetic expansion interprets a leading zero as octal. With recorded length 08 and an advanced WIP HEAD, PostToolUse exits 1 under sh and 2 under dash. | A corrupt attribution record aborts the advisory hook before the counter reset, violating invariant 1 and leaving stale Gate-B state. | Reject noncanonical or out-of-range numeric records before arithmetic, or safely normalize and bound them; test malformed lengths through PostToolUse under sh and dash. +MINOR | high | plugins/dev-workflow/hooks/codex-gate.sh:1024 | The shared WIP note says preservation uses how HEAD moved and clears counters when the result cannot be attributed to the command, but it also serves --amend --no-edit. That unchanged path records no base and checks only command shape, repository rewrite conditions, and the current WIP subject. | The shipped prompt claims attribution protection that the no-edit path does not implement, contrary to prompt-standards item 11. | Keep the requested no-edit behavior unchanged and shorten the shared note to reference the applicable policy conditions, or distinguish the two paths in the message. +MINOR | high | docs/getting-started.md:55-56 | The revised example still says the hook knows the plain git commit -m 'WIP: …' command does not end the cycle, without qualifying preservation by the resulting subject and attribution checks. The exact command resets if a repository hook rewrites its subject or attribution fails. | The getting-started guide retains the unconditional WIP-preservation claim that the original task explicitly required correcting. | Replace the parenthetical promise with a reference to the implemented WIP conditions in CLAUDE.md section 5 or the 0.13.3 CHANGELOG. +END OF FINDINGS (4 total) diff --git a/.context/codex-reviews/gate-b-quality-hpg0kyk2nz-pass-5.md b/.context/codex-reviews/gate-b-quality-hpg0kyk2nz-pass-5.md new file mode 100644 index 0000000..3ea8011 --- /dev/null +++ b/.context/codex-reviews/gate-b-quality-hpg0kyk2nz-pass-5.md @@ -0,0 +1,2 @@ +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:869 | Unchanged HEAD and reflog are treated as a failed completed commit without checking whether Bash returned while the command is still running. Reproduced under sh and dash: start on a WIP HEAD with two passes, run the accepted git commit --allow-empty -m 'WIP: next' with run_in_background=true and a waiting prepare-commit-msg hook, deliver PostToolUse with backgroundTaskId, then release the hook to rewrite the subject to real subject. | PostToolUse preserves both passes and removes the attribution record; the real commit subsequently lands with the count still 2, missing the required reset at a real boundary. Background Bash returns immediately while execution continues, so this is not evidence of a failed commit. | Reject backgrounded or incomplete Bash results before granting the new -m WIP exemption, covering explicit background input and automatic/manual background response signals; reset conservatively and add the delayed real-commit regression under sh and dash. +END OF FINDINGS (1 total) diff --git a/.context/codex-reviews/gate-b-quality-hpg0kyk2nz-pass-6.md b/.context/codex-reviews/gate-b-quality-hpg0kyk2nz-pass-6.md new file mode 100644 index 0000000..8d781f4 --- /dev/null +++ b/.context/codex-reviews/gate-b-quality-hpg0kyk2nz-pass-6.md @@ -0,0 +1,2 @@ +NO FINDINGS +END OF FINDINGS (0 total) diff --git a/.context/codex-reviews/gate-b-spec-hpg0kyk2nz-pass-1.md b/.context/codex-reviews/gate-b-spec-hpg0kyk2nz-pass-1.md new file mode 100644 index 0000000..f0771fc --- /dev/null +++ b/.context/codex-reviews/gate-b-spec-hpg0kyk2nz-pass-1.md @@ -0,0 +1,5 @@ +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:818 | The supposed value-less flag group accepts operand-taking flags, including -m and -F. With an edited tracked file named WIP, git commit -m -m WIP matches the exemption although Git uses the second -m as its message; git commit -F -m WIP similarly reads the message from a file named -m. These are successful real commits, not attributable WIP commands. | PreToolUse emits the WIP note and suppresses the ordinary Gate-B reminder before a real boundary, violating the requested conservative recognition and invariant 2. | Allow only explicitly known value-less options before the message option; reject ambiguous argument forms and add real-command regressions under sh and dash. +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:831 | The pre-commit decision checks repository hooks/settings but accepts message-changing command options. The matcher permits --edit/-e and ignores everything after the WIP prefix: git commit --allow-empty -m WIP --fixup=HEAD creates a fixup! subject; adding --edit lets the configured editor replace WIP with a real subject. Both received the WIP note in real runs under sh and dash. | The ordinary Gate-B reminder is suppressed even when the command itself can rewrite the message; resetting afterward does not satisfy the separate before-commit requirement. | Validate the complete supported command form and make editor or message-transforming options take the ordinary reminder path; add real-run coverage for --edit and --fixup. +MINOR | high | docs/architecture.md:97; CLAUDE.md:1325; plugins/dev-workflow/commands/workflow-init.md:1514 | The new claims that any other spelling resets and a snapshot made any other way discards counters imply recognition of arbitrary Bash commit forms. The detector still requires literal commit text: git com""mit --allow-empty -m real succeeds while both hook phases are silent and the counters remain, reproduced under sh and dash. | The documentation promises a reset the implementation does not perform, contrary to the explicit requirement to bound command-attribution claims. | Qualify these statements to commit attempts recognized by the hook and avoid promising coverage for arbitrary Bash spellings; keep the Mechanics copies identical. +MINOR | high | CLAUDE.md:1319; plugins/dev-workflow/commands/workflow-init.md:1508; docs/architecture.md:93; docs/getting-started.md:55 | The recommended double-quoted git commit -m "WIP: …" is described as preserving the cycle, but without optional jq the fallback reader truncates at the escaped opening quote and the new matcher rejects the remaining backslash. A successful WIP commit then loses its counters, reproduced under sh and dash. | The documented WIP workaround does not preserve the review cycle in a supported jq-free environment; the new real-run tests use single quotes and miss this discrepancy. | Recommend a supported single-quoted WIP command consistently, or explicitly document the conservative jq-free reset for double-quoted messages; cover the documented spelling without jq. +END OF FINDINGS (4 total) diff --git a/.context/codex-reviews/gate-b-spec-hpg0kyk2nz-pass-2.md b/.context/codex-reviews/gate-b-spec-hpg0kyk2nz-pass-2.md new file mode 100644 index 0000000..109eb83 --- /dev/null +++ b/.context/codex-reviews/gate-b-spec-hpg0kyk2nz-pass-2.md @@ -0,0 +1,4 @@ +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:839 | PostToolUse treats a WIP HEAD as sufficiently attributable even when repository hooks create additional history. Reproduced under sh and dash: run the accepted git commit --allow-empty -m 'WIP: requested' with prepare-commit-msg rewriting the subject to 'real boundary' and post-commit running git -c core.hooksPath=/dev/null commit --allow-empty -m 'WIP: follow-up'; the command succeeds and history becomes WIP follow-up -> real boundary -> initial, but passCount remains 2 | Counters survive a real boundary, violating the requested reset on real boundaries or undecidable history and invariant 2; checking the final subject does not identify the requested commit | Preserve counters only when the result can be attributed without intervening real commits; otherwise reset conservatively, including hook environments where that attribution cannot be established. Add this real lifecycle regression under both shells without changing the amend/no-edit rules +MINOR | high | CLAUDE.md:1327; plugins/dev-workflow/commands/workflow-init.md:1516 | The Mechanics text prescribes single quotes and a flag subset, then says any detected commit made any other way resets; the matcher also preserves --all, --quiet, bare WIP messages and double-quoted WIP messages with jq | Both synchronized prompt copies still misdescribe implemented WIP recognition, contrary to the documentation task and prompt-standards item 11 | Identify the shown command as a recommended subset and qualify the reset claim using the actual supported forms; keep both copies identical +MINOR | high | plugins/dev-workflow/CHANGELOG.md:34; plugins/dev-workflow/hooks/codex-gate.sh:782 | The new text says only the single-quoted form qualifies without jq, but the fallback reads git commit -m WIP intact and the matcher accepts that bare form under both sh and dash | The release notes and implementation comment incorrectly state the jq-free recognition boundary | Say single-quoted and bare forms qualify without jq, while double-quoted messages fail the fallback reader +END OF FINDINGS (3 total) diff --git a/.context/codex-reviews/gate-b-spec-hpg0kyk2nz-pass-3.md b/.context/codex-reviews/gate-b-spec-hpg0kyk2nz-pass-3.md new file mode 100644 index 0000000..2167c7d --- /dev/null +++ b/.context/codex-reviews/gate-b-spec-hpg0kyk2nz-pass-3.md @@ -0,0 +1,5 @@ +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:853-855 | The endpoint comparisons do not detect a real commit followed by an amend back to WIP. Reproduced under sh and dash: prepare-commit-msg changes the requested WIP subject to 'real subject', then post-commit runs git -c core.hooksPath=/dev/null commit --allow-empty --amend -m 'WIP: amended real'. The reflog contains the real commit followed by the WIP amend, but the final HEAD still has the recorded HEAD as its parent and passCount stays 2. | Counters cross a real boundary despite the requirement to reset at a real boundary or an undecidable history; recording only HEAD and its parent does not establish that the final WIP was the command's sole commit result. | Validate intervening HEAD transitions, or reset conservatively when they cannot be established; add this real-hook regression under sh and dash and bound the corresponding attribution claims. +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:856 | The --amend test searches the entire command, including the quoted -m value. Reproduced under sh and dash with git commit -q --allow-empty -m 'WIP: explain --amend option' and a post-commit hook that resets --soft HEAD~2 and creates a WIP sibling: the new HEAD shares the recorded parent, so the counters survive although the command supplied no --amend option. | Message text incorrectly enables the amend attribution exception and preserves counters for an unrelated replacement HEAD; this history should be undecidable and reset. | Determine amend mode from the allow-listed options outside the message and carry that result into attribution; test that mentioning --amend inside a message cannot enable the sibling-parent exception. +MINOR | high | plugins/dev-workflow/hooks/codex-gate.sh:1013 | The revised WIP note still says counters are kept if the resulting commit is WIP, omitting the required attribution record and ancestry checks. A real first WIP commit in an unborn repository reproduces the contradiction under both shells: PreToolUse emits this note, record_wip_base cannot record HEAD, and PostToolUse clears the counters despite the WIP result. | The shipped prompt promises preservation in a case where the implemented conservative behavior resets, contrary to the requested behavior-accurate WIP guidance and prompt-standard enforcement-claim rule. | Make the note conditional on a WIP result that also passes attribution, or state simply that PostToolUse decides preservation; align the matching CHANGELOG statement. +MINOR | high | docs/architecture.md:94-99; todos.md:512-513 | Architecture lists only a WIP result directly on the recorded HEAD plus the separate --amend --no-edit case, then says any other detected commit resets; todos likewise says counters stay only when the result follows directly on the recorded HEAD. Both omit the implemented unchanged-WIP-HEAD and --amend -m WIP sibling-parent cases. | These descriptions contradict the implementation and the Mechanics copies for failed WIP commands and explicit-message WIP amends, leaving the requested documentation synchronization incomplete. | Include both additional accepted attribution cases or refer to the canonical 0.13.3 description instead of restating an incomplete decision procedure. +END OF FINDINGS (4 total) diff --git a/.context/codex-reviews/gate-b-spec-hpg0kyk2nz-pass-4.md b/.context/codex-reviews/gate-b-spec-hpg0kyk2nz-pass-4.md new file mode 100644 index 0000000..45dffc8 --- /dev/null +++ b/.context/codex-reviews/gate-b-spec-hpg0kyk2nz-pass-4.md @@ -0,0 +1,4 @@ +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:864 | A missing HEAD reflog is counted as zero and the unchanged-HEAD branch accepts matching zero counts. Reproduced under sh and dash with core.logAllRefUpdates=false and no logs: prepare-commit-msg rewrites the requested WIP to real, then post-commit resets HEAD to its previous WIP commit; PostToolUse retains the counters. | A real boundary survives as an apparently failed WIP attempt, violating the required reset on undecidable history and contradicting the documented no-reflog reset. | Require a successfully read, nonempty HEAD reflog before accepting either attribution branch; reset when reflog evidence is unavailable and add this real lifecycle regression under both shells. +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:861 | The digits-only record check admits 08, which is then used in POSIX shell arithmetic at line 865. With a changed WIP HEAD and a wipBase record containing 08 as its reflog count, the hook exits 1 under sh and 2 under dash with an invalid-octal error. | Corrupt persistent state violates the always-exit-0 invariant and aborts before clearing the old gate counters, instead of conservatively resetting. | Validate or safely normalize the count before arithmetic, including leading zeros and arithmetic bounds; reject invalid records through the ordinary reset path and add regression coverage for both shells. +MINOR | high | docs/getting-started.md:55 | The revised example still says the hook knows a plain git commit -m 'WIP: …' does not end the cycle, without conditioning preservation on the resulting subject and attribution evidence. Even a successful plain WIP commit resets when the PreToolUse record or HEAD reflog is unavailable. | The requested documentation correction remains incomplete: command spelling alone is presented as sufficient to preserve the counters, while the implementation deliberately requires more. | Qualify the statement with the post-commit attribution condition or point to the authoritative 0.13.3 rule, as architecture.md does. +END OF FINDINGS (3 total) diff --git a/.context/codex-reviews/gate-b-spec-hpg0kyk2nz-pass-5.md b/.context/codex-reviews/gate-b-spec-hpg0kyk2nz-pass-5.md new file mode 100644 index 0000000..8d781f4 --- /dev/null +++ b/.context/codex-reviews/gate-b-spec-hpg0kyk2nz-pass-5.md @@ -0,0 +1,2 @@ +NO FINDINGS +END OF FINDINGS (0 total) diff --git a/.context/codex-reviews/gate-b-spec-hpg0kyk2nz-pass-6.md b/.context/codex-reviews/gate-b-spec-hpg0kyk2nz-pass-6.md new file mode 100644 index 0000000..8d781f4 --- /dev/null +++ b/.context/codex-reviews/gate-b-spec-hpg0kyk2nz-pass-6.md @@ -0,0 +1,2 @@ +NO FINDINGS +END OF FINDINGS (0 total) diff --git a/CLAUDE.md b/CLAUDE.md index b9cec66..609c325 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1315,9 +1315,22 @@ like the rest of §5; the detection is a reader comparing the pass against the s object name `HEAD` resolves to at that moment, never the symbolic `HEAD` — see the branch-agreement rule below for why); pre-commit, `baseSha` = HEAD is an empty range (HEAD..HEAD) — make a WIP commit - and set `baseSha` to its parent. **Name that commit `WIP: …`** — the hook treats a - `wip`-prefixed commit message as cycle-internal, so it neither fires a Gate-B STOP - nor resets your pass counters. A pre-review snapshot named anything else reads to the hook as a real commit: the hook treats the + and set `baseSha` to its parent. **Make that commit as `git commit -m 'WIP: …'`** — the + recommended form, because the hook recognises a WIP commit from the command and the commit + that results, not from the message alone. What it accepts is a one-line `git commit` with + exactly one `-m` whose value starts with `wip` — single-quoted, double-quoted or bare, though + without `jq` a double-quoted message cannot be read whole — beside nothing but `-a`/`--all`, + `-q`/`--quiet`, `-n`/`--no-verify`, `--amend` or `--allow-empty`; so no chain, `cd`, + `git -C`, editor or `--fixup`, and none of `; & | < > $ ( )`, a backtick or a backslash + anywhere, the message included. Beforehand, where a hook or setting in the repository could + rewrite the message, it shows the ordinary Gate-B reminder instead of the WIP note. + Afterwards it keeps your pass counters only if the resulting commit's subject starts with + `wip` and `HEAD` moved by exactly this one commit since the command started — directly on + the previous `HEAD`, or on its parent for `--amend`, with nothing else in between — or did + not move at all, if the command failed; a commit run in the background resets. A + plain `git commit --amend --no-edit` on a WIP commit counts too, under narrower conditions + the dev-workflow CHANGELOG lists at 0.13.2. Any other commit the hook detects reads to it + as a real commit — a snapshot named anything else, or made any other way: the hook treats the cycle as closed and **discards its count of the passes you just accumulated**, while the cycle itself stays open until the closure ordering's conditions hold. **What the hook loses is its counter state**, and that counter is not what makes a pass valid — so the reminder now understates what you hold, and no diff --git a/docs/architecture.md b/docs/architecture.md index 6d6e23b..7315360 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -88,6 +88,13 @@ Two consequences worth knowing: `git status --porcelain`. It is excluded from the hash — otherwise the hash would change every time the hook wrote to it, and could never match itself. - CLAUDE.md §5 tells you to make a throwaway commit so `mcp__codex__review` has a - non-empty range to read. A commit whose message starts with `WIP` is treated as - cycle-internal: no STOP, and the pass counters survive. Otherwise the documented - workaround would destroy the cycle it exists to serve. + non-empty range to read. That commit is treated as cycle-internal — otherwise the + documented workaround would destroy the cycle it exists to serve — when it is a one-line + `git commit` with one WIP `-m` and only a few value-less flags beside it (CHANGELOG + 0.13.3): the pass counters survive if the commit that results starts with `WIP` and is + attributable to that command by the rule the 0.13.3 CHANGELOG entry states, and a + repository whose hooks or settings could rewrite the message gets the ordinary reminder + beforehand. A plain `git commit --amend --no-edit` on a WIP commit also counts, under + narrower conditions (CHANGELOG 0.13.2). The hook reads the command and the resulting + commit, not the message alone; any other commit it detects resets. A commit spelled so + that the hook does not detect it as a commit at all is outside both rules. diff --git a/docs/getting-started.md b/docs/getting-started.md index 47d557a..3d7edf0 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -52,8 +52,9 @@ so right when execution starts. code + duplication + tests) must be green locally. CI runs the same command, so skipping locally only postpones the red. -**7. Gate B on the diff.** Claude makes a `WIP:`-prefixed commit (gives Codex a -range to read; the hook knows WIP doesn't end the cycle), then loops +**7. Gate B on the diff.** Claude makes a plain `git commit -m 'WIP: …'` (gives Codex a +range to read; the hook keeps the cycle open across it under the conditions `CLAUDE.md` §5 +Mechanics states), then loops `mcp__codex__review` the same way: the derived floor, final clean. Invalidation is by **content** — any change to included content present when the hook runs, even from a formatter, makes the hook report that it cannot confirm the reviewed content. What that proves is bounded, and the hook's own diff --git a/plugins/dev-workflow/.claude-plugin/plugin.json b/plugins/dev-workflow/.claude-plugin/plugin.json index 27db375..8d06d7c 100644 --- a/plugins/dev-workflow/.claude-plugin/plugin.json +++ b/plugins/dev-workflow/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "dev-workflow", "displayName": "Cross-Model Review Workflow", - "version": "0.13.2", + "version": "0.13.3", "description": "Spec-driven workflow with two independent cross-model review gates, an append-only hardening ledger with an escalation ladder, and repo-enforced quality. Requires the superpowers plugin.", "author": { "name": "Daniel Sänger", diff --git a/plugins/dev-workflow/CHANGELOG.md b/plugins/dev-workflow/CHANGELOG.md index 95715b4..722d733 100644 --- a/plugins/dev-workflow/CHANGELOG.md +++ b/plugins/dev-workflow/CHANGELOG.md @@ -22,6 +22,36 @@ unambiguously, still fails. Deleting only a plugin's *manifest* while the direct keeps shipping fails too. AGENTS.md invariant 12 carries the complete list. +## 0.13.3 + +- **`git commit -m "WIP: …"` is judged before and after the commit separately.** The hook + used one check for both: any command containing `-m "wip…"` got the WIP note beforehand and + kept the counters afterwards, 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 cycle. + Now only an allow-listed one-line form qualifies: `git commit`, then only `-a`, `--all`, + `-q`, `--quiet`, `--no-verify`, `-n`, `--amend` or `--allow-empty`, and exactly one `-m` + whose quoted or bare value starts with `wip` — no editor, `--fixup`, `-F`, second `-m`, + chain, redirect, `cd`, `git -C`, shell metacharacter or backslash. Without `jq` the + single-quoted and bare forms qualify, since the fallback reader truncates a double-quoted + one. Beforehand, a repository whose hooks or + settings could rewrite the message gets the ordinary Gate-B reminder instead of the WIP note + — a reminder, not a reset. Afterwards, the counters stay only if the resulting commit's + subject starts with `wip` and it is attributable to the command: PreToolUse records `HEAD`, + its parent, the length of `HEAD`'s reflog and whether `--amend` is an option (outside quoted + text) in `.context/codex-gate.wipBase`. Afterwards either `HEAD` and the reflog are + unchanged (the commit failed, and the WIP commit already there stays), or the reflog grew + by exactly one and the new `HEAD` sits directly on the recorded one, or shares its parent + for `--amend`. A hook that commits, amends or resets in between, a missing record, an empty + or unreadable `HEAD` reflog, or a Bash call sent to the background (its result arrives + before the commit finishes) resets. The record is removed after every detected commit. + The WIP note no longer promises the counters are kept; it says that is decided after the + commit. The `--amend --no-edit` rules from 0.13.2 are unchanged. +- **`CLAUDE.md` §5 Mechanics and its `/dev-workflow:workflow-init` copy** now say the hook + recognises the WIP commit from the command and the resulting commit rather than from a + `wip`-prefixed message, recommend the single-quoted command form and state what else is + accepted; `docs/architecture.md` and + `docs/getting-started.md` follow. + ## 0.13.2 - **`git commit --amend --no-edit` on a `WIP:` commit no longer resets the Gate-B cycle.** The diff --git a/plugins/dev-workflow/commands/workflow-init.md b/plugins/dev-workflow/commands/workflow-init.md index 8c480f1..70d3f63 100644 --- a/plugins/dev-workflow/commands/workflow-init.md +++ b/plugins/dev-workflow/commands/workflow-init.md @@ -1504,9 +1504,22 @@ like the rest of §5; the detection is a reader comparing the pass against the s object name `HEAD` resolves to at that moment, never the symbolic `HEAD` — see the branch-agreement rule below for why); pre-commit, `baseSha` = HEAD is an empty range (HEAD..HEAD) — make a WIP commit - and set `baseSha` to its parent. **Name that commit `WIP: …`** — the hook treats a - `wip`-prefixed commit message as cycle-internal, so it neither fires a Gate-B STOP - nor resets your pass counters. A pre-review snapshot named anything else reads to the hook as a real commit: the hook treats the + and set `baseSha` to its parent. **Make that commit as `git commit -m 'WIP: …'`** — the + recommended form, because the hook recognises a WIP commit from the command and the commit + that results, not from the message alone. What it accepts is a one-line `git commit` with + exactly one `-m` whose value starts with `wip` — single-quoted, double-quoted or bare, though + without `jq` a double-quoted message cannot be read whole — beside nothing but `-a`/`--all`, + `-q`/`--quiet`, `-n`/`--no-verify`, `--amend` or `--allow-empty`; so no chain, `cd`, + `git -C`, editor or `--fixup`, and none of `; & | < > $ ( )`, a backtick or a backslash + anywhere, the message included. Beforehand, where a hook or setting in the repository could + rewrite the message, it shows the ordinary Gate-B reminder instead of the WIP note. + Afterwards it keeps your pass counters only if the resulting commit's subject starts with + `wip` and `HEAD` moved by exactly this one commit since the command started — directly on + the previous `HEAD`, or on its parent for `--amend`, with nothing else in between — or did + not move at all, if the command failed; a commit run in the background resets. A + plain `git commit --amend --no-edit` on a WIP commit counts too, under narrower conditions + the dev-workflow CHANGELOG lists at 0.13.2. Any other commit the hook detects reads to it + as a real commit — a snapshot named anything else, or made any other way: the hook treats the cycle as closed and **discards its count of the passes you just accumulated**, while the cycle itself stays open until the closure ordering's conditions hold. **What the hook loses is its counter state**, and that counter is not what makes a pass valid — so the reminder now understates what you hold, and no diff --git a/plugins/dev-workflow/hooks/codex-gate.sh b/plugins/dev-workflow/hooks/codex-gate.sh index dafb749..cc637d3 100755 --- a/plugins/dev-workflow/hooks/codex-gate.sh +++ b/plugins/dev-workflow/hooks/codex-gate.sh @@ -57,6 +57,7 @@ countA_file="$state_dir/codex-gate.passCountA" # Gate A (exec) passes since l bgadv_file="$state_dir/codex-gate.bgAdvice" unver_file="$state_dir/codex-gate.unverified" pend_file="$state_dir/codex-gate.unverifiedPending" +wipbase_file="$state_dir/codex-gate.wipBase" # HEAD (and its parent) when a `-m wip` commit starts # FINDING G: the plugin is installed globally, but the workflow is adopted per project. # A repo that never ran /workflow-init has no gate to enforce, so the hook does NOTHING @@ -761,32 +762,78 @@ is_commit() { printf '%s' "$1" | grep -Eq '(^|[^[:alnum:]])git[[:space:]].*commi # spurious STOP and reset the very counters the review loop is accumulating — the # documented workaround would fight the hook. So: gentle note, no reset. # -# `git commit --amend --no-edit` carries no `-m` and normally keeps HEAD's message, so on -# a WIP HEAD it is usually a WIP commit too, and PreToolUse and PostToolUse both read -# HEAD's subject for it. "Usually" is why this exemption refuses whenever anything could -# change that message: it reads HEAD of the repository the hook runs in, and a command -# string is not its arguments, so it is an allow-list, never a deny-list: one line of exactly -# `git commit` followed only by `--amend`, `--no-edit`, `--no-verify`, -# `-a`, `--all`, `-q` or `--quiet`, both of the first two present. No quote, `#`, backslash -# (the jq-free reader truncates at an escaped quote and leaves one) or shell metacharacter -# anywhere, so no quoting trick, comment, pathspec, `cd`, `git -C` or chained command can -# qualify. Global `git -c` is refused too: its operand can expand into `-C ` or set -# `commit.cleanup` and strip the WIP subject — and for the same reason a repository that -# sets its own comment character is refused, since cleanup can then strip a `WIP` line. -# So is one whose hooks directory holds anything but `*.sample` files — a deliberately -# stricter rule than Git's own (Git runs only executable files with hook names): any other -# entry refuses, executable or not, because a message hook can rewrite the message -# (`--no-verify` does not skip `prepare-commit-msg`) and an earlier hook such as -# `pre-commit` can install one mid-commit. Hook contents are never read. A hooks directory -# this check cannot list refuses too. Any `core.hooksPath` at all is refused outright rather -# than resolved, since a path this shell cannot carry exactly (a trailing newline, say) -# would send the check to the wrong directory. The check sees the directory as it is when -# the hook runs, not what another process does afterwards. The `-m "wip…"` path has the -# same exposure to hooks; it predates this and is left as it was, recorded in todos.md. -# Anything else falls through to the reset, the safe direction. The `-m "wip…"` match above -# is separate and unchanged. -is_wip_commit() { - printf '%s' "$1" | grep -Eiq -- "-m[[:space:]]*['\"]?[[:space:]]*wip" && return 0 +# Two questions, answered separately, because they are asked at different times: +# PreToolUse decides whether to show the WIP note instead of the Gate-B reminder, before the +# commit runs; PostToolUse decides whether to keep the counters, after it ran. A reminder +# shown in doubt costs nothing, so is_wip_commit_pre refuses whenever anything could change +# the message; is_wip_commit_post can read the commit that resulted, so it keeps the counters +# only when that commit is WIP. Everything else resets, the safe direction. +# +# `-m "wip…"`: an allow-list again — one line of exactly `git commit`, then only `-a`, +# `--all`, `-q`, `--quiet`, `--no-verify`, `-n`, `--amend` or `--allow-empty`, and exactly one +# `-m` whose single-quoted, double-quoted or bare value starts with `wip`; nothing else, so no +# editor (`-e`), `--fixup`, `-F`, second `-m`, `cd`, `git -C`, chain or redirect, and no shell +# metacharacter or backslash anywhere (the jq-free reader truncates a double-quoted message at +# its escaped quote and leaves one, so without jq only the single-quoted and bare forms +# qualify). Before the +# commit, a repository whose hooks or settings could rewrite the message gets the Gate-B +# reminder instead of the note; after it, the counters stay only if HEAD's subject starts with +# `wip` AND HEAD is attributable to this command. PreToolUse records HEAD, its parent, the +# number of entries in HEAD's reflog, and whether `--amend` is one of the command's options +# (looked for outside quoted text, so a message mentioning it does not count). Afterwards +# either HEAD and the reflog are both unchanged (the commit failed; the WIP commit already +# there stays), or the reflog grew by exactly one entry and HEAD sits directly on the +# recorded HEAD (a new commit) or, for `--amend`, shares the recorded parent. Any other +# change to HEAD in between — a hook that commits, amends or resets — adds reflog entries and +# resets, as does a missing or unreadable record, and so does a repository whose HEAD +# reflog is empty or unreadable before or after: without it neither branch can be told apart +# from a hook that moved HEAD and moved it back. A Bash call that was sent to the background +# (`run_in_background`, or a result carrying a `backgroundTaskId`) is refused too: its +# PostToolUse arrives before the commit has finished, so an unchanged HEAD proves nothing. The record lives only from one PreToolUse to the next +# PostToolUse and is removed there. +# +# `git commit --amend --no-edit` carries no `-m` and normally keeps HEAD's message. It is an +# allow-list, never a deny-list: one line of exactly `git commit` followed only by `--amend`, +# `--no-edit`, `--no-verify`, `-a`, `--all`, `-q` or `--quiet`, both of the first two present, +# with no quote, `#`, backslash or shell metacharacter anywhere, so no quoting trick, comment, +# pathspec, `cd`, `git -C` or chained command can qualify, and global `git -c` is refused +# (its operand can expand into `-C ` or set `commit.cleanup`). It also requires, in +# both phases, that nothing could rewrite the message (below) and that HEAD's subject starts +# with `wip`. +# +# "Could rewrite the message": a custom comment character (cleanup can then strip a `WIP` +# line); any `core.hooksPath` at all, refused rather than resolved, since a path this shell +# cannot carry exactly (a trailing newline, say) would send the check to the wrong directory; +# or a hooks directory holding anything but `*.sample` files — deliberately stricter than +# Git's own rule (Git runs only executable files with hook names), because a message hook +# can rewrite the message (`--no-verify` does not skip `prepare-commit-msg`) and an earlier +# hook such as `pre-commit` can install one mid-commit. Hook contents are never read, and a +# hooks directory this check cannot list counts as able to. The check sees the repository +# as it is when the hook runs, not what another process does afterwards. +msg_may_be_rewritten() { + git -C "$repo_root" config --get-regexp '^core\.comment(char|string)$' >/dev/null 2>&1 && return 0 + git -C "$repo_root" config --get core.hooksPath >/dev/null 2>&1 && return 0 + _hooks=$(git -C "$repo_root" rev-parse --git-path hooks 2>/dev/null) || return 0 + [ -n "$_hooks" ] || return 0 + case $_hooks in /*) ;; *) _hooks="$repo_root/$_hooks" ;; esac + if [ -e "$_hooks" ] || [ -L "$_hooks" ]; then + [ -d "$_hooks" ] && [ -r "$_hooks" ] && [ -x "$_hooks" ] || return 0 + for _f in "$_hooks"/* "$_hooks"/.[!.]* "$_hooks"/..?*; do + [ -e "$_f" ] || [ -L "$_f" ] || continue + case ${_f##*/} in *.sample) ;; *) return 0 ;; esac + done + fi + return 1 +} +head_is_wip() { git -C "$repo_root" log -1 --format=%s 2>/dev/null | grep -Eiq '^[[:space:]]*wip'; } +is_wip_message_cmd() { + case $1 in *' +'*) return 1 ;; esac + printf '%s' "$1" | grep -q '[;&|<>$`()\\]' && return 1 + _ok='(-a|--all|-q|--quiet|--no-verify|-n|--amend|--allow-empty)' + printf '%s' "$1" | grep -Eiq -- "^[[:space:]]*git[[:space:]]+commit([[:space:]]+$_ok)*[[:space:]]+-m[[:space:]]*('[[:space:]]*wip[^']*'|\"[[:space:]]*wip[^\"]*\"|wip[^[:space:]'\"]*)([[:space:]]+$_ok)*[[:space:]]*$" +} +is_wip_amend_cmd() { case $1 in *' '*) return 1 ;; esac printf '%s' "$1" | grep -q '[;&|<>$`()\\#"]' && return 1 @@ -794,19 +841,46 @@ is_wip_commit() { printf '%s' "$1" | grep -Eq '^[[:space:]]*git[[:space:]]+commit([[:space:]]+(--amend|--no-edit|--no-verify|-a|--all|-q|--quiet))+[[:space:]]*$' || return 1 printf '%s ' "$1" | grep -Eq '[[:space:]]--amend[[:space:]]' || return 1 printf '%s ' "$1" | grep -Eq '[[:space:]]--no-edit[[:space:]]' || return 1 - git -C "$repo_root" config --get-regexp '^core\.comment(char|string)$' >/dev/null 2>&1 && return 1 - git -C "$repo_root" config --get core.hooksPath >/dev/null 2>&1 && return 1 - _hooks=$(git -C "$repo_root" rev-parse --git-path hooks 2>/dev/null) || return 1 - [ -n "$_hooks" ] || return 1 - case $_hooks in /*) ;; *) _hooks="$repo_root/$_hooks" ;; esac - if [ -e "$_hooks" ] || [ -L "$_hooks" ]; then - [ -d "$_hooks" ] && [ -r "$_hooks" ] && [ -x "$_hooks" ] || return 1 - for _f in "$_hooks"/* "$_hooks"/.[!.]* "$_hooks"/..?*; do - [ -e "$_f" ] || [ -L "$_f" ] || continue - case ${_f##*/} in *.sample) ;; *) return 1 ;; esac - done - fi - git -C "$repo_root" log -1 --format=%s 2>/dev/null | grep -Eiq '^[[:space:]]*wip' + ! msg_may_be_rewritten && head_is_wip +} +head_reflog_len() { git -C "$repo_root" reflog show --format=%H HEAD 2>/dev/null | grep -c ''; } +record_wip_base() { # PreToolUse: " "; best effort + rm -f "$wipbase_file" 2>/dev/null + is_wip_message_cmd "$1" || return 0 + _h=$(git -C "$repo_root" rev-parse -q --verify HEAD 2>/dev/null) || return 0 + _p=$(git -C "$repo_root" rev-parse -q --verify HEAD^ 2>/dev/null) || _p=- + _n=$(head_reflog_len) + case $_n in '' | 0 | 0* | *[!0-9]* | ??????????*) return 0 ;; esac + _m=new + printf '%s ' "$1" | sed "s/'[^']*'//g; s/\"[^\"]*\"//g" | grep -Eiq '[[:space:]]--amend[[:space:]]' && _m=amend + mkdir -p "$state_dir" 2>/dev/null + { printf '%s %s %s %s\n' "$_h" "$_p" "$_n" "$_m" > "$wipbase_file"; } 2>/dev/null || true +} +bash_backgrounded() { printf '%s' "$payload" | grep -Eq '"run_in_background"[[:space:]]*:[[:space:]]*true|"backgroundTaskId"'; } +wip_base_holds() { # PostToolUse: is HEAD this command's own result, or unchanged? + _rh='' _rp='' _rn='' _rm='' _rx='' + [ -f "$wipbase_file" ] || return 1 + read -r _rh _rp _rn _rm _rx 2>/dev/null < "$wipbase_file" + rm -f "$wipbase_file" 2>/dev/null + [ -n "$_rh" ] && [ -n "$_rp" ] && [ -n "$_rm" ] && [ -z "$_rx" ] || return 1 + # a positive decimal with no leading zero and at most nine digits, so the arithmetic + # below can neither read it as octal nor overflow + case $_rn in '' | 0* | *[!0-9]* | ??????????*) return 1 ;; esac + _h=$(git -C "$repo_root" rev-parse -q --verify HEAD 2>/dev/null) || return 1 + _n=$(head_reflog_len) + case $_n in '' | 0* | *[!0-9]* | ??????????*) return 1 ;; esac + if [ "$_h" = "$_rh" ]; then [ "$_n" = "$_rn" ]; return; fi + [ "$_n" = "$((_rn + 1))" ] || return 1 + _p=$(git -C "$repo_root" rev-parse -q --verify HEAD^ 2>/dev/null) || return 1 + if [ "$_rm" = amend ]; then [ "$_p" = "$_rp" ]; else [ "$_p" = "$_rh" ]; fi +} +is_wip_commit_pre() { + is_wip_message_cmd "$1" && ! msg_may_be_rewritten && return 0 + is_wip_amend_cmd "$1" +} +is_wip_commit_post() { + is_wip_message_cmd "$1" && ! bash_backgrounded && head_is_wip && wip_base_holds && return 0 + is_wip_amend_cmd "$1" } # A commit that stages all tracked changes (-a / -am / --all) also sweeps in @@ -918,7 +992,7 @@ case "$event" in Bash) cmd=$(input_field command) # RESET on commit closes the Gate-B cycle. A WIP commit does NOT close it - # (see is_wip_commit). + # (see is_wip_commit_post). # # We reset regardless of whether the commit actually SUCCEEDED. The Bash # tool_response shape is documented as {stdout, stderr, interrupted, isImage} @@ -930,9 +1004,11 @@ case "$event" in # wrongly judged the commit failed — would carry passes across a real cycle # boundary and produce a false ✓, which is the failure this hook exists to # prevent. Revisit if an exit-status field is ever documented. - if is_commit "$cmd" && ! is_wip_commit "$cmd"; then + if is_commit "$cmd" && ! is_wip_commit_post "$cmd"; then rm -f "$state_file" "$count_file" "$fresh_file" fi + # The `-m wip` record belongs to this one command, whatever the decision above was + is_commit "$cmd" && rm -f "$wipbase_file" 2>/dev/null ;; Skill) case "$(input_field skill)" in @@ -951,8 +1027,9 @@ case "$event" in Bash) cmd=$(input_field command) if is_commit "$cmd"; then - if is_wip_commit "$cmd"; then - note "WIP commit — cycle-internal, per $policy: this exists so mcp__codex__review has a non-empty range to read (baseSha = this commit's parent). Gate B is not evaluated here and your pass counters are preserved. Use this commit as the review range; whether this cycle runs a review now, and when its closing act may be performed, are both $policy's closure ordering's, read there in full." "ℹ WIP commit (Codex cycle preserved)" + record_wip_base "$cmd" + if is_wip_commit_pre "$cmd"; then + note "WIP commit — cycle-internal, per $policy: this exists so mcp__codex__review has a non-empty range to read (baseSha = this commit's parent). Gate B is not evaluated here. Whether your pass counters are kept is decided after the commit, under the conditions $policy states for a WIP commit; where those do not hold they are cleared. Use this commit as the review range; whether this cycle runs a review now, and when its closing act may be performed, are both $policy's closure ordering's, read there in full." "ℹ WIP commit (Codex cycle decided after the commit)" else # Docs-only commits (spec/plan .md files) carry no code diff, # so Gate B (mcp__codex__review reviews a code diff) cannot apply — emit a diff --git a/plugins/dev-workflow/hooks/codex-gate.test.sh b/plugins/dev-workflow/hooks/codex-gate.test.sh index 0c7b66f..e9c3a56 100644 --- a/plugins/dev-workflow/hooks/codex-gate.test.sh +++ b/plugins/dev-workflow/hooks/codex-gate.test.sh @@ -151,7 +151,7 @@ reset_gate_state() { rm -f "$state" "$count" "$fresh" "$countA"; } # that wants the gate off must set the marker after calling this. reset_all() { rm -f "$state" "$count" "$fresh" "$countA" "$floorf" "$toolsf" "$notedf" \ - "$bgadvf" "$unverf" "$pendf" "$offf" + "$bgadvf" "$unverf" "$pendf" "$offf" .context/codex-gate.wipBase } # 0. THE jq-FREE PATH IS USABLE. Asserted before anything depends on it: if tree_hash @@ -570,10 +570,21 @@ printf '%s' "$out" | grep -q 'Codex gate state:' && fail "WIP commit must not em printf '%s' "$out" | grep -q 'WIP commit' && pass "WIP commit -> gentle note" || fail "WIP commit -> gentle note" out=$(wip "git commit -m 'WIP: caps variant'") printf '%s' "$out" | grep -q 'WIP commit' && pass "WIP matcher is case-insensitive" || fail "WIP matcher is case-insensitive" -# PostToolUse: a WIP commit must PRESERVE the counters (the cycle is still open) +# PostToolUse: a WIP commit must PRESERVE the counters (the cycle is still open). Since +# 0.13.3 PostToolUse reads the commit that resulted, so a WIP commit is made for real here +# (empty, so no fingerprint input moves) and undone afterwards. +wip "git commit -m 'wip: snapshot'" >/dev/null +git commit -q --allow-empty -m 'wip: snapshot' >/dev/null 2>&1 run '{"hook_event_name":"PostToolUse","tool_name":"Bash","tool_input":{"command":"git commit -m \"wip: snapshot\""}}' >/dev/null [ "$(cat "$count" 2>/dev/null)" = 2 ] && pass "WIP commit preserves pass count (Finding 11)" || fail "WIP commit preserves pass count (Finding 11)" [ -f "$state" ] && pass "WIP commit preserves Gate B state" || fail "WIP commit preserves Gate B state" +git reset -q --soft HEAD~1 >/dev/null 2>&1 +# ...and the same command on a HEAD that is not WIP (the commit did not happen, or a hook +# rewrote it) resets +rev +run '{"hook_event_name":"PostToolUse","tool_name":"Bash","tool_input":{"command":"git commit -m \"wip: snapshot\""}}' >/dev/null +[ ! -f "$count" ] && pass "-m wip with a non-WIP HEAD afterwards resets" || fail "-m wip with a non-WIP HEAD afterwards resets" +reset_all; rev; rev # A real (non-WIP) commit still resets commitpost [ ! -f "$count" ] && pass "non-WIP commit still resets counters" || fail "non-WIP commit still resets counters" @@ -585,7 +596,7 @@ git commit -q --allow-empty -m 'WIP: snapshot' >/dev/null 2>&1 amendpost() { run "{\"hook_event_name\":\"PostToolUse\",\"tool_name\":\"Bash\",\"tool_input\":{\"command\":\"$1\"}}" >/dev/null; } reset_all; rev; rev out=$(wip "git commit --amend --no-edit") -printf '%s' "$out" | grep -q 'Codex cycle preserved' && pass "amend --no-edit on WIP HEAD -> WIP note" || fail "amend --no-edit on WIP HEAD -> WIP note" +printf '%s' "$out" | grep -q 'Codex cycle decided after' && pass "amend --no-edit on WIP HEAD -> WIP note" || fail "amend --no-edit on WIP HEAD -> WIP note" amendpost "git commit --amend --no-edit" [ "$(cat "$count" 2>/dev/null)" = 2 ] && [ -f "$state" ] && pass "amend --no-edit on WIP HEAD preserves the cycle" || fail "amend --no-edit on WIP HEAD preserves the cycle" # A new message is a closing act (or a stray commit) and still resets @@ -602,14 +613,14 @@ for c in "git commit --amend --no-edit '-m' 'real'" "git commit --amend --no-edi "git commit --amend --no-edit --no-amend" "git -c {core.quotePath=false,-C,/elsewhere} commit --amend --no-edit" \ "git -c core.quotePath=false commit --amend --no-edit"; do reset_all; rev - printf '%s' "$(wip "$c")" | grep -q 'Codex cycle preserved' && fail "not WIP: $c" || pass "not WIP: $c" + printf '%s' "$(wip "$c")" | grep -q 'Codex cycle decided after' && fail "not WIP: $c" || pass "not WIP: $c" amendpost "$c" [ ! -f "$count" ] && pass "resets: $c" || fail "resets: $c" done # A custom comment character lets cleanup strip the WIP line, so the plain form is refused too git config core.commentChar W reset_all; rev -printf '%s' "$(wip "git commit --amend --no-edit")" | grep -q 'Codex cycle preserved' && fail "custom commentChar: no WIP note" || pass "custom commentChar: no WIP note" +printf '%s' "$(wip "git commit --amend --no-edit")" | grep -q 'Codex cycle decided after' && fail "custom commentChar: no WIP note" || pass "custom commentChar: no WIP note" amendpost "git commit --amend --no-edit" [ ! -f "$count" ] && pass "custom commentChar: resets" || fail "custom commentChar: resets" git config --unset core.commentChar @@ -617,7 +628,7 @@ git config --unset core.commentChar # backslash must still decline the exemption. (JSON: \" inside the command string.) reset_all; rev out=$(nojq_run '{"hook_event_name":"PreToolUse","tool_name":"Bash","tool_input":{"command":"git commit --amend --no-edit \"-m\" \"real\""}}') -printf '%s' "$out" | grep -q 'Codex cycle preserved' && fail "jq-free: double-quoted -m gets no WIP note" || pass "jq-free: double-quoted -m gets no WIP note" +printf '%s' "$out" | grep -q 'Codex cycle decided after' && fail "jq-free: double-quoted -m gets no WIP note" || pass "jq-free: double-quoted -m gets no WIP note" nojq_run '{"hook_event_name":"PostToolUse","tool_name":"Bash","tool_input":{"command":"git commit --amend --no-edit \"-m\" \"real\""}}' >/dev/null [ ! -f "$count" ] && pass "jq-free: double-quoted -m still resets" || fail "jq-free: double-quoted -m still resets" # 19c. The real lifecycle: PreToolUse, an ACTUAL `git commit --amend --no-edit`, PostToolUse. @@ -653,25 +664,25 @@ lifecycle() { # runs one real amend between the two hook events; leaves $pre, $s } # (a) no hooks at all: the amend keeps the WIP subject and the cycle survives lifecycle -printf '%s' "$pre" | grep -q 'Codex cycle preserved' && [ "$moved" = yes ] && [ "$subj" = 'WIP: snapshot' ] && [ "$(cat "$count" 2>/dev/null)" = 2 ] \ +printf '%s' "$pre" | grep -q 'Codex cycle decided after' && [ "$moved" = yes ] && [ "$subj" = 'WIP: snapshot' ] && [ "$(cat "$count" 2>/dev/null)" = 2 ] \ && pass "real amend, no hooks: WIP note, subject kept, cycle preserved" || fail "real amend, no hooks: WIP note, subject kept, cycle preserved" # (a2) *.sample files are the one thing the hooks directory may hold printf '#!/bin/sh\nexit 1\n' > "$hooksd/commit-msg.sample"; chmod +x "$hooksd/commit-msg.sample" lifecycle -printf '%s' "$pre" | grep -q 'Codex cycle preserved' && [ "$moved" = yes ] && [ "$(cat "$count" 2>/dev/null)" = 2 ] \ +printf '%s' "$pre" | grep -q 'Codex cycle decided after' && [ "$moved" = yes ] && [ "$(cat "$count" 2>/dev/null)" = 2 ] \ && pass "real amend, only *.sample hooks: cycle preserved" || fail "real amend, only *.sample hooks: cycle preserved" rm -f "$hooksd/commit-msg.sample" # (b) an amend aborted with no hook present (a held index lock) leaves HEAD WIP: cycle kept : > "$(git rev-parse --git-dir)/index.lock" lifecycle rm -f "$(git rev-parse --git-dir)/index.lock" -printf '%s' "$pre" | grep -q 'Codex cycle preserved' && [ "$moved" = no ] && [ "$subj" = 'WIP: snapshot' ] && [ "$(cat "$count" 2>/dev/null)" = 2 ] \ +printf '%s' "$pre" | grep -q 'Codex cycle decided after' && [ "$moved" = no ] && [ "$subj" = 'WIP: snapshot' ] && [ "$(cat "$count" 2>/dev/null)" = 2 ] \ && pass "aborted real amend, no hooks: cycle preserved" || fail "aborted real amend, no hooks: cycle preserved" # (b2) Before PR #30's review this case expected preservation: a pre-commit hook that # aborts. Under the any-hook rule its mere presence refuses the exemption, so it resets. printf '#!/bin/sh\nexit 1\n' > "$hooksd/pre-commit"; chmod +x "$hooksd/pre-commit" lifecycle -{ printf '%s' "$pre" | grep -q 'Codex cycle preserved'; } && fail "pre-commit hook present: no WIP note" || pass "pre-commit hook present: no WIP note" +{ printf '%s' "$pre" | grep -q 'Codex cycle decided after'; } && fail "pre-commit hook present: no WIP note" || pass "pre-commit hook present: no WIP note" [ "$moved" = no ] && [ ! -f "$count" ] && pass "pre-commit hook present, amend aborted: cycle reset" || fail "pre-commit hook present, amend aborted: cycle reset" rm -f "$hooksd/pre-commit" # (f) a pre-commit hook that installs a rewriting prepare-commit-msg mid-commit @@ -679,19 +690,19 @@ printf '%s' "$rewrite_hook" > "$sandbox/rewrite-hook" printf '#!/bin/sh\ncp "%s" "%s/prepare-commit-msg" && chmod +x "%s/prepare-commit-msg"\n' "$sandbox/rewrite-hook" "$hooksd" "$hooksd" > "$hooksd/pre-commit" chmod +x "$hooksd/pre-commit" lifecycle -{ printf '%s' "$pre" | grep -q 'Codex cycle preserved'; } && fail "pre-commit installing a message hook: no WIP note" || pass "pre-commit installing a message hook: no WIP note" +{ printf '%s' "$pre" | grep -q 'Codex cycle decided after'; } && fail "pre-commit installing a message hook: no WIP note" || pass "pre-commit installing a message hook: no WIP note" [ "$subj" = 'real subject' ] && [ ! -f "$count" ] && pass "pre-commit installing a message hook: subject rewritten, cycle reset" || fail "pre-commit installing a message hook: subject rewritten, cycle reset" rm -f "$hooksd/pre-commit" "$hooksd/prepare-commit-msg" "$sandbox/rewrite-hook"; git commit -q --amend -m 'WIP: snapshot' >/dev/null 2>&1 # (c) prepare-commit-msg rewrites the subject into a real one: no WIP note before, reset after printf '%s' "$rewrite_hook" > "$hooksd/prepare-commit-msg"; chmod +x "$hooksd/prepare-commit-msg" lifecycle -{ printf '%s' "$pre" | grep -q 'Codex cycle preserved'; } && fail "message hook: no WIP note before the amend" || pass "message hook: no WIP note before the amend" +{ printf '%s' "$pre" | grep -q 'Codex cycle decided after'; } && fail "message hook: no WIP note before the amend" || pass "message hook: no WIP note before the amend" [ "$subj" = 'real subject' ] && [ ! -f "$count" ] && pass "message hook: subject rewritten, cycle reset" || fail "message hook: subject rewritten, cycle reset" rm -f "$hooksd/prepare-commit-msg"; git commit -q --amend -m 'WIP: snapshot' >/dev/null 2>&1 # (d) commit-msg rejects the amend: HEAD stays WIP, but a message hook is present, so reset printf '#!/bin/sh\nexit 1\n' > "$hooksd/commit-msg"; chmod +x "$hooksd/commit-msg" lifecycle -{ printf '%s' "$pre" | grep -q 'Codex cycle preserved'; } && fail "rejecting commit-msg: no WIP note" || pass "rejecting commit-msg: no WIP note" +{ printf '%s' "$pre" | grep -q 'Codex cycle decided after'; } && fail "rejecting commit-msg: no WIP note" || pass "rejecting commit-msg: no WIP note" [ "$moved" = no ] && [ "$subj" = 'WIP: snapshot' ] && [ ! -f "$count" ] && pass "rejecting commit-msg: HEAD still WIP, cycle reset" || fail "rejecting commit-msg: HEAD still WIP, cycle reset" rm -f "$hooksd/commit-msg" # (e) any core.hooksPath refuses the exemption; this one also rewrites the subject @@ -699,16 +710,123 @@ mkdir -p "$sandbox/hooks" printf '%s' "$rewrite_hook" > "$sandbox/hooks/prepare-commit-msg"; chmod +x "$sandbox/hooks/prepare-commit-msg" git config core.hooksPath "$sandbox/hooks" lifecycle -{ printf '%s' "$pre" | grep -q 'Codex cycle preserved'; } && fail "core.hooksPath message hook: no WIP note" || pass "core.hooksPath message hook: no WIP note" +{ printf '%s' "$pre" | grep -q 'Codex cycle decided after'; } && fail "core.hooksPath message hook: no WIP note" || pass "core.hooksPath message hook: no WIP note" [ "$subj" = 'real subject' ] && [ ! -f "$count" ] && pass "core.hooksPath message hook: subject rewritten, cycle reset" || fail "core.hooksPath message hook: subject rewritten, cycle reset" git config --unset core.hooksPath; rm -rf "$sandbox/hooks"; git commit -q --amend -m 'WIP: snapshot' >/dev/null 2>&1 # a hooksPath ending in a newline is refused too, not resolved to a neighbouring directory git config core.hooksPath "$sandbox/nl " reset_all; rev -[ "$(git log -1 --format=%s)" = 'WIP: snapshot' ] && ! { printf '%s' "$(wip "git commit --amend --no-edit")" | grep -q 'Codex cycle preserved'; } \ +[ "$(git log -1 --format=%s)" = 'WIP: snapshot' ] && ! { printf '%s' "$(wip "git commit --amend --no-edit")" | grep -q 'Codex cycle decided after'; } \ && pass "newline-ending core.hooksPath on a WIP HEAD: no WIP note" || fail "newline-ending core.hooksPath on a WIP HEAD: no WIP note" git config --unset core.hooksPath +# 19d. The `-m "wip…"` path, as real commits: PreToolUse, the command itself, PostToolUse. +# Commands use single quotes so they embed in the JSON payload as they are. +base19d=$(git rev-parse HEAD) +mlife() { # $1 = the command; runs it for real between the two hook events + reset_all; rev; rev + pre=$(wip "$1") + before=$(git rev-parse HEAD) + printf 'x\n' >> wip.txt; git add wip.txt >/dev/null 2>&1 + sh -c "$1" >/dev/null 2>&1 + [ "$(git rev-parse HEAD)" != "$before" ] && moved=yes || moved=no + subj=$(git log -1 --format=%s) + amendpost "$1" +} +noted() { printf '%s' "$pre" | grep -q 'Codex cycle decided after'; } +kept() { [ "$(cat "$count" 2>/dev/null)" = 2 ]; } +# (m1) plain `-m 'WIP…'`, no hooks: note before, commit is WIP, cycle kept +mlife "git commit -q -m 'WIP: next'" +noted && [ "$moved" = yes ] && [ "$subj" = 'WIP: next' ] && kept && pass "-m wip, no hooks: note, WIP commit, cycle kept" || fail "-m wip, no hooks: note, WIP commit, cycle kept" +# (m2) a hook that leaves the message alone: reminder before (it could have), cycle kept after +printf '#!/bin/sh\nexit 0\n' > "$hooksd/pre-commit"; chmod +x "$hooksd/pre-commit" +mlife "git commit -q -m 'WIP: next'" +! noted && [ "$moved" = yes ] && [ "$subj" = 'WIP: next' ] && kept && pass "-m wip, harmless hook: reminder before, cycle kept" || fail "-m wip, harmless hook: reminder before, cycle kept" +rm -f "$hooksd/pre-commit" +# (m3) a hook rewrites the WIP message into a real one: reminder before, reset after +printf '%s' "$rewrite_hook" > "$hooksd/prepare-commit-msg"; chmod +x "$hooksd/prepare-commit-msg" +mlife "git commit -q -m 'WIP: next'" +! noted && [ "$subj" = 'real subject' ] && [ ! -f "$count" ] && pass "-m wip rewritten by a hook: reminder before, reset after" || fail "-m wip rewritten by a hook: reminder before, reset after" +rm -f "$hooksd/prepare-commit-msg"; git commit -q --amend -m 'WIP: snapshot' >/dev/null 2>&1 +# (m4) the commit fails with a WIP HEAD already there: no boundary, cycle kept +: > "$(git rev-parse --git-dir)/index.lock" +mlife "git commit -q -m 'WIP: next'" +rm -f "$(git rev-parse --git-dir)/index.lock" +noted && [ "$moved" = no ] && kept && pass "-m wip that fails on a WIP HEAD: cycle kept" || fail "-m wip that fails on a WIP HEAD: cycle kept" +# (m5) chains either way round, and a first `-m` that is not WIP: reminder before, reset after +for c in "git commit -q -m 'real' && git commit -q --allow-empty -m 'WIP: next'" \ + "git commit -q -m 'WIP: next' && git commit -q --allow-empty -m 'real'" \ + "git commit -q -m 'real' -m 'WIP: next'" "git commit -q -m -m WIP" \ + "git commit -q -m 'WIP: next' --fixup=HEAD" "git commit -q -F -m WIP"; do + mlife "$c" + ! noted && [ ! -f "$count" ] && pass "reminder and reset: $c" || fail "reminder and reset: $c" + git commit -q --amend -m 'WIP: snapshot' >/dev/null 2>&1 +done +# (m6) an editor could replace the message, so -e gets the reminder (not run: no editor here) +pre=$(wip "git commit -q -e -m 'WIP: next'") +! noted && pass "reminder before: -e with a WIP message" || fail "reminder before: -e with a WIP message" +# (m7) without jq only the single-quoted form can be read whole: it keeps the cycle there too +reset_all; rev; rev +nojq_run "{\"hook_event_name\":\"PreToolUse\",\"tool_name\":\"Bash\",\"tool_input\":{\"command\":\"git commit -m 'WIP: next'\"}}" >/dev/null +git commit -q --allow-empty -m 'WIP: next' >/dev/null 2>&1 +nojq_run "{\"hook_event_name\":\"PostToolUse\",\"tool_name\":\"Bash\",\"tool_input\":{\"command\":\"git commit -m 'WIP: next'\"}}" >/dev/null +kept && pass "jq-free: single-quoted -m wip keeps the cycle" || fail "jq-free: single-quoted -m wip keeps the cycle" +# (m8) a post-commit hook that makes a real commit and then a WIP one: HEAD is WIP, but it +# no longer sits on the commit that was HEAD when the command started, so reset +printf '#!/bin/sh\ngit -c core.hooksPath=/dev/null commit -q --allow-empty -m real >/dev/null 2>&1\ngit -c core.hooksPath=/dev/null commit -q --allow-empty -m "WIP: from a hook" >/dev/null 2>&1\n' > "$hooksd/post-commit" +chmod +x "$hooksd/post-commit" +mlife "git commit -q -m 'WIP: next'" +[ "$subj" = 'WIP: from a hook' ] && [ ! -f "$count" ] && pass "-m wip, then a hook adds real + WIP commits: reset" || fail "-m wip, then a hook adds real + WIP commits: reset" +rm -f "$hooksd/post-commit" +# (m9) no record from PreToolUse (it never ran): the result cannot be attributed, so reset +reset_all; rev; rev +git commit -q --allow-empty -m 'WIP: next' >/dev/null 2>&1 +amendpost "git commit -m 'WIP: next'" +[ ! -f "$count" ] && pass "-m wip with no PreToolUse record: reset" || fail "-m wip with no PreToolUse record: reset" +# (m10) --amend -m 'WIP…' keeps the cycle: the new HEAD shares the recorded parent +mlife "git commit -q --amend -m 'WIP: amended'" +[ "$moved" = yes ] && [ "$subj" = 'WIP: amended' ] && kept && [ ! -f .context/codex-gate.wipBase ] \ + && pass "--amend -m wip: same parent, cycle kept, record removed" || fail "--amend -m wip: same parent, cycle kept, record removed" +# (m11) a hook rewrites the WIP message to a real one, then amends that real commit back to +# WIP: the final HEAD still sits on the recorded HEAD, but the reflog grew by two, so reset +printf '%s' "$rewrite_hook" > "$hooksd/prepare-commit-msg"; chmod +x "$hooksd/prepare-commit-msg" +printf '#!/bin/sh\ngit -c core.hooksPath=/dev/null commit -q --allow-empty --amend -m "WIP: amended real" >/dev/null 2>&1\n' > "$hooksd/post-commit" +chmod +x "$hooksd/post-commit" +mlife "git commit -q -m 'WIP: next'" +[ "$subj" = 'WIP: amended real' ] && [ ! -f "$count" ] && pass "hook amends a real commit back to WIP: reset" || fail "hook amends a real commit back to WIP: reset" +rm -f "$hooksd/prepare-commit-msg" "$hooksd/post-commit" +# (m12) `--amend` inside the message is not the option: a hook that replaces HEAD with a WIP +# sibling must not pass as an amend +printf '#!/bin/sh\ngit reset -q --soft HEAD~2 >/dev/null 2>&1\ngit -c core.hooksPath=/dev/null commit -q --allow-empty -m "WIP: sibling" >/dev/null 2>&1\n' > "$hooksd/post-commit" +chmod +x "$hooksd/post-commit" +mlife "git commit -q -m 'WIP: --amend explained'" +[ "$subj" = 'WIP: sibling' ] && [ ! -f "$count" ] && pass "--amend inside the message is not an option: reset" || fail "--amend inside the message is not an option: reset" +rm -f "$hooksd/post-commit"; git commit -q --amend -m 'WIP: snapshot' >/dev/null 2>&1 +# (m13) no HEAD reflog: a hook that makes a real commit and moves HEAD back would look like a +# failed commit, so without reflog evidence the exemption is refused and the cycle resets +gd=$(git rev-parse --git-dir) +mv "$gd/logs" "$sandbox/logs-aside"; git config core.logAllRefUpdates false +printf '%s' "$rewrite_hook" > "$hooksd/prepare-commit-msg"; chmod +x "$hooksd/prepare-commit-msg" +printf '#!/bin/sh\ngit reset -q --soft HEAD~1 >/dev/null 2>&1\n' > "$hooksd/post-commit"; chmod +x "$hooksd/post-commit" +mlife "git commit -q -m 'WIP: next'" +[ "$moved" = no ] && [ "$subj" = 'WIP: snapshot' ] && [ ! -f "$count" ] && pass "no HEAD reflog, real commit undone by a hook: reset" || fail "no HEAD reflog, real commit undone by a hook: reset" +rm -f "$hooksd/prepare-commit-msg" "$hooksd/post-commit" +git config --unset core.logAllRefUpdates; rm -rf "$gd/logs"; mv "$sandbox/logs-aside" "$gd/logs" +git commit -q --amend -m 'WIP: snapshot' >/dev/null 2>&1 +# (m14) a corrupt record (a leading-zero count, read as octal by shell arithmetic) must neither +# make the hook fail nor keep the cycle +reset_all; rev; rev +printf '%s - 08 new\n' "$(git rev-parse HEAD)" > .context/codex-gate.wipBase +git commit -q --allow-empty -m 'WIP: next' >/dev/null 2>&1 +run '{"hook_event_name":"PostToolUse","tool_name":"Bash","tool_input":{"command":"git commit -m '"'"'WIP: next'"'"'"}}' >/dev/null; rc=$? +[ "$rc" -eq 0 ] && [ ! -f "$count" ] && pass "corrupt record (08): exit 0 and reset" || fail "corrupt record (08): exit 0 and reset" +# (m15) a backgrounded Bash call: PostToolUse arrives before the commit ran, HEAD looks +# unchanged, and that is no evidence of a failed commit — reset +reset_all; rev; rev +wip "git commit -m 'WIP: next'" >/dev/null +run '{"hook_event_name":"PostToolUse","tool_name":"Bash","tool_input":{"command":"git commit -m '"'"'WIP: next'"'"'","run_in_background":true},"tool_response":{"backgroundTaskId":"b1"}}' >/dev/null +[ ! -f "$count" ] && pass "backgrounded -m wip commit: reset" || fail "backgrounded -m wip commit: reset" +git reset -q --soft "$base19d" >/dev/null 2>&1 git reset -q --soft HEAD~1 >/dev/null 2>&1; git rm -q --cached wip.txt >/dev/null 2>&1; rm -f wip.txt rm -rf "$hooksd"; { [ -e "$sandbox/orig-hooks" ] || [ -L "$sandbox/orig-hooks" ]; } && mv "$sandbox/orig-hooks" "$hooksd" # On a non-WIP HEAD the kept message is a real one, so the reset stands diff --git a/todos.md b/todos.md index efaa7c3..701e1cb 100644 --- a/todos.md +++ b/todos.md @@ -506,14 +506,13 @@ backlog. allow-list, because four Gate-B passes found a new bypass in every deny-list. Still open, deliberately, all in the safe direction: any other spelling of a WIP amend (global `git -c`, `cd … &&`, `-F`/`-C` with a WIP message) still resets, and a heredoc - merely containing `WIP:` or `git commit` still fires. **Open in the unsafe direction, - and older than this fix:** the `-m "wip…"` path trusts the typed message, so a - hook that rewrites it into a real one — or installs one that does — gets the WIP note - and keeps the cycle (PR #30 review). Not fixed there, by scope. Also collected - there: `CLAUDE.md` §5 Mechanics (and the `workflow-init` copy) and - `docs/architecture.md` say a `wip`-prefixed commit *message* keeps the counters, but - the hook reads the command and a few git settings, not the message — a two-copy prompt - fix for a later change. + merely containing `WIP:` or `git commit` still fires. **Fixed in 0.13.3:** the + `-m "wip…"` path used to trust the typed message; now the reminder is decided before + the commit (a repository that could rewrite the message gets the Gate-B reminder) and + the reset after it (counters stay only for a WIP result attributable to the command, + by the rule in the 0.13.3 CHANGELOG entry), for an allow-listed one-line + `git commit` only. The §5 Mechanics text and both docs now describe that. + Still open, in the safe direction: a chained or redirected WIP commit resets. - [x] **P2 — risk/security profiles, and the derived validation mode.** Shipped: two human-confirmed axes in the story header, a mode derived as `max(risk, security)`, From 1542663fe9fb8571e6f25edf94aa565f8ff2930d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20S=C3=A4nger?= <20968534+dsnger@users.noreply.github.com> Date: Tue, 29 Sep 2026 15:57:05 +0200 Subject: [PATCH 2/2] Keep one -m wip record per tool call (PR #31 review) 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.. 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 --- .../gate-b-quality-p0nhw0wb2d-pass-1.md | 3 + .../gate-b-quality-p0nhw0wb2d-pass-2.md | 2 + .../gate-b-quality-p0nhw0wb2d-pass-3.md | 2 + .../gate-b-spec-p0nhw0wb2d-pass-1.md | 3 + .../gate-b-spec-p0nhw0wb2d-pass-2.md | 2 + .../gate-b-spec-p0nhw0wb2d-pass-3.md | 2 + CLAUDE.md | 4 +- plugins/dev-workflow/CHANGELOG.md | 20 +++-- .../dev-workflow/commands/workflow-init.md | 4 +- plugins/dev-workflow/hooks/codex-gate.sh | 36 ++++++-- plugins/dev-workflow/hooks/codex-gate.test.sh | 86 ++++++++++++++++--- 11 files changed, 132 insertions(+), 32 deletions(-) create mode 100644 .context/codex-reviews/gate-b-quality-p0nhw0wb2d-pass-1.md create mode 100644 .context/codex-reviews/gate-b-quality-p0nhw0wb2d-pass-2.md create mode 100644 .context/codex-reviews/gate-b-quality-p0nhw0wb2d-pass-3.md create mode 100644 .context/codex-reviews/gate-b-spec-p0nhw0wb2d-pass-1.md create mode 100644 .context/codex-reviews/gate-b-spec-p0nhw0wb2d-pass-2.md create mode 100644 .context/codex-reviews/gate-b-spec-p0nhw0wb2d-pass-3.md diff --git a/.context/codex-reviews/gate-b-quality-p0nhw0wb2d-pass-1.md b/.context/codex-reviews/gate-b-quality-p0nhw0wb2d-pass-1.md new file mode 100644 index 0000000..63e35e4 --- /dev/null +++ b/.context/codex-reviews/gate-b-quality-p0nhw0wb2d-pass-1.md @@ -0,0 +1,3 @@ +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:853-855 | Validate the original JSON string before shell normalization: with jq, tool_use_id="toolu_b\n" becomes toolu_b because command substitution strips trailing newlines; numeric and boolean IDs are also converted to accepted text. | An invalid-ID call can select a valid call's record instead of resetting. Reproduced under sh and dash: the malformed-ID PostToolUse preserved the counter and deleted toolu_b's record. | Require a string and validate its exact decoded characters and length before command substitution; add malformed-ID collision regressions asserting reset and preservation of the foreign record. +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:853 | The jq-free field() fallback searches all nesting levels, so wip_record_path accepts tool_response.tool_use_id when the top-level ID is missing, or selects a nested ID appearing before the actual top-level ID. | Call A can consume call B's record despite lacking B's top-level ID. Reproduced under sh and dash with A's real commit followed by B's WIP commit: A's PostToolUse retained the counters and deleted B's record, violating conservative reset and call isolation. | Use extraction that establishes top-level string scope, refusing attribution when it cannot do so; add jq-free missing-top-level and nested-before-top-level regressions asserting reset and that foreign records remain. +END OF FINDINGS (2 total) diff --git a/.context/codex-reviews/gate-b-quality-p0nhw0wb2d-pass-2.md b/.context/codex-reviews/gate-b-quality-p0nhw0wb2d-pass-2.md new file mode 100644 index 0000000..6285770 --- /dev/null +++ b/.context/codex-reviews/gate-b-quality-p0nhw0wb2d-pass-2.md @@ -0,0 +1,2 @@ +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:861 | Using the raw mixed-case tool_use_id as the filename makes distinct valid ids such as toolu_a and toolu_A share a record on case-insensitive filesystems; reproduced with the actual hook on Darwin under both sh and dash | Both original failures recur: the m16 interleaving loses valid counters, while m17 preserves count 2 after A was rewritten into a real commit; A's PostToolUse deletes B's record in both cases | Derive a bounded case-fold-safe filename that preserves id identity, or conservatively reject ids that cannot be isolated; add both interleavings with case-variant ids and verify that neither call reads or deletes the other's record +END OF FINDINGS (1 total) diff --git a/.context/codex-reviews/gate-b-quality-p0nhw0wb2d-pass-3.md b/.context/codex-reviews/gate-b-quality-p0nhw0wb2d-pass-3.md new file mode 100644 index 0000000..8d781f4 --- /dev/null +++ b/.context/codex-reviews/gate-b-quality-p0nhw0wb2d-pass-3.md @@ -0,0 +1,2 @@ +NO FINDINGS +END OF FINDINGS (0 total) diff --git a/.context/codex-reviews/gate-b-spec-p0nhw0wb2d-pass-1.md b/.context/codex-reviews/gate-b-spec-p0nhw0wb2d-pass-1.md new file mode 100644 index 0000000..ccc9b47 --- /dev/null +++ b/.context/codex-reviews/gate-b-spec-p0nhw0wb2d-pass-1.md @@ -0,0 +1,3 @@ +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:853-855 | Invalid IDs are validated only after lossy extraction: with jq, JSON tool_use_id "toolu_b\n" becomes "toolu_b" because command substitution strips trailing newlines; numeric 123 also becomes the accepted string "123". The task requires invalid IDs to produce no record and reset. | Reproduced under sh and dash: A with the newline-suffixed ID crosses a real commit boundary, B records toolu_b and commits WIP on top, then A PostToolUse reads and deletes B's record and incorrectly retains the counters. This also falsifies the isolation guarantee in the hook comment and CHANGELOG. | Validate the top-level JSON value's string type, full character set and length before emitting it into command substitution; reject extraction failures and add regressions proving invalid IDs reset without touching a valid foreign record. +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:853 | The jq-free field() fallback searches the whole payload for the first matching key, so a missing top-level tool_use_id is replaced by a nested tool_response.tool_use_id. This new use of field() does not satisfy the required top-level identity and missing-ID reset rule. | Reproduced without jq under sh and dash: a call lacking its own ID but carrying nested toolu_b consumes and deletes B's record and retains counters after A made a real commit and B added a WIP commit. Distinct valid top-level IDs can likewise be shadowed by an earlier nested key. | Use a depth-aware top-level ID reader, or conservatively reject payloads whose top-level identity cannot be established; add jq-free regressions for missing and shadowed IDs that assert both reset and preservation of foreign records. +END OF FINDINGS (2 total) diff --git a/.context/codex-reviews/gate-b-spec-p0nhw0wb2d-pass-2.md b/.context/codex-reviews/gate-b-spec-p0nhw0wb2d-pass-2.md new file mode 100644 index 0000000..f1f742b --- /dev/null +++ b/.context/codex-reviews/gate-b-spec-p0nhw0wb2d-pass-2.md @@ -0,0 +1,2 @@ +MAJOR | 1.00 | plugins/dev-workflow/hooks/codex-gate.sh:859-861 | Raw mixed-case tool_use_id filenames do not isolate calls on case-insensitive filesystems: accepted IDs toolu_A and toolu_a resolve to the same record. Expected distinct records for distinct IDs, but B's PreToolUse overwrites A's record. | Reproduced under sh and dash on this macOS filesystem: A Pre, A commit rewritten to real, B Pre, B WIP commit, A Post leaves passCount=2 and deletes B's record, recreating the missed-reset failure and falsifying the isolation claim in CHANGELOG.md:39-40. | Derive a bounded filename whose identity preserves case distinctions on case-insensitive filesystems (for example a lowercase hexadecimal digest of the validated ID), and add regressions for both interleavings with IDs differing only by case. +END OF FINDINGS (1 total) diff --git a/.context/codex-reviews/gate-b-spec-p0nhw0wb2d-pass-3.md b/.context/codex-reviews/gate-b-spec-p0nhw0wb2d-pass-3.md new file mode 100644 index 0000000..8d781f4 --- /dev/null +++ b/.context/codex-reviews/gate-b-spec-p0nhw0wb2d-pass-3.md @@ -0,0 +1,2 @@ +NO FINDINGS +END OF FINDINGS (0 total) diff --git a/CLAUDE.md b/CLAUDE.md index 609c325..4cc4ad0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1318,8 +1318,8 @@ like the rest of §5; the detection is a reader comparing the pass against the s and set `baseSha` to its parent. **Make that commit as `git commit -m 'WIP: …'`** — the recommended form, because the hook recognises a WIP commit from the command and the commit that results, not from the message alone. What it accepts is a one-line `git commit` with - exactly one `-m` whose value starts with `wip` — single-quoted, double-quoted or bare, though - without `jq` a double-quoted message cannot be read whole — beside nothing but `-a`/`--all`, + exactly one `-m` whose value starts with `wip` — single-quoted, double-quoted or bare; without + `jq` the hook cannot attribute the result and resets — beside nothing but `-a`/`--all`, `-q`/`--quiet`, `-n`/`--no-verify`, `--amend` or `--allow-empty`; so no chain, `cd`, `git -C`, editor or `--fixup`, and none of `; & | < > $ ( )`, a backtick or a backslash anywhere, the message included. Beforehand, where a hook or setting in the repository could diff --git a/plugins/dev-workflow/CHANGELOG.md b/plugins/dev-workflow/CHANGELOG.md index 722d733..bd74c5a 100644 --- a/plugins/dev-workflow/CHANGELOG.md +++ b/plugins/dev-workflow/CHANGELOG.md @@ -31,19 +31,21 @@ AGENTS.md invariant 12 carries the complete list. Now only an allow-listed one-line form qualifies: `git commit`, then only `-a`, `--all`, `-q`, `--quiet`, `--no-verify`, `-n`, `--amend` or `--allow-empty`, and exactly one `-m` whose quoted or bare value starts with `wip` — no editor, `--fixup`, `-F`, second `-m`, - chain, redirect, `cd`, `git -C`, shell metacharacter or backslash. Without `jq` the - single-quoted and bare forms qualify, since the fallback reader truncates a double-quoted - one. Beforehand, a repository whose hooks or + chain, redirect, `cd`, `git -C`, shell metacharacter or backslash. Beforehand, a repository whose hooks or settings could rewrite the message gets the ordinary Gate-B reminder instead of the WIP note — a reminder, not a reset. Afterwards, the counters stay only if the resulting commit's subject starts with `wip` and it is attributable to the command: PreToolUse records `HEAD`, its parent, the length of `HEAD`'s reflog and whether `--amend` is an option (outside quoted - text) in `.context/codex-gate.wipBase`. Afterwards either `HEAD` and the reflog are - unchanged (the commit failed, and the WIP commit already there stays), or the reflog grew - by exactly one and the new `HEAD` sits directly on the recorded one, or shares its parent - for `--amend`. A hook that commits, amends or resets in between, a missing record, an empty - or unreadable `HEAD` reflog, or a Bash call sent to the background (its result arrives - before the commit finishes) resets. The record is removed after every detected commit. + text) in a record of its own, `.context/codex-gate.wipBase.` — encoded so ids + differing only in letter case stay apart on a case-insensitive filesystem — so overlapping + tool calls cannot read or remove each other's. The id is read with `jq` as a top-level + string and checked there; a call without a usable id gets no record, and neither does any + call without `jq`, so there the WIP commit resets. Afterwards either `HEAD` and the reflog are unchanged (the commit failed, and the WIP + commit already there stays), or the reflog grew by exactly one and the new `HEAD` sits + directly on the recorded one, or shares its parent for `--amend`. A hook that commits, + amends or resets in between, a missing record, an empty or unreadable `HEAD` reflog, or a + Bash call sent to the background (its result arrives before the commit finishes) resets. + Each call's record is removed by that call's PostToolUse. The WIP note no longer promises the counters are kept; it says that is decided after the commit. The `--amend --no-edit` rules from 0.13.2 are unchanged. - **`CLAUDE.md` §5 Mechanics and its `/dev-workflow:workflow-init` copy** now say the hook diff --git a/plugins/dev-workflow/commands/workflow-init.md b/plugins/dev-workflow/commands/workflow-init.md index 70d3f63..e837131 100644 --- a/plugins/dev-workflow/commands/workflow-init.md +++ b/plugins/dev-workflow/commands/workflow-init.md @@ -1507,8 +1507,8 @@ like the rest of §5; the detection is a reader comparing the pass against the s and set `baseSha` to its parent. **Make that commit as `git commit -m 'WIP: …'`** — the recommended form, because the hook recognises a WIP commit from the command and the commit that results, not from the message alone. What it accepts is a one-line `git commit` with - exactly one `-m` whose value starts with `wip` — single-quoted, double-quoted or bare, though - without `jq` a double-quoted message cannot be read whole — beside nothing but `-a`/`--all`, + exactly one `-m` whose value starts with `wip` — single-quoted, double-quoted or bare; without + `jq` the hook cannot attribute the result and resets — beside nothing but `-a`/`--all`, `-q`/`--quiet`, `-n`/`--no-verify`, `--amend` or `--allow-empty`; so no chain, `cd`, `git -C`, editor or `--fixup`, and none of `; & | < > $ ( )`, a backtick or a backslash anywhere, the message included. Beforehand, where a hook or setting in the repository could diff --git a/plugins/dev-workflow/hooks/codex-gate.sh b/plugins/dev-workflow/hooks/codex-gate.sh index cc637d3..c1fd59e 100755 --- a/plugins/dev-workflow/hooks/codex-gate.sh +++ b/plugins/dev-workflow/hooks/codex-gate.sh @@ -57,7 +57,7 @@ countA_file="$state_dir/codex-gate.passCountA" # Gate A (exec) passes since l bgadv_file="$state_dir/codex-gate.bgAdvice" unver_file="$state_dir/codex-gate.unverified" pend_file="$state_dir/codex-gate.unverifiedPending" -wipbase_file="$state_dir/codex-gate.wipBase" # HEAD (and its parent) when a `-m wip` commit starts +wipbase_prefix="$state_dir/codex-gate.wipBase." # + tool_use_id: one `-m wip` record per tool call # FINDING G: the plugin is installed globally, but the workflow is adopted per project. # A repo that never ran /workflow-init has no gate to enforce, so the hook does NOTHING @@ -774,8 +774,8 @@ is_commit() { printf '%s' "$1" | grep -Eq '(^|[^[:alnum:]])git[[:space:]].*commi # `-m` whose single-quoted, double-quoted or bare value starts with `wip`; nothing else, so no # editor (`-e`), `--fixup`, `-F`, second `-m`, `cd`, `git -C`, chain or redirect, and no shell # metacharacter or backslash anywhere (the jq-free reader truncates a double-quoted message at -# its escaped quote and leaves one, so without jq only the single-quoted and bare forms -# qualify). Before the +# its escaped quote and leaves one; without jq the result is never attributed anyway, see the +# record below). Before the # commit, a repository whose hooks or settings could rewrite the message gets the Gate-B # reminder instead of the note; after it, the counters stay only if HEAD's subject starts with # `wip` AND HEAD is attributable to this command. PreToolUse records HEAD, its parent, the @@ -789,8 +789,17 @@ is_commit() { printf '%s' "$1" | grep -Eq '(^|[^[:alnum:]])git[[:space:]].*commi # reflog is empty or unreadable before or after: without it neither branch can be told apart # from a hook that moved HEAD and moved it back. A Bash call that was sent to the background # (`run_in_background`, or a result carrying a `backgroundTaskId`) is refused too: its -# PostToolUse arrives before the commit has finished, so an unchanged HEAD proves nothing. The record lives only from one PreToolUse to the next -# PostToolUse and is removed there. +# PostToolUse arrives before the commit has finished, so an unchanged HEAD proves nothing. +# The record belongs to one tool call: it is named after the payload's `tool_use_id`, which +# that call's PreToolUse and PostToolUse share, so overlapping calls cannot read, take or +# delete each other's record. The id is read with jq only, as a top-level string checked +# inside jq; a call whose id is missing, not a string, longer than 100 characters or not +# made of [A-Za-z0-9_-] gets no record and so resets — and so does every call without jq, +# whose fallback reader cannot establish that the key is the top-level one. The file name +# encodes the id so that ids differing only in letter case stay apart on a case-insensitive +# filesystem. PostToolUse removes the call's own +# record; one whose PostToolUse never arrives stays behind as an inert file in `.context/`, +# outside the fingerprint. # # `git commit --amend --no-edit` carries no `-m` and normally keeps HEAD's message. It is an # allow-list, never a deny-list: one line of exactly `git commit` followed only by `--amend`, @@ -844,7 +853,19 @@ is_wip_amend_cmd() { ! msg_may_be_rewritten && head_is_wip } head_reflog_len() { git -C "$repo_root" reflog show --format=%H HEAD 2>/dev/null | grep -c ''; } +wip_record_path() { # this call's record path; fails when the id is unusable + # jq only: the fallback reader cannot tell a top-level key from a nested one, and a + # shell capture would strip a trailing newline before any check could see it, so the + # type, characters and length are checked inside jq on the decoded value + command -v jq >/dev/null 2>&1 || return 1 + # and then written as a name no case-insensitive filesystem can merge with another id's: + # `_` becomes `__` and an upper-case letter `X` becomes `_x`, which decodes uniquely + _id=$(printf '%s' "$payload" | jq -r '.tool_use_id | if type == "string" and test("\\A[A-Za-z0-9_-]{1,100}\\z") then (gsub("_"; "__") | gsub("(?[A-Z])"; "_" + (.c | ascii_downcase))) else empty end' 2>/dev/null) || return 1 + case $_id in '' | *[!a-z0-9_-]*) return 1 ;; esac + printf '%s%s' "$wipbase_prefix" "$_id" +} record_wip_base() { # PreToolUse: " "; best effort + wipbase_file=$(wip_record_path) || return 0 rm -f "$wipbase_file" 2>/dev/null is_wip_message_cmd "$1" || return 0 _h=$(git -C "$repo_root" rev-parse -q --verify HEAD 2>/dev/null) || return 0 @@ -859,6 +880,7 @@ record_wip_base() { # PreToolUse: " /dev/null < "$wipbase_file" rm -f "$wipbase_file" 2>/dev/null @@ -1007,8 +1029,8 @@ case "$event" in if is_commit "$cmd" && ! is_wip_commit_post "$cmd"; then rm -f "$state_file" "$count_file" "$fresh_file" fi - # The `-m wip` record belongs to this one command, whatever the decision above was - is_commit "$cmd" && rm -f "$wipbase_file" 2>/dev/null + # This call's `-m wip` record belongs to this one command, whatever the decision above was + if is_commit "$cmd" && wipbase_file=$(wip_record_path); then rm -f "$wipbase_file" 2>/dev/null; fi ;; Skill) case "$(input_field skill)" in diff --git a/plugins/dev-workflow/hooks/codex-gate.test.sh b/plugins/dev-workflow/hooks/codex-gate.test.sh index e9c3a56..6c7ca27 100644 --- a/plugins/dev-workflow/hooks/codex-gate.test.sh +++ b/plugins/dev-workflow/hooks/codex-gate.test.sh @@ -151,7 +151,7 @@ reset_gate_state() { rm -f "$state" "$count" "$fresh" "$countA"; } # that wants the gate off must set the marker after calling this. reset_all() { rm -f "$state" "$count" "$fresh" "$countA" "$floorf" "$toolsf" "$notedf" \ - "$bgadvf" "$unverf" "$pendf" "$offf" .context/codex-gate.wipBase + "$bgadvf" "$unverf" "$pendf" "$offf" .context/codex-gate.wipBase.* } # 0. THE jq-FREE PATH IS USABLE. Asserted before anything depends on it: if tree_hash @@ -564,7 +564,7 @@ reset_all # 19. FINDING 11 — WIP commit is cycle-internal: gentle note, no gate-state reminder, no reset reset_all rev; rev # 2 passes accumulated -wip() { run "{\"hook_event_name\":\"PreToolUse\",\"tool_name\":\"Bash\",\"tool_input\":{\"command\":\"$1\"}}"; } +wip() { run "{\"hook_event_name\":\"PreToolUse\",\"tool_name\":\"Bash\",\"tool_use_id\":\"${2:-toolu_t1}\",\"tool_input\":{\"command\":\"$1\"}}"; } out=$(wip "git commit -m 'wip: pre-review snapshot'") printf '%s' "$out" | grep -q 'Codex gate state:' && fail "WIP commit must not emit the gate-state reminder" || pass "WIP commit does not emit the gate-state reminder" printf '%s' "$out" | grep -q 'WIP commit' && pass "WIP commit -> gentle note" || fail "WIP commit -> gentle note" @@ -575,7 +575,7 @@ printf '%s' "$out" | grep -q 'WIP commit' && pass "WIP matcher is case-insensiti # (empty, so no fingerprint input moves) and undone afterwards. wip "git commit -m 'wip: snapshot'" >/dev/null git commit -q --allow-empty -m 'wip: snapshot' >/dev/null 2>&1 -run '{"hook_event_name":"PostToolUse","tool_name":"Bash","tool_input":{"command":"git commit -m \"wip: snapshot\""}}' >/dev/null +run '{"hook_event_name":"PostToolUse","tool_name":"Bash","tool_use_id":"toolu_t1","tool_input":{"command":"git commit -m \"wip: snapshot\""}}' >/dev/null [ "$(cat "$count" 2>/dev/null)" = 2 ] && pass "WIP commit preserves pass count (Finding 11)" || fail "WIP commit preserves pass count (Finding 11)" [ -f "$state" ] && pass "WIP commit preserves Gate B state" || fail "WIP commit preserves Gate B state" git reset -q --soft HEAD~1 >/dev/null 2>&1 @@ -593,7 +593,7 @@ commitpost # cycle-internal too — but it carries no `-m`, and a command-string match alone reset it. # The empty commit keeps HEAD's tree, so no fingerprint input moves; it is undone below. git commit -q --allow-empty -m 'WIP: snapshot' >/dev/null 2>&1 -amendpost() { run "{\"hook_event_name\":\"PostToolUse\",\"tool_name\":\"Bash\",\"tool_input\":{\"command\":\"$1\"}}" >/dev/null; } +amendpost() { run "{\"hook_event_name\":\"PostToolUse\",\"tool_name\":\"Bash\",\"tool_use_id\":\"${2:-toolu_t1}\",\"tool_input\":{\"command\":\"$1\"}}" >/dev/null; } reset_all; rev; rev out=$(wip "git commit --amend --no-edit") printf '%s' "$out" | grep -q 'Codex cycle decided after' && pass "amend --no-edit on WIP HEAD -> WIP note" || fail "amend --no-edit on WIP HEAD -> WIP note" @@ -765,12 +765,13 @@ done # (m6) an editor could replace the message, so -e gets the reminder (not run: no editor here) pre=$(wip "git commit -q -e -m 'WIP: next'") ! noted && pass "reminder before: -e with a WIP message" || fail "reminder before: -e with a WIP message" -# (m7) without jq only the single-quoted form can be read whole: it keeps the cycle there too +# (m7) without jq the call id cannot be read safely, so no record is kept and the WIP commit +# resets (the safe direction) reset_all; rev; rev -nojq_run "{\"hook_event_name\":\"PreToolUse\",\"tool_name\":\"Bash\",\"tool_input\":{\"command\":\"git commit -m 'WIP: next'\"}}" >/dev/null +nojq_run "{\"hook_event_name\":\"PreToolUse\",\"tool_name\":\"Bash\",\"tool_use_id\":\"toolu_t1\",\"tool_input\":{\"command\":\"git commit -m 'WIP: next'\"}}" >/dev/null git commit -q --allow-empty -m 'WIP: next' >/dev/null 2>&1 -nojq_run "{\"hook_event_name\":\"PostToolUse\",\"tool_name\":\"Bash\",\"tool_input\":{\"command\":\"git commit -m 'WIP: next'\"}}" >/dev/null -kept && pass "jq-free: single-quoted -m wip keeps the cycle" || fail "jq-free: single-quoted -m wip keeps the cycle" +nojq_run "{\"hook_event_name\":\"PostToolUse\",\"tool_name\":\"Bash\",\"tool_use_id\":\"toolu_t1\",\"tool_input\":{\"command\":\"git commit -m 'WIP: next'\"}}" >/dev/null +[ ! -f "$count" ] && pass "jq-free: -m wip resets (no call id without jq)" || fail "jq-free: -m wip resets (no call id without jq)" # (m8) a post-commit hook that makes a real commit and then a WIP one: HEAD is WIP, but it # no longer sits on the commit that was HEAD when the command started, so reset printf '#!/bin/sh\ngit -c core.hooksPath=/dev/null commit -q --allow-empty -m real >/dev/null 2>&1\ngit -c core.hooksPath=/dev/null commit -q --allow-empty -m "WIP: from a hook" >/dev/null 2>&1\n' > "$hooksd/post-commit" @@ -785,7 +786,7 @@ amendpost "git commit -m 'WIP: next'" [ ! -f "$count" ] && pass "-m wip with no PreToolUse record: reset" || fail "-m wip with no PreToolUse record: reset" # (m10) --amend -m 'WIP…' keeps the cycle: the new HEAD shares the recorded parent mlife "git commit -q --amend -m 'WIP: amended'" -[ "$moved" = yes ] && [ "$subj" = 'WIP: amended' ] && kept && [ ! -f .context/codex-gate.wipBase ] \ +[ "$moved" = yes ] && [ "$subj" = 'WIP: amended' ] && kept && [ ! -f .context/codex-gate.wipBase.toolu__t1 ] \ && pass "--amend -m wip: same parent, cycle kept, record removed" || fail "--amend -m wip: same parent, cycle kept, record removed" # (m11) a hook rewrites the WIP message to a real one, then amends that real commit back to # WIP: the final HEAD still sits on the recorded HEAD, but the reflog grew by two, so reset @@ -816,16 +817,77 @@ git commit -q --amend -m 'WIP: snapshot' >/dev/null 2>&1 # (m14) a corrupt record (a leading-zero count, read as octal by shell arithmetic) must neither # make the hook fail nor keep the cycle reset_all; rev; rev -printf '%s - 08 new\n' "$(git rev-parse HEAD)" > .context/codex-gate.wipBase +printf '%s - 08 new\n' "$(git rev-parse HEAD)" > .context/codex-gate.wipBase.toolu__t1 git commit -q --allow-empty -m 'WIP: next' >/dev/null 2>&1 -run '{"hook_event_name":"PostToolUse","tool_name":"Bash","tool_input":{"command":"git commit -m '"'"'WIP: next'"'"'"}}' >/dev/null; rc=$? +run '{"hook_event_name":"PostToolUse","tool_name":"Bash","tool_use_id":"toolu_t1","tool_input":{"command":"git commit -m '"'"'WIP: next'"'"'"}}' >/dev/null; rc=$? [ "$rc" -eq 0 ] && [ ! -f "$count" ] && pass "corrupt record (08): exit 0 and reset" || fail "corrupt record (08): exit 0 and reset" # (m15) a backgrounded Bash call: PostToolUse arrives before the commit ran, HEAD looks # unchanged, and that is no evidence of a failed commit — reset reset_all; rev; rev wip "git commit -m 'WIP: next'" >/dev/null -run '{"hook_event_name":"PostToolUse","tool_name":"Bash","tool_input":{"command":"git commit -m '"'"'WIP: next'"'"'","run_in_background":true},"tool_response":{"backgroundTaskId":"b1"}}' >/dev/null +run '{"hook_event_name":"PostToolUse","tool_name":"Bash","tool_use_id":"toolu_t1","tool_input":{"command":"git commit -m '"'"'WIP: next'"'"'","run_in_background":true},"tool_response":{"backgroundTaskId":"b1"}}' >/dev/null [ ! -f "$count" ] && pass "backgrounded -m wip commit: reset" || fail "backgrounded -m wip commit: reset" +# (m16) overlapping calls, Greptile's order: A's Pre, A's commit, B's Pre, A's Post, B's +# commit, B's Post. Each call reads its own record, so both valid WIP commits keep the cycle +reset_all; rev; rev +wip "git commit -q -m 'WIP: a'" toolu_a >/dev/null +git commit -q --allow-empty -m 'WIP: a' >/dev/null 2>&1 +wip "git commit -q -m 'WIP: b'" toolu_b >/dev/null +amendpost "git commit -q -m 'WIP: a'" toolu_a +git commit -q --allow-empty -m 'WIP: b' >/dev/null 2>&1 +amendpost "git commit -q -m 'WIP: b'" toolu_b +kept && pass "overlapping WIP calls each keep the cycle" || fail "overlapping WIP calls each keep the cycle" +# (m17) the other direction: a hook turns A's WIP commit into a real one, then B makes a WIP +# commit on top. A's decision must not come from B's record: A resets +reset_all; rev; rev +printf '%s' "$rewrite_hook" > "$hooksd/prepare-commit-msg"; chmod +x "$hooksd/prepare-commit-msg" +wip "git commit -q -m 'WIP: a'" toolu_a >/dev/null +git commit -q --allow-empty -m 'WIP: a' >/dev/null 2>&1 +rm -f "$hooksd/prepare-commit-msg" +wip "git commit -q -m 'WIP: b'" toolu_b >/dev/null +git commit -q --allow-empty -m 'WIP: b' >/dev/null 2>&1 +amendpost "git commit -q -m 'WIP: a'" toolu_a +[ ! -f "$count" ] && pass "A rewritten to real, B WIP on top: A's Post resets" || fail "A rewritten to real, B WIP on top: A's Post resets" +# (m18) an unusable call id gets no record, so the WIP commit resets +reset_all; rev; rev +wip "git commit -q -m 'WIP: c'" 'bad/id' >/dev/null +git commit -q --allow-empty -m 'WIP: c' >/dev/null 2>&1 +amendpost "git commit -q -m 'WIP: c'" 'bad/id' +[ ! -f "$count" ] && ! ls .context/codex-gate.wipBase.* >/dev/null 2>&1 && pass "unusable call id: no record, reset" || fail "unusable call id: no record, reset" +# (m19) malformed ids that a lossy read would turn into a valid one: a trailing newline, and +# a number. Each must reset without reading or removing call B's record +for bad in '"toolu_b\n"' '123'; do + reset_all; rev; rev + printf '%s' "$rewrite_hook" > "$hooksd/prepare-commit-msg"; chmod +x "$hooksd/prepare-commit-msg" + run "{\"hook_event_name\":\"PreToolUse\",\"tool_name\":\"Bash\",\"tool_use_id\":$bad,\"tool_input\":{\"command\":\"git commit -q -m 'WIP: a'\"}}" >/dev/null + git commit -q --allow-empty -m 'WIP: a' >/dev/null 2>&1 + rm -f "$hooksd/prepare-commit-msg" + wip "git commit -q -m 'WIP: b'" toolu_b >/dev/null + git commit -q --allow-empty -m 'WIP: b' >/dev/null 2>&1 + run "{\"hook_event_name\":\"PostToolUse\",\"tool_name\":\"Bash\",\"tool_use_id\":$bad,\"tool_input\":{\"command\":\"git commit -q -m 'WIP: a'\"}}" >/dev/null + [ ! -f "$count" ] && [ -f .context/codex-gate.wipBase.toolu__b ] && pass "malformed id $bad: reset, B's record untouched" || fail "malformed id $bad: reset, B's record untouched" +done +# (m20) without jq, a nested tool_use_id with no top-level one must not be taken for this +# call's id (the fallback reader cannot tell the levels apart): reset, B untouched +reset_all; rev; rev +wip "git commit -q -m 'WIP: b'" toolu_b >/dev/null +git commit -q --allow-empty -m 'WIP: b' >/dev/null 2>&1 +nojq_run '{"hook_event_name":"PostToolUse","tool_name":"Bash","tool_input":{"command":"git commit -q -m '"'"'WIP: b'"'"'"},"tool_response":{"tool_use_id":"toolu_b"}}' >/dev/null +[ ! -f "$count" ] && [ -f .context/codex-gate.wipBase.toolu__b ] && pass "nested-only id: reset, B's record untouched" || fail "nested-only id: reset, B's record untouched" +rm -f .context/codex-gate.wipBase.* +# (m21) ids differing only in letter case are different calls, also on a case-insensitive +# filesystem: A (toolu_A) is rewritten to real, B (toolu_a) makes a WIP commit on top; A resets +# and B's record survives +reset_all; rev; rev +printf '%s' "$rewrite_hook" > "$hooksd/prepare-commit-msg"; chmod +x "$hooksd/prepare-commit-msg" +wip "git commit -q -m 'WIP: a'" toolu_A >/dev/null +git commit -q --allow-empty -m 'WIP: a' >/dev/null 2>&1 +rm -f "$hooksd/prepare-commit-msg" +wip "git commit -q -m 'WIP: b'" toolu_a >/dev/null +git commit -q --allow-empty -m 'WIP: b' >/dev/null 2>&1 +amendpost "git commit -q -m 'WIP: a'" toolu_A +[ ! -f "$count" ] && [ -f .context/codex-gate.wipBase.toolu__a ] && pass "ids differing in case stay apart: A resets, B's record kept" || fail "ids differing in case stay apart: A resets, B's record kept" +rm -f .context/codex-gate.wipBase.* git reset -q --soft "$base19d" >/dev/null 2>&1 git reset -q --soft HEAD~1 >/dev/null 2>&1; git rm -q --cached wip.txt >/dev/null 2>&1; rm -f wip.txt rm -rf "$hooksd"; { [ -e "$sandbox/orig-hooks" ] || [ -L "$sandbox/orig-hooks" ]; } && mv "$sandbox/orig-hooks" "$hooksd"