diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 818395f9..67d5dcc2 100644 --- a/.claude-plugin/marketplace.json +++ b/.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" }, diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 7c198310..c1e349af 100644 --- a/.claude-plugin/plugin.json +++ b/.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", diff --git a/CHANGELOG.md b/CHANGELOG.md index b3c2c97e..99f345de 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/CLAUDE_CODE_SDLC_WIZARD.md b/CLAUDE_CODE_SDLC_WIZARD.md index 8bb9e62c..87cda06d 100644 --- a/CLAUDE_CODE_SDLC_WIZARD.md +++ b/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": ""` 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 - + @@ -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": ""` 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": ""` 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. diff --git a/ROADMAP.md b/ROADMAP.md index 837e5b8b..801fb93d 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -207,7 +207,7 @@ Living tracker of projects shipped using this wizard. **Rule:** only list projec | Project | Repo | Status | |---------|------|--------| -| SDLC Wizard itself | BaseInfinity/claude-sdlc-wizard | Dogfooded, v1.85.0 | +| SDLC Wizard itself | BaseInfinity/claude-sdlc-wizard | Dogfooded, v1.86.0 | | Codex SDLC Adapter | BaseInfinity/codex-sdlc-wizard | v0.7.x, shipped with SDLC workflow | | GDLC Wizard (games sibling) | BaseInfinity/claude-gdlc-wizard | v0.2.x, persona-driven playtest cycles | | _(add as projects are marked)_ | | | @@ -321,6 +321,6 @@ Living tracker of projects shipped using this wizard. **Rule:** only list projec | 434 | AI Setup Lanes v3 — Sonnet 5 + model-aware effort + auto-escalation | **Maintainer pain event:** AI_SETUP_LANES.md still references Sonnet 4.6 and recommends blanket `max` effort. Sonnet 5 launched June 30 and beats Opus 4.6 on every benchmark (SWE-bench Verified 85.2% vs 80.8%, Terminal-Bench 80.4% vs 65.4%). Opus 4.6 cannot advise Sonnet 5 (pairing table rejects it). Fable+Codex cross-model review both confirmed: Sonnet 5 high + Fable advisor is the new recommended primary, Opus 4.8 xhigh for escalation. **Scope (multi-part):** (a) ✅ DONE: Update AI_SETUP_LANES.md with Sonnet 5, model-aware effort per model, escalation ladder (PR pending); (b) Update `model-effort-check.sh` to recommend effort per model (not blanket max) — Sonnet 5: high/xhigh, Opus 4.8: xhigh, Opus 4.6: max, Fable: high; (c) Update `/sdlc` skill frontmatter from `effort: max` to model-aware; (d) Add auto-escalation detection to `sdlc-prompt-check.sh` — after 3+ failures or context >200K, suggest `/effort xhigh` then `/model opus`; (e) Add quota-burn awareness — detect when session is burning Max limits fast and suggest effort downgrade; (f) Sync updated lanes to sibling wizards (gdlc, rdlc) — **still open 2026-07-05.** Maintainer intent: fold this into the Fable-led sibling upgrade pass (#438) once this repo's own v1.84.0 retrospective fixes land, rather than a separate sync pass — one Fable run per sibling repo covers both the Lanes v3 sync and the #438 bloat/review sweep. **Entry gate:** maintainer pain event — stale model recommendations, Opus 4.6 pairing broken with Sonnet 5. Cross-model verified: Fable recommended Opus 4.6 stability, Codex recommended Sonnet 5 switch — data favors Sonnet 5 but wizard lets users choose. | | 425 | Dynamic Workflows + ultracode evaluation — CC v2.1.154 feature we missed | **Maintainer pain event (#350 cadence gap manifested again).** CC v2.1.154 (May 28, 2026) shipped Dynamic Workflows — JavaScript scripts orchestrating subagents at scale. We're on v2.1.173 and missed it for 3 weeks. ROADMAP #71 (KAIROS/Coordinator Mode) was watching for this class of feature — Dynamic Workflows may or may not be related to KAIROS internally (not verifiable from public sources). Deep-research (2026-06-19, Codex 7/10 verified) confirmed: (a) only built-in workflow is `/deep-research`; custom workflows save to `.claude/workflows/` (project) or `~/.claude/workflows/` (user); (b) `ultracode` is a new effort level (`/effort ultracode`) = xhigh reasoning + automatic workflow orchestration, session-only; (c) `agent()`, `parallel()`, `pipeline()`, `phase()` primitives are NOT in official Anthropic docs — internal implementation, treat as non-public API; (d) Agent Teams are separate (experimental, `CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS`), not superseded. **Scope:** (a) Update #71 wording — Dynamic Workflows shipped as the public multi-agent orchestration feature, relationship to KAIROS unknown, Agent Teams still separate/experimental; (b) Evaluate `ultracode` vs `max` — Codex says ultracode should NOT replace max as default (it's xhigh + auto-workflow, not deeper reasoning), document as opt-in escalation for audits/migrations/research; (c) Document Workflows in wizard — when to use, cost awareness (deep-research = 2.5M tokens), `.claude/workflows/` save path; (d) Evaluate whether wizard should ship custom SDLC workflows (Prove-It Gate applies — prove quality before adding). **Entry gate:** maintainer pain event — #350 cadence gap. Sources: [code.claude.com/docs/en/workflows](https://code.claude.com/docs/en/workflows), [Anthropic blog](https://claude.com/blog/introducing-dynamic-workflows-in-claude-code), Codex review `.reviews/codex-deep-research-triage.md`. | | 436 | Fable self-enforcement audit — codex-gate-check.sh + tdd-pretool-check.sh never actually blocked | **Maintainer pain event 2026-07-04:** "I hate having to remind 'don't forget to cross-model review with codex'." Root-cause audit found two enforcement hooks that printed the right warning text but always `exit 0` — including on their "REQUIRED"/blocking branches. Claude Code only denies a `PreToolUse` call on `exit 2` + stderr; `exit 0` always allows regardless of stdout content. Both hooks had shipped this way since introduction, silently decorative the whole time. **Scope shipped:** (a) `codex-gate-check.sh` — the two "CROSS-MODEL REVIEW REQUIRED" branches now `exit 2`; (b) `tdd-pretool-check.sh` — new edit-ordering gate blocks (`exit 2`) writes to `src/**` unless a test file was touched earlier in the session (proxy for TDD RED — a bash hook can't run the suite to confirm a real failing test, this is the best mechanically-available approximation); (c) new `Stop` hook (`codex-review-stop-check.sh`) closes a third gap — a session can end with `git commit` never invoked, so the Bash-triggered gate never fires; non-blocking (Stop can't deny), warns once per session on significant uncommitted changes with no REVIEWED/CERTIFIED handoff; (d) fixed 3 pre-existing distribution-parity bugs surfaced by a new stronger test (`test_hooks_json_script_parity`, compares actual registered scripts per hook event, not just event-name presence) — `codex-gate-check.sh` was never wired into the plugin's `hooks/hooks.json` (plugin users had zero cross-model gate), `goal-confidence-check.sh` was missing from the project's own `.claude/settings.json`, `token-spike-check.sh` was missing from the CLI template and from `cli/init.js`'s FILES array entirely (never actually copied to consumer repos by `npx ... init`). **Entry gate:** maintainer pain event — the exact mechanism this whole wizard exists to provide (mechanical enforcement instead of verbal reminders) was non-functional. **Verified:** 22 test files run clean post-fix (185 hook tests run twice for isolation, plus doc-consistency/cowork-drift/cli/plugin/audit-session-load/tdd-fires-once and 15 more referencing the changed files). | -| 437 | Codex-gate handoff staleness — `.reviews/handoff.json` status isn't checked for freshness or branch match | **Found while shipping #436, 2026-07-04:** `codex-gate-check.sh` only checks `.reviews/handoff.json`'s `status` field (`CERTIFIED`/`REVIEWED` → allow commit). It does not check `review_id`, `branch`, `requested_at` staleness, or that `files_changed` overlaps the actually-staged diff. Live evidence: a week-old `REVIEWED` handoff (`review_id: "drift-baseline-fix-001"`, dated 2026-06-28, for an already-merged and unrelated PR #431) was still sitting in `.reviews/` and would have silently satisfied the gate for #436's entirely different, never-reviewed changes had it not been manually overwritten before committing. **Independently re-confirmed 2026-07-05** via the v1.84.0 post-ship retrospective (Fable + Codex dual audit): 2 real commits landed after round-11 CERTIFIED and both sailed through the gate on the same stale certification — proving the gap live a second time, on this exact hook. **Design settled (Fable-reviewed 2026-07-05), simpler than the branch/files_changed idea above:** certification writes `commit_sha` (HEAD at cert time) into `handoff.json`; the gate compares current `git rev-parse HEAD` to that SHA — mismatch (or missing `commit_sha`, e.g. old-format handoff files) = stale, exit 2. This directly answers the open design question below: it allows exactly one commit after certification (HEAD still equals the recorded SHA at that commit's `PreToolUse` check) and blocks the next one until re-cert, rather than needing a branch/superset-diff heuristic. `CODEX_GATE_SKIP=1` remains the logged-justification escape hatch for legitimate small follow-ups. **Original open design question (superseded by the above):** how to avoid false-positiving on legitimate small follow-up commits after a review already covered the substance (e.g. a one-line typo fix Codex itself suggested) — a naive "any new diff invalidates the handoff" rule would defeat the recheck/dialogue loop the protocol already relies on. **Entry gate:** maintainer pain event — demonstrated live twice now, not theoretical. Not fixed in #436 (separate, non-trivial concern; scope discipline). **Status:** design settled, implementation pending (Pass 2 of the v1.84.0 retrospective fix plan, TDD, standalone PR). | +| 437 | Codex-gate handoff staleness — `.reviews/handoff.json` status isn't checked for freshness or branch match | **Found while shipping #436, 2026-07-04:** `codex-gate-check.sh` only checks `.reviews/handoff.json`'s `status` field (`CERTIFIED`/`REVIEWED` → allow commit). It does not check `review_id`, `branch`, `requested_at` staleness, or that `files_changed` overlaps the actually-staged diff. Live evidence: a week-old `REVIEWED` handoff (`review_id: "drift-baseline-fix-001"`, dated 2026-06-28, for an already-merged and unrelated PR #431) was still sitting in `.reviews/` and would have silently satisfied the gate for #436's entirely different, never-reviewed changes had it not been manually overwritten before committing. **Independently re-confirmed 2026-07-05** via the v1.84.0 post-ship retrospective (Fable + Codex dual audit): 2 real commits landed after round-11 CERTIFIED and both sailed through the gate on the same stale certification — proving the gap live a second time, on this exact hook. **Design settled (Fable-reviewed 2026-07-05), simpler than the branch/files_changed idea above:** certification writes `commit_sha` (HEAD at cert time) into `handoff.json`; the gate compares current `git rev-parse HEAD` to that SHA — mismatch (or missing `commit_sha`, e.g. old-format handoff files) = stale, exit 2. This directly answers the open design question below: it allows exactly one commit after certification (HEAD still equals the recorded SHA at that commit's `PreToolUse` check) and blocks the next one until re-cert, rather than needing a branch/superset-diff heuristic. `CODEX_GATE_SKIP=1` remains the logged-justification escape hatch for legitimate small follow-ups. **Original open design question (superseded by the above):** how to avoid false-positiving on legitimate small follow-up commits after a review already covered the substance (e.g. a one-line typo fix Codex itself suggested) — a naive "any new diff invalidates the handoff" rule would defeat the recheck/dialogue loop the protocol already relies on. **Entry gate:** maintainer pain event — demonstrated live twice now, not theoretical. Not fixed in #436 (separate, non-trivial concern; scope discipline). **Status: ✅ DONE 2026-07-05** (v1.86.0 — Pass 2 of the v1.84.0 retrospective fix plan, standalone TDD PR, 2 new tests + 2 existing tests updated to real git-repo fixtures, 191/191 hook tests green). | | 438 | Cross-repo Fable audit sweep — track and follow up on the 5 GH issues filed across the ecosystem | **Maintainer call-out 2026-07-04:** "use fable to review/upgrade these repos while it's free... reduce bloat and fix and review all that stuff," extended across every repo that matters, not just this one. Fable-led review/upgrade/bloat-reduction issues were filed on 5 repos as a time-boxed sweep before the July 1-7 free-inclusion window closes: [claude-gdlc-wizard#14](https://github.com/BaseInfinity/claude-gdlc-wizard/issues/14), [claude-rdlc-wizard#9](https://github.com/BaseInfinity/claude-rdlc-wizard/issues/9), [audit-agent#1](https://github.com/BaseInfinity/audit-agent/issues/1) (ADLC), [anticheat#1](https://github.com/BaseInfinity/anticheat/issues/1), [pdlc#6](https://github.com/BaseInfinity/pdlc/issues/6). This repo's own equivalent is tracked separately as #236 (bloat-hunt, re-triggered 2026-07-04 with the same Fable-window urgency) — #438 is specifically the cross-repo tracking/follow-up, since those 5 issues live in repos this ROADMAP doesn't otherwise cover. **Scope:** (a) follow up on each filed issue once picked up — verify Fable was actually used (not silently misrouted to another model, per the `CLAUDE_CODE_SUBAGENT_MODEL` bug found the same night, see #436 commit history) via the subagent self-identifying its model in its report; (b) after the July 7 window, note in each issue whether it was completed in time or needs re-scoping for metered Fable; (c) if a genuine pattern/finding recurs across 3+ of these repos (e.g. the same stale-doc-vs-lessons-learned contradiction found in this repo's own SDLC.md), consider whether it's worth a wizard-level fix (a hook/test/template change here) rather than 5 separate one-off fixes; (d) **maintainer add 2026-07-04:** extend the audit to measure token efficiency and ROI per repo — for each swept repo, capture roughly how many tokens the Fable pass spent (`/usage` or session cost) against what it actually fixed/found (bugs caught, bloat removed, doc drift corrected), so the sweep reports back "was this worth it" per repo, not just "did it run." Design not yet started — needs a lightweight before/after measure (e.g. char-count or token-bloat-audit delta plus a qualitative finding count) that doesn't require its own new tooling per repo. **Entry gate:** maintainer pain event — issues filed but nothing tracks whether the cross-repo sweep actually happened or paid off. **Recurring pattern confirmed 2026-07-05** (this is the #438(c) trigger): the v1.84.0 post-ship retrospective independently re-found the exact stale-doc-vs-lessons-learned pattern #438(c) called out by name (see #437's staleness gap, and the CI-Feedback-Loop/Convergence gaps fixed in `CLAUDE_CODE_SDLC_WIZARD.md` the same day) — a wizard-level fix (this repo's own doc + hook) is underway rather than 5 one-off fixes. **Maintainer intent 2026-07-05:** once this repo's fixes land, maintainer will run the Fable-led upgrade on each sibling repo (gdlc, rdlc, etc.) pointed at this repo's now-updated guidelines, so the siblings pick up similar patterns/discipline rather than each independently reinventing them — the 5 filed issues are the vehicle for that, not just a bloat-reduction pass. | diff --git a/SDLC.md b/SDLC.md index f77dd93d..622440e7 100644 --- a/SDLC.md +++ b/SDLC.md @@ -1,4 +1,4 @@ - + @@ -10,7 +10,7 @@ | Property | Value | |----------|-------| -| Wizard Version | 1.85.0 | +| Wizard Version | 1.86.0 | | Last Updated | 2026-07-04 | | Claude Code Minimum | v2.1.154+ (required for `opus[1m]` alias resolution); v2.1.105+ for `PreCompact` hook | | Claude Code Recommended | v2.1.195+ — comma-separated hook matcher fix (v2.1.191), hyphenated matcher exact-match fix (v2.1.195), `sandbox.credentials` setting (v2.1.187), `autoMode.classifyAllShell` setting (v2.1.193) | diff --git a/cowork/.claude-plugin/plugin.json b/cowork/.claude-plugin/plugin.json index adb2520b..d99fe287 100644 --- a/cowork/.claude-plugin/plugin.json +++ b/cowork/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "sdlc-wizard-cowork", - "version": "1.85.0", + "version": "1.86.0", "description": "SDLC enforcement for Claude Cowork — TDD, planning, self-review via prompt-based hooks and skills", "author": { "name": "Stefan Ayala", diff --git a/cowork/README.md b/cowork/README.md index 6fde56a3..ef263ab3 100644 --- a/cowork/README.md +++ b/cowork/README.md @@ -96,4 +96,4 @@ This plugin is for Cowork-only users who want methodology guidance without the f ## Version -Tracks the main wizard version: **1.85.0** +Tracks the main wizard version: **1.86.0** diff --git a/cowork/skills/sdlc/SKILL.md b/cowork/skills/sdlc/SKILL.md index ad290ae9..19aa2bd9 100644 --- a/cowork/skills/sdlc/SKILL.md +++ b/cowork/skills/sdlc/SKILL.md @@ -9,7 +9,7 @@ argument-hint: [task description] This skill is loaded from **`.claude/skills/sdlc/SKILL.md`** in the active repo (symlinked to `skills/sdlc/SKILL.md` in the wizard's source tree). Claude Code prefers repo-local skills over global (`~/.claude/skills/sdlc/SKILL.md`) when both exist with the same name — the repo-local copy is the project's authoritative workflow contract. Use global skills only for cross-repo personal tooling (e.g. `feedback`, `revise-claude-md`); use repo-local for implementation, tests, release, and verification in this repo. -If unsure which copy is active, compare `head -5 .claude/skills/sdlc/SKILL.md` against `head -5 ~/.claude/skills/sdlc/SKILL.md`. The repo-local copy wins. Don't mix guidance from both — pick the source for this repo and stay there. +If unsure which copy is active, compare `head -5` of both — the repo-local copy wins. Don't mix guidance from both. ## Task $ARGUMENTS @@ -143,14 +143,14 @@ PROTOCOL is universal across domains; only `review_instructions` and `verificati 1. **Preflight** (`.reviews/preflight-{review_id}.md`) — what you already checked: `/code-review` passed, tests passing, manual verifications, known limits. Reduces reviewer findings to 0-1/round. 2. **Mission-first handoff** (`.reviews/handoff.json`) — required keys: `"review_id"`, `"status": "PENDING_REVIEW"`, `"round": 1`, `"mission"`/`"success"`/`"failure"` (without them you get "looks good"), `"files_changed"`, `"verification_checklist"` (verification checklist with file:line refs — NOT generic), `"review_instructions"`, `"preflight_path"`. Optional `"pr_number":` opts into PreCompact self-heal (#209: MERGED → implicit CERTIFIED). -3. **Run reviewer:** `codex exec -c 'model_reasoning_effort="xhigh"' -s danger-full-access -o .reviews/latest-review.md "" < /dev/null`. Always `xhigh`. Bash tool requires `run_in_background: true` + `dangerouslyDisableSandbox: true`; always append `< /dev/null`. **Why:** `< /dev/null` prevents codex stdin-hang at S/0% CPU; `run_in_background: true` avoids the Bash 10-min (`600000` ms) `timeout` cap that force-kills foreground codex (multi-artifact bundles take 5–30 min). xhigh 1–30 min; wrapper's `STALL_SECONDS=1800` controls it. Heartbeat: `scripts/codex-review-with-progress.sh`. Foreground burned 70 min on a 7-min review (#364). -4. **Dialogue loop:** per-finding response (`{"finding": "1", "action": "FIXED|DISPUTED|ACCEPTED", "summary": "..."}` in `.reviews/response.json`). Bump round, set status `PENDING_RECHECK`, add `fixes_applied` (numbered, file:line). Recheck prompt: "TARGETED RECHECK. FIXED → verify certify condition. DISPUTED → ACCEPT if sound, REJECT with reasoning. ACCEPTED → verify applied. No new findings unless P0." **NEVER unilaterally dismiss** — always run the recheck. It's a conversation: the reviewer may accept your dispute or counter with evidence you missed. +3. **Run reviewer:** `codex exec -c 'model_reasoning_effort="xhigh"' -s danger-full-access -o .reviews/latest-review.md "" < /dev/null`. Always `xhigh`. Bash tool requires `run_in_background: true` + `dangerouslyDisableSandbox: true`; always append `< /dev/null`. **Why:** `< /dev/null` prevents codex stdin-hang at S/0% CPU; `run_in_background: true` avoids the Bash 10-min (`600000` ms) `timeout` cap that force-kills foreground codex (multi-artifact bundles take 5–30 min). xhigh 1–30 min; wrapper's `STALL_SECONDS=1800` controls it. Foreground burned 70 min on a 7-min review (#364). +4. **Dialogue loop:** per-finding response (`{"finding": "1", "action": "FIXED|DISPUTED|ACCEPTED", "summary": "..."}` in `.reviews/response.json`). Bump round, set status `PENDING_RECHECK`, add `fixes_applied` (numbered, file:line). Recheck prompt: "TARGETED RECHECK. FIXED → verify certify condition. DISPUTED → ACCEPT if sound, REJECT with reasoning. ACCEPTED → verify applied. No new findings unless P0." **NEVER unilaterally dismiss** — always run the recheck. It's a conversation: the reviewer may accept your dispute or counter with evidence you missed. **On CERTIFIED, write `"commit_sha": ""` into `handoff.json`** — the gate hook (#437) treats a missing/mismatched SHA as stale, not just the status string. **Convergence:** 2 rounds sweet spot, 3 max, escalate after — except large migrations: judge by finding trend, not count. **Enforcement:** `hooks/goal-confidence-check.sh` warns when `/goal` skips the 95%-confidence or DLC-binding gates (#360). -**Multi-reviewer:** respond to each independently (no shared anchoring). **Non-code domains:** add `"audience"`/`"stakes"` keys. +**Multi-reviewer:** respond to each independently. **Non-code domains:** add `"audience"`/`"stakes"` keys. ### Release Review Focus @@ -275,7 +275,7 @@ Don't fix only the symptom. Add a gate so it can't happen again. Example: PR #14 - Auto-compact fires at ~95%; `/usage` shows what's driving token spend - After committing a PR, `/clear` before next feature - `--bare` mode (v2.1.81+) skips ALL hooks/skills/LSP/plugins. Scripted headless only — never normal development. -- Custom subagents (`.claude/agents/`) run autonomously and return results. Skills guide behavior; agents do work. Use for parallel tasks or fresh context. Examples: `sdlc-reviewer`, `ci-debug`, `test-writer`. +- Custom subagents (`.claude/agents/`) run autonomously and return results. Skills guide behavior; agents do work. Use for parallel tasks or fresh context. ## Design System Check (UI Changes Only) diff --git a/hooks/codex-gate-check.sh b/hooks/codex-gate-check.sh index c6edcde1..bd24582b 100755 --- a/hooks/codex-gate-check.sh +++ b/hooks/codex-gate-check.sh @@ -52,7 +52,26 @@ STATUS=$(grep -o '"status"[[:space:]]*:[[:space:]]*"[^"]*"' "$REVIEW_FILE" \ | sed 's/.*"status"[[:space:]]*:[[:space:]]*"//; s/"$//') case "$STATUS" in - CERTIFIED|REVIEWED) exit 0 ;; + CERTIFIED|REVIEWED) + # #437: a CERTIFIED/REVIEWED status string alone doesn't mean the + # certification is still current — commits made after it was issued + # would otherwise sail through on the same stale status forever. + # commit_sha records HEAD at cert time; a mismatch (or a missing + # field, e.g. an old-format handoff.json predating this fix) means + # new commits landed since certification, so treat it as stale. This + # allows exactly one commit after certification (HEAD still equals + # the recorded SHA at that commit's PreToolUse check) and blocks the + # next one until re-cert. No legacy-compat fallback for missing SHA. + COMMIT_SHA=$(grep -o '"commit_sha"[[:space:]]*:[[:space:]]*"[^"]*"' "$REVIEW_FILE" \ + | head -1 \ + | sed 's/.*"commit_sha"[[:space:]]*:[[:space:]]*"//; s/"$//') + CURRENT_HEAD=$(git rev-parse HEAD 2>/dev/null) || CURRENT_HEAD="" + if [ -z "$COMMIT_SHA" ] || [ "$COMMIT_SHA" != "$CURRENT_HEAD" ]; then + echo "CROSS-MODEL REVIEW REQUIRED: .reviews/handoff.json certification is stale (commit_sha does not match current HEAD — new commits landed since certification). Re-run Codex cross-model review. Set CODEX_GATE_SKIP=1 to bypass with justification." >&2 + exit 2 + fi + exit 0 + ;; *) echo "CROSS-MODEL REVIEW REQUIRED: .reviews/handoff.json status is '$STATUS' (need REVIEWED or CERTIFIED). Run Codex cross-model review before committing. Set CODEX_GATE_SKIP=1 to bypass with justification." >&2 exit 2 diff --git a/package.json b/package.json index afdb01e5..222fc7cd 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "agentic-sdlc-wizard", - "version": "1.85.0", + "version": "1.86.0", "description": "SDLC enforcement for Claude Code — hooks, skills, and wizard setup in one command", "bin": { "sdlc-wizard": "cli/bin/sdlc-wizard.js" diff --git a/skills/sdlc/SKILL.md b/skills/sdlc/SKILL.md index ad290ae9..19aa2bd9 100644 --- a/skills/sdlc/SKILL.md +++ b/skills/sdlc/SKILL.md @@ -9,7 +9,7 @@ argument-hint: [task description] This skill is loaded from **`.claude/skills/sdlc/SKILL.md`** in the active repo (symlinked to `skills/sdlc/SKILL.md` in the wizard's source tree). Claude Code prefers repo-local skills over global (`~/.claude/skills/sdlc/SKILL.md`) when both exist with the same name — the repo-local copy is the project's authoritative workflow contract. Use global skills only for cross-repo personal tooling (e.g. `feedback`, `revise-claude-md`); use repo-local for implementation, tests, release, and verification in this repo. -If unsure which copy is active, compare `head -5 .claude/skills/sdlc/SKILL.md` against `head -5 ~/.claude/skills/sdlc/SKILL.md`. The repo-local copy wins. Don't mix guidance from both — pick the source for this repo and stay there. +If unsure which copy is active, compare `head -5` of both — the repo-local copy wins. Don't mix guidance from both. ## Task $ARGUMENTS @@ -143,14 +143,14 @@ PROTOCOL is universal across domains; only `review_instructions` and `verificati 1. **Preflight** (`.reviews/preflight-{review_id}.md`) — what you already checked: `/code-review` passed, tests passing, manual verifications, known limits. Reduces reviewer findings to 0-1/round. 2. **Mission-first handoff** (`.reviews/handoff.json`) — required keys: `"review_id"`, `"status": "PENDING_REVIEW"`, `"round": 1`, `"mission"`/`"success"`/`"failure"` (without them you get "looks good"), `"files_changed"`, `"verification_checklist"` (verification checklist with file:line refs — NOT generic), `"review_instructions"`, `"preflight_path"`. Optional `"pr_number":` opts into PreCompact self-heal (#209: MERGED → implicit CERTIFIED). -3. **Run reviewer:** `codex exec -c 'model_reasoning_effort="xhigh"' -s danger-full-access -o .reviews/latest-review.md "" < /dev/null`. Always `xhigh`. Bash tool requires `run_in_background: true` + `dangerouslyDisableSandbox: true`; always append `< /dev/null`. **Why:** `< /dev/null` prevents codex stdin-hang at S/0% CPU; `run_in_background: true` avoids the Bash 10-min (`600000` ms) `timeout` cap that force-kills foreground codex (multi-artifact bundles take 5–30 min). xhigh 1–30 min; wrapper's `STALL_SECONDS=1800` controls it. Heartbeat: `scripts/codex-review-with-progress.sh`. Foreground burned 70 min on a 7-min review (#364). -4. **Dialogue loop:** per-finding response (`{"finding": "1", "action": "FIXED|DISPUTED|ACCEPTED", "summary": "..."}` in `.reviews/response.json`). Bump round, set status `PENDING_RECHECK`, add `fixes_applied` (numbered, file:line). Recheck prompt: "TARGETED RECHECK. FIXED → verify certify condition. DISPUTED → ACCEPT if sound, REJECT with reasoning. ACCEPTED → verify applied. No new findings unless P0." **NEVER unilaterally dismiss** — always run the recheck. It's a conversation: the reviewer may accept your dispute or counter with evidence you missed. +3. **Run reviewer:** `codex exec -c 'model_reasoning_effort="xhigh"' -s danger-full-access -o .reviews/latest-review.md "" < /dev/null`. Always `xhigh`. Bash tool requires `run_in_background: true` + `dangerouslyDisableSandbox: true`; always append `< /dev/null`. **Why:** `< /dev/null` prevents codex stdin-hang at S/0% CPU; `run_in_background: true` avoids the Bash 10-min (`600000` ms) `timeout` cap that force-kills foreground codex (multi-artifact bundles take 5–30 min). xhigh 1–30 min; wrapper's `STALL_SECONDS=1800` controls it. Foreground burned 70 min on a 7-min review (#364). +4. **Dialogue loop:** per-finding response (`{"finding": "1", "action": "FIXED|DISPUTED|ACCEPTED", "summary": "..."}` in `.reviews/response.json`). Bump round, set status `PENDING_RECHECK`, add `fixes_applied` (numbered, file:line). Recheck prompt: "TARGETED RECHECK. FIXED → verify certify condition. DISPUTED → ACCEPT if sound, REJECT with reasoning. ACCEPTED → verify applied. No new findings unless P0." **NEVER unilaterally dismiss** — always run the recheck. It's a conversation: the reviewer may accept your dispute or counter with evidence you missed. **On CERTIFIED, write `"commit_sha": ""` into `handoff.json`** — the gate hook (#437) treats a missing/mismatched SHA as stale, not just the status string. **Convergence:** 2 rounds sweet spot, 3 max, escalate after — except large migrations: judge by finding trend, not count. **Enforcement:** `hooks/goal-confidence-check.sh` warns when `/goal` skips the 95%-confidence or DLC-binding gates (#360). -**Multi-reviewer:** respond to each independently (no shared anchoring). **Non-code domains:** add `"audience"`/`"stakes"` keys. +**Multi-reviewer:** respond to each independently. **Non-code domains:** add `"audience"`/`"stakes"` keys. ### Release Review Focus @@ -275,7 +275,7 @@ Don't fix only the symptom. Add a gate so it can't happen again. Example: PR #14 - Auto-compact fires at ~95%; `/usage` shows what's driving token spend - After committing a PR, `/clear` before next feature - `--bare` mode (v2.1.81+) skips ALL hooks/skills/LSP/plugins. Scripted headless only — never normal development. -- Custom subagents (`.claude/agents/`) run autonomously and return results. Skills guide behavior; agents do work. Use for parallel tasks or fresh context. Examples: `sdlc-reviewer`, `ci-debug`, `test-writer`. +- Custom subagents (`.claude/agents/`) run autonomously and return results. Skills guide behavior; agents do work. Use for parallel tasks or fresh context. ## Design System Check (UI Changes Only) diff --git a/skills/update/SKILL.md b/skills/update/SKILL.md index 096a977e..4e96fb74 100644 --- a/skills/update/SKILL.md +++ b/skills/update/SKILL.md @@ -95,16 +95,16 @@ Parse CHANGELOG entries between the user's installed version and the resolved la ``` Installed: 1.42.0 -Latest: 1.85.0 +Latest: 1.86.0 What changed: -- [1.85.0] Post-ship retrospective fixes: CI Feedback Loop synced to SKILL.md's stronger version (NEVER AUTO-MERGE, read-logs-even-when-green, cross-model CI audit), CERTIFIED≠CI lesson, Policy Migration Inventory checklist, stale round-count correction (#437 hook-staleness fix follows separately). +- [1.86.0] Fix #437: codex-gate-check.sh now blocks stale certifications — a CERTIFIED handoff.json no longer sails through forever; it records commit_sha at cert time and blocks once HEAD moves past it without a re-cert. +- [1.85.0] Post-ship retrospective: CI Feedback Loop synced to SKILL.md's stronger version, CERTIFIED≠CI lesson, Policy Migration Inventory checklist, stale round-count correction. - [1.84.0] Hook enforcement fix: cross-model review gate + TDD RED gate now actually block (#436); model-aware effort docs replace blanket max recommendation. - [1.83.0] Model config batch: multi-model hook recommendation (#403), global [1m] pin detection (#391), version race fix (#405), effort config check (#384). - [1.82.0] Usage diagnostics: fix /usage row, Reading Usage Signals guide, advisor fallback procedure, Fable effort guidance, autocompact cross-reference. - [1.81.0] Native `advisorModel` support: Setup A gets Fable advisor, Setup B gets Opus advisor. Replaces manual subagent spawning. Requires CC v2.1.170+. - [1.80.0] Flip default: Opus 4.6 max becomes recommended flagship; Opus 4.8 demoted to opt-in `[l] Latest` tier. -- [1.77.0] release-dry-run.yml + cc-version-drift.yml (#350) + /goal SDLC gates (95% + DLC binding). - [1.75.1] release-workflow fix — Node 22 → 24 (ships npm 11.x), dropped flaky `npm install -g` self-upgrade (hit MODULE_NOT_FOUND on v1.75.0 publish). Explicit npm-version guard. - [1.75.0] npm Trusted Publishing — `release.yml` swapped from `NPM_TOKEN` to OIDC. No more token rotation. Requires one-time publisher config on the npm package page. - [1.74.0] v1.43 salvage: #338 precedence preamble; #235ab `/insights`; codex `< /dev/null` stdin-hang fix; test env-isolation. diff --git a/tests/test-doc-consistency.sh b/tests/test-doc-consistency.sh index 4a2d033e..716183b0 100755 --- a/tests/test-doc-consistency.sh +++ b/tests/test-doc-consistency.sh @@ -1026,6 +1026,47 @@ test_wizard_doc_autocompact_mentions_sonnet_5() { test_wizard_doc_autocompact_mentions_sonnet_5 +# ROADMAP #437: the wizard doc has 2 separate copies of the cross-model +# review protocol (a condensed summary section and a fuller tutorial +# section), each with its own prose instruction AND its own flow diagram — +# 3 prose "If CERTIFIED" lines plus 4 diagram terminal states (2 per +# section), 7 decision points total. The v1.86.0 codex-gate-check.sh fix +# requires every one of them to write commit_sha into handoff.json. +# +# Codex round 1 caught 2 of 3 prose lines fixed (1 missed); a global-count +# comparison (commit_sha mentions >= "If CERTIFIED" mentions) briefly +# replaced it but round 2's mutation testing proved that heuristic has slack +# — extra commit_sha mentions elsewhere in the doc could mask an individual +# line silently losing its own instruction, and it never covered the +# diagram terminal states at all (round 2 also found a 3rd, entirely +# separate diagram — the first section's own flow chart at lines ~2515/2526 +# — that neither the prose fix nor the original test had ever touched). +# +# Per-line check instead: every line matching one of the 3 known decision- +# point shapes (prose "If CERTIFIED", diagram "CERTIFIED? ", or diagram +# "→ CERTIFIED") must contain "commit_sha" on that SAME line — no aggregate +# slack possible. Verified the exact pattern below matches all 7 real +# decision-point lines and none of the ~13 other CERTIFIED mentions in the +# doc (explanatory prose, reviewer-prompt "End with CERTIFIED or NOT +# CERTIFIED" text, unrelated "implicit CERTIFIED" references). +test_wizard_doc_certified_paths_all_mention_commit_sha() { + local DOC="$REPO_ROOT/CLAUDE_CODE_SDLC_WIZARD.md" + if [ ! -f "$DOC" ]; then fail "CLAUDE_CODE_SDLC_WIZARD.md not found"; return; fi + local decision_lines total missing offenders + decision_lines=$(grep -nE "If CERTIFIED|CERTIFIED\? |→ CERTIFIED" "$DOC" 2>/dev/null || true) + total=$(echo "$decision_lines" | { grep -c . || true; }) + offenders=$(echo "$decision_lines" | { grep -v "commit_sha" || true; }) + missing=$(echo "$offenders" | { grep -c . || true; }) + if [ "$total" -gt 0 ] && [ "$missing" -eq 0 ]; then + pass "every CERTIFIED decision point in the wizard doc mentions commit_sha on the same line ($total checked)" + else + fail "$missing of $total CERTIFIED decision points in the wizard doc are missing a same-line commit_sha mention — at least one path silently skips the ROADMAP #437 staleness-fix instruction. Offending lines: +$offenders" + fi +} + +test_wizard_doc_certified_paths_all_mention_commit_sha + # ──────────────────────────────────────────── # Summary # ──────────────────────────────────────────── diff --git a/tests/test-hooks.sh b/tests/test-hooks.sh index 298c0691..36299a96 100755 --- a/tests/test-hooks.sh +++ b/tests/test-hooks.sh @@ -3745,34 +3745,85 @@ test_codex_gate_blocks_commit_without_review() { } test_codex_gate_allows_commit_with_certified_review() { - local tmpdir + local tmpdir head_sha tmpdir=$(mktemp -d) + (cd "$tmpdir" && git init -q && git commit -q --allow-empty -m init) > /dev/null 2>&1 + head_sha=$(cd "$tmpdir" && git rev-parse HEAD) mkdir -p "$tmpdir/.reviews" - printf '{"status":"CERTIFIED","score":9}' > "$tmpdir/.reviews/handoff.json" + printf '{"status":"CERTIFIED","score":9,"commit_sha":"%s"}' "$head_sha" > "$tmpdir/.reviews/handoff.json" local out exit_code out=$(printf '%s' '{"tool_input":{"command":"git commit -m \"fix: something\""}}' | (cd "$tmpdir" && "$HOOKS_DIR/codex-gate-check.sh") 2>&1) exit_code=$? rm -rf "$tmpdir" if [ "$exit_code" -eq 0 ] && [ -z "$out" ]; then - pass "codex gate allows (exit 0) commit with CERTIFIED review" + pass "codex gate allows (exit 0) commit with CERTIFIED review matching current HEAD" else - fail "codex gate should exit 0 silent with CERTIFIED review, got exit=$exit_code out: $out" + fail "codex gate should exit 0 silent with CERTIFIED review matching HEAD, got exit=$exit_code out: $out" fi } test_codex_gate_allows_commit_with_reviewed_status() { - local tmpdir + local tmpdir head_sha tmpdir=$(mktemp -d) + (cd "$tmpdir" && git init -q && git commit -q --allow-empty -m init) > /dev/null 2>&1 + head_sha=$(cd "$tmpdir" && git rev-parse HEAD) mkdir -p "$tmpdir/.reviews" - printf '{"status":"REVIEWED"}' > "$tmpdir/.reviews/handoff.json" + printf '{"status":"REVIEWED","commit_sha":"%s"}' "$head_sha" > "$tmpdir/.reviews/handoff.json" local out exit_code out=$(printf '%s' '{"tool_input":{"command":"git commit -m \"fix: something\""}}' | (cd "$tmpdir" && "$HOOKS_DIR/codex-gate-check.sh") 2>&1) exit_code=$? rm -rf "$tmpdir" if [ "$exit_code" -eq 0 ] && [ -z "$out" ]; then - pass "codex gate allows (exit 0) commit with REVIEWED status" + pass "codex gate allows (exit 0) commit with REVIEWED status matching current HEAD" + else + fail "codex gate should exit 0 silent with REVIEWED review matching HEAD, got exit=$exit_code out: $out" + fi +} + +# ROADMAP #437: a CERTIFIED/REVIEWED handoff.json has no freshness check — any +# number of new commits can land after certification and still sail through +# the gate on the same stale status string. Proven live in the v1.84.0 +# release: 2 real post-certification commits both passed the gate on a +# round-11 CERTIFIED handoff that never got re-issued. Fix: certification +# records commit_sha (HEAD at cert time); the gate compares it to current +# HEAD and treats a mismatch as stale. This allows exactly one commit after +# certification (HEAD still equals the recorded SHA at that commit's +# PreToolUse check) and blocks the next one until re-cert. +test_codex_gate_blocks_stale_certification_after_new_commit() { + local tmpdir cert_sha + tmpdir=$(mktemp -d) + (cd "$tmpdir" && git init -q && git commit -q --allow-empty -m init) > /dev/null 2>&1 + cert_sha=$(cd "$tmpdir" && git rev-parse HEAD) + mkdir -p "$tmpdir/.reviews" + printf '{"status":"CERTIFIED","commit_sha":"%s"}' "$cert_sha" > "$tmpdir/.reviews/handoff.json" + # A commit lands after certification (simulates the real v1.84.0 incident: + # a post-certification CI-shepherd fix committed without re-review). + (cd "$tmpdir" && git commit -q --allow-empty -m "post-cert fix") > /dev/null 2>&1 + local out exit_code + out=$(printf '%s' '{"tool_input":{"command":"git commit -m \"another fix\""}}' | (cd "$tmpdir" && "$HOOKS_DIR/codex-gate-check.sh") 2>&1) && exit_code=0 || exit_code=$? + rm -rf "$tmpdir" + if [ "$exit_code" -eq 2 ] && echo "$out" | grep -qi "stale"; then + pass "codex gate BLOCKS (exit 2) a stale certification after a new commit landed" + else + fail "codex gate should exit 2 + mention staleness once HEAD has moved past the certified commit_sha, got exit=$exit_code out: $out" + fi +} + +# Missing commit_sha (an old-format handoff.json from before this fix) is +# treated as stale, not silently allowed — no legacy-compat fallback. +test_codex_gate_blocks_missing_commit_sha_as_stale() { + local tmpdir + tmpdir=$(mktemp -d) + (cd "$tmpdir" && git init -q && git commit -q --allow-empty -m init) > /dev/null 2>&1 + mkdir -p "$tmpdir/.reviews" + printf '{"status":"CERTIFIED"}' > "$tmpdir/.reviews/handoff.json" + local out exit_code + out=$(printf '%s' '{"tool_input":{"command":"git commit -m \"fix: something\""}}' | (cd "$tmpdir" && "$HOOKS_DIR/codex-gate-check.sh") 2>&1) && exit_code=0 || exit_code=$? + rm -rf "$tmpdir" + if [ "$exit_code" -eq 2 ] && echo "$out" | grep -qi "stale"; then + pass "codex gate BLOCKS (exit 2) an old-format handoff.json with no commit_sha" else - fail "codex gate should exit 0 silent with REVIEWED review, got exit=$exit_code out: $out" + fail "codex gate should exit 2 + mention staleness when commit_sha is missing, got exit=$exit_code out: $out" fi } @@ -3864,6 +3915,8 @@ test_codex_gate_blocks_on_invalid_status test_codex_gate_skip_override test_codex_gate_blocks_commit_with_quote_before_git_commit test_codex_gate_silent_when_only_description_mentions_commit +test_codex_gate_blocks_stale_certification_after_new_commit +test_codex_gate_blocks_missing_commit_sha_as_stale # ---- codex-review-stop-check.sh tests ---- # Fable self-enforcement audit finding: a full SDLC workflow can complete — diff --git a/tests/test-self-update.sh b/tests/test-self-update.sh index d11c31bb..53f704d1 100755 --- a/tests/test-self-update.sh +++ b/tests/test-self-update.sh @@ -1455,8 +1455,14 @@ echo "" echo "--- #32 N-Reviewer CI Pipeline ---" # Test: Wizard has multi-reviewer guidance +# -E (extended regex) is required for the bare `?` in `multi.?review` to act +# as a quantifier — in basic regex (grep's default) an unescaped `?` matches +# a LITERAL question mark, so this always fell through to the other +# alternatives. Previously masked by an incidental "parallel ... reviewer" +# phrase collision elsewhere in the file (unrelated content, not real +# multi-reviewer guidance) — exposed once that unrelated text was trimmed. test_wizard_multi_reviewer() { - if grep -qi 'multi.?review\|N.?review\|parallel.*review\|multiple.*review' "$WIZARD"; then + if grep -qiE 'multi.?review|N.?review|parallel.*review|multiple.*review' "$WIZARD"; then pass "Wizard has multi-reviewer guidance" else fail "Wizard missing multi-reviewer CI guidance" @@ -1465,7 +1471,7 @@ test_wizard_multi_reviewer() { # Test: SDLC skill has multi-reviewer section test_skill_multi_reviewer() { - if grep -qi 'multi.?review\|N.?review\|parallel.*review\|multiple.*review' "$SKILL"; then + if grep -qiE 'multi.?review|N.?review|parallel.*review|multiple.*review' "$SKILL"; then pass "SDLC skill has multi-reviewer guidance" else fail "SDLC skill missing multi-reviewer guidance" @@ -1474,7 +1480,7 @@ test_skill_multi_reviewer() { # Test: Multi-reviewer guidance mentions per-reviewer response test_multi_reviewer_response_pattern() { - if grep -qi 'per.?review\|each.*review\|respond.*each\|address.*each' "$SKILL"; then + if grep -qiE 'per.?review|each.*review|respond.*each|address.*each' "$SKILL"; then pass "Multi-reviewer guidance has per-reviewer response pattern" else fail "Multi-reviewer should describe responding to each reviewer independently"