feat(hooks): classify gate results and stop counting the ones that reviewed nothing (0.8.0)

.github/workflows/ci.yml

@@ -70,8 +70,16 @@ jobs:
           docker run --rm -v "$PWD:/mnt" -w /mnt koalaman/shellcheck:v0.11.0 \
             --shell=sh scripts/check-version-bump.test.sh
 
-      - name: Hook state-machine tests
-        run: sh plugins/dev-workflow/hooks/codex-gate.test.sh
+      # Two runs, because the hook has to be correct under both shells and the runner's
+      # /bin/sh is dash while a contributor's may be bash. HOOK_SH selects the shell the
+      # HOOK runs under; without it, running the file under dash would only exercise the
+      # harness under dash. Ubuntu also gives mawk here, where a developer machine gives
+      # BWK or GNU awk — this step is the only place the locator meets a second awk.
+      - name: Hook state-machine tests (sh)
+        run: HOOK_SH=sh sh plugins/dev-workflow/hooks/codex-gate.test.sh
+
+      - name: Hook state-machine tests (dash)
+        run: HOOK_SH=dash dash plugins/dev-workflow/hooks/codex-gate.test.sh
 
       # Invariants 5 and 6 plus two prompt-conformance checks, mechanically, and BOTH
       # checkers' regression suites. The

AGENTS.md

@@ -45,6 +45,7 @@ plugins/dev-workflow/
   agents/finding-triage.md        # read-only PR-comment checker (convention-loaded)
   commands/{workflow-init,process-pr-review}.md
   hooks/{hooks.json,codex-gate.sh,codex-gate.test.sh}
+  hooks/fixtures/                 # captured PostToolUse payloads the suite replays
   examples/                       # ships, but never scaffolded — one stack's answers
 docs/
   architecture.md                 # layout + the two non-obvious design decisions
@@ -241,17 +242,22 @@ Every command below was run in this session and observed to exit 0.
 
 | Role | Command |
 |---|---|
-| quality (the whole battery — what CI runs) | `shellcheck --shell=sh plugins/dev-workflow/hooks/codex-gate.sh && shellcheck --shell=sh --exclude=SC2015 plugins/dev-workflow/hooks/codex-gate.test.sh && shellcheck --shell=sh scripts/check-invariants.sh && shellcheck --shell=sh --exclude=SC2015 scripts/check-invariants.test.sh && shellcheck --shell=sh scripts/check-version-bump.sh && shellcheck --shell=sh scripts/check-version-bump.test.sh && sh plugins/dev-workflow/hooks/codex-gate.test.sh && sh scripts/check-invariants.test.sh && sh scripts/check-invariants.sh && sh scripts/check-version-bump.test.sh && sh scripts/check-version-bump.sh main && claude plugin validate . --strict` |
+| quality (the whole battery — what CI runs) | `shellcheck --shell=sh plugins/dev-workflow/hooks/codex-gate.sh && shellcheck --shell=sh --exclude=SC2015 plugins/dev-workflow/hooks/codex-gate.test.sh && shellcheck --shell=sh scripts/check-invariants.sh && shellcheck --shell=sh --exclude=SC2015 scripts/check-invariants.test.sh && shellcheck --shell=sh scripts/check-version-bump.sh && shellcheck --shell=sh scripts/check-version-bump.test.sh && HOOK_SH=sh sh plugins/dev-workflow/hooks/codex-gate.test.sh && HOOK_SH=dash dash plugins/dev-workflow/hooks/codex-gate.test.sh && sh scripts/check-invariants.test.sh && sh scripts/check-invariants.sh && sh scripts/check-version-bump.test.sh && sh scripts/check-version-bump.sh main && claude plugin validate . --strict` |
 | typecheck | n/a — no typed sources (shell + markdown) |
 | lint | `shellcheck --shell=sh plugins/dev-workflow/hooks/codex-gate.sh && shellcheck --shell=sh --exclude=SC2015 plugins/dev-workflow/hooks/codex-gate.test.sh && shellcheck --shell=sh scripts/check-invariants.sh && shellcheck --shell=sh --exclude=SC2015 scripts/check-invariants.test.sh && shellcheck --shell=sh scripts/check-version-bump.sh && shellcheck --shell=sh scripts/check-version-bump.test.sh` |
-| test | `sh plugins/dev-workflow/hooks/codex-gate.test.sh` |
+| test | `HOOK_SH=sh sh plugins/dev-workflow/hooks/codex-gate.test.sh && HOOK_SH=dash dash plugins/dev-workflow/hooks/codex-gate.test.sh` — two runs; `HOOK_SH` selects the shell the HOOK runs under, and without it a dash invocation only exercises the harness |
 | invariant checks (5 pinning, 6 manifest, prompt conformance) | `sh scripts/check-invariants.test.sh && sh scripts/check-invariants.sh` |
 | invariant check (12 version bump) | `sh scripts/check-version-bump.test.sh && sh scripts/check-version-bump.sh main` |
 | build | n/a — nothing is compiled or bundled |
 
 **Prerequisites and pinning.** The quality command needs `shellcheck` (0.11.0 locally;
-CI runs the pinned image `koalaman/shellcheck:v0.11.0`) and the `claude` CLI (CI pins
-`@anthropic-ai/claude-code@2.1.207`). Bump both deliberately, per invariant 5.
+CI runs the pinned image `koalaman/shellcheck:v0.11.0`), the `claude` CLI (CI pins
+`@anthropic-ai/claude-code@2.1.207`), and **`dash`** — the battery runs the hook suite
+twice, once with the hook under `sh` and once under `dash`, because the hook has to be
+correct under both and Ubuntu's `/bin/sh` IS dash. Bump the first two deliberately, per
+invariant 5; `dash` is addressed by name because it is the system shell, not a pinned
+tool. Without it the second run cannot start, and dropping that run is what let a
+`dash`-only defect ship once already.
 
 **The `--exclude=SC2015` on the test file** is a single-code exclusion, not a blanket
 disable: every other shellcheck rule still applies to that file. Its hits are all

CLAUDE.md

@@ -165,17 +165,27 @@ write or just returns its prior summary is not established; if it returns the su
 that was the attempt. Spent and still incomplete → STOP and surface, naming which check
 failed.
 
-**What this does not do.** The hook counts on `PostToolUse`, keyed on tool name, and
-never sees the file. Claude Code fires `PostToolUse` after a *successful* call and routes
-a failed one to `PostToolUseFailure`, which the plugin registers no handler for — but do
-not infer from that which failures escape counting: the pinned `mcp-codex-dev` catches
-its own errors, executor timeouts and aborts included, and returns them as a normal
-result carrying `success: false` rather than throwing or setting `isError`
-(`dist/tools/codex-review.js`). A failed review therefore looks like a successful tool
-call and increments the counter. So does a call that returns and then fails validation.
-The rule that follows is the simple one: **discount every incomplete pass regardless of
-what the counter says** — a "satisfied" count can overstate the passes you actually hold,
-and reasoning about which failure took which event path will get it wrong. Nothing checks the terminator mechanically; this is
+**What this does not do.** The hook counts on `PostToolUse`, keyed on tool name **and on
+the result envelope**, and still never sees the file. Claude Code fires `PostToolUse`
+after a *successful* call and routes a failed one to `PostToolUseFailure`, which the
+plugin registers no handler for — but do not infer from that which failures escape
+counting: the pinned `mcp-codex-dev` catches its own errors, executor timeouts and aborts
+included, and returns them as a normal result carrying `success: false` rather than
+throwing or setting `isError` (`dist/tools/codex-review.js`). A failed review therefore
+still looks like a successful *tool call* — but as of 0.8.0 the hook reads the result of
+gate calls it can route, and withholds the count for three **recognized** shapes: an
+envelope whose **first** property is `success: false`, the harness backgrounding notice
+**in the wording it currently uses**, and a result from which no usable text can be
+obtained. Every other routed gate call counts, including any located text the hook cannot
+interpret — a reordered envelope, a reworded notice, an unknown third-party shape — which
+counts **with** a disclosure that is attempted and normally shown once per workspace, but
+can be lost or repeated when its marker cannot be persisted. So does a call that returns
+and then fails validation. The counter is therefore closer to the truth than it was and
+is still not evidence: a "satisfied" count can still overstate the passes you actually
+hold, and reasoning about which failure took which event path will get it wrong. The rule
+that follows is the simple one: **discount every incomplete pass regardless of what the
+counter says**, because classification cannot see whether the findings file was written.
+Nothing checks the terminator mechanically; this is
 instruction-backed by design, and a recurring truncation incident is the trigger to build
 the checker, not a reason to build it now. Detection is conditional: it catches an absent
 or malformed terminator, a count mismatch and a missing branch file *in the artifact you
@@ -395,11 +405,14 @@ like the rest of §5; the detection is a reader comparing the pass against the s
 - **Timeout / abort:** a codex call that dies at the MCP tool-call timeout is retried
   once before surfacing to the user, and that retry *is* the single shared recovery
   attempt above — not a second one. An abort is an incomplete pass, so treat it as one:
-  it may already have moved the hook's counter (the pinned server returns its own
-  timeouts as ordinary results), and it may have left a partial or stale target file, so
-  delete the targets and confirm them gone before retrying, then validate the result like
-  any other pass. Counter and workspace state persist in `.context/`; the *pass* does
-  not.
+  it may have left a partial or stale target file, so delete the targets and confirm them
+  gone before retrying, then validate the result like any other pass. Whether it moved the
+  hook's counter depends on the shape it returned and on which hook version is installed:
+  as of 0.8.0 a recognized failure envelope, the recognized backgrounding notice and a
+  result yielding no usable text are all withheld from the count, while a reordered,
+  reworded or unrecognized shape still counts fail-open. Do not reason from the counter
+  either way — an incomplete pass is discounted whatever it says. Counter and workspace
+  state persist in `.context/`; the *pass* does not.
 
 ## 6. Context Canary
 

README.md

@@ -56,7 +56,10 @@ old version until you restart it or run `/reload-plugins`.
 - **Codex** — the reviewer behind both gates. Needs the **Codex CLI, authenticated with
   an OpenAI account** — a real external dependency, not just the `.mcp.json` entry
   `/workflow-init` writes for you. It also needs **a Codex MCP server that exposes
-  `exec` and `review`** — the gates and their pass counters key on those two tool names.
+  `exec` and `review`** — the gates key on those two tool names, and the pass counters
+  additionally skip routed calls whose result the hook reads as failed, backgrounded, or
+  yielding no usable text; a result it can read but not interpret still counts, and
+  normally says so once.
   Use the `mcp-codex-dev` server `/workflow-init` pins, which has both. The official
   `codex mcp-server` is a *different* server exposing a single `codex` tool, which can't
   be attributed to Gate A (reviews text) or Gate B (reviews a diff): with it connected,
@@ -73,6 +76,37 @@ old version until you restart it or run `/reload-plugins`.
   findings out of the response.
 - **`gh`** — optional; only `/dev-workflow:process-pr-review` uses it.
 
+**2b. Set `CLAUDE_CODE_MCP_AUTO_BACKGROUND_MS`** — strongly recommended, and the primary
+defence against a gate call being auto-backgrounded. Claude Code moves a long MCP call to
+the background at a threshold (120 s by default) and fires `PostToolUse` *at that moment*,
+carrying the harness's own notice instead of any Codex result; the eventual real
+completion fires no second `PostToolUse`, so the hook never sees the review's outcome.
+
+Requires **Claude Code 2.1.212 or newer**. Set it in the environment Claude Code is
+**launched from** — it is read at process start, so exporting it inside a tool shell
+leaves the running session unchanged and you must restart Claude Code. Use `0` to disable
+auto-backgrounding, or a positive value that **exceeds** your longest expected gate call;
+a positive value shorter than the call still backgrounds it.
+
+```sh
+export CLAUDE_CODE_MCP_AUTO_BACKGROUND_MS=0   # or e.g. 900000, exceeding your longest gate call
+```
+
+**Without it, both outcomes are possible, and only one is safe.** While the harness notice
+keeps the wording the hook recognizes, the backgrounded pass is *discarded* — correct, but
+**re-running it is not simply a retry**: the original call may still be running and can still
+write into the findings slot afterwards, leaving a correctly terminated file from the wrong
+run that no downstream check can detect. Stop that task by the id in the tool result — or
+await it if the result carries none — and delete the findings slot first. The hook's own
+backgrounding message carries that procedure; follow it there rather than from memory. If that harness prose ever changes, the hook can no longer recognize
+the notice and the call is *counted* instead, with a disclosure that says the count was
+made without inspection. That second outcome is why this setting is the defence rather
+than the hook: the hook fails safe against today's wording, not against every future one.
+
+(`CLAUDE_CODE_AUTO_BACKGROUND_TIMEOUT_MS` appears in the 2.1.220 string table but was
+never exercised here, so it is deliberately not documented — naming the wrong one of two
+similar variables is this project's own docs-drift class.)
+
 **3. Run `/dev-workflow:workflow-init` in each project.** It verifies the rest and tells
 you what's missing — git repo, superpowers, Codex (not configured / not loaded / ok),
 `gh`, `AGENTS.md`, stack — before writing a single file. Then follow
@@ -94,8 +128,8 @@ plugin is installed once per machine; every other repo you open hears nothing fr
 Per-workspace knobs, all files under `.context/`:
 
 | `codex-gate.floor` | a positive integer; moves the 3-passes-per-gate floor. |
-| `codex-gate.off` | silences the reminders; state keeps tracking, so re-enabling is accurate. |
-| `codex-gate.tools` | `execTool=<name>` and/or `reviewTool=<name>` — counts a Codex server whose tools aren't named `exec`/`review`, and only worth it if that server really does separate text-review from diff-review; aiming both gates at one general-purpose tool moves the counters while neither gate means what it says. Unparseable lines are ignored, so a typo can't quietly unhook a gate. |
+| `codex-gate.off` | silences the reminders; classification and state tracking keep running, so re-enabling lands on counters carrying the same semantics as gate-on — which is not the same as evidence that a review happened. |
+| `codex-gate.tools` | `execTool=<name>` and/or `reviewTool=<name>` — counts a Codex server whose tools aren't named `exec`/`review`, and only worth it if that server really does separate text-review from diff-review; aiming both gates at one general-purpose tool moves the counters while neither gate means what it says. Each mapped name must itself lie in `mcp__codex__*`: the hook's `hooks.json` matcher is `^(Bash\|Skill\|mcp__codex__.*)$`, so an out-of-namespace name is either never delivered (the mapping looks applied and does nothing) or, for the reserved names `Bash`/`Skill`, hijacks a lifecycle event; the hook refuses both — register the server as `codex` to place its tools there. Unparseable, out-of-namespace and reserved (`Bash`/`Skill`) lines are ignored, and the gate keeps its default `exec`/`review` name. A typo **inside** the namespace — `mcp__codex__exce` — is still honoured: the hook does not check that a mapped tool exists, so the gate now counts that name and nothing else. Whether it ever counts depends on whether a tool by that name is actually invoked; for a typo, normally never. |
 
 **Without Codex**, `/workflow-init` degrades honestly instead of scaffolding gates that
 can't run: it silences the hook and marks CLAUDE.md §5 `INACTIVE` with the re-enable

docs/architecture.md

@@ -24,6 +24,7 @@ plugins/dev-workflow/
   agents/finding-triage.md
   commands/{workflow-init,process-pr-review}.md
   hooks/{hooks.json,codex-gate.sh,codex-gate.test.sh}
+  hooks/fixtures/                 # captured PostToolUse payloads the suite replays
   examples/                       # ships, but never scaffolded — one stack's answers
 docs/{getting-started,coding-workflow,prompt-standards,architecture}.md
 docs/{hardening-log,hardening-taxonomy,pr-review-bots}.md