fix(hooks): codex-gate-check.sh stale-certification loophole (#437)

.claude-plugin/marketplace.json

@@ -13,7 +13,7 @@
       "name": "sdlc-wizard",
       "source": ".",
       "description": "SDLC enforcement for AI agents — TDD, planning, self-review, CI shepherd",
-      "version": "1.85.0",
+      "version": "1.86.0",
       "author": {
         "name": "Stefan Ayala"
       },

.claude-plugin/plugin.json

@@ -1,6 +1,6 @@
 {
   "name": "sdlc-wizard",
-  "version": "1.85.0",
+  "version": "1.86.0",
   "description": "SDLC enforcement for AI agents — TDD, planning, self-review, CI shepherd",
   "author": {
     "name": "Stefan Ayala",

CHANGELOG.md

@@ -4,6 +4,16 @@ All notable changes to the SDLC Wizard.
 
 > **Note:** This changelog is for humans to read. Don't manually apply these changes - just run the wizard ("Check for SDLC wizard updates") and it handles everything automatically.
 
+## [1.86.0] - 2026-07-05
+
+### Fixed
+- **#437: `codex-gate-check.sh` stale-certification loophole.** A CERTIFIED/REVIEWED `handoff.json` status was checked as a literal string with no freshness check — any number of commits made after certification would sail through the gate on the same stale status forever. Proven live during the v1.84.0 release: 2 real post-certification commits both passed the gate on a round-11 CERTIFIED handoff that was never re-issued. Fixed: certification now records `commit_sha` (HEAD at cert time) in `handoff.json`; the gate compares it to current HEAD and treats a mismatch — or a missing field, e.g. an old-format handoff.json predating this fix — as stale (exit 2, same as an uncertified commit). This allows exactly one commit after certification and blocks the next one until re-cert. `CODEX_GATE_SKIP=1` remains the logged-justification escape hatch. TDD: 2 new tests (`test_codex_gate_blocks_stale_certification_after_new_commit`, `test_codex_gate_blocks_missing_commit_sha_as_stale`) proven RED against the unmodified hook, GREEN after; 2 existing tests updated to real git-repo fixtures with a matching `commit_sha` (191/191 hook tests green, up from 189).
+- **The protocol docs needed updating in 3 separate places, not 1 — Codex cross-model review caught 2 of them.** `CLAUDE_CODE_SDLC_WIZARD.md` documents the cross-model review protocol twice (a condensed summary section and a fuller tutorial section), each with its own prose instruction *and* its own ASCII flow diagram — 7 total "reached CERTIFIED, now what" decision points. Round 1 caught a missed prose instruction in the tutorial section; round 2's mutation testing proved my first regression test (a whole-file count comparison) had real slack and, while investigating that, surfaced a 3rd, entirely separate miss: the condensed section's own flow diagram, never touched by anything. All 7 decision points (3 prose instructions + 4 diagram exits across both sections) now write `"commit_sha"` on the same line. `skills/sdlc/SKILL.md` updated too. Replaced the count-based regression test with a per-line proximity check (`test_wizard_doc_certified_paths_all_mention_commit_sha` in `tests/test-doc-consistency.sh`) that can't be fooled by aggregate slack — proved it independently by mutating each of the 3 known-fragile spots one at a time and confirming each is individually caught (53/53 at baseline).
+- **Latent bug in `tests/test-self-update.sh`'s multi-reviewer checks**, exposed (not caused) while trimming `skills/sdlc/SKILL.md` back under its 20K-char cap: 3 test functions used an unescaped `?` as a quantifier (`multi.?review`) inside plain `grep -qi`, which runs basic regex where a bare `?` is a literal character, not "0 or 1 of the preceding." The check had only ever passed via an incidental phrase collision elsewhere in the file ("parallel tasks... `sdlc-reviewer`" in an unrelated Context Management bullet, not actual multi-reviewer guidance) — trimming that unrelated bullet removed the accidental match and surfaced the real bug. Fixed by adding `-E` (extended regex) so `?` behaves as intended; verified it now matches the genuine `**Multi-reviewer:**` guidance directly (153/153 tests green).
+
+### Why
+Post-ship retrospective on v1.84.0 (2026-07-05) independently re-confirmed a gap already tracked as ROADMAP #437 (filed 2026-07-04 while shipping #436). Design settled with Fable's input in the same retrospective, implemented as a standalone TDD PR per that plan — kept separate from the doc-only v1.85.0 release since this one changes actual hook behavior on a security-relevant gate.
+
 ## [1.85.0] - 2026-07-05
 
 ### Fixed

CLAUDE_CODE_SDLC_WIZARD.md

@@ -2453,7 +2453,7 @@ PLANNING → DOCS → TDD RED → TDD GREEN → Tests Pass → Self-Review
 
    > **Always launch codex via `run_in_background: true` on the Bash tool.** The Bash tool clamps `timeout` to 600000 ms (10 min) regardless of the value passed, and force-kills the foreground process at that wall. Multi-artifact bundle reviews (release reviews per the checklist below, multi-finding rechecks, etc.) routinely run 6–30 minutes — they need background mode to complete. The wrapper `scripts/codex-review.sh` already has a 30-min stall watchdog (`STALL_SECONDS=1800`) as the real timeout control. A foreground call killed mid-review plus the Stop-hook re-invocation loop can burn 60+ minutes of session compute on what should be a single 7-minute run (issue #364, 2026-05-27 incident). The general rule: **any long-running wrapper invoked through the CC Bash tool — codex, slow builds, long test suites — should use `run_in_background: true` unconditionally and let the wrapper's own stall watchdog be the timeout authority.**
 
-3. If CERTIFIED → proceed to CI. If NOT CERTIFIED → go to Round 2.
+3. If CERTIFIED → **write `"commit_sha": "<git rev-parse HEAD>"` into `handoff.json` before proceeding to CI.** `hooks/codex-gate-check.sh` compares this SHA to current HEAD at commit time and treats a mismatch (or a missing field) as a stale certification (ROADMAP #437) — a CERTIFIED status string alone doesn't prove the certification still covers what's about to be committed. If NOT CERTIFIED → go to Round 2.
 
 ### Round 2+: Dialogue Loop
 
@@ -2497,7 +2497,7 @@ When the reviewer finds issues, respond per-finding instead of silently fixing e
      < /dev/null
    ```
 
-4. If CERTIFIED → done. If NOT CERTIFIED (rejected disputes or failed fixes) → fix rejected items and repeat.
+4. If CERTIFIED → **write/update `"commit_sha"` in `handoff.json` to current HEAD** (same reason as Round 1 step 3 — the gate hook checks it). If NOT CERTIFIED (rejected disputes or failed fixes) → fix rejected items and repeat.
 
 ### Convergence
 
@@ -2512,7 +2512,7 @@ Self-review passes → handoff.json (round 1, PENDING_REVIEW)
                             |
                    Reviewer: FULL REVIEW (structured findings)
                             |
-                   CERTIFIED? → YES → CI feedback loop
+                   CERTIFIED? → YES → write commit_sha → CI feedback loop
                             |
                             NO (findings with IDs + certify conditions)
                             |
@@ -2523,7 +2523,7 @@ Self-review passes → handoff.json (round 1, PENDING_REVIEW)
                             |
                    Reviewer: TARGETED RECHECK (previous findings only)
                             |
-                   All resolved? → YES → CERTIFIED
+                   All resolved? → YES → CERTIFIED (write commit_sha)
                             |
                             NO → fix rejected items, repeat
                             (max 3 rechecks, then escalate to user)
@@ -3075,7 +3075,7 @@ If deployment fails or post-deploy verification catches issues:
 
 **SDLC.md:**
 ```markdown
-<!-- SDLC Wizard Version: 1.85.0 -->
+<!-- SDLC Wizard Version: 1.86.0 -->
 <!-- Setup Date: [DATE] -->
 <!-- Completed Steps: step-0.1, step-0.2, step-0.4, step-1, step-2, step-3, step-4, step-5, step-6, step-7, step-8, step-9 -->
 <!-- Git Workflow: [PRs or Solo] -->
@@ -3909,7 +3909,7 @@ codex exec \
 
 > **Always launch via `run_in_background: true` on the Bash tool.** Same reason as the parallel callout in the Cross-Model Review Loop section above — Bash tool clamps `timeout` to 600000 ms (10 min) regardless of the value passed and force-kills foreground at the wall. Multi-artifact bundle reviews need background mode to complete. See issue #364 (2026-05-27 incident: 70 min session compute on a 7-min review).
 
-4. If CERTIFIED → done. If NOT CERTIFIED → enter the dialogue loop.
+4. If CERTIFIED → **write `"commit_sha": "<git rev-parse HEAD>"` into `handoff.json`** — `hooks/codex-gate-check.sh` (ROADMAP #437) blocks a commit if this is missing or doesn't match current HEAD, so a bare `CERTIFIED` status isn't enough. Then done. If NOT CERTIFIED → enter the dialogue loop.
 
 **The Dialogue Loop (Round 2+):**
 
@@ -3979,7 +3979,7 @@ Claude writes code → self-review passes → handoff.json (round 1)
     |                              Reviewer: FULL REVIEW
     |                              (structured findings with IDs)
     |                                          |
-    |                              CERTIFIED? -+→ YES → Done
+    |                              CERTIFIED? -+→ YES → write commit_sha → Done
     |                                          |
     |                                          +→ NO (findings)
     |                                          |
@@ -3989,12 +3989,14 @@ Claude writes code → self-review passes → handoff.json (round 1)
     |                              Reviewer: TARGETED RECHECK
     |                              (previous findings only, no new P1/P2)
     |                                          |
-    |                              All resolved? → YES → CERTIFIED
+    |                              All resolved? → YES → CERTIFIED (write commit_sha)
     |                                          |
     └────────── Fix rejected items ←───────────┘
                     (max 3 rechecks, then escalate to user)
 ```
 
+**Every CERTIFIED path above writes `"commit_sha": "<git rev-parse HEAD>"` into `handoff.json`** — `hooks/codex-gate-check.sh` (ROADMAP #437) treats a missing or mismatched SHA as a stale certification, so a bare `CERTIFIED` status string is never enough on its own.
+
 **Key flags:**
 - `-c 'model_reasoning_effort="xhigh"'` — Maximum reasoning depth. This is where you get the most value. Testing showed `xhigh` caught 3 findings that `high` missed on the same content.
 - `-s danger-full-access` — Full filesystem read/write so the reviewer can read your actual code.