feat(hooks): classify gate results and stop counting the ones that reviewed nothing (0.8.0) - #21
Merged
Merged
Conversation
Second consumer of the PreToolUse timing gap: is_docs_only, not tree_hash. Commit 1950739 staged exactly one docs/**.md path, yet the hook emitted the Gate-B STOP, because the commit was issued as a single Bash call whose git add had not run when the hook read the index. Direction is safe here (a false positive), unlike the tree_hash consumer. No fix; counts toward the row's trigger. Gate B skipped: behaviourally trivial (a prose occurrence note in the backlog; no executable path changes) and the change cites no profiled story, so it keeps the pre-existing judgement-based skip. Quality battery green: shellcheck clean, hook tests all passed, invariant checks 123 assertions, version-bump check 36 assertions, claude plugin validate passed.
Design for the false-positive gate-pass defect: a classifier that reads the Codex result before counting. Four classes (success / failure / backgrounded / unrecognized), envelope-keyed, failing open with disclosure on an unrecognized envelope. Records the four settled decisions with their reasons, the two anchors and their disjointness, the backgrounding anchor's residual, and what the change does not do. Also files a follow-up todos entry for the /workflow-init preflight, kept out of the story's scope deliberately. Gate B N/A: no code and no prompt paths are staged. Gate B reviews a code diff, and the routing rule forbids reviewing a document with it -- the spec's review is Gate A (mcp__codex__exec), running next per CLAUDE.md section 5. Invariant checks re-run clean against the new files.
Eight Gate-A passes (17, 13, 14, 4, 7, 7, 6, 6 MAJOR), every one validated. Closed on judgement, NOT on a NO FINDINGS pass -- recorded as such in .context/codex-reviews/gate-a-spec-CLOSURE.md, which also carries the obligations the plan must check off. Design changes across the cycle: a five-class classifier (success, failure, backgrounded, no-result, unrecognized) with the class table as the single normative definition; classification collapsed from two representations to one, with the jq path verifying its re-encoded block occurs exactly once inside the located span so both parser environments read the same bytes or refuse together; no-result stated by its complement so unanticipated shapes fail closed; malformed outer JSON reclassified as unrouteable rather than a class, since the hook derives its own routing from that document. Story amended four times, each recorded inline with what it replaced: §2 at passes 2 and 5, §3 criteria at passes 4 and 8. §2 is now marked a summary deferring to the spec, so a future amendment has one target. todos.md: two trigger-gated rows for pre-existing conditions this design states as contracts rather than fixes -- unserialized counter mutation, and the hook's trust of a repository-controlled .context/. Gate B N/A: no code or prompt paths staged. Invariant checks re-run clean.
Slot names carry no cycle-unique component, and the section 5 protocol mandates deleting every target before each call -- correctly, since a surviving prior file is indistinguishable from a fresh one. So a second cycle in the same repo silently erases the first cycle's findings artifacts. Observed rather than theorised: this cycle's Gate-A pass-1 call deleted the 2026-07-26 profiles cycle's gate-a-spec-pass-1.md, which is unrecoverable because .context is git-ignored. Section 5 covers concurrent calls racing on one slot; it does not cover sequential cycles reusing them. Gate B skipped: behaviourally trivial (one backlog row; no executable path changes) and the change cites no profiled story, so it keeps the pre-existing judgement-based skip. Invariant checks green.
Eleven tasks, TDD steps throughout. Opens with the carried-obligations checklist from the Gate-A closure record: seven deferred implementation contracts (A1-A7), the shipped-doc scope discovered at spec pass 8 (B1-B3), and three accepted residuals that must survive unchanged (C1-C3). Each names the task that discharges it. Ordering is load-bearing: fixtures and test helpers land before any behaviour change, so the suite is green on both sides of the classifier; emit's status change lands before the markers that depend on it; the shipped-doc edits land after the behaviour they describe is real. Carries the watch-item forward: if this plan's Gate A concentrates on A5-A7, stop and surface rather than elaborate -- a simpler composition semantics is the named pressure valve and that trade is decided upstream. Gate B N/A: docs-only. Gate A on this plan runs next, per CLAUDE.md section 5.
… parser row The result-classification story builds a POSIX awk locator with proper string-state and backslash-parity handling. Once proven, that machine is the likely reuse path for the escaped-quote defect in input_field's fallback rather than inventing a second escape-aware scanner. Scopes stay separate: that story does not touch input_field. Gate B skipped: behaviourally trivial (a note on an existing backlog row; no executable path changes) and the change cites no profiled story, so it keeps the pre-existing judgement-based skip. Invariant checks green.
Spec §3.1 is amended to one locator: the jq path's in-span check needed byte offsets jq does not report, so the span came from the scan anyway, leaving jq as a component whose only reachable effect was downgrading an agreed result to unrecognized. The re-encode step, the uniqueness and in-span checks and §7.3's extraction-parity matrix are deleted with it rather than fenced off. The plan is narrowed to contracts, tables, message pairs and per-test oracles; the harness shell moves to Gate B as code that runs and lints. Gate A closed at pass 9 as a pre-authorized judgement exit. The execution notes record all 37 pass-9 findings as named Gate-B obligations, the three fixed at closure because they would have broken execution before Gate B could observe anything, and why nine passes never converged. Gate B: N/A — every staged path is docs/**.md prose.
…viewed nothing Validation evidence — docs/superpowers/stories/2026-07-30-failed-codex-call-counts-as-a-pass-story.md battery: full quality command green — shellcheck (6 files), the hook suite 467/467 with 0 failures and 1 named skip under BOTH shells, check-invariants 123/123 + ok, check-version-bump 36/36 + ok against BASE 0dc93d8, claude plugin validate --strict passed. "Both shells" is now literal: HOOK_SH selects the shell the HOOK runs under, and the sh and dash runs produce byte-identical row sets apart from the banner naming the interpreter. An earlier draft of this entry claimed sh-and-dash while every runner invoked the hook via `sh` — bash on macOS — so only ~7 explicit rows ever reached dash. Gate B pass 1 caught it; the runner was parameterized rather than the claim softened. check 1 (counterfactual for the classifier): the 0.8.0 suite and fixtures run against the pre-change hook materialized from BASE in a temp directory. New hook: exit 0, 0 FAIL. Base hook: exit 1, 144 FAIL (measured against the suite as it stood at that point; the suite has grown since with the pass-2 and pass-3 oracles, so the base-hook totals are a record of that run rather than of the current row set). Four named rows, verbatim from the base run and absent from the new one: FAIL - failure/review/default/jq preserves passCount/freshCount/fingerprint/passCountA FAIL - failure/review/default/jq writes no gate-pass state from clean FAIL - backgrounded/review/default/jq writes no gate-pass state from clean FAIL - class: fixture shape2-executor-timeout (got [success], want [failure]) The base hook classifies every captured failure, timeout and backgrounding notice as `success`. The invariant-1 regression is counterfactual too: FAIL - dash: exits 0 with a directory at the marker path (got 2) check 2 (counterfactual for the scan rewrites, each written BEFORE its fix): timed regression rows assert bounds and each failed first. FAIL - perf: 150KB payload classified within 2s (took 11s) -> now 0s FAIL - perf: anchor-prefixed near miss (no-newline) within 2s (took 6s) -> now 1s FAIL - perf: anchor-prefixed near miss (late-newline) within 2s (took 5s) -> now 0s Three quadratics, each measured: `readstr` accumulated byte-by-byte while `substr(s,j,1)` is O(len) per call in BWK awk; the `backgrounded` test ran `${b%%\n*}` on every block, which bash 3.2 evaluates by trying successively longer suffixes; and guarding that on the anchor prefix still left it running in full for a block that starts with the anchor and never completes the notice — the first timed fixture began with an envelope, took the guard's cheap path, and was blind to that branch entirely. check 3 (correctness, found at Gate-B pass 2 and fixed here): with a pending disclosure owed, an event name the hook could not route emitted a MALFORMED document — reproduced as `"hookEventName":"Bogus\\","`, where the trailing backslash escapes the closing quote and Claude Code receives invalid JSON. Two defects introduced by this diff combined: `flush_notes` ran unconditionally so an unroutable event reached `emit`, and `$event` was interpolated raw beside two escaped fields (the jq branch was never affected — `--arg` encodes it). Both fixed. Five oracle rows across both runners, written before the fix and failing first; the jq-free rows are the ones that caught the malformed document, so testing one runner would have missed it. check 5 (correctness, found at Gate-B pass 3 and fixed here): the tool-mapping parser accepted any plausible token, and the mapped cases are tested BEFORE the native `Bash` and `Skill` cases — the only two names outside `mcp__codex__*` that the hooks matcher delivers at all. So `reviewTool=Bash` made a `git commit` COUNT a Gate-B pass instead of resetting the cycle, and `execTool=Skill` counted a skill invocation as a Gate-A pass. Both are false checkmarks in recorded state, reachable from a plausible typo, and this diff is what introduced the contract they contradicted. Reproduced, fixed by requiring mapped names to lie in the namespace, and pinned by three rows including one proving a legitimate in-namespace mapping still counts. check 6 (the 4096-unit notice bound, both sides): the bound that made the anchor-prefixed near miss cheap also NARROWS the backgrounded class — a notice whose segment falls past it counts instead of being discarded. Now tested with a genuine notice immediately inside the cutoff (-> backgrounded) and immediately outside it (-> unrecognized), and stated in spec §4 rather than only in the implementation comment. check 4 (spec conformance): spec §6 fixes the composed order as disclosure first, then the per-occurrence message. The implementation appended instead and the golden froze the inversion. Implementation moved to match the approved spec; goldens updated. WHAT IS NOT BOUNDED, and this entry says so rather than repeating a claim that has now needed narrowing three times. TWO paths, not one: (a) `skipval` walks containers one character at a time, so a large VALID sibling container before `tool_response` costs 3.2 s at 200 KB and 11.5 s at 400 KB; (b) the record accumulator `s = s $0 "\n"` rebuilds the whole input once per input line, so a newline-rich payload is quadratic in line count independently of (a) — 0.35 s at 4k lines, 2.69 s at 16k. Naming only (a) was itself a finding at pass 3: every timed row uses a large single-record payload and never reaches (b). Only the 1 Mi-unit ceiling stops that path, and a payload just under it still costs tens of seconds. Gate-B pass 2 found this after pass 1 found the first two; the earlier drafts of this entry claimed "the curve is now linear", generalizing from the one shape they had measured. That claim is withdrawn: the measured shapes are bounded, the container walk is not, and no timed row covers a branch its own fixture does not reach. verification (named), part 1 — replay of the captured payloads through the changed hook, 9/9 rows, every row guarded on fixture-read status, sed status AND effect, the routed tool name, the exact class and an explicit per-row expected value, with an exact row-count check. The guards were negative-checked in three directions before the green result was accepted: a wrong expected class, a missing fixture, and a wrong row count each fail the step. verification (named), part 2 — story criterion 10 was AMENDED, human-confirmed 2026-08-02, and the amendment is in the story itself with what it gives up stated. The live probe re-run is dropped, not deferred: it requires instrumenting the installed hook, which serves every concurrent Claude Code session here, and no isolated profile can be driven from this session. awk portability — NOT established against this tree at the time of this commit. The local battery runs one awk; CI's ubuntu runner uses mawk, and CI now runs the suite twice (sh and dash) so that run is also the only place the locator meets a second awk implementation. That result binds at PR time and this commit body does not carry it. 467/467 local is shell coverage, not awk-implementation coverage. rollback — verified by inspection, deliberately not executed, per an explicit decision recorded 2026-08-02. The plugin cache IS version-keyed (cache/dev-workflow-kit/dev-workflow/<version>/) and installed_plugins.json selects the active one. 0.7.1 is PRESENT and its hook bytes are bee47e59f9b9d682cbfc68a5db0351bf183a27fb, matching the repo bytes at 0dc93d8 — the newest commit whose manifest still contains 0.7.1 — so that resolution method is confirmed here empirically rather than assumed. This repo has NO tags, so "released bytes" remains a repository-bytes claim. NOT verified, and named rather than implied: the operator switch path. 0.8.0 has never been installed, `claude plugin marketplace add` takes no version, and editing installed_plugins.json is undocumented and untested. Rollback is the CURRENT state of this machine, not a procedure anyone has run — which also means every gate-pass counter in this cycle was maintained by 0.7.1, the hook that counts failed and backgrounded calls. The counters are worth nothing here; each pass was judged on its findings file, per CLAUDE.md section 5. What a rollback costs is in the CHANGELOG, not only what it leaves behind: it restores the original defect and the dash special-builtin exit, and it does not undo the three diagnostic markers already in .context/. gate B — closed clean-with-dispositions at pass 9 of 9. Pass 9 returned ZERO new Blocker/Major on both branches; the spec branch was dispositions-only. Four findings are dismissed with a named home in todos.md and a trigger, listed in .context/codex-reviews/gate-b-0.8.0-dispositions.md: the two quadratic locator paths (skipval's container walk and the record accumulator), A5 marker-matrix breadth, A6 composition breadth, and the hardening ledger's missing supersession convention. One one-clause docs precision fix was applied after pass 9 and was NOT re-reviewed; it changed no behaviour, prompt or test. Both pass-9 branches independently reproduced this evidence from their own checkout rather than reading it. What the cycle caught, each fixed with an oracle written before the fix: a denial of service (150 KB of ordinary review text cost 10.9 s in one synchronous hook invocation, now 0.46 s, three separate quadratics); invalid emitted JSON, where an unroutable event plus one unescaped field produced "hookEventName":"Bogus\","; a false-checkmark configuration hijack, where reviewTool=Bash made a git commit COUNT a Gate-B pass instead of resetting the cycle; and a composition order that contradicted spec section 6 while its own golden froze the inversion. Five overclaims in this evidence entry were caught and narrowed across the nine passes.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThe plugin updates Codex gate processing to classify hook results before counting passes, buffer diagnostic output, validate tool mappings, and preserve state on output failures. It adds shell-portable tests, captured fixtures, release metadata, and operational documentation for version 0.8.0. ChangesCodex gate result classification
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant ClaudeCode
participant codex-gate.sh
participant GateState
participant HookOutput
ClaudeCode->>codex-gate.sh: PostToolUse payload
codex-gate.sh->>codex-gate.sh: Extract and classify tool_response
codex-gate.sh->>GateState: Count, discard, or record disclosure
codex-gate.sh->>HookOutput: Flush buffered message
HookOutput-->>ClaudeCode: Hook response
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThe PR classifies routed Codex gate results before mutating pass state, preventing recognized failures, backgrounded calls, and empty results from counting as completed reviews.
Confidence Score: 5/5The PR appears safe to merge; no concrete, unacknowledged defect remains in the reviewed changes. Recognized incomplete review outcomes no longer mutate gate-pass state, uncertain outcomes retain the documented fail-open behavior, and the new routing, emission, marker, mapping, and shell-portability paths have focused regression coverage. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[PostToolUse payload] --> B{Routed gate tool?}
B -- No --> C[No gate classification]
B -- Yes --> D[Locate first usable tool_response text]
D --> E{Classification}
E -- success --> F[Count pass and store applicable state]
E -- unrecognized --> G[Queue uncertainty disclosure]
G --> F
E -- failure --> H[Do not mutate pass state]
E -- backgrounded --> H
E -- no-result --> H
F --> I[Flush buffered output]
H --> I
I --> J{Emission succeeded?}
J -- Yes --> K[Persist applicable one-shot markers]
J -- No --> L[Preserve disclosure debt best-effort]
Reviews (1): Last reviewed commit: "feat(hooks): classify gate results and s..." | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
plugins/dev-workflow/hooks/codex-gate.sh (1)
456-473: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAdd a timed regression row for the newline-rich accumulator path.
The record accumulator
s = s $0 "\n"rebuilds the whole buffer once per input line, so cost is quadratic in line count.docs/superpowers/plans/2026-08-01-gate-pass-result-classification.mdnames this as remaining quadratic path (b) and reports 0.35 s at 4k lines, 0.75 s at 8k, and 2.69 s at 16k. Only the 1 Mi-unit ceiling bounds it.The perf suite does not cover it.
plugins/dev-workflow/hooks/codex-gate.test.shsection 41 builds single-record payloads, and the pretty-printed row at lines 1397–1403 is a handful of lines. The plan already records that generalizing from one measured shape has been wrong every time. A timed row over a many-line payload would pin this path the same way section 41b pins the anchor-prefixed near miss.This is a suggestion, not a blocker: the residual is documented in the hook comment and the plan.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/dev-workflow/hooks/codex-gate.sh` around lines 456 - 473, Add a timed regression case to section 41 of codex-gate.test.sh that feeds a newline-rich, many-line single-record payload through the accumulator path and verifies its classification and runtime threshold. Use the documented 4k/8k/16k-line shape or equivalent, keeping the test focused on s = s $0 "\n" rather than the existing single-record and pretty-printed cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@AGENTS.md`:
- Around line 245-248: Update the prerequisites section in AGENTS.md to document
the required dash executable alongside shellcheck and claude, since the quality
and test commands invoke the harness with dash. Keep the existing command
battery unchanged.
In `@docs/superpowers/plans/2026-08-01-gate-pass-result-classification.md`:
- Around line 88-103: Reconcile the count in the “Before committing” description
with the command block: replace “four script runs” with wording that accurately
identifies the five invocations shown, or explicitly state “four distinct
scripts” while noting that codex-gate.test.sh runs under both sh and dash. Keep
the verbatim command block unchanged and ensure the sentence does not claim a
count unsupported by it.
In `@docs/superpowers/specs/2026-07-20-codex-file-first-output.md`:
- Around line 203-204: Update the stale citation in the retained 0.5.1 analysis
by removing the `codex-gate.sh:361` line-number reference and identifying the
`PostToolUse` counting branch by name instead. Keep the surrounding statement
about counters and tool names unchanged.
In
`@docs/superpowers/stories/2026-07-30-failed-codex-call-counts-as-a-pass-story.md`:
- Line 118: Update the jq parity checklist criterion in the story to explicitly
scope identical behavior to routable, well-formed payloads, rather than all
payloads. Preserve the existing comparison while excluding malformed outer
documents whose routing differs between jq and the grep fallback.
In `@README.md`:
- Around line 131-132: Update the codex-gate.tools documentation to remove the
unsupported claim that typos cannot quietly unhook a gate. State instead that
malformed, out-of-namespace, or reserved mappings fall back to the default
exec/review names, unless the implementation also validates configured tool
availability.
---
Nitpick comments:
In `@plugins/dev-workflow/hooks/codex-gate.sh`:
- Around line 456-473: Add a timed regression case to section 41 of
codex-gate.test.sh that feeds a newline-rich, many-line single-record payload
through the accumulator path and verifies its classification and runtime
threshold. Use the documented 4k/8k/16k-line shape or equivalent, keeping the
test focused on s = s $0 "\n" rather than the existing single-record and
pretty-printed cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ff25122a-46f8-411f-9d4c-ef734a0c7148
📒 Files selected for processing (31)
.github/workflows/ci.ymlAGENTS.mdCLAUDE.mdREADME.mddocs/architecture.mddocs/superpowers/plans/2026-08-01-gate-pass-result-classification-execution-notes.mddocs/superpowers/plans/2026-08-01-gate-pass-result-classification.mddocs/superpowers/specs/2026-07-20-codex-file-first-output.mddocs/superpowers/specs/2026-07-31-failed-codex-call-counts-as-a-pass-design.mddocs/superpowers/stories/2026-07-30-failed-codex-call-counts-as-a-pass-story.mdplugins/dev-workflow/.claude-plugin/plugin.jsonplugins/dev-workflow/CHANGELOG.mdplugins/dev-workflow/commands/workflow-init.mdplugins/dev-workflow/hooks/codex-gate.shplugins/dev-workflow/hooks/codex-gate.test.shplugins/dev-workflow/hooks/fixtures/README.mdplugins/dev-workflow/hooks/fixtures/collision-failure-quotes-true.jsonplugins/dev-workflow/hooks/fixtures/collision-failure-quotes-true.response.jsonplugins/dev-workflow/hooks/fixtures/collision-success-quotes-both.jsonplugins/dev-workflow/hooks/fixtures/collision-success-quotes-both.response.jsonplugins/dev-workflow/hooks/fixtures/shape0-success-review.jsonplugins/dev-workflow/hooks/fixtures/shape0-success-review.response.jsonplugins/dev-workflow/hooks/fixtures/shape0-success.jsonplugins/dev-workflow/hooks/fixtures/shape0-success.response.jsonplugins/dev-workflow/hooks/fixtures/shape1-fast-fail.jsonplugins/dev-workflow/hooks/fixtures/shape1-fast-fail.response.jsonplugins/dev-workflow/hooks/fixtures/shape2-executor-timeout.jsonplugins/dev-workflow/hooks/fixtures/shape2-executor-timeout.response.jsonplugins/dev-workflow/hooks/fixtures/shape3-backgrounding-notice.jsonplugins/dev-workflow/hooks/fixtures/shape3-backgrounding-notice.response.jsontodos.md
All five were true findings, validated by me rather than by a finding-triage subagent: this PR edits instruction-bearing paths (CLAUDE.md, AGENTS.md, plugins/, commands/), and process-pr-review's precheck forbids delegating triage there — a subagent loads the whole CLAUDE.md hierarchy, so it would review under rules the diff is rewriting. Four were introduced by the 0.8.0 change itself: - AGENTS.md listed only shellcheck and the claude CLI as prerequisites while the battery invokes `HOOK_SH=dash dash` twice. dash is now documented, with why it is named rather than pinned (system shell, not a fetched tool) and why the run matters — dropping it is how the dash-only special-builtin exit shipped. - The plan said "four script runs" for a block holding five invocations over four distinct scripts; the dash run was added and the count left stale, which is the dropped-row failure that sentence warns about. - The 2026-07-20 spec cited codex-gate.sh:361 for the PostToolUse counting branch. That was correct at the merge base — the 0.8.0 hook rewrite moved it to FAILURE_MSG. Now cited by branch name so it cannot drift again. - The story's jq parity criterion read as unqualified, but routability itself diverges for a malformed outer document, which this cycle's own sweep wrote into the design. Scoped to routable payloads, naming field() as the owner. The fifth was pre-existing but local to a row this change already edited: README claimed "a typo can't quietly unhook a gate". Verified false by running both parser guards against mcp__codex__exce — it passes the charset check and the mcp__codex__?* namespace check, so the mapping is honoured. Validation evidence — docs/superpowers/stories/2026-07-30-failed-codex-call-counts-as-a-pass-story.md battery: full quality command green — shellcheck (6 files), the hook suite 467/467 with 0 failures and 1 named skip under both sh and dash, invariants 123/123, version-bump 36/36 against the parent commit, claude plugin validate --strict. No plugins/** path is touched, so no version bump is due. check: the counterfactual for each correction is the statement it replaces — each was verified false against the artifact it describes before being rewritten, and the codex-gate.sh:361 case was checked against the merge base to establish that this change is what falsified it rather than pre-existing drift. verification (named): Gate B, 2 passes, closed clean. Pass 1's quality branch was clean; its spec branch found one Major — the README replacement said an in-namespace typo means the gate "counts nothing", turning a conditional into a categorical, since honouring the mapping only replaces the expected name and the gate would count if such a tool were invoked. Fixed to state the condition. Pass 2 returned NO FINDINGS on both branches, with the quality branch independently re-running the full battery from its own checkout. Not done in this cycle, and owed: harden-finding on these five. Both matching ledger classes — docs-drift and unverified-enforcement-claim — already sit at five occurrences, so a sixth escalates a rung rather than adding a prose row. Deferred deliberately; it changes the repository and needs its own gate.
Owner
Author
Per-head review count — diagnostic, blocks nothingRecorded per
So the final head was not re-reviewed by CodeRabbit, while its check is green — the Greptile posted a summary with no defect claims (5/5). Cursor Bugbot is disabled for this Merging on that basis: the required check passes, both opportunistic bots are |
dsnger
added a commit
that referenced
this pull request
Aug 5, 2026
# Validation evidence — hardening round 0.8.1
Story: docs/superpowers/stories/2026-08-03-hardening-round-0-8-0-and-pr-21-story.md
Branch: harden-0-8-0-and-pr-21
## battery
exit status: 0
hook suite under sh: "all passed" (no assertion total, as expected)
hook suite under dash: "all passed" (no assertion total, as expected)
invariants suite: "all passed (123 assertions)"
version-bump suite: "all passed (36 assertions)"
check-invariants.sh: "invariant checks: ok"
check-version-bump.sh main: "version-bump check: ok"
claude plugin validate --strict: "✔ Validation passed"
derived hook-suite assertion count (the suite prints none) — suite exit status captured
separately from the count, so a partial count from a dead suite is not recorded:
HOOK_SH=sh -> status 0, count 467
HOOK_SH=dash -> status 0, count 467
## check (named verification), with its counterfactual
§5.2 applied to the four cases in spec §10 — each must FAIL on the wiring half:
1. $EVIDENCE dry run -> FAIL: the falsifying observation is the commands erroring on an unset variable, and the wiring supplies its own input by assigning $EVIDENCE itself.
2. single-shell regression -> FAIL: the falsifying observation is a non-zero exit of the `:` builtin, which occurs under dash; the check runs under macOS sh, where the defect cannot appear.
3. timed regression row -> FAIL: the falsifying observation is a newline-rich sub-ceiling payload costing far more than measured; single-record fixtures never reach the record accumulator's quadratic branch.
4. dash release evidence -> FAIL: the falsifying observation is a hook-level dash failure; the hook ran under /bin/sh, so the defect cannot appear in that run.
§5.4 applied to "It bounds the scan, not memory" — must FAIL on exhaustiveness: FAIL: it names the two axes it was checked against (scan, memory) and never states whether that list is exhaustive, which is exactly what the appended clause requires.
counterfactual: all four were reviewed during the 0.8.0 cycle under the §5 text as it
read before this change and each was accepted by at least one review looking for exactly
this; one (the single-shell test) reached the released artifact.
## prompt conformance
workflow-init.md (invariant 11 surface) — all 12 items:
1 Target model named — PASS on the naming half: line 9 names Claude via Claude Code and the edit does not touch it. Second half, stated rather than glossed: the check against that model's current prompting page was made against this repo's distilled reading of those pages, docs/prompt-standards.md § "Verified model-specific notes (read 2026-07-04 — re-verify per Revalidation)", not against a live fetch in this cycle. Revalidation's stated trigger is a model generation change, which has not occurred since that read. Two of those notes bear on these additions and both hold: "literal instruction following — state the scope" (the lens sentence enumerates what to name rather than asking generally), and "coverage first, filter later" (unchanged; the additions ask for more coverage, never less). A live re-read was not performed, and this line says so instead of implying one.
2 Success criteria explicit — PASS: each inserted sentence states the artifact the reviewer must produce (the named list plus its grep; the named observation plus the wiring judgement; the settled mechanical facts).
3 Stop conditions defined — PASS: the sentences sit inside existing loops that already carry their stop rules (clean final pass, stop-and-surface); none introduces a new terminal state.
4 Output format with example — PASS: the surrounding finding-line format and the literal NO FINDINGS example are unchanged and are what these questions feed.
5 Structured sections — PASS: each sentence lands inside its existing paragraph; no section boundary moves.
6 Rules carry their why — PASS after a pass-3 fix. The lens sentence carries "because asked as an open question alone this lens missed three such statements in one cycle"; the counterfactual carries "reports success because of how it was wired, not because the thing it checks succeeded". The sweep sentence originally carried a reason only for its subordinate constraint (inspect rather than run, "since a command quoted in a spec may be destructive or an intentional failure") and none for the constraint itself; Gate-B pass 3's quality branch found that, and the sentence now carries "because a read pass spends expensive judgement on what a parser settles in seconds and misses it anyway". This line previously recorded a PASS on the strength of the subordinate clause, which is the item-11 shape this round hardens — noted rather than quietly corrected.
7 No contradictions with CLAUDE.md / AGENTS.md — PASS: the cross-finding conflict check returned none, and mirror parity confirms the template says what CLAUDE.md says.
8 Token-lean — PASS under the exception item 11 states by name: an inline template cannot point at a file the downstream project does not have, so it restates and the copies are kept in sync (invariant 8). Mirror parity is the sync evidence.
9 Positive instructions — PASS, no exception needed: all three are positive imperatives (Name what…, Name the observation…, settle mechanically…).
10 Diagnostic states name their causes — PASS: the edit introduces no failure-state report, so it adds no unresolvable symptom.
11 Enforcement claims name their mechanism — PASS: none of the three claims anything is enforced or caught; they are asks, and the CHANGELOG entry and ledger rows record "none of the three is a check" explicitly rather than leaving the reader to infer coverage.
12 Calibrated emphasis — PASS: bold falls only on the imperative clause, matching the §5 discipline-gate emphasis this template already carries as a deliberate exception.
CLAUDE.md, AGENTS.md (not on invariant 11's list) — items 6,7,8,9,11,12:
6 PASS — each of the three §5 sentences and the AGENTS.md clause carries its reason ("because an enumeration read as complete guarantees the axes it omits").
7 PASS — conflict check verdict: none; the four sites are distinct and no pair weakens another.
8 PASS — CLAUDE.md is the authoritative copy; only the template restates, under invariant 8.
9 PASS by the exemption item 9 states in its own text, cited rather than assumed: §5.4's clause continues an existing prohibition whose subject IS the prohibition ("delete any part of the sentence that outruns it"), and restating it positively would lose the boundary it draws. The three §5 sentences are positive imperatives and need no exemption.
11 PASS — no sentence claims enforcement; each ledger row states STILL INSTRUCTION-BACKED and names what is not done (nothing runs the grep, nothing tests whether a check could have failed, nothing runs or records the sweep).
12 PASS — emphasis limited to the imperative clause, consistent with §5's existing gate language.
five split stories — in-spirit brief review: PASS. Each names its success criteria as acceptance criteria; each carries an explicit stop condition as criterion 1 (design pauses for Daniel's confirmation of the profile); each states what is established versus assumed — the four trigger stories carry a kept/moved/dropped inventory of their source row's conditions, and the unprofiled headers say why no profile was written rather than fabricating one. The fifth (passive metrics over the ledger) was added at Gate-B pass 1 and reviewed on the same terms.
## mirror parity
each §5 sentence appears once in CLAUDE.md and once in workflow-init.md: CONFIRMED — all three matched exactly once per file under whitespace normalization (identical word sequences; wrapping differs only), each under the corresponding heading.
## gate-b fix record
Pass 1 (both branches, 1 Major each, same subject): the four appended ledger rows take the ledger from 18 to 22 and cross P8's stated 20-row trigger, which the round had not recorded. Validated against the counts (18 at main, 22 after) and against spec D3, whose scope is every trigger THIS ROUND fires and whose rejected alternative excludes only triggers already fired in the backlog. Applied: P8's row marked TRIGGER FIRED, a fifth split story written, and the statements the fix falsified corrected. Found by the standing lens this round adds: the diff changed a count, and a statement keyed to that count sat in a file the diff did not touch.
Pass 2 (spec 6 findings, quality 1; 6 Major, 1 Minor): the pass-1 fix corrected the headline counts and missed the sites that carry the same claim in other words — spec §2's summary, §8's inventory sentence, completion sentence and numbered list, the cited story's acceptance criteria and amendment log, and the plan's Task 5 and Task 6 bodies, step counts and must-be-true checks. All corrected here, and the P8 story is now a numbered item in §8 with the same detail as items 1–4. The quality branch found item 1 of the prompt-standards checklist recorded on its naming half only; the record above now states what was and was not checked. The pass-1 fix record originally claimed every falsified statement had been corrected — pass 2 disproved that claim, which is why this entry names the sites rather than asserting completeness.
Pass 3 (spec 1 Major, quality 2 Major; one raised by both branches): Task 5's heading, file count and classification sentence still said "three" though Step 3b makes it four — the same count-keyed drift a third time, now inside the amendment itself; and the Gate-A sweep sentence carried a reason for its subordinate constraint and none for the constraint itself, which prompt-standards item 6 requires. Both fixed. The sweep fix changed a sentence quoted verbatim in four other places, so ledger row D's ref, spec §5.3's block quote, the plan's Task 1 Step 3 block and the plan's copy of row D were all updated in the same commit; mirror parity re-verified after the rewording (one occurrence per file, identical word sequence). Row D was amended rather than superseded because it is being authored in this cycle and has not landed — the append-only rule protects the committed record, and the absence of a sanctioned move for a landed row is what the ledger-supersession story exists to settle.
Pass 4: NO FINDINGS on both branches, validated on disk (two lines each, `NO FINDINGS` then `END OF FINDINGS (0 total)`). Four passes against a floor of three, final pass clean, every fix amended into the WIP commit before the next call so each review read a range containing it.
## cross-finding conflict check
Task 0 Step 2 verdict: no conflict. The four hardenings touch four distinct sites and act at different moments on different objects — the lens on a Gate-B diff, the counterfactual on a Gate-B evidence claim, the sweep before a Gate-A read pass, the Don't on prose about mechanisms. No pair asks a reviewer for contradictory behaviour, and §5.3's "inspect rather than run" limits the sweep without exempting anything from §5.1 or §5.2.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this fixes
The gate hook advanced its pass counters — and, for Gate B, stored a content fingerprint — on Codex calls that reviewed nothing. Observed directly:
passCountAmoved 3 → 4 on a call that ran 272 ms and never started a review. The pinnedmcp-codex-devcatches its own errors and returns them as ordinary results carryingsuccess: false, so Claude Code sees a successful tool call; nothing in the hook looked further. A satisfied count therefore overstated the passes held, and Gate B's stored fingerprint described content nobody read — a false ✓ in the hook's own recorded state, the direction invariant 2 names as dangerous.What it does now
Five classes (design §3.3, the single normative definition):
successandunrecognizedcount and store a fingerprint;failure,backgroundedandno-resultdo neither. An escape-awareawklocator finds the firsttextelement oftool_response— neverjq, which would reserialize away the very escape variants the matcher reads. The failure marker must be the envelope's immediately-first property, so a reordered envelope degrades tounrecognizedrather than to a wrong verdict.Fail-open where locating is uncertain, fail-closed where it is certain. An unambiguous "nothing usable here" is
no-resultand does not count. Not being able to determine anything counts, with a once-per-workspace disclosure that the count was made without inspection.Also fixes an invariant-1 violation that had shipped: with a directory at a marker path the old
: > "$file"form exits 2 underdashwhile exiting 0 under macOSsh.Four defects this review cycle caught before they shipped
Each was reproduced first, then fixed with a regression oracle written before the fix and observed failing:
substr(s,j,1)is O(len) per call in BWKawk),${b%%\n*}on every block (bash 3.2 evaluates%%by trying successively longer suffixes), and that expansion still running in full for a block that starts with the notice anchor. Now 0.46 s."hookEventName":"Bogus\","— the backslash escapes the closing quote. Routing now gates the flush, and the field is escaped like its neighbours.reviewTool=Bashmade agit commitcount a Gate-B pass instead of resetting the cycle. Reachable from a plausible typo.Review
Gate B: 9 passes, closed clean-with-dispositions. Pass 9 returned zero new Blocker/Major on both branches; the spec branch was dispositions-only. Both branches independently reproduced the evidence from their own checkout rather than reading it.
Four findings are dismissed with a named
todos.mdhome and a trigger — the two remaining quadratic locator paths, A5 marker-matrix breadth, A6 composition breadth, and the hardening ledger's missing supersession convention. Each is a design call or a coverage build-out rather than a correction; the full reasoning is in.context/codex-reviews/gate-b-0.8.0-dispositions.md(git-ignored, so it lives with the working copy). One one-clause docs precision fix landed after pass 9 and was not re-reviewed; it changed no behaviour, prompt or test.Verification
shand underdash.HOOK_SHnow selects the shell the hook runs under; previously every runner hardcodedsh, so adashinvocation only exercised the harness. CI runs the suite twice.claude plugin validate --strict.Not established:
awk-implementation portability. The local battery runs oneawk; this PR's CI run is the first time the locator meetsmawk, which is why the branch exists.Accepted residuals
C1–C4 in the CHANGELOG, named rather than left to be discovered — the backgrounding anchor recognizes today's harness wording only; an
unrecognizedcall whose disclosure is neither delivered nor persisted counts silently; counter mutation is unserialized; concurrent marker writes can duplicate or lose a disclosure.Rolling back to 0.7.1 restores the original defect and the
dashspecial-builtin exit. It does not remove the three diagnostic markers already in.context/.Summary by CodeRabbit
New Features
Documentation
Tests