diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6c8b3ce..b0499d4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -81,7 +81,7 @@ jobs: - 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 + # Invariants 5 and 6 plus three prompt-conformance checks, mechanically, and BOTH # checkers' regression suites. The # invariant-12 checker itself is not here — it needs a PR base and runs in the # step below, so naming this step "version bump" would show a green version-bump diff --git a/AGENTS.md b/AGENTS.md index ed904f6..9702f1a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -170,10 +170,12 @@ reader can judge whether it still holds. 11. **Prompt changes pass `docs/prompt-standards.md`** — all 12 checklist items, for any skill, command, agent definition, hook message, or scaffolded template. The prompts are the product, and **no comprehensive mechanical checker exists for them**: - review is the gate. Two narrow checks in `scripts/check-invariants.sh` cover one + review is the gate. Three narrow checks in `scripts/check-invariants.sh` cover one spelling each — a `Target model:` line naming exactly one recognized model in files - claiming conformance, and a prose checklist-count claim matching the checklist — and - they are a floor, not coverage. Every other item is judged by a reader. + claiming conformance, a prose checklist-count claim matching the checklist, and the + finding-severity vocabulary stated as a closed set in both prompt copies (in the + scaffolded template's own section, in the command file) — and they are a floor, not + coverage. Every other item is judged by a reader. ## Don'ts diff --git a/CLAUDE.md b/CLAUDE.md index fbfc833..9c1c1f9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -95,7 +95,9 @@ Append to the gate prompt: > `gate-a-spec-pass-

`, `gate-a-plan-pass-

`, or `gate-b--pass-

`. > > One finding per line in the format above; escape a literal pipe inside a field as -> `\|`. Every line before the terminator is exactly one finding line — no blank lines, +> `\|`. +> Severity is one of exactly: BLOCKER | MAJOR | MINOR | NIT — no other token. +> Every line before the terminator is exactly one finding line — no blank lines, > headings, prose or wrapped continuations. End the file with a final line reading > exactly `END OF FINDINGS ( total)`, `` being the number of finding lines. A > clean pass is the single body line `NO FINDINGS` with `END OF FINDINGS (0 total)`. @@ -149,6 +151,19 @@ file, an `INCOMPLETE` reply — is an **INCOMPLETE pass**, which is not a review act on the partial list, don't count it toward the 3-pass floor, and don't read "no Blocker/Major visible" as clean. +**Reader:** the severity field is taken by splitting the line on **unescaped** pipes and +trimming the ASCII whitespace the finding format puts either side of each separator; a field +that is empty or all whitespace is a **structural** failure, so the line is INCOMPLETE and is +never normalized. Otherwise the field is matched **case-insensitively** against the four tokens +first — `Minor`, `minor` and `MINOR` are all `MINOR`, because `CLAUDE.md` Mechanics +legitimately spells them in Title case and a model copying that spelling is doing as it was +told, not drifting. A field that matches no token case-insensitively, and is non-empty, is +read as `MAJOR`. Every **structural** failure stays INCOMPLETE — a malformed +line, a wrong field count, an empty severity field, a bad terminator, a count mismatch. Only +the severity token is tolerated, and only when everything else about the line is right. +(PR #23's Gate-B pass 3 returned all four findings at `IMPORTANT`; discarding that pass over a +token would have thrown away four real findings.) + **Recovery: one attempt per pass**, shared across timeout, an `INCOMPLETE` reply and failed validation — the Mechanics timeout-retry rule widened, not a second budget beside it, since two budgets let a pass alternate between them indefinitely. The attempt is a @@ -415,6 +430,66 @@ like the rest of §5; the detection is a reader comparing the pass against the s amend replaces the WIP message wholesale, so an entry written only into the WIP body is destroyed exactly when the cycle closes. The final commit body is the durable record; a PR shows commit messages, so there is no second home to keep in sync. + + **On squash-merge, copy every evidence entry and every human-exception record in the squash range into the squash body — the squash commit is the only body the merge carries into `main`'s history, so anything left behind is unreachable from it.** + + **Recording a human exception.** Where a human decides that something **no applicable rule + required** was nonetheless worth skipping — an optional check this environment cannot run, a + review someone asked for and then stood down, a courtesy step — that decision goes in the + closing commit body: + + ``` + Human exception: · + Not done: + Accepted because: + ``` + + **Which commit:** an ungated change records it in that commit; a Gate-A cycle in the spec or + plan commit; a Gate-B cycle in the WIP commit, restated by the closing amend. Several records + accumulate; order means nothing. + + **A decision made after its commit closed** — during PR review, say — goes in whichever of + these exists: the next commit on the branch, the squash body, or a follow-up commit after the + merge. If none does — the branch is closed, unmerged, and heading for an ordinary or rebase + merge — **add a commit for it.** An empty commit carrying only the record is a legitimate + destination: it changes no content, so it raises no review obligation. A record with nowhere + to go would otherwise be a record that does not exist. + + **Do not expect silence from the gate hook, and do not read a reminder as a gate + reopening.** It is advisory, so it never blocks the commit attempt. What is exempt is the + **empty diff**, which `git show --stat` confirms — never a reminder that merely looks the + same on a commit carrying content. + + Copy every record into the squash body alongside the evidence entry (Mechanics, + squash-merge carry). **Nothing performs that carry and nothing checks afterwards that it + happened** — it is on whoever prepares the merge. If two copies of one record disagree, that + is a copying error: stop and fix it rather than picking one. + + **Scope, and it is narrow. This form supplies no permission.** It records a decision that + was already the human's to make about something genuinely optional. It is **never** the answer to a + below-floor pass, an unclean final pass, a `STOP and surface`, a Gate-A or Gate-B + obligation, or a profile-derived evidence requirement — and more generally **it authorizes + nothing that any mandatory rule in this file or in `AGENTS.md` requires.** Those have their + own terminal actions and this paragraph changes none of them: on a STOP you still stop, and + neither a human's assent nor this record lets an agent close or continue a cycle. + + **"Mandatory" is not limited to this file.** A rule in `AGENTS.md`, a project doc, CI, a + branch policy or the platform is equally out of reach — under **Wait for**, + `docs/pr-review-bots.md` requires a bot review unless an explicit recorded human decision + permits proceeding without it, and this form is not that decision. If you are reaching for it to get past something mandatory, the answer + is no — take the operational route or stop. + + **Nor is it for things that were simply never owed.** An absent review from a bot routed + **opportunistically** blocks nothing and needs no exception and no record; + `docs/pr-review-bots.md` says so deliberately, and writing one anyway would rebuild the + per-quiet-bot ceremony that routing removed. Record a decision, not a non-event. + + **What the record is worth.** It is an **unverified assertion**, and reads as one: nothing + checks that the handle belongs to whoever decided, that a human was asked, or that the + reason is honest. A reader of history learns that *the commit claims* a human chose, what + it says was skipped, and why — no more. It supports no claim of authorization or review, + and satisfies no evidence obligation. It exists because an exception nobody wrote down is + invisible, not because writing it down makes it sound. - **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: diff --git a/README.md b/README.md index 3877c51..5e8dc14 100644 --- a/README.md +++ b/README.md @@ -157,7 +157,7 @@ honest gap ([reasoning](docs/coding-workflow.md#adapting-it-to-another-project)) CI ([`.github/workflows/ci.yml`](.github/workflows/ci.yml)) runs four checks on every PR and push to main: `shellcheck --shell=sh` over all three executables and their test files, the hook's test suite, -[`scripts/check-invariants.sh`](scripts/check-invariants.sh) (invariants 5 and 6, plus two prompt-conformance checks) plus +[`scripts/check-invariants.sh`](scripts/check-invariants.sh) (invariants 5 and 6, plus three prompt-conformance checks) plus both checkers' regression suites, and `claude plugin validate . --strict`. A fifth check runs **on pull requests only**: diff --git a/docs/coding-workflow.md b/docs/coding-workflow.md index c69fcab..c3b520c 100644 --- a/docs/coding-workflow.md +++ b/docs/coding-workflow.md @@ -183,6 +183,110 @@ real check is noise, and noise trains people to ignore the gate that will eventu matter. Being explicitly gateless is a known gap you can close; being implicitly self-reviewed is an unknown one you cannot. +**Choosing which model reviews — and switching when one runs dry.** The invariant names a +model **family**, not a vendor: a pass satisfies a gate when the reviewer is a different family +from the implementer. That leaves the vendor free, which matters because the common failure is +not a bad review, it is **no review** — a quota limit hit mid-cycle, with work blocked and the +gate unsatisfiable. An alternative reviewer is the operational answer, and it is worth wiring +up *before* you need it. + +**This section describes the mechanism, not a choice of model.** It names no models and no +recommended default deliberately: model availability, pricing and quality move faster than a +document does, and a list here would be stale before it was useful. A gateway such as +OpenRouter publishes a live catalog — read that for what exists. What follows is how the +plumbing works, so that picking a model is a one-string edit rather than a research project. + +**Adding a gateway** to the Codex CLI is one provider block naming the base URL and the +environment variable holding the key: + +```toml +[model_providers.""] +name = "" +base_url = "" +env_key = "" +wire_api = "responses" +``` + +(The table key is quoted because `` is a placeholder: TOML bare keys allow only +`A-Za-z0-9_-`, so the block would not parse with the angle brackets unquoted. Substitute a bare +id and the quotes become optional.) + +Adding it changes nothing by itself; `model_provider` still decides who answers. Check +`wire_api` against your CLI version, and check it with `codex doctor` rather than at the first +call. Measured on **codex-cli 0.147.0**: `wire_api = "chat"` makes the whole config fail to +load — `codex doctor` reports `config could not be loaded` — while `"responses"` loads clean. +An arbitrary value fails identically, so `"chat"` is not specially diagnosed, it is simply no +longer accepted. That is the good failure, surfacing at load rather than silently; the version +is named because it is the one this was run against, not because earlier or later ones are +known to differ. + +**Four switch surfaces, each a one-string edit**, in the order `mcp-codex-dev` resolves them +(later overrides earlier): + +| Surface | Scope | Use it when | +|---|---|---| +| The config the CLI reads (`~/.codex/config.toml`) — its `model` and `model_provider` | every call, all repos | you are changing the standing default | +| `~/.mcp/mcp-codex-dev/config.json` | every repo, this MCP server only | the gate calls need a different model from what the CLI uses by hand | +| `/.mcp/mcp-codex-dev.config.json` | one repository | a project needs a different reviewer from your default | +| `CODEX_DEV_MODEL` / `CODEX_DEV_REVIEW_MODEL` | current environment — all tools / **Gate B only** | switching per-shell; the `REVIEW` variant changes the code reviewer without touching Gate A | + +**One catch worth knowing before you reach for a profile:** `mcp-codex-dev` passes `--model` +and **never `--profile`**, so a CLI profile does not reach the gate calls at all. Because only +the model name is passed, `model_provider` has to be active in the config the CLI reads — a +profile cannot carry the switch. Profiles remain useful for driving the CLI by hand. + +**One config, both providers.** Keep the native provider and the gateway entry in the same +config the CLI reads: the top level names no `model_provider`, so the native default answers, +and the appended gateway block is inert until a top-level `model_provider = ""` line +selects it. The switch is that one line — inserted in the top-level block, since a key placed +after any `[table]` header belongs to that table — and the revert is deleting it; the default +returns to the native provider at the next call. Do not point `CODEX_HOME` at a second config +directory to get isolation: the CLI's login state lives beside the config it reads, and a +redirected directory strands the existing login. + +Three timing facts decide where an edit lands and when it takes effect. The CLI is spawned +per call and reads its config at start, so the provider switch needs no restart of anything. +`mcp-codex-dev` resolves its *model* chain once per resolved project root and caches it until +the server restarts — the launch root at startup, any other root on its first call — so a model +edit must be in place before the root is first loaded, or be made under a project root the +server has not seen yet. And the key named by `env_key` must be present in the +environment the MCP server was launched with — the CLI inherits it from the server, the +server from its parent at spawn — so an export made after launch reaches nothing until that +parent restarts. + +Provider selection cannot travel per-repo: the `mcp-codex-dev` config schema has no +provider key and strips unknown keys, so the per-repo file picks a *model* while +the *provider* stays global to the config the CLI reads. + +**Record which model took each pass.** The gate's value comes from independence, so a pass is +only interpretable if you know who gave it. Put the model the pass *ran under* in the pass record +beside the finding count, never one recalled from memory or copied from a document. That is not +always what the config says now: per the timing facts above the model chain is resolved once per +project root and cached until the server restarts — the launch root at startup, any other root on +its first call — so a model edit landed after a root was loaded leaves the configured value and +the running one disagreeing until restart, and the configured value is the wrong one. A root the +server has not loaded yet is the exception: there the edit does take effect. Where they +can disagree, confirm by probing: call `mcp__codex__health` with the same `workingDirectory` you pass to the +gate call. It reports the server's cached per-root resolution, which is what the gate call for +that root uses — the point being that reading the config file yourself is exactly the thing that +can disagree. **Read the per-tool field, not the top-level one:** the server resolves a gate's +model as `tools..model ?? model`, so Gate B is `checks.config.effective.tools.review.model` +falling back to `checks.config.effective.model`, and Gate A the same with `tools.exec.model`. The +top-level field alone is the wrong answer precisely where the override documented above is in +use, since `CODEX_DEV_REVIEW_MODEL` is stored at `tools.review.model`. If neither level names a +model the probe establishes nothing — the CLI then picks its own default, and the only honest +record is to set an explicit model or record the model as undetermined. Record the result beside +the finding count in the pass record: the commit body's evidence entry, or the slot's +dispositions file. This is +bookkeeping, not enforcement: nothing checks it, and a wrong entry looks exactly like a right +one. + +**The one permanent rule here is family-level.** No model from the **implementer's own family** +satisfies a gate — whatever the vendor, whatever the gateway, whatever the transport. Routing +an Anthropic model through a third-party gateway while Claude is implementing does not make it +independent; it is the same family behind a different bill. Everything else in this section is +configuration and will change. That sentence will not. + ### The self-hardening ledger The system learns from its own findings through an **append-only ledger**. Every diff --git a/docs/hardening-log.md b/docs/hardening-log.md index 093a112..628cb64 100644 --- a/docs/hardening-log.md +++ b/docs/hardening-log.md @@ -72,6 +72,9 @@ correction, mechanical readers do not, and nothing checks the difference. **Superseded rows:** - 2026-08-12 · supersedes 2026-07-20 `truncated-tool-output-read-as-complete` · its `ref` states that an incomplete pass still increments the counter, including a failed review returning `{success: false}`; true when written, and it no longer holds for every pass · CLAUDE.md §5, the paragraph opening `` **What this does not do.** The hook counts on `PostToolUse` `` +- 2026-08-17 · supersedes 2026-08-16 `docs-drift` "both told the reader to record the reviewer model by reading" · its hardening names `checks.config.effective.model` as the probe field, which is the wrong field wherever a per-tool override is set: the pinned server resolves a gate's model as `tools..model ?? model` (`dist/config/config.js`, `getToolConfig`), and the `CODEX_DEV_REVIEW_MODEL` surface the same row cites is stored at `tools.review.model` — so the named probe reproduced the misattribution the row claims to close, and it was wrong when written · docs/coding-workflow.md, the paragraph opening `**Record which model took each pass.**`, and the matching rule in docs/sparring-briefing.md: read `tools.review.model` for Gate B or `tools.exec.model` for Gate A, falling back to the top-level `model`, and record the model as undetermined where neither level names one +- 2026-08-17 · supersedes 2026-08-16 `docs-drift` "both told the reader to record the reviewer model by reading" · its hardening names `checks.config.effective.model` as the probe field, and that was wrong when written; the entry immediately above, correcting it the same day, is itself wrong in two ways — it attributes a `CODEX_DEV_REVIEW_MODEL` citation to that row, which the row does not contain, and it restates the corrected rule instead of citing where the rule lives, which this ledger's own format forbids because a restatement is a second copy that can drift · docs/coding-workflow.md, the paragraph opening `**Record which model took each pass.**`; docs/sparring-briefing.md, the reviewer bullet opening `**The reviewer is whatever actually ran**` +- 2026-08-17 · supersedes 2026-08-16 `docs-drift` "both told the reader to record the reviewer model by reading" · its hardening names a top-level probe field, which was wrong when written · docs/coding-workflow.md, the paragraph opening `**Record which model took each pass.**`; docs/sparring-briefing.md, the reviewer bullet opening `**The reviewer is whatever actually ran**` Columns: `date` (YYYY-MM-DD), `fingerprint` (canonical class), `finding` (short, escape `\|`, one line), `source` (gate-a|gate-b|bot|manual), @@ -102,3 +105,5 @@ escape `\|`, one line), `source` (gate-a|gate-b|bot|manual), | 2026-08-04 | unverified-enforcement-claim | fifth occurrence: "It bounds the **scan**, not memory" named one uncovered axis and left the reader to infer the others were covered — the bound was a size bound, the work was quadratic in size, and the runtime it was read as bounding held only for the shapes that had been measured | gate-a | major | 1 prose | AGENTS.md Don't "Never describe what a gate proves without checking what it actually compares", closing rule extended: where a sentence says what a mechanism does NOT cover, name the axes it was checked against and state whether that list is exhaustive. PRIOR ROW: 2026-07-19 (1 prose), the same Don't, whose operative instruction covers "every sentence about a gate" — INSIDE it, so this is a regression, and the amendment closes a gap the comparison-focused wording left open. GUARD, exactly: the exhaustiveness statement. The motivating sentence names two axes and gives each a verdict, so a rule asking only for the axes checked would approve it; declaring the list exhaustive or not is what it never does. NOT LOGGED HERE: PR #21's C4 and C5 fall inside the same Don't but need nothing it does not already say, so no repair exists to name — they are parked in todos.md as a compliance recurrence with their own trigger. STILL INSTRUCTION-BACKED: no checker reads a coverage claim | | 2026-08-04 | verification-masks-failure | second occurrence, four cases in one cycle: a dry run that defined `$EVIDENCE` itself, proving the git mechanics and never that the plan defines the variable; a regression test that only ever ran under macOS `sh` while the defect it guards appears under `dash`; timed rows whose single-record fixtures never reach the record accumulator's quadratic path; and release evidence claiming `dash` coverage from a run that executed the harness under `dash` and the hook under `/bin/sh` | gate-b | major | P std | CLAUDE.md §5 Profiles counterfactual + the same block in the workflow-init inline template: name the observation that would exist if the claim were false, and confirm the wiring could have produced it. GUARD, exactly: the second half — a check that supplies its own input, runs where the defect cannot appear, or uses a fixture that never reaches the branch it covers. Each of the four cases fails on it. PRIOR ROW: 2026-07-20 (1 prose), whose ref states its own scope — "nothing checks new plans for the same shape" — so all four are OUTSIDE it and this is the fitting rung rather than an escalation on the count. RUNG P NOT 1: the artifact is a prompt; the rung follows the artifact, not the count. SOURCE: two gate-a cases and two gate-b, so the tie-break applies — the triggering case is the `dash` release evidence, which is gate-b. STILL INSTRUCTION-BACKED: nothing tests whether a check could have failed | | 2026-08-04 | mechanical-check-skipped-before-review | NEW CLASS, minted this change: eight read-only Gate-A passes over one plan missed eight defects that thirteen machine checks then found in a single sweep, including a rollback that would have byte-verified against the wrong hook | manual | major | P std | CLAUDE.md §5 Gate-A pass procedure + the same block in the workflow-init inline template: before each read pass, settle mechanically what the artifact asserts and a machine can decide without side effects — cited paths, quoted passages, stated counts, the syntax of standalone fenced blocks — because a read pass spends expensive judgement on what a parser settles in seconds and misses it anyway, inspecting quoted commands rather than running them, since a command quoted in a spec may be destructive or an intentional failure. Class added to docs/hardening-taxonomy.md in this same change, with its boundary against verification-masks-failure stated: there a check ran and could not fail; here the cheap check never ran at all. NO PRIOR ROW — this is the first occurrence. STILL INSTRUCTION-BACKED: nothing runs the sweep, records that it ran, or checks what it settled | +| 2026-08-16 | unverified-enforcement-claim | sixth occurrence, and the first where the standing rung FIRED AND HELD: PR #24's Greptile P1 found §5 claiming an empty record-only commit "does not reopen any gate" — true of the obligation, false of the shipped hook, which routes an empty staged-path list through the ordinary Gate-B decision. Four consecutive Gate-B rounds then reproduced the class *inside the correction itself*: an enumeration read as complete ("STOP or below-floor"), a single-cause label for a three-way condition ("stale-fingerprint STOP"), a landing guarantee inferred from the hook's exit status ("so the commit lands"), and a categorical "the hook fires" that is false on the non-adopted and `codex-gate.off` silent paths | bot | major | P std | NO NEW RUNG — `docs/prompt-standards.md`'s existing rule resolved it: "when a claim about a mechanism needs a fourth correction, delete the claim rather than refine it a fifth time". It was applied at exactly the fourth correction and the state-machine narration was deleted from all five restatements, leaving the obligation ("an empty diff raises no review obligation"), the advisory fact, and the `git show --stat` test. This is the row that says the rung WORKS: it was minted after an incident where four corrections each introduced a subtler version of the same claim, and here it stopped the identical spiral at the same round rather than a fifth. What it does NOT do is prevent the first three corrections — it bounds the spiral, it does not catch the original overclaim, and nothing mechanical decides when a claim "needs a fourth correction"; the reviewer counts. Also recorded: deleting the narration made the two §5 copies' edited regions byte-identical, removing the invariant-citation divergence that had itself produced a finding | +| 2026-08-16 | docs-drift | sixth occurrence: PR #24 (CodeRabbit) — `docs/coding-workflow.md` and `docs/sparring-briefing.md` both told the reader to record the reviewer model by reading "the configured value at that moment", while the same section's own timing facts said `mcp-codex-dev` caches its model chain per project root. Two sentences in one document disagreeing, and the wrong one was the actionable instruction: after any model edit the config names a model the running server is not using, so a pass record built from it misstates who reviewed — which is the only thing that makes reviewer-family independence checkable | bot | major | 1 prose | Both documents now name the model the pass *ran under* as the thing recorded, state the cache boundary exactly (launch root at startup, any other root on its first call, cached until restart, and an unseen root is the exception where a post-startup edit does take effect), and name a DETERMINISTIC probe rather than "check the config": `mcp__codex__health` with the same `workingDirectory` as the gate call, reading `checks.config.effective.model`, which is the cached per-root resolution the gate call itself uses. NOT ESCALATED past the 2026-07-26 `2 lint` row: that check guards prose count claims and cannot reach a stale mechanism description, the over-escalation those rows warn about. NO DETERMINISTIC RUNG EXISTS for this: nothing can tell that a sentence about a cache is stale, and the probe is a recipe a human runs, not a check — it raises the floor by making the right value obtainable, and does not close the class. Sibling row this same date under `unverified-enforcement-claim` covers the empty-commit half of the same PR review | diff --git a/docs/sparring-briefing.md b/docs/sparring-briefing.md index 4a13190..fb9ec83 100644 --- a/docs/sparring-briefing.md +++ b/docs/sparring-briefing.md @@ -42,6 +42,26 @@ role's authors repeatedly — treat that as the system working, not as an affron workflow (intake → gates → PR). Do not design around the gates, and do not treat a satisfied human as a substitute for a clean pass. Why: cross-model independence is the core invariant, and you are not the other model. +- **The reviewer is whatever actually ran — never a model you name, and not + always what is configured.** When the primary reviewer is out of quota there is + a configured fallback, and you read its value rather than carrying one: from + the config the Codex CLI reads, from + `~/.mcp/mcp-codex-dev/config.json`, from `/.mcp/mcp-codex-dev.config.json`, + or from `CODEX_DEV_MODEL` / `CODEX_DEV_REVIEW_MODEL` — the latter for Gate B + alone. Configuration is the expected input, not the authority: the model chain is + resolved once per project root and cached until the server restarts, so an edit + landed after that root was loaded leaves the two disagreeing until restart. Record + the model the pass actually *ran under*, and where the two can differ, probe: + `mcp__codex__health` with the same `workingDirectory` as the gate call reports the + server's cached resolution for that root. Read the per-tool field — a gate's model + is `tools..model ?? model`, so Gate B is `checks.config.effective.tools.review.model` + with `checks.config.effective.model` as fallback, Gate A the same with `tools.exec.model`. + The top-level field alone is wrong exactly where an override is in use. If neither + level names a model, the probe settles nothing and the record says undetermined. + Why: a model name written into a briefing is stale the week after, and a stale + name in a pass record makes the record say something untrue about who reviewed. + The one thing that does not change: **no model from the implementer's own + family satisfies a gate**, whatever the vendor or gateway. - **One decision at a time, with its price.** When the human must choose, present the options, name what each costs, recommend one, and mark the recommendation as yours. Why: unpriced recommendations get followed, not diff --git a/docs/superpowers/plans/2026-08-15-reviewer-availability-salvage.md b/docs/superpowers/plans/2026-08-15-reviewer-availability-salvage.md new file mode 100644 index 0000000..f6cd2d6 --- /dev/null +++ b/docs/superpowers/plans/2026-08-15-reviewer-availability-salvage.md @@ -0,0 +1,686 @@ +# Reviewer-availability salvage — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Ship the salvage from the closed reviewer-availability story — a human-exception +record form, a closed severity enum with a tolerant reader, and a squash-carry sentence — into +`CLAUDE.md` §5 and its `/workflow-init` mirror, with one mechanical assertion behind it. + +**Architecture:** Four prose blocks, written identically into two files, plus one check in the +existing `scripts/check-invariants.sh`. No new file, no CI change, no new executable surface. + +**Tech Stack:** Markdown prompts; POSIX `sh` + `awk` for the checker; `shellcheck` and the +existing regression suite. + +**Spec:** `docs/superpowers/specs/2026-08-14-reviewer-availability-fallback-design.md` — +§4 is the path table, §2.1 and §3 carry the shipped text verbatim, §5.2 the assertion. +**Story:** `docs/superpowers/stories/2026-08-13-reviewer-availability-fallback-story.md` — +profile: risk `standard` · security `none` · `battery+check`. Read it fresh; never from here. + +## Global Constraints + +- **The two prompt copies are `CLAUDE.md` and `plugins/dev-workflow/commands/workflow-init.md`.** + Every prose edit below goes into **both**, identically. The mirror lives inside the + ````markdown fence at lines 192–629 and is flush-left; §5 is at `CLAUDE.md:65` and + `workflow-init.md:257`. +- **Parity is a named verification, not a check.** After each prose task, diff the region you + edited between the two copies and note the result in your working notes — **not** in a commit + body, because Tasks 2–5 only amend a WIP commit whose message deliberately does not change. + All parity results are written once, into the final closing amend. Nothing verifies any of + this (spec §5.3). +- **Cite `CLAUDE.md` §5, never restate it.** The shipped text below is the whole change; do not + paraphrase surrounding rules into it. +- **No `Co-Authored-By` / `Generated with` trailers** (AGENTS.md Don'ts). +- **Quality command** = the `AGENTS.md` § Commands battery row. Run it, not a subset. +- **One WIP commit for the whole cycle. No task ends with a real commit.** `CLAUDE.md` and + everything under `plugins/` are product, so this is one full Gate-B cycle, and §5 Mechanics + is explicit: Gate B runs *before* `git commit`, and a non-`WIP` commit reads to the hook as + the cycle closing and discards the accumulated passes. So: + - **Task 1 creates the snapshot, and it carries everything** — including the twice-amended + spec, this plan, and `scripts/check-invariants.{sh,test.sh}`, which were built and run + during Gate A so the plan could cite executed code. **`baseSha` = `c0a6ed2`.** + An earlier draft committed those four separately, first: that commit then *became* the WIP + parent, and a `baseSha..HEAD` range **excludes `baseSha` itself**, so Gate B would have + excluded the very artifacts the separate commit existed to include. It was also a non-WIP + commit of executable code with the battery red and no Gate-B loop, which §5 forbids twice + over. One snapshot, one range, one close. + - **Tasks 2–5 amend it:** `git commit --amend --no-edit`. The WIP message stays as-is; the + real message is written once at the close. + - **After Task 5:** run the battery and the counterfactual, then the Gate-B loop against + `baseSha` = the WIP commit's parent, then close with + `git commit --amend -m ""` carrying the validated evidence entry. + - Per-task commit *messages* are given below as **what the closing message must cover**, not + as commands to run. If you want per-task history, produce it only after the reviewed + closing state exists. +- **The 0.9.0 manifest bump happens in Task 1, not Task 5.** Task 1's snapshot is the first + commit carrying plugin changes, so the bump must be in it: otherwise every intermediate + battery in Tasks 2–4 runs `scripts/check-version-bump.sh` against a committed, unbumped + plugin diff and **fails on a branch** — passing only on `main`, where the range is empty and + the run decides nothing. Task 5 still writes the CHANGELOG entry last, because that text + describes work the earlier tasks produce; only the `version` field moves early. +- **The WIP parent is recomputed, never carried.** A shell variable does not survive between + tool calls or a resumed session, so every consumer derives it itself, from the snapshot that + is still `WIP`: + ```bash + WIP_PARENT=$(git rev-parse HEAD^) || exit 1 + git log -1 --format=%s | grep -q '^WIP:' || { echo "HEAD is not the WIP snapshot"; exit 1; } + ``` + The guard matters, and its reason is narrower than it looks: an amend **preserves** the + parent, so `HEAD^` does not move when the cycle closes. What changes is the **subject** — so + the guard is what stops the procedure being run after the WIP state has ended, when `HEAD^` + would be the parent of a *closed* commit and the range would mean something else. `scripts/check-version-bump.sh` compares commits, so it + must be given that sha — run against `main` from `main`, the merge-base is HEAD, the range is + empty, and it passes without deciding anything. + +--- + +### Task 1: The closed severity enum, its tolerant reader, and check 4c + +**Check 4c and its fixtures are already written and green in the working tree.** They were +built and run rather than specified, because four Gate-A passes on this plan showed that shell +embedded in a document gets reviewed by reading and gets it wrong — three of those four passes +had a blocker in the same thirty lines of `awk`, each introduced by the previous pass's fix. +The shell below is not a proposal; it is what is in the file, and the run output is what it +printed. **Your job is to verify it, not to write it.** + +**Files:** +- Already modified (verify): `scripts/check-invariants.sh` — `BEGIN check 4c` … `END check 4c` + after 4b's end marker; `scripts/check-invariants.test.sh` — `init_prompt_fixtures` extended, + `sev_case`/`sev_put`/`sev_tpl` builders, **24 `sev_case` cases and one `inject_case`** +- To modify: `CLAUDE.md` — findings-protocol blockquote at `:97`, acceptance rule after `:150` +- To modify: `plugins/dev-workflow/commands/workflow-init.md` — the same two places in the mirror +- To modify: `scripts/check-invariants.sh` header inventory at `:24` and `:32`; **not** `:261` +- To modify: `scripts/check-invariants.test.sh` — the mutation-evidence block at `:325` +- To modify: `AGENTS.md` — invariant 11 at `:173` +- To modify: `plugins/dev-workflow/.claude-plugin/plugin.json` — `version` `0.8.2` → `0.9.0` + +**Interfaces:** +- Consumes: nothing. +- Produces: `SEV_CANON` (checker) and `SEV_LINE` (suite) — the same literal. The WIP parent is + **derived locally** by every consumer, never carried (see Global Constraints). + +- [ ] **Step 1: Verify the check and its suite, as built** + +```bash +shellcheck --shell=sh scripts/check-invariants.sh +shellcheck --shell=sh --exclude=SC2015 scripts/check-invariants.test.sh +sh scripts/check-invariants.test.sh +dash scripts/check-invariants.test.sh +``` + +**Observed when written:** shellcheck clean on both; `all passed (148 assertions)` under `sh` +and under `dash` — 123 before, so 25 new. If your run differs, something moved and the rest of +this task is not safe to start. + +**What 4c checks, as built** — two rules, priced separately in spec §5.2: + +- **Duplicates, both files.** The canonical line must appear **exactly once per file**, as a + whole line, after stripping a leading blockquote marker and indentation, compared for + **equality** and **case-sensitively**. Nothing is stripped from the right. +- **Placement, the command file only.** That occurrence must sit inside the **`### 2.1`** + scaffold section, terminated by the next **numbered** `### ` heading. Only that region is + scaffolded into a user's project, so a copy in the command file's own prose ships nothing — + and whole-file counting alone permitted exactly that, verified by running it. The terminator + is *numbered* on purpose: the template carries its own `### Profiles` and `### Mechanics` + subsections, and a next-`###` rule would truncate the range at them. A missing, renamed or + duplicated `### 2.1` **fails loudly** rather than skipping the rule. + +The repo's own `CLAUDE.md` gets no placement rule — the whole file is the artifact. + +The 25 assertions cover presence, duplicates (including two copies on one line), equality +(blockquoted and indented accept; leading text, trailing text, trailing space, title-case and +paraphrase reject), placement (**the outside-the-template exploit as a reject case**, inside as +accept, after an unnumbered subsection as accept, missing and duplicate anchor as reject, +`CLAUDE.md` needing no anchor as accept), **terminator drift** (unnumbered terminator, a line +planted in the widened gap — the second verified exploit — and an absent terminator, all +reject), and fail-closed paths (missing file, unreadable file +— skipped as root, which satisfies `-r` on mode 000 — and parser failure through the suite's +`inject_case` PATH seam keyed on the `sev-canon-count` marker). + +- [ ] **Step 2: Observe the counterfactual** + +```bash +sh scripts/check-invariants.sh; echo "exit: $?" +``` + +**Observed:** exit 1, with exactly two diagnostics — `CLAUDE.md must state the closed severity +set exactly once; found 0.` and the same for the command file. Nothing else fired. **This is +the `+check` evidence**, and it is an observation rather than a claim: the check is in the +tree, the canonical line is not yet in either prompt copy, and the failure names only 4c. + +Record this output. Once Step 3 lands, it cannot be reproduced without reverting the prose. + +- [ ] **Step 3: Add the canonical line to both prompt copies** + +In `CLAUDE.md`, inside the findings-protocol blockquote, make `:97–98` read: + +```markdown +> One finding per line in the format above; escape a literal pipe inside a field as +> `\|`. +> Severity is one of exactly: BLOCKER | MAJOR | MINOR | NIT — no other token. +> Every line before the terminator is exactly one finding line — no blank lines, +``` + +It must be **its own line**: 4c compares whole lines. Make the identical edit in the mirror at +`plugins/dev-workflow/commands/workflow-init.md:295`. + +- [ ] **Step 4: Add the tolerant reader rule to both copies** + +**Copied byte-for-byte from spec §3.** An earlier draft reworded it — changing the opening +rule, collapsing the structural-failure inventory, importing prose from elsewhere. The product +is prompts, so reworded is *changed*. Extract from the spec and diff against what you paste. + +Immediately after the "Accept a pass only when" paragraph (`CLAUDE.md:150`), and at the +matching place in the mirror: + +```markdown +**Reader:** the severity field is taken by splitting the line on **unescaped** pipes and +trimming the ASCII whitespace the finding format puts either side of each separator; a field +that is empty or all whitespace is a **structural** failure, so the line is INCOMPLETE and is +never normalized. Otherwise the field is matched **case-insensitively** against the four tokens +first — `Minor`, `minor` and `MINOR` are all `MINOR`, because `CLAUDE.md` Mechanics +legitimately spells them in Title case and a model copying that spelling is doing as it was +told, not drifting. A field that matches no token case-insensitively, and is non-empty, is +read as `MAJOR`. Every **structural** failure stays INCOMPLETE — a malformed +line, a wrong field count, an empty severity field, a bad terminator, a count mismatch. Only +the severity token is tolerated, and only when everything else about the line is right. +``` + +- [ ] **Step 5: Run the full battery — 4c should now pass** + +Run: the `AGENTS.md` § Commands quality command. + +Before Step 3 the checker exits 1 on 4c (Step 2's counterfactual). After Steps 3–4 it exits 0. +**That transition is the evidence**, and it is the reason those steps come before the rest of +this task rather than after. + +- [ ] **Step 6: Update `AGENTS.md` invariant 11 and the checker's own inventory** + +Replace the **whole** three-sentence tail of invariant 11 — from `review is the gate.` through +`judged by a reader.` — so the replacement does not duplicate the sentences bracketing it: + +```markdown + review is the gate. Three narrow checks in `scripts/check-invariants.sh` cover one + spelling each — a `Target model:` line naming exactly one recognized model in files + claiming conformance, a prose checklist-count claim matching the checklist, and the + finding-severity vocabulary stated as a closed set in both prompt copies — and they + are a floor, not coverage. Every other item is judged by a reader. +``` + +Then the checker's own comments, and **not by changing every "two" to "three"**: + +- `:24` — "The two prompt-conformance checks below" is a **count of checks** → three. +- `:32` — the mutation procedure. Parameterise it over all three markers, with the + baseline-green and load-bearing guards, rather than adding a 4c-specific line; a + marker-specific procedure is what went stale before. +- `:261` — "Scan domain for the two prompt-conformance checks below" describes what + `PROMPT_EXCL` and the recursive Markdown scan govern, and that is **still 4a and 4b only**. + 4c reads two fixed paths directly. Say so explicitly. Incrementing it would put a fresh + enforcement-scope overclaim into the change meant to calibrate one. + +- [ ] **Step 7: Verify the mutation evidence — already measured and recorded** + +The block at `scripts/check-invariants.test.sh` § *Prompt conformance: checks 4a, 4b and 4c* +carries a **RE-RUN TRIGGER** this work fired by name: a marked check added, fixtures added, +harness changed. **It has been re-run and the block rewritten.** Measured: + +| Deleted block | Assertions flipped to FAIL | +|---|---| +| 4a | **20** — unchanged from the previous record | +| 4b | **22** — see below | +| 4c | **19** — its 18 reject fixtures plus `4c canonical-line parser failure fires` | + +**No accept case moved in any of the three** — the second half of the check, and the one a +non-empty flip set alone does not establish. + +**4b measured 21 on the first run against a recorded 22, and that was a real regression this +work introduced.** `checklist parser failure fires` greps the checker's output for the bare +`parser failed`; 4c's new diagnostic also ends in those words, so the fixture had stopped +testing 4b — deleting the 4b block left it green. The pattern is now `checklist parser failed` +and the count is 22 again. **This is the argument for building before planning:** four review +passes read that shell without finding it; one mutation run did. + +Confirm the numbers if you change anything. If they differ, the record is wrong and must be +rewritten — nothing re-runs it for you. + +- [ ] **Step 8: Bump the manifest, lint, and create the one WIP snapshot** + +`plugins/dev-workflow/.claude-plugin/plugin.json`: `version` `0.8.2` → `0.9.0`. **Minor, not +patch** — §5's decision procedure gains a record form and a reader rule, which is new product +behaviour. It happens here, not in Task 5, because this snapshot is the first commit carrying +plugin changes and invariant 12 wants the bump in it. + +Run the `AGENTS.md` § Commands quality command **again** — every Task 1 edit is now in place — +then: + +```bash +shellcheck --shell=sh scripts/check-invariants.sh +shellcheck --shell=sh --exclude=SC2015 scripts/check-invariants.test.sh +git add CLAUDE.md plugins/dev-workflow/commands/workflow-init.md \ + scripts/check-invariants.sh scripts/check-invariants.test.sh AGENTS.md \ + plugins/dev-workflow/.claude-plugin/plugin.json +git commit -m "WIP: reviewer-availability salvage" +``` + +**This is the cycle's only commit until Gate B closes it.** Tasks 2–5 amend it. Closing-message +share for this task: rider (b) — the writer's vocabulary stated rather than exemplified in both +copies; the reader matching case-insensitively so a pass is never discarded over a token; check +4c asserting the statement is present exactly once per file, case-sensitively; the mutation +record re-measured, including the 4b isolation regression found and fixed; and the two edited +regions diffed and matched. + +### Task 2: The human-exception record form + +**Files:** +- Modify: `CLAUDE.md` — § Mechanics, immediately after the closing-message rule at `:413` +- Modify: `plugins/dev-workflow/commands/workflow-init.md` — same place, at `:603` + +**Interfaces:** +- Consumes: nothing from Task 1. +- Produces: the `Human exception:` block form, quoted by Task 5's changelog entry. + +- [ ] **Step 1: Add the paragraph to `CLAUDE.md`** + +**Copied byte-for-byte from spec §2.1.** An earlier draft of this plan paraphrased it — +dropping the closed-and-unmerged condition on the no-destination case, the +does-not-reopen-any-gate sentence, and part of the mandatory-scope wording — while +claiming to be verbatim. Those are the limiting clauses; do not re-edit them here. +Verify by extracting the block from the spec and diffing against what you paste. + +**One clause of the block below was superseded after this plan ran, as of plugin 0.9.1.** The +sentence *"An empty commit carrying only the record is a legitimate destination and does not +reopen any gate"* is false about the shipped hook: such a commit can still draw a Gate-B +reminder. The block is deliberately **left unedited** — it is the record of what was +approved, and the byte-for-byte relationship to spec §2.1 is the thing this step verifies, so +correcting it in place would break the check while hiding that a correction happened. Do not +paste this block into `CLAUDE.md` as-is; §5 and the `/workflow-init` template carry the +corrected wording, and spec §2.1 carries the same supersession note. + +```markdown +> **Recording a human exception.** Where a human decides that something **no applicable rule +> required** was nonetheless worth skipping — an optional check this environment cannot run, a +> review someone asked for and then stood down, a courtesy step — that decision goes in the +> closing commit body: +> +> ``` +> Human exception: · +> Not done: +> Accepted because: +> ``` +> +> **Which commit:** an ungated change records it in that commit; a Gate-A cycle in the spec or +> plan commit; a Gate-B cycle in the WIP commit, restated by the closing amend. Several records +> accumulate; order means nothing. +> +> **A decision made after its commit closed** — during PR review, say — goes in whichever of +> these exists: the next commit on the branch, the squash body, or a follow-up commit after the +> merge. If none does — the branch is closed, unmerged, and heading for an ordinary or rebase +> merge — **add a commit for it.** An empty commit carrying only the record is a legitimate +> destination and does not reopen any gate: it changes no content, so it raises no review +> obligation. A record with nowhere to go would otherwise be a record that does not exist. +> +> Copy every record into the squash body alongside the evidence entry (Mechanics, +> squash-merge carry). **Nothing performs that carry and nothing checks afterwards that it +> happened** — it is on whoever prepares the merge. If two copies of one record disagree, that +> is a copying error: stop and fix it rather than picking one. +> +> **Scope, and it is narrow. This form supplies no permission.** It records a decision that +> was already the human's to make about something genuinely optional. It is **never** the answer to a +> below-floor pass, an unclean final pass, a `STOP and surface`, a Gate-A or Gate-B +> obligation, or a profile-derived evidence requirement — and more generally **it authorizes +> nothing that any mandatory rule in this file or in `AGENTS.md` requires.** Those have their +> own terminal actions and this paragraph changes none of them: on a STOP you still stop, and +> neither a human's assent nor this record lets an agent close or continue a cycle. +> +> **"Mandatory" is not limited to this file.** A rule in `AGENTS.md`, a project doc, CI, a +> branch policy or the platform is equally out of reach — under **Wait for**, +> `docs/pr-review-bots.md` requires a bot review unless an explicit recorded human decision +> permits proceeding without it, and this form is not that decision. If you are reaching for it to get past something mandatory, the answer +> is no — take the operational route or stop. +> +> **Nor is it for things that were simply never owed.** An absent review from a bot routed +> **opportunistically** blocks nothing and needs no exception and no record; +> `docs/pr-review-bots.md` says so deliberately, and writing one anyway would rebuild the +> per-quiet-bot ceremony that routing removed. Record a decision, not a non-event. +> +> **What the record is worth.** It is an **unverified assertion**, and reads as one: nothing +> checks that the handle belongs to whoever decided, that a human was asked, or that the +> reason is honest. A reader of history learns that *the commit claims* a human chose, what +> it says was skipped, and why — no more. It supports no claim of authorization or review, +> and satisfies no evidence obligation. It exists because an exception nobody wrote down is +> invisible, not because writing it down makes it sound. +``` + +It goes into `CLAUDE.md` § Mechanics immediately after the closing-message rule at +`:413`, as a list item at that section's indentation. + +- [ ] **Step 2: Make the identical edit in the mirror** + +Same text at `workflow-init.md:603`, flush-left inside the fence, matching the surrounding +list indentation of the template's Mechanics section. + +- [ ] **Step 3: Verify parity** + +Extract the block from both files and diff them. They must be identical apart from the +template's list indentation. Record the result — nothing checks it. + +- [ ] **Step 4: Run the full battery** + +Run: the `AGENTS.md` § Commands quality command. +Expected: green. What that establishes is only what those checks compare — shellcheck, the two +hook suites, the invariant checks including 4c's duplicate and placement counts, the version +check, and `claude plugin validate`. It establishes **nothing** about this paragraph, which no mechanical +check reads; that is the 12-item review's job (Task 5 Step 5) and Gate B's. + +- [ ] **Step 5: Fold into the WIP snapshot** + +```bash +git add CLAUDE.md plugins/dev-workflow/commands/workflow-init.md +git commit --amend --no-edit +``` + +What this task's share of the closing message must cover: the record form, for a decision about +work no applicable rule required; that it records a decision and authorizes none — never a +gate, a floor, a pass count, an evidence obligation, or any mandatory rule from anywhere; that +the record is an unverified assertion and the shipped text says so; and that the two edited +regions were diffed and matched. **Not** that it closes the story's remaining scope — rider +(c), the backlog, the release metadata and Gate B are all still ahead, and only the final +closing commit can say the cycle is done. + +### Task 3: Rider (c) — the squash-merge carry sentence + +**Files:** +- Modify: `CLAUDE.md` — § Mechanics, beside the closing-message rule at `:413` +- Modify: `plugins/dev-workflow/commands/workflow-init.md` — same place, at `:603` + +**Interfaces:** +- Consumes: Task 2's record form (the sentence names it). +- Produces: nothing. + +- [ ] **Step 1: Add the sentence to both copies** + +**One line, copied byte-for-byte from spec §3** — the spec pins it as a single source +line, so wrapping it here would break the equality it is pinned for: + +```markdown +> **On squash-merge, copy every evidence entry and every human-exception record in the squash range into the squash body — the squash commit is the only body the merge carries into `main`'s history, so anything left behind is unreachable from it.** +``` + +Place it immediately after the existing closing-message rule, before Task 2's paragraph, so the +evidence entry and the record are described in the order they are written. + +- [ ] **Step 2: Verify parity, run the battery, fold into the WIP snapshot** + +Diff the sentence between the two copies; run the `AGENTS.md` § Commands battery; then: + +```bash +git add CLAUDE.md plugins/dev-workflow/commands/workflow-init.md +git commit --amend --no-edit +``` + +Closing-message share: `main`'s tip is the durable record, so a squash that drops the evidence +entry or a human-exception record drops it from `main`'s history; the sentence says which +artifacts and which range; nothing enforces it. + +### Task 4: Backlog + +**Files:** +- Modify: `todos.md` — the compound-commands row; `unverified-enforcement-claim` at `:90`; + `prompt-vague-criteria` at `:103`; **six** new rows (Step 3's parked items) and **one amended** + (the slot-collision row, which already exists — Step 4) + +**`docs/hardening-log.md` is NOT in this task.** An earlier draft staged it with no row +specified, which is not executable. Spec §6 requires no ledger append: the ledger records +*findings that recurred and were hardened a rung*, and nothing here is one — 4c is a new check +for a new rule, not an escalation of a repeat finding. If Gate B disagrees, the row is written +then, against a named recurrence. + +**Interfaces:** none. + +- [ ] **Step 1: Edit the compound-commands row in place** + +`todos.md` is edited in place — append-only-never-edit is `docs/hardening-log.md`'s rule, not +this file's. Add occurrence 3: `git add` and `git commit` in one Bash call, empty staged set at +`PreToolUse`, loose STOP; observed on PR #23's close. Same shape as occurrence 2 and, like it, +a **false positive**. + +- [ ] **Step 2: Close one fingerprint, re-point another** + +`prompt-vague-criteria` (`:103`) **closes** — rider (b) states the enum rather than exemplifying +it, which is what that row asked for. `unverified-enforcement-claim` (`:90`) **stays open**, +re-pointed at `docs/superpowers/stories/2026-08-14-sequential-branch-calls-hook-story.md`. + +- [ ] **Step 3: Add all six parked rows from spec §6** + +**Copy each row's full text from spec §6, not a summary.** An earlier draft of this plan +compressed them — the external-authority row lost its trusted-signer, role-policy and +independent-availability-attestation requirements and its "one untried direction, not the only +one that could work" calibration, which are the whole content of that row. Extract each bullet +from the spec and fit it to `todos.md`'s row format without dropping a clause. The six, by +their spec headings: + +1. **Attribution for the shipped record form.** *Trigger: the first record whose authorship is + disputed or unattributable.* +2. **External-authority zero-pass research.** *Trigger: a renewed need to close a gate cycle + with no review — a second multi-day reviewer outage, or the operational bridges of the + design's §7 proving unavailable.* +3. **Tracked re-review debt.** *Trigger: a human explicitly asks for follow-up review on a + recorded exception and that follow-up is later found not to have happened.* +4. **A recording mechanism for severity normalization.** *Trigger: a pass is normalized and the + drift goes unnoticed in review.* +5. **The hook's `is_docs_only` breadth** — it exempts any `.md` path outside a prompt directory, + broader than §5's prose list. *Trigger: a root `.md` file acquiring gate-relevant state.* +6. **Tier-2 counting and containment**, pointing at + `docs/superpowers/stories/2026-08-14-tier-2-same-family-reviewer-story.md`. + +- [ ] **Step 4: Amend the slot-collision row — it already exists** + +**Do not create one.** `todos.md` already carries this item, with its history (the +result-classification cycle's pass-1 call deleting the 2026-07-26 profiles cycle's +its pass-1 findings file), a *NOT FIRED (2026-08-04)* note, and the trigger *"the next round +touching the §5 file protocol."* An earlier draft of this plan said to create it, which would +have split one concern across two rows and discarded that history. + +**Amend it in place, recording TRIGGER FIRED.** Spec §6 settles this: *trigger fired, row stays +open*. Do not re-open that as a judgement — an earlier draft of this plan did, with a +*NOT FIRED* rationale that was also wrong on its facts, since Task 1 adds the severity reader +rule **to the pass-acceptance area** of §5. + +Record: this story's cycles destroyed a predecessor's findings file **and** its dispositions +before the surviving artifacts were archived by hand — a second observed occurrence, after the +2026-07-26 profiles cycle. The row stays open; the fix is still naming or archiving, never +relaxing the pre-call delete. + +- [ ] **Step 5: Fold into the WIP snapshot** + +```bash +git add todos.md +git commit --amend --no-edit +``` + +Closing-message share: the six new backlog rows with their triggers, the slot-collision row +amended with its second occurrence, `prompt-vague-criteria` +closed, `unverified-enforcement-claim` re-pointed at the sequential-branch-calls hook story, and +occurrence 3 on the compound-commands row. + +### Task 5: Changelog, and the closing checks + +The manifest bump landed in Task 1's snapshot (invariant 12 wants it in the first commit +carrying plugin changes). What is left here is the entry that describes everything, plus the +two obligations that can only run once the change is complete. + +**Files:** +- Modify: `plugins/dev-workflow/CHANGELOG.md` — new entry at the top + +**Interfaces:** +- Consumes: nothing. The WIP parent is **derived locally** in each step that needs it, with + the guard shown there — never carried from Task 1. + +- [ ] **Step 1: Write the changelog entry** + +Newest first, no date — the file says versions are recorded, not release dates, and inventing +one would be fiction. Written from `git log` over `plugins/`, not from memory: read the WIP +snapshot's diff. Cover the closed severity enum and its case-insensitive reader, check 4c, the +human-exception record form, rider (c), and that the **zero-pass gate closure the story began +as was withdrawn**, with the finding recorded in the design's §1 and shipped as `c0a6ed2`. + +- [ ] **Step 2: Fold into the WIP snapshot** + +```bash +git add plugins/dev-workflow/CHANGELOG.md +git commit --amend --no-edit +``` + +- [ ] **Step 3: Verify the bump against a base that can decide** + +```bash +# Self-contained: derive the parent here. A shell variable does not survive between tool +# calls or a resumed session, and the guard is what makes re-deriving safe -- it refuses +# to run once the cycle has closed and HEAD is no longer the WIP snapshot. +git log -1 --format=%s | grep -q '^WIP:' || { echo "HEAD is not the WIP snapshot"; exit 1; } +WIP_PARENT=$(git rev-parse HEAD^) || exit 1 +git diff --quiet "$WIP_PARENT" HEAD -- plugins/ \ + && { echo "VOID: no plugin changes in range"; exit 1; } +sh scripts/check-version-bump.sh "$WIP_PARENT" +``` + +**Not `main`.** From `main` the merge-base is HEAD, the range is empty, and the checker passes +without comparing anything — the plan would be citing a run that decided nothing. The +non-empty-range guard is what turns that from a silent pass into a VOID. + +- [ ] **Step 4: Run the full battery** + +Run: the `AGENTS.md` § Commands quality command. Expected: green, on the complete change. + +- [ ] **Step 5: The prompt-standards review — the obligation no check covers** + +`AGENTS.md` invariant 11 requires all 12 items of `docs/prompt-standards.md` for every changed +prompt artifact, and says in terms that `scripts/check-invariants.sh` is a **floor, not +coverage**. A green battery therefore establishes nothing about the reader rule, the record +form or the carry sentence. + +Go through all 12 items, in order, for **each** changed prompt copy — `CLAUDE.md` and +`plugins/dev-workflow/commands/workflow-init.md` — and record the result by name in the closing +commit body. This is a person reading a checklist. Nothing enforces it, and it is owed anyway. + +Closing-message share: the 0.9.0 bump and its CHANGELOG entry; the version check's result +against `WIP_PARENT`; the battery; the counterfactual; the parity results from Tasks 1–3; and +the 12-item review, named per copy. + +## Gate B + +**One cycle, one WIP commit, closed once.** `CLAUDE.md` and everything under `plugins/` are +product, so no path here is prose-exempt and the triviality skip does not apply. + +- **`baseSha`** = the WIP commit's parent. `reviewType: full`. +- **Carry, in `additionalContext`:** the story path + `docs/superpowers/stories/2026-08-13-reviewer-availability-fallback-story.md` and the current + evidence entry quoted verbatim. Read the profile from that header, not from this plan. +- **After every Gate-B fix, before re-reviewing:** stage the complete intended diff and + `git commit --amend --no-edit` it into the still-`WIP` snapshot; re-run the battery and the + counterfactual; re-run parity and the 12-item prompt review if prompt text changed; then + re-review with the **unchanged** WIP parent as `baseSha`. Re-run the **complete self-contained block from Task 5 Step 3** — guard, derivation, + non-empty-range check, checker — in **every** iteration and once immediately before the + closing amend — a fix can change plugin content or lose the manifest bump, and the + battery's own `main` argument is an empty range on this branch, so it would not notice. `mcp__codex__review` reads a git + range, so a fix left in the worktree is invisible to it — the re-review would return a clean + pass over stale content, which is the one failure mode a clean pass cannot be distinguished + from. Revalidate the evidence entry each time; a fix changes the diff. +- **Close** with `git commit --amend -m ""` carrying the validated entry and the + five tasks' message shares. + +**Evidence owed — `battery+check`:** + +- **Battery** — the `AGENTS.md` § Commands quality command, green on the complete change. +- **The check that fails without the change** — 4c. Counterfactual, isolated so the failure has + exactly one cause: + + ```bash + set -e + TMP=$(mktemp -d); [ -n "$TMP" ] && [ -d "$TMP" ] + trap 'rm -rf "$TMP"' EXIT HUP INT TERM + git ls-files -z \ + | xargs -0 -I{} sh -c 'mkdir -p "$0/$(dirname "{}")" && cp "{}" "$0/{}"' "$TMP/repo" + ( cd "$TMP/repo" && sh scripts/check-invariants.sh ) \ + || { echo "VOID: baseline not green"; exit 1; } + git show df850ab:CLAUDE.md > "$TMP/repo/CLAUDE.md" + git show df850ab:plugins/dev-workflow/commands/workflow-init.md \ + > "$TMP/repo/plugins/dev-workflow/commands/workflow-init.md" + set +e + out=$( cd "$TMP/repo" && sh scripts/check-invariants.sh 2>&1 ); st=$? + [ "$st" -ne 0 ] || { echo "VOID: mutant still green"; exit 1; } + sev=$(printf '%s\n' "$out" | grep -c 'closed severity set') + all=$(printf '%s\n' "$out" | grep -cE '^(Invariant|Prompt standards)') + [ "$sev" -gt 0 ] || { echo "VOID: no 4c diagnostic"; exit 1; } + [ "$sev" -eq "$all" ] || { echo "VOID: $((all - sev)) other diagnostic(s) present"; exit 1; } + echo "counterfactual OK — baseline 0, mutant $st, $sev 4c diagnostic(s) and nothing else" + ``` + + Required: baseline **exit 0**, mutant **exit 1** carrying the 4c diagnostic (`closed severity + set`) **and no other diagnostic**. 4c ships inside the full checker, so a bare two-file tree + would fail unrelated invariants and prove nothing — the isolation is what makes this evidence + rather than an assertion. + +- **The prompt-standards review** (Task 5 Step 5) is named in the entry too. It satisfies no + part of `battery+check`; it is invariant 11's own obligation, and it is recorded so a reader + can see it was done rather than assumed. + +**What the evidence does not establish:** that a reader applies the reader rule, that the record +form is used correctly, or that the carry happens. Those are behaviour, and nothing here +observes them. Say so in the entry rather than letting a green battery imply otherwise. + +**Lens sets:** none — risk `standard`, security `none`. + +**The standing falsification lens still applies.** Name what this change alters the size, value +or position of, then grep for where each is described elsewhere: the checker's check count +(`scripts/check-invariants.sh:24`, `:261`; `AGENTS.md` invariant 11), the mutation evidence's +flip counts (`scripts/check-invariants.test.sh:325`), the manifest version, and the command file's +`### 2.1` scaffold heading — 4c's placement rule anchors on it and on the next **numbered** +`### `. Renaming or renumbering that heading fails the check loudly, which is intended; the +template's own unnumbered `### Profiles` and `### Mechanics` do not affect it. + +## Self-review + +**Spec §4 path table — all ten rows, individually.** Two are **already satisfied** by commit +`c0a6ed2` and belong to no task here: the parent story and the tier-2 story, both amended when +the closure landed. Verify that before starting rather than assuming it — `git show --stat +c0a6ed2`. The remaining eight map as: `CLAUDE.md` → Tasks 1–3 · +`plugins/dev-workflow/commands/workflow-init.md` → Tasks 1–3 · +`scripts/check-invariants.sh` → Task 1 · `scripts/check-invariants.test.sh` → Task 1 · +`AGENTS.md` → Task 1 · `plugins/dev-workflow/.claude-plugin/plugin.json` → **Task 1** (the bump +moved there so the first plugin-carrying commit contains it) · +`plugins/dev-workflow/CHANGELOG.md` → Task 5 · `todos.md` → Task 4. An earlier draft claimed +"all ten appear across Tasks 1–5", which was false and hid the line between landed closure work +and remaining salvage work. + +**Other spec coverage.** §2.1 → Task 2 (byte-for-byte). §3 rider (b) → Task 1, rider (c) → Task +3 (byte-for-byte). §5.2 assertion and fixtures → Task 1. §5.3 parity → Global Constraints plus +each prose task's diff step. §6 → **all ten bullets**, mapped individually: the six parked rows and the slot-collision row +to Task 4 Steps 3–4; the `prompt-vague-criteria` / `unverified-enforcement-claim` bullet to Task +4 Step 2; the compound-commands bullet to Task 4 Step 1; and the version-and-CHANGELOG bullet +split across Task 1 (the bump) and Task 5 (the entry). An earlier draft said nine. §1 needs no task; it is the closure +record and shipped with `c0a6ed2`. + +**Placeholders.** None. Every edit carries its exact text or exact shell, and the two verbatim +blocks were extracted from the spec programmatically rather than retyped — an earlier draft +paraphrased §2.1's limiting clauses while claiming to be verbatim. + +**Type consistency, read off the built code rather than remembered.** The checker holds +`SEV_CANON`; the suite holds `SEV_LINE`; same literal. The function is +`severity_rule_scan ` — one argument — and it prints four space-separated fields: +whole-file count, in-template count, `### 2.1` anchor count, and the terminator's number or +`none`. The caller splits them with `set --` and branches on each: awk status, whole count, +anchor count, terminator identity, in-template count. The `inject_case` seam keys on the +`sev-canon-count` marker comment. + +**Branch coverage.** Every branch has a fixture: missing file, unreadable file (skipped as +root), parser failure, wrong whole-file count, missing anchor, duplicate anchor, wrong +terminator, absent terminator, and line-outside-template. Nothing is reviewed-but-untested. + +**One fixture deliberately dropped:** missing-template. Check 4b reads that path, so its +absence fails 4b first and the case could never be isolated to 4c. The missing-`CLAUDE.md` +case covers the same branch and *is* isolated. + +**Mutation numbers are measured, never carried forward.** 4c read 13, then 16, then **19** as +fixtures were added; only the last was ever true of the suite that shipped. Each superseded +number was replaced by re-running. diff --git a/docs/superpowers/specs/2026-08-14-reviewer-availability-fallback-design.md b/docs/superpowers/specs/2026-08-14-reviewer-availability-fallback-design.md new file mode 100644 index 0000000..1b972c1 --- /dev/null +++ b/docs/superpowers/specs/2026-08-14-reviewer-availability-fallback-design.md @@ -0,0 +1,661 @@ +# Reviewer-availability fallback — closure record, and what ships — Design + +**Story:** `docs/superpowers/stories/2026-08-13-reviewer-availability-fallback-story.md` +— the story header is the single writable copy of the profile. Every gate call carries that +path and reads the axes, the mode and the lens sets fresh from it; this document never +restates them as values. + +**Status: the design question is closed with a negative answer.** Three design cycles across +nine Gate-A passes failed to produce a safe authorized zero-pass closure. §1 records the +finding and its evidence, because the finding *is* the result. §2 specifies the small thing +that survives — which, after Gate-A pass 1 of this cycle, is **narrower than the first draft +of this section made it**: see §2.0. + +## 1. The finding + +**No safe design for a sanctioned zero-pass gate closure was found, and each of the three +classes tried failed for a structural reason rather than an incidental one.** + +### 1.1 What "safe" was required to mean + +The claim is only as sharp as the properties it is measured against, so they are named. A +sanctioned zero-pass closure would have had to be: + +1. **Grounded** — the condition that justifies it (the reviewer cannot run) is *established*, + not asserted, and not producible on demand by whoever benefits. +2. **Guarded** — its preconditions are decidable in the situation it fires in, not only in + situations where the gate was available anyway. +3. **Bounded** — it closes the one cycle it was granted for, and does not become a general + route past review. +4. **Attributable** — the record identifies who decided, durably, in a way a later reader can + weigh. + +Properties 1 and 2 are where every cycle died. Property 4 was never achieved beyond an +unverified assertion, which §2.1 now says out loud rather than working around. + +### 1.2 The evidence + +| Cycle | Shape | Blockers by pass | Findings | Outcome | +|---|---|---|---|---| +| Three-tier | tier 1 / tier 2 same-family reviewer / tier 3 human | 4 → 4 → 6 | 116 | stopped | +| Two-tier + debt | tier 1 / tier 3, with a tracked re-review debt | 7 → 8 → 12 | 96 | stopped | +| Stripped | tier 1 / tier 3, no state at all | 4 → 14 → 15 | 91 | stopped | + +**303 findings across nine passes. None was ever dismissed.** The per-pass artifacts sit in the +current worktree under `.context/codex-reviews/*.stopped-3tier.md`, `*.stopped-2tier-debt.md` +and `*.stopped-tier3-core.md` — the last including the rejected 1016-line design in full. +**Those are ephemeral**: `.context/` is git-ignored, so they do not survive a clone and slot +collisions have already destroyed some. Everything a later reader must be able to rely on is +therefore *in this section*, not behind those paths; the files are corroboration while they +last, not the record. + +### 1.3 The three failure modes, mapped to the design class each defeated + +**Unenforceable, or recursive — defeats compensating state that the repository itself +authors.** Every +control added to make the waiver safe landed as a prose assertion authored by the party being +waived, or as real state that itself needed the gate that was unavailable. Cycle 2 died of +this in its debt machinery; cycle 3 removed that machinery entirely and pass 3 landed the +identical pair on the core. **Structural, because** among the **repository-controlled** +mechanisms considered here, the gate is the only one available to protect the record of its +own waiver. Authority held outside the repository is expressly outside this conclusion (§1.5). + +**The outage is manufacturable — defeats property 1 for any client-observed trigger.** Cycle 3 +narrowed the evidence to canonical calls, removed `auth-failed` because withholding a +credential fakes an outage, then removed `status-page` as unreachable. Pass 3 showed the same +argument defeats what remained: deliberately exhaust the quota, point at a spent account, +block DNS. A local transport failure does not establish that the vendor answered. **Structural, +because** the approver may be the author, so the condition authorizing the waiver is +producible by whoever benefits from it. + +**Unprotected local history cannot establish the preconditions in the case they exist for — +defeats property 2.** The +central precondition (no unresolved adverse findings) needs to know which passes happened. +Nothing durably records that: session memory is not evidence, `.context/` slot names collide, +the hook counter is explicitly not evidence, and at Gate A no prior commit body exists. So +`Passes completed: none` can never be *established* precisely when it is true, and accepting +"none observed" reopens the path where a lost adverse pass is laundered into a clean closure. +**Structural, because** proving a negative about review history requires *protected* history, +and history the repository authors returns to the first failure mode. Externally protected +history is a different class and was not tried (§1.5). + +### 1.4 What the fixes did + +The stripped cycle's artifact went 489 → 750 → 1016 lines across two rounds of remediation. A +prose amendment to §5 acquired a git algorithm — a dedicated index, `read-tree` from a fixed +parent, changed-path classification, a ref compare-and-swap, a three-way commit read-back, +squash-carry envelopes and an invalidation-record convention. Pass 3 found five defects +**inside that algorithm**, and one above it: none of that logic was exercised by any +validation, so the mode's evidence obligation was unmet for a risk path the fixes had created. + +Meanwhile that design's own "What this does not do" section conceded throughout that nothing +verifies the human pause, that every record is a recorded assertion, and that an agent +departing from the procedure produces a conforming-looking commit. **The machinery was *intended* to protect the record against +accident — no comparison here establishes that it did, and pass 3 found five defects in it — +while the finding was that the record cannot be protected against intent.** + +### 1.5 What this finding does and does not claim + +**Does claim:** three design classes — **repository-authored** compensating state, +**client-observed** outage triggers, and **unprotected local** history as a source of +preconditions — each failed for a reason inherent to a prompt-only product rather than to the +particular draft. The qualifiers are load-bearing: externally protected state is still +compensating state and can establish history, and nothing here was tested against it. + +**Does not claim:** + +- **Not a proof of impossibility.** Nine passes over three related designs establish repeated + structural failure, not exhaustion of the design space. A class not tried here — notably one + resting on authority *outside* the repository — is untouched by this evidence. +- **Not that a human exception is wrong.** Humans make them, correctly. §2 ships the form. +- **Not that no enforcement is possible anywhere.** Authority held outside the repository is + the untried class. It is a **candidate, not a proof**: a signature establishes property 4 + only with a trusted signer identity *and* a role policy saying which identities may approve, + since an ordinary signer can be the author. A platform **protected-branch approval** can + establish property 4 more directly — an authenticated approver under an enforced, + named branch policy — but establishes **nothing about property 1**, because an approval + says nothing about whether the reviewer was available. Property 1 needs an availability + attestation from a party that is not the author, which neither mechanism supplies. Parked in §6 with its trigger, and with those requirements named so the next + attempt starts from them. +- **Not that the three cycles found every defect.** They found enough. + +### 1.6 Two lessons about revising a spec, learned expensively here + +Recorded because they cost eight passes of the salvage cycle to see, and neither is specific +to this feature. + +**Cutting a requirement is not like cutting an answer.** Removing the drift record removed an +*obligation*, and the finding count fell (20 → 16). Removing the `Ref:` identifier removed an +*answer to a question the document had already asked* — "which record is this?" — and the count +rose (16 → 17), with six findings caused by the cut itself, because the question was still +standing and every dependent rule now dangled. **Delete the question, not just the answer.** + +**A spec must not ask mechanical questions about artifacts only humans read.** Eleven of pass +8's seventeen findings were in three sections specifying procedures no tool executes: how to +anchor-extract and normalize Markdown before diffing two prompt copies, how to order evidence +entries across a squash range, how to establish a commit-body record's identity. Every answer +to such a question is prose about prose — unenforced, unexecuted, and reviewable forever. The +honest form is an instruction plus the admission that nothing checks it. *(Candidate for +`docs/prompt-standards.md` when the field-intake round runs.)* + +## 2. What ships — the record form of a human exception + +### 2.0 The scope correction, and why it is the load-bearing part of this section + +The first draft of §2 wrote the form as covering *"a pass short of the floor, an absent bot +review, a check that could not be run"* while asserting that it authorized nothing. **Gate-A +pass 1 of this cycle rejected that as a distinction without a difference** (blockers 1 and 11): +a normative instruction shaped *"when a human decides to proceed past X, write this"*, plus a +required commit form, plus §6 routing the stall through it, is operationally a route past the +gate no matter what the surrounding sentence says. A compliant agent reads human assent plus +three lines as sufficient to continue after §5 says STOP — which is exactly the waiver §1 +found unbuildable, re-entering through the prose. + +**And the precedent it named turned out not to be one.** Pass 1 corrected the first draft by +pointing at PR #14 — a merge past an absent supplementary bot review on a change that had +already passed Gate B. Pass 5 corrected that in turn, and pass 6 corrected the correction: +#14 happened while CodeRabbit was under **Wait for**, where `docs/pr-review-bots.md` makes the +review required **unless** an explicit recorded human decision permits proceeding without it. +The two are alternatives, not a conjunction — #14 had no review and took the recorded-decision +branch. So #14 was a mandatory rule being consciously answered by the mechanism that rule +provides, which the boundary above puts out of reach: this form is not that decision. #14 is therefore history, not +derivation: it shows that humans make exceptions and that recording them is worth doing, and +nothing more. The form is justified on its own terms below. + +**So the form is scoped by the optional-work boundary itself, and no further:** + +> **It applies only to work that no applicable rule required.** Not a gate, a floor, a pass +> count or a profile-derived evidence obligation — and equally not anything required by +> `AGENTS.md`, a project doc, CI, a branch policy or the platform. The source of the +> obligation is irrelevant; that it *was* an obligation is decisive. + +**Stated that way, not as "where the gates are satisfied"** — pass 2 found that phrasing +temporally impossible: §2.4 places a record in a Gate-A spec commit, at which point the later +Gate-A run and Gate B are not satisfied and cannot be. The boundary is not *when* the record +is written but *what it is about*: something nothing required. A Gate-A spec +commit may carry a record about an absent supplementary bot review; it may not carry one about +its own gate, before or after. + +That is not a softening of the first draft — it is the difference between a record and a +waiver. It also makes §4's claim true: with core-gate cases excluded, `docs/getting-started.md` +("Gate A is not skippable at any level"), `docs/coding-workflow.md` ("mandatory… not optional") and +`docs/sparring-briefing.md` ("do not treat a satisfied human as a substitute for a clean pass") are +all still true as written, with nothing to amend. + +### 2.1 The paragraph added to CLAUDE.md §5 + +One paragraph, in Mechanics, beside the evidence-entry rule: + +**Superseded in one clause, as of plugin 0.9.1 — the block below is left unedited as the +record of what was approved.** The sentence *"An empty commit carrying only the record is a +legitimate destination and does not reopen any gate"* is false about the shipped hook, and +Greptile found it on PR #24: such a commit can still draw a Gate-B reminder. Only the *no gate +reopening* half survives, and only in the sense that mattered — an empty diff raises no review +**obligation**, and the hook is advisory, so it does not block the commit attempt. `CLAUDE.md` +§5 and the `/workflow-init` template carry the corrected wording; prefer them over this block. + +> **Recording a human exception.** Where a human decides that something **no applicable rule +> required** was nonetheless worth skipping — an optional check this environment cannot run, a +> review someone asked for and then stood down, a courtesy step — that decision goes in the +> closing commit body: +> +> ``` +> Human exception: · +> Not done: +> Accepted because: +> ``` +> +> **Which commit:** an ungated change records it in that commit; a Gate-A cycle in the spec or +> plan commit; a Gate-B cycle in the WIP commit, restated by the closing amend. Several records +> accumulate; order means nothing. +> +> **A decision made after its commit closed** — during PR review, say — goes in whichever of +> these exists: the next commit on the branch, the squash body, or a follow-up commit after the +> merge. If none does — the branch is closed, unmerged, and heading for an ordinary or rebase +> merge — **add a commit for it.** An empty commit carrying only the record is a legitimate +> destination and does not reopen any gate: it changes no content, so it raises no review +> obligation. A record with nowhere to go would otherwise be a record that does not exist. +> +> Copy every record into the squash body alongside the evidence entry (Mechanics, +> squash-merge carry). **Nothing performs that carry and nothing checks afterwards that it +> happened** — it is on whoever prepares the merge. If two copies of one record disagree, that +> is a copying error: stop and fix it rather than picking one. +> +> **Scope, and it is narrow. This form supplies no permission.** It records a decision that +> was already the human's to make about something genuinely optional. It is **never** the answer to a +> below-floor pass, an unclean final pass, a `STOP and surface`, a Gate-A or Gate-B +> obligation, or a profile-derived evidence requirement — and more generally **it authorizes +> nothing that any mandatory rule in this file or in `AGENTS.md` requires.** Those have their +> own terminal actions and this paragraph changes none of them: on a STOP you still stop, and +> neither a human's assent nor this record lets an agent close or continue a cycle. +> +> **"Mandatory" is not limited to this file.** A rule in `AGENTS.md`, a project doc, CI, a +> branch policy or the platform is equally out of reach — under **Wait for**, +> `docs/pr-review-bots.md` requires a bot review unless an explicit recorded human decision +> permits proceeding without it, and this form is not that decision. If you are reaching for it to get past something mandatory, the answer +> is no — take the operational route or stop. +> +> **Nor is it for things that were simply never owed.** An absent review from a bot routed +> **opportunistically** blocks nothing and needs no exception and no record; +> `docs/pr-review-bots.md` says so deliberately, and writing one anyway would rebuild the +> per-quiet-bot ceremony that routing removed. Record a decision, not a non-event. +> +> **What the record is worth.** It is an **unverified assertion**, and reads as one: nothing +> checks that the handle belongs to whoever decided, that a human was asked, or that the +> reason is honest. A reader of history learns that *the commit claims* a human chose, what +> it says was skipped, and why — no more. It supports no claim of authorization or review, +> and satisfies no evidence obligation. It exists because an exception nobody wrote down is +> invisible, not because writing it down makes it sound. + +### 2.2 What that paragraph is not + +- **Not a gate waiver.** It does not clear the hook, satisfy a floor, or make an unreviewed + change reviewed. §5's gates are unchanged in every particular. +- **Not a decision procedure.** It is retrospective bookkeeping about a decision already + legitimately available; it adds no branch to any of §5's stop conditions. +- **Not a permission with preconditions.** It has a strict **applicability boundary** — §2.0, + and that boundary is absolutely a condition on using the form. What it does not have is a + set of conditions whose satisfaction would authorize bypassing an obligation, because + §1.3 is the finding that no such set exists. Calling the boundary a non-condition, as an + earlier draft did, weakened the one thing keeping this from being a waiver. +- **Not evidence.** It appears in no evidence entry and satisfies no mode. +- **Not attribution.** §2.1 says so in the shipped text, not only here. + +### 2.3 Old-condition accounting — CLAUDE.md §5 + +The AGENTS.md Don't requires this, and pass-1 finding 4 established that four rows were not +enough: the first draft's wording created new control-flow edges beside several of §5's +terminal actions, and "everything untouched" concealed them. Every clause that draft could +have touched is therefore listed individually, with the §2.0 scope applied. + +| §5 clause | Disposition under §2 as scoped | +|---|---| +| Hard floor: min 3 passes **per run** — Gate A's spec and plan are separate runs, each with its own loop | **Kept, untouched** — §2.1 excludes below-floor closure by name | +| Final pass must be clean | **Kept, untouched** — same exclusion | +| Only early exit is a zero-finding pass | **Kept, untouched** — no second exit is added | +| "Clearly stuck → STOP and surface" | **Kept, untouched** — §2.1 states that a STOP still stops and that assent does not clear it | +| Recovery: one attempt per pass; spent and still incomplete → STOP | **Kept, untouched** — the record is not a substitute for the attempt or for the STOP | +| An INCOMPLETE pass is discounted, whatever the counter says | **Kept, untouched** | +| Gate-B triviality skip needs two independent conditions | **Kept, untouched** — the skip has its own reason field; §2.1 is a different record for a different case | +| The skip reason is recorded in the commit body | **Kept**, and now has a sibling form for the non-skip case | +| What the author owes by mode | **Kept, untouched** — §2.1 satisfies no mode and §2.2 says so | +| Unobservable counterfactual → blocking evidence gap, human may lower the mode by logged override | **Kept, untouched** — that path stays the override, not this record; §2.1 excludes evidence obligations by name | +| Work gap vs setup gap | **Kept, untouched** — an unrunnable *required* check is still a gap to fix or surface; §2.1 covers optional checks only | +| Profile changes are proposed, human-confirmed, logged | **Kept, untouched** | +| Evidence entry in the commit body, revalidated before close | **Kept, untouched** for the evidence entry. The exception record travels the same carry chain and has **no revalidation step at all** — not a weaker one. §2.4 records that as a deliberate cut, not an omission | +| "Dismissed finding → one-line why" | **Kept**, and §2.1 is the same instinct applied to a decision rather than a finding | +| "Accept a pass only when" — readable file, exact terminator, exactly `` finding lines and nothing else | **Kept, untouched** — the acceptance grammar is unchanged. Normalization applies *after* acceptance, to an already-valid line; a structural failure is still INCOMPLETE and never reaches it | +| `.context/codex-gate.off` silences reminders; "the gates still apply" | **Kept, untouched** — the opt-out's meaning is unchanged, and §2.1 is not an opt-out | +| Gate A is two runs — spec then plan — each with its own loop | **Kept, untouched** | +| A profile present but unresolvable → stop and surface | **Kept, untouched.** §2.1's "on a STOP you still stop" is general and reaches this one; it is listed separately because a general assurance is what this repository's Don't rejects as accounting | +| Mechanics' severity semantics — Blocker (wrong/unsafe/breaks invariant) · Major (design flaw → rework) → both must resolve; Minor · Nit → collect, never iterate | **Kept, untouched.** Rider (b) changes how an *unrecognized* token is read, never what a recognized severity means or does. Its Title-case spelling is also kept: the reader matches case-insensitively (§3) precisely so this sentence stays legitimate input rather than becoming drift | +| "You filter to Blocker/Major, Codex never does" | **Kept, untouched** — normalization happens before the filter and feeds it a token; the filter itself is unchanged | +| The writer-facing finding-line format, one per line, escaped pipes | **Kept**; rider (b) adds the closed severity vocabulary the format previously showed only by example, and changes nothing else about the line | +| Companions are advisory; they never participate in pass validation | **Kept, untouched.** An earlier draft narrowed this for normalization-dependent passes; that narrowing is withdrawn with the drift record (§3), so no companion clause changes | +| Every file-protocol clause — pre-call deletion, slot naming, one pass at a time, recovery and single-branch resume, both-branch acceptance under `full`, the one-line reply | **Kept, untouched.** These were in scope only while the drift record needed an attempt-suffixed slot model; with it cut, none of them moves | +| Anything else is an INCOMPLETE pass, discounted | **Kept, untouched** | +| Recovery: one attempt per pass, shared; prefer a single-branch resume with `reviewType` + `sessionId` | **Kept, untouched.** An earlier draft had rider (b) specify retry filenames and pairing; that model went with the drift record (§3), so §5's existing recovery text is unmodified | +| `WIP:` naming; amend to close; `reset --soft` for several snapshots | **Kept, untouched**; §2.4 adds only what the exception record does inside that flow | +| The closing message carries the validated evidence entry | **Kept**; §2.1's record sits beside it in the same body, under the same carry instruction and the same absence of any check | + +**There is no `every other clause` row.** An earlier draft ended this table with one, which is +the catch-all this repository's decision-procedure Don't rejects by name: it asserts +completeness while naming nothing, so a dropped condition is indistinguishable from a +deliberate one. The rows above are what was walked. Anything not here was not examined, and a +reader relying on this table should re-walk §5 — five successive versions of this accounting +across this story's cycles each claimed completeness and each was wrong. + +### 2.4 What is not specified about the carry, and why + +The placement and carry rules are **in §2.1's shipped paragraph and nowhere else**. Three +drafts of this section built more: a per-gate placement table, a stable `Ref:` identifier, a +supersession grammar, a four-state collision procedure, duplicate-detection and restoration +matching rules, and a deterministic ordering. All of it specified how a **person** should +handle a three-line note in a commit message, and none of it was executed by anything. + +**It is cut, and the cut is the design decision.** What replaces it is one sentence in the +shipped text — *copy the records across; nothing checks that you did* — plus the admission +that a disagreeing duplicate is a copying error to fix rather than a case to adjudicate. A +reader who needs to tell two similar records apart does what they would do for anything else +in history: look at the commits. + +**What this gives up, stated rather than hidden:** there is no machine-followable procedure for +deduplicating records, for naming which earlier record a correction corrects, or for resolving +two records that collide in every visible field. Those situations are rare, and a person +resolves them by reading. If they turn out not to be rare, §6's attribution row is where that +evidence goes. + +## 3. Riders — these carried real discriminating checks all along + +**(b) Canonical syntax, and a tolerant reader.** **Canonical:** severity is +`BLOCKER | MAJOR | MINOR | NIT`, uppercase, stated in §5 and in `/workflow-init`'s template as +a closed set of permitted tokens for what the prompt demands of the writer — not shown by +example. **Reader:** the severity field is taken by splitting the line on **unescaped** pipes and +trimming the ASCII whitespace the finding format puts either side of each separator; a field +that is empty or all whitespace is a **structural** failure, so the line is INCOMPLETE and is +never normalized. Otherwise the field is matched **case-insensitively** against the four tokens +first — `Minor`, `minor` and `MINOR` are all `MINOR`, because `CLAUDE.md` Mechanics +legitimately spells them in Title case and a model copying that spelling is doing as it was +told, not drifting. A field that matches no token case-insensitively, and is non-empty, is +read as `MAJOR`. Every **structural** failure stays INCOMPLETE — a malformed +line, a wrong field count, an empty severity field, a bad terminator, a count mismatch. Only +the severity token is tolerated, and only when everything else about the line is right. + +Motivating incident: PR #23's Gate-B pass 3 returned all four findings at `IMPORTANT`. +Discarding that pass over a token would have thrown away four real findings. + +**The drift record is cut, and this is the interesting part of the rider.** Three drafts +required that pass's dispositions file to carry an `Enum drift:` line, with the pass invalid +without it. Gate-A pass 4 falsified its premise: **the normalization was never silent.** The findings +file carries the original token verbatim on the finding line, so the reader who validates and +filters the pass sees the drift **at the moment the decision is made** — which is when it +matters and who it matters to. + +**The stronger claim is not available, and pass 5 was right to reject it.** An earlier version +of this paragraph said the findings file is a *permanent* record that "may not be deleted". +False: §5 imposes no retention after validation, `.context/` is git-ignored, and §6's own row +records that slot collisions have already destroyed a predecessor's findings in this +repository. So the honest form is the narrower one — **visible to the reader at decision +time, not durable afterwards** — and the decision to cut is taken on that basis: the companion +bought durability the rest of the system does not provide anyway, at the cost below. + +What that clause cost before it was cut, all of it now moot: a token-identity rule with a +whitespace case contradicting its own example; a six-field parse `CLAUDE.md` §5 never defines; +a bijection check, because verifying that cited lines resolve does not verify that every +drifted line was cited; a freshness rule; a two-artifact audit with a discount path; a +logical-pass / attempt / credited-count identity model to survive single-branch recovery; +edits to **four shipped hook reminder strings and their test assertions**, which instruct a +retry to delete the very slot the rider wanted preserved; and an append-only supersession row +in `docs/hardening-log.md` against a ledger row stating that dispositions never participate in +pass validation. + +**So §5's "companions are advisory; neither participates in pass validation" stays untouched.** +Nothing in this change narrows it — §2.3 carries that clause as an explicit kept-untouched row +rather than omitting it, since three drafts did narrow it and a reader needs to see it walked +back. A recording +mechanism is parked in §6 with its trigger. + +**(c) Squash-merge carry.** The shipped sentence, byte-for-byte: + +> **On squash-merge, copy every evidence entry and every human-exception record in the squash range into the squash body — the squash commit is the only body the merge carries into `main`'s history, so anything left behind is unreachable from it.** + +**And nothing more.** Earlier drafts specified which entry wins when a story has several in +range, defined an ancestor relation over the range to decide "latest", and made incomparable +entries stop the squash. That was a selection algorithm for a human copying text between commit +messages, and it generated findings in three consecutive passes. §5's existing rule already +governs evidence entries — each is revalidated before its close, and the closing message +carries the validated one — so this rider's only job is to say that squash does not exempt you +from carrying them. It says that. + +## 4. Sites + +Small, because **nothing that claims the gates are mandatory becomes false** once §2.0's scope +holds. + +| Path | Change | +|---|---| +| `CLAUDE.md` §5 Mechanics | §2.1's paragraph in full — it carries its own placement and carry sentences; rider (b)'s closed enum and reader rule; rider (c)'s carry sentence | +| `plugins/dev-workflow/commands/workflow-init.md` | The same edits to the inline §5 mirror — story AC 9, verified per §5.3 | +| `scripts/check-invariants.sh` | §5.2's closed-enum assertion, **plus** the script's own stale inventory: its header says "the two prompt-conformance checks" and its marked mutation procedure covers only 4a/4b. Both go to three, with `BEGIN/END check 4c` markers matching the existing convention | +| `scripts/check-invariants.test.sh` | **First**, `init_prompt_fixtures` — it creates no `CLAUDE.md` at all and gives `plugins/dev-workflow/commands/workflow-init.md` no template section, so adding 4c without extending it turns **every existing fixture repo** red on an unrelated baseline failure. It must build a valid bounded §5 region in both files. **Then** 4c's own reject/accept cases, and the recorded flipped-set mutation result the header's procedure requires | +| `plugins/dev-workflow/.claude-plugin/plugin.json` | **`version` 0.8.2 → 0.9.0** — invariant 12. Its `description` is untouched; the *file* is not | +| `plugins/dev-workflow/CHANGELOG.md` | The 0.9.0 entry | +| `todos.md` | Compound-commands row, occurrence 3; the `unverified-enforcement-claim` re-point; `prompt-vague-criteria` closed (§6) | +| `AGENTS.md` invariant 11 | Its count of the narrow checks in `scripts/check-invariants.sh` and their guarded-spelling inventory — currently "two narrow checks… a `Target model:` line… and a prose checklist-count claim". §5.2 makes it three. The invariant's calibration ("a floor, not coverage") is kept verbatim | +| `docs/superpowers/stories/2026-08-14-tier-2-same-family-reviewer-story.md` | Its live text assumed tier 3 ships. Amended with old-condition dispositions | +| `docs/superpowers/stories/2026-08-13-reviewer-availability-fallback-story.md` | Closure banner, reversed criteria, profile log | + +**No new file, and that is a deliberate reversal.** Draft two added a parity checker and its +suite; §5.1 records why that was withdrawn. Adding one is what would have pulled +`.github/workflows/ci.yml`, `AGENTS.md`'s Boundaries inventory and its exact checker-count +claims, the architecture tree, and the battery and lint rows into scope — a chain draft two +listed only half of. **That inventory churn is what withdrawing the checker avoids**, and +`.github/workflows/ci.yml` needs no edit because `scripts/check-invariants.sh` is already in +the invariant-check step. + +**`AGENTS.md` is not unchanged, though**, and an earlier draft said it was: invariant 11 states +that the checker carries "two narrow checks" and enumerates them, and §5.2 makes it three. That +one narrow count update is required; the tree, Boundaries and command rows are not. + +An earlier draft listed only the first two paths while requiring the rest in its backlog +section, and called the plugin manifest "checked and unchanged" against a required version +bump — an invariant-12 failure written into the design. + +**Checked for truth and unchanged, each verified rather than assumed:** `docs/getting-started.md`, +`docs/coding-workflow.md`, `docs/sparring-briefing.md`, `README.md`, +`.claude-plugin/marketplace.json`, `docs/pr-review-bots.md`. Every one was on the rejected +design's site list **because tier 3 falsified it**; with tier 3 withdrawn and §2.0's scope +excluding everything mandatory, each is true as written. *Unchanged in content* — distinct +from the row above, where the manifest's content claim is fine and its version is not. +(Verified as of the design's close, 2026-08-14. The salvage's shipping commit later edited +three of these — reviewer-model-selection docs in `docs/coding-workflow.md` and +`docs/sparring-briefing.md`, task scope outside this design's site list, and a stale +check-count correction in `README.md` — without touching the claims verified here.) + +## 5. Validation — what the profile's mode owes + +The story's mode is read fresh from its header (`battery+check` at the time of writing). Named +here rather than left in a story comment because a plan can otherwise implement the prompt +edits and skip the discriminating check (pass-1 finding 13). + +### 5.1 Two wrong answers first, because the second is more instructive than the first + +**Draft one** said the battery "already covers the two prompt copies' parity via +`scripts/check-invariants.sh`". False, and the defect class `AGENTS.md` names first. + +**Draft two** replaced it with a proposed new checker (`scripts/check-prompt-parity.sh`, never +written) extracting §5 from both copies and diffing them. Pass 3 killed it, correctly, on four counts: **the two §5 bodies +already differ materially** in hook-tool mapping, incident prose, citations and prose +exemptions, so a whole-section diff cannot pass without unrelated synchronization absent from +§4; stripping "leading template indentation" is lossy in Markdown, where indentation changes +list nesting and can make prose a code block; the fixture list required the source-extractor to +also parse runtime `Enum drift:` records, which it is not; and the counterfactual "run it at +`df850ab`" would have failed on those pre-existing differences rather than on the thing under +test. A new checker also drags `.github/workflows/ci.yml`, `AGENTS.md`'s Boundaries inventory, +its checker-count claims, the tree and two command rows behind it — churn pass 3 found +half-missing from §4. + +**And draft two's justification was itself an overclaim.** "No parity or mirror comparison +exists anywhere in this repository" is false: `scripts/check-invariants.sh` check 4b compares the +checklist item count in `docs/prompt-standards.md` against the copy in +`plugins/dev-workflow/commands/workflow-init.md`. Writing a categorical "anywhere" into the +section repairing an enforcement overclaim is the recursion this repo keeps producing. + +**The true statement is narrow:** no *textual* check compares `CLAUDE.md` §5 with its inline +mirror. Check 4b compares an item **count** between a different pair of files. + +### 5.2 The check that fails without the change + +**One assertion, added to `scripts/check-invariants.sh`** — no new file, no CI wiring, no tree +or Boundaries or command-row churn: + +> **The canonical line**, byte-for-byte, is: +> +> `Severity is one of exactly: BLOCKER | MAJOR | MINOR | NIT — no other token.` +> +> **Duplicates, both files:** it must appear **exactly once per file** — as a whole line, +> after stripping a leading blockquote marker and indentation, compared for **equality** and +> **case-sensitively**. Zero, two or more, a line that merely *contains* it, a title-case +> copy, a trailing space, an unreadable or missing file: all fail. +> +> **Placement, the command file only:** that one occurrence must sit inside the +> **`### 2.1` scaffold section**, terminated by the next **numbered** `### ` heading. Only +> that region is written into an initialized project, so a copy in the command file's own +> prose ships nothing. A missing or renamed anchor, or a duplicated one, **fails loudly** +> rather than skipping the rule — the safe direction. The repo's own `CLAUDE.md` gets no +> placement rule: the whole file is the artifact. + +**Amended twice, and the second amendment repairs the first.** + +**First (Gate-A pass 4 of the plan cycle):** this was a **bounded section-5 region** in both +files. That bounding needed fence nesting and template anchoring, and the parser was wrong in +three of the plan's four review passes — the last returning a multiple-end-boundary error on +the real command file, so the check could never have passed. It was replaced by a whole-file +count, described then as *"the same guarantee, stricter"*. + +**Second (Gate-A pass 5):** that description was **wrong, and the error is the gate-proof class +this repository names first.** Whole-file counting is stronger on duplicates and **weaker on +placement**: the canonical line can sit in the command file's own prose, outside the template +`/workflow-init` scaffolds, and the count is still 1. **Verified by running it** — appending +the line to the command prose left the checker green while an initialized project would have +received nothing. + +**So the contract is now priced exactly, one clause per guarantee.** Duplicates are fatal +file-wide in **both** files. Placement is enforced in the **command file** through the heading +range, because only the template region reaches users. The heading anchor is not the fence +parser returning: it terminates on the next **numbered** heading — the template carries its own +unnumbered `### Profiles` and `### Mechanics` subsections, which a next-`###` rule would +truncate on — and its state is one flag: set at the anchor, cleared at the first +numbered heading after it. **That terminator is itself checked** — it must be `2.2`. Without +that, renaming `### 2.2` to something unnumbered widens the range to `### 2.3` and a line +planted in the gap counts as inside the template. Verified: the battery stayed green. An +anchor rule that survives its own boundary drifting is worth nothing. + +**Its own failure mode is stated rather than assumed:** a missing, renamed or duplicated +`### 2.1` heading fails the check loudly. An anchor-based rule that skipped when its anchor +moved would be worth nothing, and that is the direction this had to get right. + +**Fixtures** — in `scripts/check-invariants.test.sh`, and **written, run and green**: 148 +assertions under both `sh` and `dash`, 123 before this change. The initializer came first: +`init_prompt_fixtures` built no `CLAUDE.md` and gave the command file no scaffold section, so +adding the assertion without extending it would have turned **every existing fixture repo** red +on a baseline unrelated to its own assertion. + +The 25 cases, by what each discriminates: + +| Group | Cases | +|---|---| +| **Presence** | both copies stating it (accept) · absent from `CLAUDE.md` · absent from the command file · absent from both | +| **Duplicates** | twice in one file · two copies on one physical line | +| **Equality** | blockquoted (accept) · indented (accept) · leading text · trailing text · trailing space · title-case copy · paraphrase | +| **Placement** | outside the scaffolded template (**the exploit whole-file counting alone permitted**) · inside it (accept) · after an unnumbered subsection (accept — a next-`###` terminator would wrongly reject this) · missing `### 2.1` anchor · duplicate anchor · `CLAUDE.md` needing no anchor (accept) | +| **Terminator** | unnumbered terminator · **a line planted in the widened gap** (the second verified exploit) · absent terminator | +| **Fail-closed** | missing file · unreadable file (skipped as root, which satisfies `-r` on mode 000) · parser failure, through the suite's `inject_case` PATH seam keyed on the `sev-canon-count` marker | + +**Mutation evidence, re-measured after every fixture change** (`4a` 20 · `4b` 22 · `4c` **19** +— 18 reject fixtures plus the parser case; no accept case moved in any of the three). 4c +measured 13 before the placement cases and 16 before the terminator ones; both were superseded +by re-running, never by extrapolation. The re-run found a real regression this work +introduced: `checklist parser failure fires` greps the checker output for the bare +`parser failed`, which 4c's diagnostic also ends in, so that fixture had stopped testing 4b — +deleting the 4b block left it green, and the count came back 21 against a recorded 22. Pattern +tightened to `checklist parser failed`; count restored. **Four review passes read that shell +without finding it; one mutation run did.** + +**Rider (b)'s behavioural half is a named verification, not a test** — and its evidence is +already in hand: PR #23's Gate-B pass 3 returned four findings at `IMPORTANT` and they were +accepted and filtered by interpretation. That is an observation of the prior state, not a +hypothesis about it. **The after-state is unobserved**, and §8 says so rather than the design +implying a symmetric result. + +### 5.3 Parity — an instruction, and what it is worth + +§5's two copies are not byte-identical today and this change does not make them so. So AC 9's +"agree after the change" means what a person actually does: **edit both copies, diff the +regions you edited, and record in the commit body that you did and that they matched.** + +**Nothing checks this.** There is no mechanical parity check for `CLAUDE.md` §5 and its inline +mirror, this design does not add one, and §4 does not claim one. §5.2's assertion covers +exactly one line of the two copies — the severity enum — and nothing else. + +Three drafts specified more: an enumerated block table with sentence anchors, an occurrence +count per block, and a paragraph-structured normalization to compare across differing line +wrapping. It was a procedure for a human diffing two Markdown files, it was wrong twice about +the repository (a claimed indentation that does not exist, anchors that did not resolve), and +it produced findings in three consecutive passes. Anyone wanting a real parity checker must +first decide what the two copies are *supposed* to share, since they legitimately differ today +— §6 parks that. + +**Battery** — the `AGENTS.md` quality command, green, including check-invariants with the new +assertion and its regression cases. + +**Prompt conformance** — all 12 items of `docs/prompt-standards.md` for both changed prompt +copies. + +## 6. Backlog — the triggers stay parked + +- **Attribution for the shipped record form** — the handle in a human-exception record is + unverified, and §2.1 says so. *Trigger: the first record whose authorship is disputed or + unattributable.* This is hardening for what ships and needs no availability attestation. +- **External-authority zero-pass research** — signed commit or protected-branch approval, as + the one untried class. *Trigger: a renewed need to close a gate cycle with no review — + a second multi-day reviewer outage, or the operational bridges of §7 proving unavailable.* + Kept separate from the row above because they are different problems: one hardens + attribution on an optional-work note, the other reopens a rejected design. §1.5 is why external authority is **one untried direction worth + reconsidering** rather than the only one that could work — the design space was never + exhausted. Whoever takes it inherits the named requirements: a trusted signer identity, a + role policy saying who may approve, and an availability attestation from someone other than + the author. +- **Tracked re-review debt.** *Trigger: a human explicitly asks for follow-up review on a + recorded exception and that follow-up is later found not to have happened.* Stated as an + observable event because the shipped form creates no follow-up obligation, so "never + happened" would otherwise never become true (pass-1 finding 15). +- **A recording mechanism for severity normalization.** Rider (b) normalizes an unrecognized + token to `MAJOR` and records nothing; §3 explains why the drift record was cut — the findings + file carries the original token verbatim, so the drift is **visible to the reader at the + moment the pass is validated**, and the companion duplicated that at the cost of an identity + model, an audit, four hook-message edits and a ledger supersession. No permanence is claimed + for either: `.context/` is git-ignored and slot collisions have destroyed findings here. *Trigger: a pass is normalized and the drift goes unnoticed in review.* Whoever + takes it starts from the cost §3 records — an identity model, an audit, four hook-message + edits and a ledger supersession — rather than rediscovering it. (Per-pass dispositions exist + under `.context/codex-reviews/` while this worktree lasts, but §3 is the durable statement; + see §1.2 on why those paths are not citable.) +- **The hook's `is_docs_only` exempts any `.md` path outside a prompt directory**, broader than + §5's prose list. *Trigger: a root `.md` file acquiring gate-relevant state.* +- **Gate-cycle slot collision — trigger fired, row stays open.** This story's cycles destroyed + a predecessor's findings file and dispositions before the surviving artifacts were archived + by hand. §1.3's third failure mode is downstream of the same weakness. +- **Tier-2 counting and containment**, pointing at the tier-2 story (§4). +- `todos.md`: **occurrence 3** on the compound-commands row (story AC 8) — `git add` and + `git commit` in one Bash call, empty staged set at `PreToolUse`, loose STOP; observed on PR + #23's close. Same shape as occurrence 2 and, like it, a **false positive**. The existing item + is **edited in place**; append-only-never-edit is `docs/hardening-log.md`'s rule. +- `prompt-vague-criteria` closes. `unverified-enforcement-claim` **stays open**, re-pointed at + the hook story. +- Version **0.8.2 → 0.9.0** with a `plugins/dev-workflow/CHANGELOG.md` entry — invariant 12. + **Verified** by `scripts/check-version-bump.sh` against the PR's base *after* the WIP commit + carries both the plugin edits and the manifest bump; run before then it reports clean, + uselessly. + +## 7. Where the stall problem goes + +The five-day quota stall (2026-08-05 to 2026-08-10) is **not solved by this change**, and +saying so is part of closing the story honestly. §2.0's scope means the exception form does +not reach it either — that was the first draft's error. It routes two ways: + +- **Operational bridges** — a second API key, or an alternate vendor. This is the answer that + restores review rather than removing it, and it needs no design permission. +- **The tier-2 story**, if its same-family containment proves buildable — a weaker review is + still a review, which is categorically different from none. + +If neither is available, the honest answer is that work on gated changes stops until the +reviewer returns. That is a real cost, and §1 is the record of three attempts to avoid paying +it. + +## 8. What this does not do + +- **Nothing enforces the exception record.** No hook recognizes, parses or validates + `Human exception:` as an exception record, and nothing checks the handle. It is a convention + read by a human, and §2.1 says so in the shipped text. (Corrected as of plugin 0.9.1: an + earlier draft said "no hook fires on it", which is false — a record-only empty commit can + still draw a Gate-B reminder. The hook does receive the commit command; what it never does + is interpret the record.) +- **Nothing detects an unrecorded exception.** A human proceeding silently is exactly as + invisible after this change as before it. +- **The gates remain waivable in practice by anyone willing to ignore them**, as they were + before — §5's mechanisms were always advisory. §1 establishes that no *sanctioned* path was + found; it does not establish that the unsanctioned one closed. +- **The record could still be misread as permission** by a reader who takes the form and skips + the scope. §2.1 puts the scope in the shipped paragraph for that reason, but prose cannot + prevent selective reading. +- **Rollback is forward, not a revert to a green tree.** The prompt edits and §5.2's + closed-enum assertion are coupled in one direction: reverting the prompt lines while the + assertion stands makes the invariant check fail. So a rollback removes **both** — the two + prompt copies' edits, the assertion, its regression cases, **`AGENTS.md` invariant 11's count + back to two, and the checker's own inventory comments and mutation procedure with it** — + plus a `plugins/dev-workflow/CHANGELOG.md` entry + and a **forward** version bump, never a decrease. `docs/hardening-log.md` is append-only, so + anything landed there is superseded rather than deleted. Installed copies live under + version-keyed cache paths and downstream `/workflow-init` copies are the user's own files, + so both follow the CHANGELOG rather than the revert — the standing cost of invariant 8's + inline templates, not something this change introduces. diff --git a/docs/superpowers/stories/2026-08-13-reviewer-availability-fallback-story.md b/docs/superpowers/stories/2026-08-13-reviewer-availability-fallback-story.md new file mode 100644 index 0000000..7659e6a --- /dev/null +++ b/docs/superpowers/stories/2026-08-13-reviewer-availability-fallback-story.md @@ -0,0 +1,333 @@ +# Reviewer-availability fallback — no sanctioned gate waiver; an optional-work record — Story + +**Date:** 2026-08-13 · **Size:** story +**Risk:** standard · **Security:** none · **Validation:** battery+check + +**Profile log:** +- 2026-08-14 · axis change · risk ↓, security ↓ · the closure: the `high` rationale named a sanctioned fail-open path, which design §2.2 now says explicitly does not exist, leaving §5 prompt discipline — the hardening-round precedent class, with its recorded delimitation; and the trust relationship "who reviews" left with the ladder, so a record form touches no asset, boundary, role or external system · drops both lens sets; mode recomputes to `battery+check` + +> **CLOSED 2026-08-14 with a negative answer, and a small salvage.** Three design cycles and +> nine Gate-A passes (303 findings, none dismissed) found **no safe design for a sanctioned +> zero-pass gate closure** in the three classes tried, each failing for a recorded structural +> reason. That finding is this story's result, in +> `docs/superpowers/specs/2026-08-14-reviewer-availability-fallback-design.md` §1 — which is +> explicit that this is repeated structural failure, **not a proof of impossibility**: a class +> resting on authority outside the repository was never tried. +> +> What ships is the **record form** of a human exception (design §2), plus riders (b) and (c), +> which carried real discriminating checks throughout. **The form does not reach the stall** +> — it is scoped to things §5 never required, so it cannot close a gate cycle (design §2.0). +> Gated work blocked by an unavailable reviewer routes to **operational bridges** (a second +> key, another vendor) or to the tier-2 story if its containment proves buildable; if neither +> is available, work on gated changes stops until the reviewer returns (design §7). +> +> **The profile moved with the closure** (axis change, 2026-08-14, logged): the `high` risk +> rationale named a sanctioned fail-open path that §2.0 now says does not exist, and the +> "who reviews" trust relationship left with the ladder. What remains is §5 prompt discipline. +> Both lens sets drop; the derived mode is what the header now carries. + + + +## 1. Problem statement + +Both review gates depend on a single external reviewer, and there is no sanctioned +degraded path when it is unavailable. §5 requires a **minimum of three passes** per gate +with only the **final** pass clean, and permits an early exit below three only on a pass +returning zero findings; when the vendor is out of quota, no pass can be taken at all, so +no cycle can close and all work stops. +Observed: a five-day full process stall, 2026-08-05 to 2026-08-10, from one vendor's +quota. + +The one degraded mode that exists is `/workflow-init` §2.13, and it does not reach this +case: it fires at **init time** when the preflight finds Codex unconfigured, and its +answer is to scaffold a project that is explicitly gateless. A project already running +the gates that loses its reviewer mid-flight falls outside it. + +So the gap is narrow and specific: **there is no authorized way to close a gate cycle +when no reviewer can run.** Work stops, and the only alternative reachable today is to +abandon the gates entirely — which §2.13 offers a *new* project and offers no one else. + +*(Amended 2026-08-14, fifth amendment. **Kept, and still true** — every sentence of this +problem statement, including the five-day incident and §2.13's failure to reach the +mid-flight case. The problem is real and is **not solved by this story**. What changed is the +answer: three design cycles established that the authorized closure this section asks for +cannot be built safely here, so the gap is now a **stated limitation** rather than an open +requirement, and the stall routes to operational bridges or the tier-2 story, and if neither +is available, work on gated changes stops until the reviewer returns — design §7.)* + +> **Amended 2026-08-14 (second amendment) — Gate-A pass 1 findings 21 and 22.** The first +> amendment claimed tier 2 moved "in its entirety" and left this section still framing the +> problem as *"the workflow has no vocabulary for a review that happened but was weaker +> than tier 1"*. That was untrue of the text, and the claim is withdrawn. +> **Old-condition accounting for this section:** **kept** — the availability stall, the +> five-day incident, and §2.13's failure to reach the mid-flight case. **Moved by name** — +> the four-prohibition-site survey and the `docs/sparring-briefing.md` family-granularity +> premise, to +> `docs/superpowers/stories/2026-08-14-tier-2-same-family-reviewer-story.md`, which is the +> only story whose fallback those sites forbid; a human exception is not a model reviewing +> its own work. **Overturned** — the "no vocabulary for a weaker review" framing, which +> described tier 2's problem, not this story's. This story's problem is that a cycle +> cannot be *closed* at all. + +## 2. Desired outcome + +~~A reviewer-availability ladder exists for **both** gates, with each tier's standing stated +and its degradation visible in the durable record~~ — **the ladder is overturned** (below); +what survives is the second half of that sentence, applied to a human's decision rather than +to a tier. The original outcome is kept in place so the reversal is legible: + +- **tier 1** — cross-model, a different model family than the implementer. Today's + normal, unchanged, and the only tier that satisfies a gate without qualification. +- [~] ~~**tier 3** — the human exception, mid-flight: a zero-pass closure, a gate waiver + authorized by a human, disclosed in the durable record~~ — **OVERTURNED 2026-08-14, fifth + amendment.** Recorded rather than deleted, because a criterion that vanishes is + indistinguishable from one never written. **Old-condition accounting:** **overturned** — + that a human may *authorize* a zero-pass closure at all, since three cycles established the + authorization cannot be given the properties it needs (design §1.3: every control landed + unenforceable or recursive; the outage that justifies the waiver is manufacturable by + whoever benefits; the preconditions cannot be established in the case they exist for). + **Kept, and now the whole of what ships** — that when a human makes an exception, the + decision is **written into the durable record** and identifiable from `main` alone. What + reversed is the direction of the claim: the record no longer certifies the decision, it + preserves it. **Kept, untouched** — §2.13's init-time gateless answer. **Moved** — nothing; + there is no third story. + *(The two earlier amendments to this bullet — second amendment / pass 1 finding 21, which + overturned "tier 3 exists today for the init-time case", and fourth amendment / pass 1 + finding 14, which narrowed "re-review debt" to "untracked re-review obligation" — are + subsumed by this one. Both concerned a tier that no longer ships.)* + +~~Degrading is a **fail-open-with-disclosure**: work continues, and the weakening is +recorded rather than silently absorbed.~~ — **OVERTURNED with tier 3.** There is no +degraded mode left to fail open into: §5's gates are unchanged in every particular, and +nothing this story ships lets work continue past a gate. **Kept in full, and it is the +reason the salvage is worth shipping at all** — the failure this must not produce is a +weakened cycle indistinguishable from a normal one after the fact. Design §2.1 answers +that for the one case that remains: a human who decides to proceed leaves a record saying +so. **Kept** — that this is the false ✓ the current prohibition exists to prevent. + +The change carries two riders and one backlog append that amend the same §5 region, so one +Gate-B cycle covers all of it. **After the closure these are the majority of the change**, +not its trim. + +> **Amended 2026-08-14 — the ladder narrows from three tiers to two.** Gate-A spec passes +> 1–3 returned 116 findings, none dismissed, with blockers rising; the cycle was stopped +> under §5's stuck condition. **Old-condition accounting for the three-tier framing:** +> **kept** — tier 1's standing, the fail-open-with-disclosure principle, and the +> requirement that degradation be visible in the durable record. **Moved by name** — tier 2 +> in its entirety, to +> `docs/superpowers/stories/2026-08-14-tier-2-same-family-reviewer-story.md`, carrying +> pass-3 blockers 2, 3 and 4 as opening evidence and the sanitized-external-checkout +> direction as its design question. **Narrowed** — "each tier's standing" now covers two +> tiers rather than three. Nothing was dropped: tier 2 is deferred, not abandoned, and the +> reason is that every stopping blocker except rider (a)'s belonged to it. + +## 3. Acceptance criteria + +- [~] ~~§5 documents a **two-tier** reviewer ladder covering both Gate A and Gate B, naming + for each tier what it is and whether a cycle closed there satisfies the gate~~ — + **OVERTURNED 2026-08-14, fifth amendment.** No ladder ships: there is one tier, the + existing one, and §5's account of it is unchanged. + *(**Kept** — that §5 must state plainly what does and does not satisfy a gate, which it + already does and which the salvage is careful not to blur: design §2.2 says in terms + that the exception record is not a waiver, clears no floor, and satisfies no gate. + **Overturned** — the ladder, the tier vocabulary, and "whether a cycle closed there + satisfies the gate", which presupposes a tier where one could.)* +- [ ] A **recorded human exception** is identifiable from `main`'s history alone — the + closing commit body carries it, without reference to the session that produced it. + *(Amended 2026-08-14, fifth amendment. **Kept in force, narrowed in subject** — the + requirement that a weakening be visible in the durable record is the one thing that + survives the closure intact, and design §2.1 delivers it. **Narrowed** — the subject + is a human's recorded decision rather than "a degraded cycle", because no degraded + cycle exists to identify. **Kept** — no reference to the producing session; the record + stands alone. Earlier amendments moved this from the pass to the cycle; this one moves + it from the cycle to the decision.)* +- [ ] Every site currently forbidding a same-model fallback is accounted for explicitly + — `/workflow-init` §2.13, `docs/coding-workflow.md` § *The two gates…*, `README.md`, + `plugins/dev-workflow/agents/finding-triage.md`, and + `docs/sparring-briefing.md`'s family-granularity premise — with each condition + marked kept, narrowed, or deliberately overturned per the AGENTS.md decision-procedure + Don't. No site is left asserting the ladder is forbidden. + *(Amended 2026-08-14. **Moved by name** — to the tier-2 story in its entirety. The + four prohibition sites forbid a **same-model reviewer**, which tier 3 is not: a + human exception is not a model reviewing its own work. So this change leaves their + wording untouched, and the narrowing principle ships only insofar as tier 3 needs + it. **Kept** — the accounting obligation itself, which the tier-2 story inherits + unchanged.)* + *(Amended again 2026-08-14, third amendment, Gate-A spec pass 3 finding 27 — the + amendment above was **partly false and is corrected here**. + `docs/sparring-briefing.md` does not only state a same-family premise. Lines 41–44 + also say: "**Advisory, never exempt.** … Do not design around the gates, and do not + treat a satisfied human as a substitute for a clean pass." **Tier 3 is exactly a + satisfied human substituting for a clean pass**, so those clauses are this story's + to face, not the tier-2 story's. Corrected accounting: **moved by name** — only the + same-family premise, to the tier-2 story; **overturned here, explicitly** — the + never-exempt and human-substitution clauses, which this change contradicts and must + therefore amend in that file rather than leave standing.)* + *(Amended a fourth time 2026-08-14, fifth amendment, Gate-A pass 2 finding 17 — the + third amendment is **overturned**. It was correct about tier 3: a satisfied human + closing a gate *was* exactly what `docs/sparring-briefing.md` lines 41–44 forbid, so + those clauses would have had to be amended. **Tier 3 does not ship.** Under design + §2.0's scope the record form applies only to things §5 never required, so it never + substitutes a satisfied human for a clean pass. **Kept, unchanged, and now relied + upon** — "Advisory, never exempt… do not treat a satisfied human as a substitute for a + clean pass", which design §4 lists among the files verified true as written. **Kept** — + the same-family premise's move to the tier-2 story, which the closure does not touch.)* +- [~] ~~Gate B's tier 2 states its own mechanism, named separately from Gate A's~~ — + **MOVED 2026-08-14** to + `docs/superpowers/stories/2026-08-14-tier-2-same-family-reviewer-story.md`. Recorded + here rather than deleted, because a criterion that disappears is indistinguishable + from one that was never written. Nothing of it is dropped: the asymmetry it names + (`exec` reviews passed text, `review` reads a git range) is the tier-2 story's + opening constraint, alongside pass-3 blocker 2, which found the mechanism it called + for is not achievable in-repo. +- [~] ~~Rider (a): sequential single-branch gate calls are the documented default~~ — + **MOVED 2026-08-14** to + `docs/superpowers/stories/2026-08-14-sequential-branch-calls-hook-story.md`. Pass-3 + blocker 1 is why: the unchanged hook counts **each** call as a pass, so the rider + double-counts the floor in **tier-1 normal operation**. It is a hook change, not the + §5 prose edit it was bundled as. Nothing dropped — the row's evidence (PR #23's + sixteen consecutive single-branch calls) moves with it, and its `todos.md` row + re-points there. +- [ ] Rider (b): the finding-line severity is stated in §5 and in `/workflow-init`'s + template as a **closed set of permitted tokens** for what the prompt demands of the + writer, not shown by example only; and the **reader** normalizes an out-of-enum + token to `MAJOR`. + *(Amended 2026-08-14, Gate-A spec pass 2 finding 23. Old-condition accounting — + **kept**: the enum is closed, stated rather than exemplified, in both §5 and the + template; **narrowed**: "closed" binds the writer's prompt, not the reader's + acceptance rule. Reason: discarding a well-formed pass over a token would have + thrown away four real findings on PR #23, which is the incident this rider exists + for.)* + *(Amended again 2026-08-15, Gate-A spec pass 4. **Overturned** — "with the mapping + recorded in that pass's dispositions". Its premise was that normalization would + otherwise be silent, and pass 4 falsified that: the findings file preserves the original + token verbatim on the finding line, so the drift is visible to the reader at the moment + the pass is validated — which is when it matters. No permanence is claimed for either + artifact: `.context/` is git-ignored and slot collisions have destroyed findings in this + repository, so the companion bought durability the rest of the system does not provide. **Kept** — the closed enum, the reader's normalization, and the + PR #23 reason, which are the whole of the rider. Design §6 parks a recording mechanism + with its trigger; §3 records what the cut clause cost across four passes.)* +- [ ] Rider (c): §5 Mechanics states that on squash-merge the evidence entry is carried + into the squash body explicitly, because main's tip is the durable record. +- [ ] The compound-commands row in `todos.md` carries occurrence 3 — `git add` and + `git commit` in one Bash call, empty staged set at PreToolUse, loose STOP — + observed on PR #23's close. +- [ ] §5 and its `/workflow-init` inline-template mirror agree after the change. + *(Amended 2026-08-15, Gate-A spec pass 5 finding 10, corrected at pass 9 finding 6. + **Narrowed** — "agree" means the regions this change edits are identical in both copies: + edit both, diff those regions, and record in the commit body that they matched. + **Overturned** — a whole-section reading, since the two §5 copies are not byte-identical + today and this change does not make them so. **Also overturned** — an interim version of + this amendment promised enumerated anchors and an extract-and-diff procedure in design + §5.3; that procedure was **cut** at the termination assessment as prose specifying prose, + so citing it would demand a mechanism the final design rejects. **Kept** — the obligation + that the two copies not diverge on what this change writes, and the explicit admission + that nothing checks the claim. **Dropped** — nothing.)* + +## 4. Affected AGENTS.md invariants + +- `## What this project is` — "two independent cross-model review gates (a different + model reviews the design at Gate A and the diff at Gate B)" — ~~tier 3 qualifies this + sentence by making the gate waivable~~. *(Amended 2026-08-14, fifth amendment, Gate-A pass 2 + finding 18. **Overturned** — no qualification ships. The sentence stays **true and + unchanged**: nothing in the salvage makes a gate waivable. *(The `AGENTS.md` **file** does + change — invariant 11's count of the narrow checks in `scripts/check-invariants.sh` goes from + two to three, design §4. The quoted cross-model-gate sentence is what stays true and + unchanged; an earlier version of this bullet said the file needed no edit at all.)* **Kept** — that this was the invariant the + withdrawn design would have had to amend, which is why it is still listed here.)* +- `## What this project is` — "**The product is prompts.** … There is no application + code, so there is no typechecker to catch a defect; review and + `docs/prompt-standards.md` are the only gates a prompt passes through." — why + weakening review costs more here than in a project with a compiler. +- `## Architecture` (Dependency direction) — "on a Codex MCP server exposing both + `exec` and `review` (the gates key on those two tool names)" — that single dependency + becoming unavailable is the whole motivating condition. +- `## Key invariants` → Hook, 1 — "**The hook always exits 0.** It is advisory; a + reminder that can fail closed would make the workflow unusable whenever Codex is down + or the environment is odd." — already names the motivating condition. +- `## Key invariants` → Hook, 2 — "**Loose in the firing direction.** On uncertainty, + fire. A missed commit (false ✓) is the dangerous direction" — the fail-open half of + the pattern must not become a false ✓. +- `## Key invariants` → Prompts and scaffolding, 11 — "**Prompt changes pass + `docs/prompt-standards.md`** — all 12 checklist items, for any skill, command, agent + definition, hook message, or scaffolded template." +- `## Don'ts` — "**Never replace a decision procedure without accounting for its old + conditions.** List what the previous prose required, then mark each one kept, moved, + or deliberately dropped." — the governing invariant for this change. +- `## Don'ts` — "**Never describe what a gate proves without checking what it actually + compares.**" +- `## Don'ts` — "**Never rename or delete a doc section without grepping for references + first.**" — `README.md` links the `docs/coding-workflow.md` anchor + `#the-two-gates-and-why-independence-is-the-point`. + +## 5. Open questions + +- None remain open for this story. All five original questions were settled during design + or moved with tier 2. + +*(Amended 2026-08-14, second amendment, Gate-A pass 1 finding 22. The five original +questions are accounted for individually rather than deleted — a question that vanishes is +indistinguishable from one never asked.)* + +- **Settled** — "what exactly does the disclosure look like in the closing commit body, + and how does it sit beside the evidence entry": answered by the design's marker schema + and carry chain, which carry both. +- **Settled** — "does the ladder extend §2.13's degraded mode or stand beside it": it + stands beside it; §2.13 is untouched. +- **Settled, then reshaped, then narrowed, then moot** — "does a `high`-risk profile owe a + cross-model final pass once availability returns": moot as of the fifth amendment, since no + closure ships that skips a review. The obligation had no owner once the debt machinery went; + now it has no subject either. Design §6 keeps the tracked-debt row parked with its trigger, + so the question returns if a recorded exception ever proves to have gone un-followed-up. + *(Amended 2026-08-14, fourth amendment, Gate-A spec pass 1 finding 14, then subsumed by the + fifth. The fourth overturned "the profile scales the repayment" — no repayment operation + existed for a profile to scale. The fifth removes the closure the obligation attached to.)* +- **Moved by name** — "how does the hook treat a non-Codex reviewer pass", to the tier-2 + story. Tier 3 takes no passes, so the hook has nothing to treat. +- **Moved by name** — "is the same-model prohibition narrowed or overturned", to the + tier-2 story, together with the four sites it governs. + +## 6. Suggested size + +`story` — one coherent decision: may a human close a gate cycle when no reviewer can run, +and what must be true when they do. Everything else (the disclosure schemas, the carry +chain, the re-review debt) follows from that one answer. + +*(Amended 2026-08-14, fifth amendment. **Kept** — the sizing judgement and the framing: one +coherent decision did drive everything else, and the answer turned out to be **no**, which +collapsed the rest exactly as this section predicted it would follow. **Narrowed** — what +remains is well under a `story` as a *decision*, though not as a diff: design §4's path table +is the actual surface, and it is not two files. Describing the residue as "two prompt edits and +a paragraph" understated it and is withdrawn (Gate-A pass 4 finding 19). It is **not** +re-sized down, because the decision that produced it cost three design cycles and nine Gate-A +passes, and a `patch` label on the residue would misrepresent what was decided. The size +records the question, not the diff.)* + +*(Amended 2026-08-14, second amendment, Gate-A pass 1 finding 22. **Overturned** — the +original sizing, which was framed around "what does a tier-2 pass mean" and named a split +condition for Gate B's tier-2 mechanism. **Kept** — the `story` size itself and the +judgement that one coherent decision drives the rest. The split it anticipated has since +happened twice, to the tier-2 and sequential-branch-calls stories.)* diff --git a/docs/superpowers/stories/2026-08-14-sequential-branch-calls-hook-story.md b/docs/superpowers/stories/2026-08-14-sequential-branch-calls-hook-story.md new file mode 100644 index 0000000..c7d757e --- /dev/null +++ b/docs/superpowers/stories/2026-08-14-sequential-branch-calls-hook-story.md @@ -0,0 +1,97 @@ +# Sequential single-branch gate calls double-count the floor — Story + +**Date:** 2026-08-14 · **Size:** story +**Risk:** high · **Security:** none · **Validation:** battery+check+verification + +## 1. Problem statement + +`CLAUDE.md` §5 prescribes one file per findings branch because `reviewType: full` runs the +spec and quality reviewers in parallel from one `additionalContext`. On PR #23's Gate-B +pass 1 both reviewers wrote **both** paths: the spec branch's seven findings were lost, and +**every acceptance condition still passed** — terminator present, count matching, nothing +but finding lines, both branch files present — because all four are *shape* checks and +provenance sits outside them. + +The recorded fix candidate was to make **sequential single-branch calls** the documented +default (`reviewType: spec`, then `quality`), eliminating the concurrency rather than +detecting it. Evidence for it is real: sixteen consecutive single-branch calls across +passes 2–9 of that cycle, no recurrence. + +**That fix cannot ship as a §5 prose edit, and the reason was found at Gate A.** The hook +increments its pass counter on **each** `mcp__codex__review` call. Two sequential calls are +two counted passes, so three logical passes become six counted ones, and the hook can +report the three-pass floor satisfied **before a pair's quality branch exists**. That is a +new mechanical false ✓ — and it lands in **tier-1 normal operation**, not in any degraded +mode. + +So the current situation is a choice between two defects: keep `full` and keep a +demonstrated data-loss path, or adopt sequential calls and create a counting defect. The +prose default cannot resolve it because the counter is in the hook. + +Found at Gate-A spec pass 3 of the reviewer-availability fallback cycle +(`.context/codex-reviews/gate-a-spec-pass-3.md`, blocker 4), which was stopped under §5's +stuck condition partly because of it. + +## 2. Desired outcome + +A Gate-B pass made of two sequential single-branch calls is **counted once**, so the floor +means what it says. Whichever way that is achieved, the observable result is: + +- Sequential single-branch calls are safe to make the documented default, closing the + `reviewType: full` two-writer data-loss path without opening a counting one. +- The hook credits a **completed branch pair**, not an individual call, or the two loops + are separated in a way that is mechanically supported rather than described. +- A partially completed pair — spec branch done, quality branch not yet run or failed — + never reads as a satisfied floor. +- The existing `full` behaviour keeps working for anyone who uses it, or is refused + explicitly rather than silently miscounted. + +## 3. Acceptance criteria + +- [ ] A Gate-B cycle of three logical passes made as sequential single-branch calls causes + the hook to report **three** passes, not six. +- [ ] A pass whose second branch has not completed does **not** count toward the floor, and + the hook's message distinguishes that state from a satisfied one. +- [ ] `reviewType: full` continues to behave as it does today, or the hook/§5 refuses it + with a named cause — no silent miscount either way. +- [ ] The regression suite covers the pair state machine: both branches succeed, first + fails, second fails, branches interleaved with a commit, and a `full` call. +- [ ] The hook still exits 0 on every one of those paths, and remains POSIX `sh` with `jq` + optional. +- [ ] `CLAUDE.md` §5 and `/workflow-init`'s inline mirror agree after the change. +- [ ] `docs/hardening-log.md`'s `unverified-enforcement-claim` row and the `todos.md` row + that pointed at the parent story re-point here. + +## 4. Affected AGENTS.md invariants + +- `## Key invariants` → Hook, 1 — "**The hook always exits 0.** It is advisory; a reminder + that can fail closed would make the workflow unusable whenever Codex is down or the + environment is odd." +- `## Key invariants` → Hook, 2 — "**Loose in the firing direction.** On uncertainty, fire. + A missed commit (false ✓) is the dangerous direction; a redundant warning is the accepted + price." — the defect is squarely in the dangerous direction. +- `## Key invariants` → Hook, 3 — "**Gate-B validity is content-derived, never + event-derived.**" — pair binding must not become an event-derived check. +- `## Key invariants` → Hook, 4 — "**POSIX `sh`, and `jq` is optional.** No bash-isms; + correct behaviour via fallback parsing when `jq` is absent." +- `## Key invariants` → Packaging, 12 — "**A plugin change requires a version bump.**" +- `## Don'ts` — "**Never describe what a gate proves without checking what it actually + compares.**" + +## 5. Open questions + +- Does the hook bind a pair by `sessionId`, by an explicit pair marker the caller writes, or + by something content-derived? An event-derived binding would violate invariant 3's spirit + even while fixing the count. +- Is `full` deprecated, refused, or left working? Deprecating it closes the original + data-loss row; leaving it working keeps that row open. +- Does the fix change the *floor semantics* (three pairs) or the *counting* (three + increments)? These are different changes with different failure modes. +- Gate A makes only `exec` calls and has no branches — is anything owed there, or is this + strictly Gate B? + +## 6. Suggested size + +`story` — one coherent defect in one executable artifact, with a bounded state machine and +an existing regression suite to extend. It is genuinely hook work, which is why it could not +stay bundled with the prose change that surfaced it. diff --git a/docs/superpowers/stories/2026-08-14-tier-2-same-family-reviewer-story.md b/docs/superpowers/stories/2026-08-14-tier-2-same-family-reviewer-story.md new file mode 100644 index 0000000..77413ba --- /dev/null +++ b/docs/superpowers/stories/2026-08-14-tier-2-same-family-reviewer-story.md @@ -0,0 +1,158 @@ +# Tier 2 — a fresh-context same-family gate reviewer — Story + +**Date:** 2026-08-14 · **Size:** story +**Risk:** high · **Security:** high · **Validation:** battery+check+verification+abuse-path + +## 1. Problem statement + +> **Amended 2026-08-14 — the parent story closed, and tier 3 does not ship.** +> `docs/superpowers/stories/2026-08-13-reviewer-availability-fallback-story.md` was closed +> with a negative answer after nine Gate-A passes: no safe sanctioned zero-pass closure was +> found, and its design records why each class failed +> (`docs/superpowers/specs/2026-08-14-reviewer-availability-fallback-design.md` §1). +> **Old-condition accounting for this section:** **kept** — the problem, unchanged and now +> sharper: when the cross-model reviewer is unavailable there is no reviewing fallback at +> all. **Overturned** — the premise that tier 3 exists as the floor beneath tier 2, and with +> it "tier 3 unblocks work", which nothing now does. **Kept** — that a same-family reviewer +> would sit between tier 1 and nothing, which is the case for building it. + +When the cross-model reviewer is unavailable, there is **no fallback that reviews anything**. +Tier 1 (cross-model) is the only tier that ships, and below it the cycle simply does not +close. Between them sits the tier the parent story was originally designed around and could +not ship: a **fresh-context same-family reviewer** whose passes actually review something. + +Nothing unblocks work today. Tier 2 would review — worse than tier 1, better +than nothing — and `docs/prompt-standards.md` line 136 records that fresh-context verifier +subagents outperform self-critique. That note grounds *tier 2 beats nothing*; it has never +grounded *tier 2 ≈ tier 1*. + +Tier 2 was cut from the parent cycle after Gate-A spec passes 1–3 (116 findings, none +dismissed) established that **its containment is not achievable in-repo**. The three +blockers are the opening evidence, not a blank page: + +- **Pass-3 blocker 2 — the trust boundary does not exist.** A custom agent definition does + not prevent the repository's `CLAUDE.md` hierarchy loading as instructions, and Claude + Code offers no per-agent switch to omit it. So the prompt product under review can + address its own reviewer *before* that reviewer treats anything as data. An agent + definition improves the tool allowlist; it never creates the boundary. +- **Pass-3 M14 — Bash is a capability, not an instruction.** The reviewer needs a git range + to review a diff, which needs Bash; Bash in the main checkout grants arbitrary repository + writes, process execution and environment reads. A reviewer that can alter the artifact it + certifies, while its findings file looks clean, is a worse failure than no reviewer. +- **Pass-3 blocker 4 and pass-2 blocker 1 — capability and input are entangled.** Read/Grep/ + Glob cannot compute a git range; handing over pasted diff text has no truncation + detection, so a clean findings file can certify a lossy diff. Each fix breaks the other. + +The direction to explore is a **sanitized external checkout** — a reviewer surface that does +not load the reviewed repository's instructions and cannot write to it — but that is a +design question, not a decided answer, which is why this is a story and not a patch. + +## 2. Desired outcome + +A same-family reviewer can take gate passes that are **worth counting**, under a containment +story that is actually true: + +- The reviewer does not load instructions from the repository it is reviewing, so the + artifact under review cannot steer it. +- The reviewer cannot write to the reviewed repository, so it cannot alter what it certifies. + *(Amended 2026-08-14, Gate-A pass 2 finding 20. **Narrowed** — "or exfiltrate what it reads" + is withdrawn: a reviewer that reads content and emits findings already has an output + channel, and an externally executed one may add network and environment channels, so a + write barrier is an **integrity** control and not a confidentiality one. **Kept** — the + write barrier itself and its reason. **Open** — whether an exfiltration control is wanted + at all, and what would demonstrate it, now belongs in §5.)* +- It can still read the reviewed content losslessly — including a complete git range for + Gate B, and the files outside the diff that §5's standing falsification lens requires. +- Its passes are **disclosed as same-family** in the cycle-closing commit body, and are never + presentable as tier 1. *(Amended 2026-08-14, Gate-A pass 2 finding 19. **Narrowed** — from + "wherever the ladder already discloses degradation": there is no ladder and no such site, so + this story owes its **own** disclosure form rather than inheriting one. **Kept** in full — + that a tier-2 pass is disclosed as same-family and never counted as tier 1, which is the + requirement, not the mechanism.)* +- Where containment cannot be demonstrated, tier 2 reports itself **unavailable** rather + than running with a boundary it does not have. + +## 3. Acceptance criteria + +- [ ] A named abuse scenario is demonstrated: an instruction planted in a reviewed artifact + attempts to steer the reviewer, and the expected control **rejects or contains it**, + with the observation recorded. (The `+abuse-path` obligation; this criterion is the + story's forcing function.) +- [ ] The reviewer provably does not load the reviewed repository's `CLAUDE.md` hierarchy — + demonstrated, not asserted. +- [ ] The reviewer cannot write to the reviewed repository, demonstrated by an attempted + write that fails. +- [ ] A Gate-B tier-2 pass reviews a **complete** range: the authoritative source set is + named, and an exact comparison — content hashes over the diff's paths plus the + out-of-diff files the falsification lens requires — establishes the reviewed input + matches it, with any residual case bounded rather than assumed. + *(Amended 2026-08-14, Gate-A pass 2 finding 21. **Narrowed** — "a check that would fail + if the input were truncated" detects one mutation and does not establish losslessness, + which is the gate-proof calibration Don't. **Kept** — that completeness must be + demonstrated rather than assumed.)* +- [ ] A tier-2 pass is identifiable as same-family from `main`'s history alone, in a record + form **this story defines**, and is never counted or presented as tier 1. + *(Amended 2026-08-14, Gate-A pass 2 finding 19. **Overturned** — the delegation to the + parent story's AC 2, which now covers a human's decision about optional work and + supplies nothing a tier-2 pass could use. **Kept** in full — the requirement itself, + which was always this story's to satisfy.)* +- [ ] When the containment preconditions cannot be met, tier 2 is unavailable and says so + with a named cause — it never degrades silently into an uncontained reviewer. +- [ ] The four prohibition sites — `/workflow-init` §2.13, `docs/coding-workflow.md` § *The + two gates…*, `README.md`, `plugins/dev-workflow/agents/finding-triage.md` — plus + `docs/sparring-briefing.md`'s family-granularity premise are accounted for explicitly, + each condition marked kept, narrowed or deliberately overturned. **Inherited unchanged + from the parent story's AC 4**, which deferred it here because a human exception is not + a model reviewing its own work. *(Amended 2026-08-14: the parent's reason is unchanged + by its closure — the four sites forbid a same-model **reviewer**, which is this story's + subject and was never the parent's.)* + +## 4. Affected AGENTS.md invariants + +- `## What this project is` — "two independent cross-model review gates (a different model + reviews the design at Gate A and the diff at Gate B)" — tier 2 is the clause that qualifies + this most directly. +- `## What this project is` — "**The product is prompts.** … review and + `docs/prompt-standards.md` are the only gates a prompt passes through." — and here the + product being reviewed is what can steer the reviewer. +- `## Architecture` (Dependency direction) — "on a Codex MCP server exposing both `exec` and + `review` (the gates key on those two tool names)" — a tier-2 reviewer is not that server. +- `## Key invariants` → Hook, 2 — "**Loose in the firing direction.** … A missed commit + (false ✓) is the dangerous direction." +- `## Key invariants` → Prompts and scaffolding, 11 — "**Prompt changes pass + `docs/prompt-standards.md`** — all 12 checklist items, for any skill, command, agent + definition, hook message, or scaffolded template." +- `## Don'ts` — "**Never describe what a gate proves without checking what it actually + compares.**" +- `## Don'ts` — "**Never replace a decision procedure without accounting for its old + conditions.**" + +## 5. Open questions + +- What is the reviewer surface? A sanitized external checkout, a separate harness with an + independently controlled system prompt, or something else — and what does each cost an + ordinary user who has no infrastructure? +- Can a reviewer be given a lossless git range **without** general Bash? If not, is a + read-only worktree plus a narrowly scoped diff helper sufficient? +- Do tier-2 passes count toward the three-pass floor at all, given the parent cycle + established the hook cannot see them? +- Is tier 2 worth building, given nothing else unblocks work? *(Amended 2026-08-14. + **Overturned** — the original framing, "once tier 3 exists", and its premise that tier 3 + unblocks work. **Kept, and strengthened** — the question itself: tier 2 adds review quality + at real complexity cost. What changed is the alternative it is weighed against, which is no + longer a zero-pass closure but **stopping until the reviewer returns**, or an operational + bridge such as a second key or another vendor.)* +- Does a contained tier-2 reviewer change what the four prohibition sites should say? + *(Amended 2026-08-14, Gate-A pass 2 finding 19. **Overturned** — "or does the narrowing + principle the parent shipped already cover it": the parent shipped no narrowing principle, + since tier 3 was withdrawn and the four sites were left untouched. **Kept** — the question, + which this story must now answer from scratch.)* +- **New, from the parent's closure:** what is the disclosure record form for a same-family + pass, and what confidentiality boundary (if any) does containment owe (findings 19, 20)? + +## 6. Suggested size + +`story` — one coherent question (can a same-family reviewer be contained well enough to be +worth counting) with a bounded set of candidate surfaces. It becomes +`epic-needs-splitting` if the answer requires new infrastructure a user must install, in +which case the split is reviewer-surface first, ladder integration second. diff --git a/plugins/dev-workflow/.claude-plugin/plugin.json b/plugins/dev-workflow/.claude-plugin/plugin.json index b3d1e9f..fcfec04 100644 --- a/plugins/dev-workflow/.claude-plugin/plugin.json +++ b/plugins/dev-workflow/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "dev-workflow", "displayName": "Cross-Model Review Workflow", - "version": "0.8.2", + "version": "0.9.1", "description": "Spec-driven workflow with two independent cross-model review gates, an append-only hardening ledger with an escalation ladder, and repo-enforced quality. Requires the superpowers plugin.", "author": { "name": "Daniel Sänger", diff --git a/plugins/dev-workflow/CHANGELOG.md b/plugins/dev-workflow/CHANGELOG.md index 2c9cd1b..580636e 100644 --- a/plugins/dev-workflow/CHANGELOG.md +++ b/plugins/dev-workflow/CHANGELOG.md @@ -22,6 +22,81 @@ unambiguously, still fails. Deleting only a plugin's *manifest* while the direct keeps shipping fails too. AGENTS.md invariant 12 carries the complete list. +## 0.9.1 + +- §5's human-exception form and its `workflow-init` template no longer claim the empty + record-only commit "does not reopen any gate". The normative half was right — an empty diff + raises no review obligation — but the sentence also promised silence from the gate hook, + which it does not deliver: such a commit can still draw a Gate-B reminder. Both copies now + say so **without restating the hook's decision logic** — and without pointing at it either, + since the scaffolded copy must stand on its own in a project that does not have the hook's + source. Both pin the exemption to the **empty diff**, confirmable with `git show --stat`, + rather than to the reminder looking inconvenient. The edited human-exception regions of the + two copies are byte-identical (the files as wholes are not, and never were). +- **Four Gate-B rounds went into narrowing that replacement sentence** — each correction a + subtler version of the same overclaim — before `docs/prompt-standards.md`'s own rule + applied: at the fourth correction, delete the mechanism claim rather than refine it a fifth + time. That deletion is what shipped, and it is why this entry describes no state machine + either. The same supersession is noted in the design spec (§2.1, §8) and the implementation + plan, whose approved quoted blocks are left unedited. Ledger row appended under + `unverified-enforcement-claim`. +- **Corrects one rule 0.9.0 shipped below**, also from PR #24 (CodeRabbit): that entry says the + model taking each pass is "read from the configured value at that moment", and it shipped + that rule into both `docs/coding-workflow.md` and `docs/sparring-briefing.md`. It is wrong — + `mcp-codex-dev` resolves its model chain once per project root and caches it until the + server restarts, so an edit landed after a root was loaded leaves the configured value naming + a model the running server is not using. Both documents now say to record the model the pass + *ran under* and to probe with `mcp__codex__health` where the two can differ — reading the + per-tool field (`tools.review.model` for Gate B, `tools.exec.model` for Gate A) with the + top-level `model` as fallback, since the server resolves a gate's model as + `tools..model ?? model` and `CODEX_DEV_REVIEW_MODEL` lands at `tools.review.model`. + Both documents are corrected across this 0.9.1 range. The 0.9.0 entry is left as the record of what shipped. + +## 0.9.0 + +- §5 and its `workflow-init` template now state the finding-line **severity vocabulary as a + closed set** — `BLOCKER | MAJOR | MINOR | NIT` — rather than showing it by example, and + define what the **reader** does with anything else: split on unescaped pipes, trim the + format's whitespace, match case-insensitively, and read an unrecognized non-empty token as + `MAJOR`. An empty or malformed field stays a structural failure and the pass stays + INCOMPLETE. Motivating incident: a Gate-B pass returned all four findings at `IMPORTANT`, + and discarding it over the token would have thrown away four real findings. +- `scripts/check-invariants.sh` gains **check 4c**, which asserts that canonical line is + present exactly once in each of `CLAUDE.md` and the command file — compared for equality, + case-sensitively — and, in the command file, that it sits inside the `### 2.1` section that + is actually scaffolded into a user's project. A copy anywhere else in that file ships + nothing. The `### 2.1` anchor and its `### 2.2` terminator are both validated: a renamed, + missing or duplicated boundary fails loudly rather than widening the range. +- §5 gains a **human-exception record form** — `Human exception:` / `Not done:` / + `Accepted because:` — for a decision about work **no applicable rule required**: an optional + check an environment cannot run, a requested review stood down, a courtesy step. It records + a decision and authorizes nothing: never a gate, a floor, a pass count, an evidence + obligation, or any mandatory rule from this file, `AGENTS.md`, a project doc, CI, a branch + policy or the platform. The record is an unverified assertion and the shipped text says so. +- §5 Mechanics now states the **squash-merge carry** explicitly: every evidence entry and + every human-exception record in the squash range is copied into the squash body, because + that commit is the only body the merge carries into `main`'s history. +- `docs/coding-workflow.md` gains a **reviewer model selection** section, and + `docs/sparring-briefing.md` the matching rule for the upstream advisor. Both describe the + **mechanism only and name no models**: how to add a gateway provider to the Codex CLI, the + switch surfaces `mcp-codex-dev` resolves in order (the config the CLI reads, the user-level + `~/.mcp/mcp-codex-dev/config.json`, the per-repo `.mcp/mcp-codex-dev.config.json`, and the + `CODEX_DEV_MODEL` / `CODEX_DEV_REVIEW_MODEL` environment overrides — the latter Gate B + alone), each a one-string edit; why a CLI *profile* does not reach the gate calls (`--model` is + passed, `--profile` never is); and that the model taking each pass is read from the + configured value at that moment rather than carried in a document. Availability and pricing + move faster than documentation, so the gateway's own catalog is the reference. The one + permanent rule is family-level: no model from the implementer's own family satisfies a gate, + whatever the vendor, gateway or transport. No §5 change — the invariant names families, not + vendors. +- **What this release deliberately does not ship:** the mid-flight *gate waiver* this work + began as. Three design cycles over nine review passes found no safe way to authorize a + zero-pass gate closure in a prompt-only system — every compensating control landed + unenforceable or recursive, the outage that would justify a waiver is producible by whoever + benefits, and the preconditions cannot be established in the case they exist for. That + finding, and what it does *not* claim, are recorded in + `docs/superpowers/specs/2026-08-14-reviewer-availability-fallback-design.md` §1. + ## 0.8.2 - `workflow-init`: the scaffolded ledger header now carries a supersession convention — diff --git a/plugins/dev-workflow/commands/workflow-init.md b/plugins/dev-workflow/commands/workflow-init.md index 341387c..e3e08bc 100644 --- a/plugins/dev-workflow/commands/workflow-init.md +++ b/plugins/dev-workflow/commands/workflow-init.md @@ -293,7 +293,9 @@ because the response stops carrying the findings at all. Append to the gate prom > `gate-a-spec-pass-

`, `gate-a-plan-pass-

`, or `gate-b--pass-

`. > > One finding per line in the format above; escape a literal pipe inside a field as -> `\|`. Every line before the terminator is exactly one finding line — no blank lines, +> `\|`. +> Severity is one of exactly: BLOCKER | MAJOR | MINOR | NIT — no other token. +> Every line before the terminator is exactly one finding line — no blank lines, > headings, prose or wrapped continuations. End the file with a final line reading > exactly `END OF FINDINGS ( total)`, `` being the number of finding lines. A > clean pass is the single body line `NO FINDINGS` with `END OF FINDINGS (0 total)`. @@ -345,6 +347,19 @@ file, an `INCOMPLETE` reply — is an **INCOMPLETE pass**, which is not a review act on the partial list, don't count it toward the 3-pass floor, and don't read "no Blocker/Major visible" as clean. +**Reader:** the severity field is taken by splitting the line on **unescaped** pipes and +trimming the ASCII whitespace the finding format puts either side of each separator; a field +that is empty or all whitespace is a **structural** failure, so the line is INCOMPLETE and is +never normalized. Otherwise the field is matched **case-insensitively** against the four tokens +first — `Minor`, `minor` and `MINOR` are all `MINOR`, because `CLAUDE.md` Mechanics +legitimately spells them in Title case and a model copying that spelling is doing as it was +told, not drifting. A field that matches no token case-insensitively, and is non-empty, is +read as `MAJOR`. Every **structural** failure stays INCOMPLETE — a malformed +line, a wrong field count, an empty severity field, a bad terminator, a count mismatch. Only +the severity token is tolerated, and only when everything else about the line is right. +(PR #23's Gate-B pass 3 returned all four findings at `IMPORTANT`; discarding that pass over a +token would have thrown away four real findings.) + **Recovery: one attempt per pass**, shared across timeout, an `INCOMPLETE` reply and failed validation — the Mechanics timeout-retry rule widened, not a second budget beside it, since two budgets let a pass alternate between them indefinitely. The attempt is a @@ -605,6 +620,66 @@ like the rest of §5; the detection is a reader comparing the pass against the s amend replaces the WIP message wholesale, so an entry written only into the WIP body is destroyed exactly when the cycle closes. The final commit body is the durable record; a PR shows commit messages, so there is no second home to keep in sync. + + **On squash-merge, copy every evidence entry and every human-exception record in the squash range into the squash body — the squash commit is the only body the merge carries into `main`'s history, so anything left behind is unreachable from it.** + + **Recording a human exception.** Where a human decides that something **no applicable rule + required** was nonetheless worth skipping — an optional check this environment cannot run, a + review someone asked for and then stood down, a courtesy step — that decision goes in the + closing commit body: + + ``` + Human exception: · + Not done: + Accepted because: + ``` + + **Which commit:** an ungated change records it in that commit; a Gate-A cycle in the spec or + plan commit; a Gate-B cycle in the WIP commit, restated by the closing amend. Several records + accumulate; order means nothing. + + **A decision made after its commit closed** — during PR review, say — goes in whichever of + these exists: the next commit on the branch, the squash body, or a follow-up commit after the + merge. If none does — the branch is closed, unmerged, and heading for an ordinary or rebase + merge — **add a commit for it.** An empty commit carrying only the record is a legitimate + destination: it changes no content, so it raises no review obligation. A record with nowhere + to go would otherwise be a record that does not exist. + + **Do not expect silence from the gate hook, and do not read a reminder as a gate + reopening.** It is advisory, so it never blocks the commit attempt. What is exempt is the + **empty diff**, which `git show --stat` confirms — never a reminder that merely looks the + same on a commit carrying content. + + Copy every record into the squash body alongside the evidence entry (Mechanics, + squash-merge carry). **Nothing performs that carry and nothing checks afterwards that it + happened** — it is on whoever prepares the merge. If two copies of one record disagree, that + is a copying error: stop and fix it rather than picking one. + + **Scope, and it is narrow. This form supplies no permission.** It records a decision that + was already the human's to make about something genuinely optional. It is **never** the answer to a + below-floor pass, an unclean final pass, a `STOP and surface`, a Gate-A or Gate-B + obligation, or a profile-derived evidence requirement — and more generally **it authorizes + nothing that any mandatory rule in this file or in `AGENTS.md` requires.** Those have their + own terminal actions and this paragraph changes none of them: on a STOP you still stop, and + neither a human's assent nor this record lets an agent close or continue a cycle. + + **"Mandatory" is not limited to this file.** A rule in `AGENTS.md`, a project doc, CI, a + branch policy or the platform is equally out of reach — under **Wait for**, + `docs/pr-review-bots.md` requires a bot review unless an explicit recorded human decision + permits proceeding without it, and this form is not that decision. If you are reaching for it to get past something mandatory, the answer + is no — take the operational route or stop. + + **Nor is it for things that were simply never owed.** An absent review from a bot routed + **opportunistically** blocks nothing and needs no exception and no record; + `docs/pr-review-bots.md` says so deliberately, and writing one anyway would rebuild the + per-quiet-bot ceremony that routing removed. Record a decision, not a non-event. + + **What the record is worth.** It is an **unverified assertion**, and reads as one: nothing + checks that the handle belongs to whoever decided, that a human was asked, or that the + reason is honest. A reader of history learns that *the commit claims* a human chose, what + it says was skipped, and why — no more. It supports no claim of authorization or review, + and satisfies no evidence obligation. It exists because an exception nobody wrote down is + invisible, not because writing it down makes it sound. - **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: diff --git a/scripts/check-invariants.sh b/scripts/check-invariants.sh index dfcc7ab..8e4ac74 100755 --- a/scripts/check-invariants.sh +++ b/scripts/check-invariants.sh @@ -21,15 +21,17 @@ # # TESTED SPELLINGS ONLY: extend the fixtures before extending the regex. # -# MUTATION RE-RUN PROCEDURE (manual; nothing automates it). The two prompt-conformance +# MUTATION RE-RUN PROCEDURE (manual; nothing automates it). The three prompt-conformance # checks below are bracketed by `# --- BEGIN check 4a ---` / `# --- END check 4a ---` -# markers so a scratch copy can be neutered cleanly: +# markers -- and likewise for 4b and 4c -- so a scratch copy can be neutered cleanly. +# Substitute the marker for each check in turn; the procedure is otherwise identical: # # TMP=$(mktemp -d) || exit 1 # [ -n "$TMP" ] && [ -d "$TMP" ] || exit 1 # else the copy below targets /repo # trap 'rm -rf "$TMP"' EXIT HUP INT TERM # mkdir -p "$TMP/repo"; tar cf - --exclude=.git . | (cd "$TMP/repo" && tar xf -) -# sed '/BEGIN check 4a/,/END check 4a/d' scripts/check-invariants.sh \ +# chk=4a # then 4b, then 4c +# sed "/BEGIN check $chk/,/END check $chk/d" scripts/check-invariants.sh \ # > "$TMP/repo/scripts/check-invariants.sh" # sh scripts/check-invariants.test.sh > "$TMP/before" 2>&1; base=$? # ( cd "$TMP/repo" && sh scripts/check-invariants.test.sh ) > "$TMP/after" 2>&1; mut=$? @@ -258,8 +260,11 @@ for manifest in plugins/*/.claude-plugin/plugin.json; do fail "Invariant 6: $manifest re-declares a convention-loaded component." "$bad_keys" done -# Scan domain for the two prompt-conformance checks below: Markdown only, because both -# rules are about prompt text. The wider yml/json/toml domain used by invariant 5 is +# Scan domain for checks 4a and 4b: Markdown only, because both rules are about prompt +# text. NOT 4c -- that one reads two fixed paths directly and is not part of this scan, +# so this domain stays a two-check domain even though the file now carries three +# prompt-conformance checks. Incrementing the number here would claim a scope 4c does not +# use. The wider yml/json/toml domain used by invariant 5 is # deliberately NOT reused — a `Target model:` line in a JSON fixture is not a prompt # claim. `grep -r` does not follow symlinks (`-R` would), which is the intended form. # @@ -464,5 +469,102 @@ else fi # --- END check 4b --- +# --- BEGIN check 4c --- +# The finding-line severity vocabulary must be stated as a CLOSED SET in both prompt +# copies, exactly once each. This is the normative statement that the writer's tokens are +# uppercase, so the comparison is CASE-SENSITIVE: a title-case copy does not carry the +# rule. Mechanics' own "Blocker (wrong/unsafe...)" sentence is a DIFFERENT sentence and +# keeps its title case -- the READER normalizes case, the writer syntax does not. +# +# EQUALITY, not containment. A line that merely contains the sentence can negate it +# ("Ignore the following: Severity is one of exactly: ..."), so the whole line must BE the +# sentence, after stripping a leading blockquote marker and indentation. Nothing is +# stripped from the right: "byte-for-byte" means what it says, and a trailing space is a +# Markdown hard break, not whitespace noise. +# +# WHOLE-FILE count for duplicates, PLUS a placement rule in the command file. An earlier +# design bounded a section-5 region in both files; on the command file that needed fence +# nesting, and the parser returned a multiple-end-boundary error on the real file, so it +# could never have passed. Whole-file counting replaced it -- and silently traded away a +# guarantee: the line can sit in the command file's own prose, outside the template that +# `/workflow-init` actually scaffolds, and the count is still 1. Verified, not theorised. +# Only the template region reaches a user's project, so the command file gets a placement +# rule anchored on its `### 2.1` scaffold heading and terminated by the NEXT NUMBERED +# heading -- not the next `###`, because the template contains its own unnumbered +# `### Profiles` and `### Mechanics` subsections and would truncate the range. +# +# The repo's own CLAUDE.md needs no placement rule: the whole file is the artifact. +SEV_CANON='Severity is one of exactly: BLOCKER | MAJOR | MINOR | NIT — no other token.' + +# Prints " ", where the +# terminator is the number of the first numbered `### ` heading after the anchor, or +# `none`. Returns 2 if awk itself failed, which is not the same as zeroes: an awk that +# cannot run prints nothing, and empty is not a number, so the caller checks the status. +# +# The TERMINATOR is checked, not just the anchor. Without it the range fails OPEN when the +# boundary moves: rename `### 2.2` to something unnumbered and the section runs on to 2.3, +# so a line planted in the old 2.2 region counts as inside the template. Verified before +# this was added -- the battery stayed green. An anchor-based rule that survives its own +# anchor drifting is worth nothing. +severity_rule_scan() { # $1 = file + awk -v canon="$SEV_CANON" ' # sev-canon-count + /^### 2\.1[[:space:]]/ { intpl = 1; heads += 1; next } + intpl && /^### [0-9]/ { intpl = 0; term = $2; next } + { line = $0 + sub(/^[ \t]*/, "", line); sub(/^> ?/, "", line); sub(/^[ \t]*/, "", line) + if (line == canon) { n += 1; if (intpl) t += 1 } } + END { printf "%d %d %d %s\n", n + 0, t + 0, heads + 0, (term == "" ? "none" : term) } + ' "$1" || return 2 +} + +# Both paths are REQUIRED. A missing one is a named failure, never a skip: the rule is +# defined over both copies, so continuing quietly would turn half the check off exactly +# when a file is deleted or moved -- the fail-open direction. +for sev_file in CLAUDE.md plugins/dev-workflow/commands/workflow-init.md; do + if [ ! -f "$sev_file" ]; then + fail "Prompt standards: $sev_file is missing, so the closed severity set cannot be checked." \ + "both prompt copies are required" + continue + fi + if [ ! -r "$sev_file" ]; then + fail "Prompt standards: $sev_file is unreadable, so the closed severity set cannot be checked." \ + "check permissions" + continue + fi + sev_out=$(severity_rule_scan "$sev_file"); sev_st=$? + if [ "$sev_st" -ne 0 ]; then + fail "Prompt standards: the closed severity set parser failed; results are not trustworthy." \ + "awk exited $sev_st on $sev_file" + continue + fi + # Word splitting is the point: the scan prints three space-separated integers. + # shellcheck disable=SC2086 + set -- $sev_out + sev_n=$1; sev_t=$2; sev_heads=$3; sev_term=$4 + if [ "$sev_n" -ne 1 ]; then + fail "Prompt standards: $sev_file must state the closed severity set exactly once; found $sev_n." \ + "$SEV_CANON" + elif [ "$sev_file" != CLAUDE.md ]; then + # Placement, command file only. A missing or renamed anchor fails loudly rather than + # skipping the rule -- the safe direction, and the one an anchor-based check has to + # get right to be worth having. + if [ "$sev_heads" -ne 1 ]; then + fail "Prompt standards: $sev_file has $sev_heads '### 2.1' scaffold headings, so the closed severity set's placement cannot be checked." \ + "expected exactly one" + elif [ "$sev_term" != 2.2 ]; then + # Loud, not lenient. A renumbered or renamed boundary is precisely when the range + # silently widens, so the check refuses rather than measuring a range it cannot + # trust. Fixing it is renaming a heading back, or updating this expectation + # deliberately. + fail "Prompt standards: $sev_file's '### 2.1' section is terminated by '$sev_term', not '2.2', so the closed severity set's placement cannot be checked." \ + "the range would silently widen past the scaffolded template" + elif [ "$sev_t" -ne 1 ]; then + fail "Prompt standards: $sev_file states the closed severity set outside the scaffolded CLAUDE.md template, so an initialized project would not receive it." \ + "expected it inside the '### 2.1' section" + fi + fi +done +# --- END check 4c --- + [ "$rc" -eq 0 ] && printf 'invariant checks: ok\n' exit "$rc" diff --git a/scripts/check-invariants.test.sh b/scripts/check-invariants.test.sh index ed5dbf6..434daff 100755 --- a/scripts/check-invariants.test.sh +++ b/scripts/check-invariants.test.sh @@ -20,8 +20,14 @@ fail() { fail_n=$((fail_n + 1)); printf 'FAIL - %s\n' "$1"; } # diagnostic-isolation failure the $5-substring guard exists to prevent. Measured before # this existed: 25 of 61 assertions failed. # -# One initializer, called from all four builders. Extending only `run_with` would leave +# One initializer, called from all five builders. Extending only `run_with` would leave # `sh_case` and the two inline blocks broken. +# +# Check 4c adds a second reason this exists: it requires the canonical severity line in +# BOTH prompt copies, and neither exists in a bare fixture repo. Without the two writes +# below, every fixture would fail 4c on a baseline unrelated to its own assertion — the +# same isolation failure the checklist pair was added for. +SEV_LINE='Severity is one of exactly: BLOCKER | MAJOR | MINOR | NIT — no other token.' init_prompt_fixtures() { # $1 = fixture repo root mkdir -p "$1/docs" "$1/plugins/dev-workflow/commands" for pf in "$1/docs/prompt-standards.md" "$1/plugins/dev-workflow/commands/workflow-init.md"; do @@ -29,8 +35,16 @@ init_prompt_fixtures() { # $1 = fixture repo root i=1 while [ "$i" -le 12 ]; do printf '%s. **item %s**\n' "$i" "$i"; i=$((i + 1)); done printf '\n## After\n\nReviewed against all 12 items.\n' + # 4c: the command file needs the line INSIDE a `### 2.1` scaffold section, because + # only that region is written into an initialized project. A copy anywhere else in + # the file satisfies the duplicate count and still ships nothing. + printf '\n### 2.1 CLAUDE-md\n\n%s\n\n### 2.2 next\n' "$SEV_LINE" } > "$pf" done + # `docs/prompt-standards.md` got the section from the loop as well; harmless, 4c does + # not read that path. CLAUDE.md is not written by the loop and needs its own copy -- + # and needs no `### 2.1`, since the whole file is the artifact there. + printf '# Fixture\n\n%s\n' "$SEV_LINE" > "$1/CLAUDE.md" } work=$(mktemp -d) || work='' @@ -322,36 +336,41 @@ for badname in 'a|b' 'a&b' 'a\b'; do else pass "violation in filename '$badname.yml' still rejected"; fi done -# --- Prompt conformance: checks 4a and 4b ------------------------------------------ +# --- Prompt conformance: checks 4a, 4b and 4c --------------------------------------- # -# MUTATION EVIDENCE (recorded 2026-07-26). This is a DOCUMENTED DEVELOPMENT-TIME RUN, +# MUTATION EVIDENCE (re-measured 2026-08-15). This is a DOCUMENTED DEVELOPMENT-TIME RUN, # not automated enforcement: nothing re-runs it, and nothing here fails if it goes stale. # The procedure lives in the checker's header comment. # -# Deleting the `BEGIN check 4a`/`END check 4a` block flipped exactly 20 assertions -# (baseline exit 0, mutant exit 1): every `4a:` reject fixture (15), the four -# `exclusion: neighbouring …` controls, which depend on 4a because they carry the -# assertion phrase, and `4a value extraction failure fires`, whose stage 4a alone reaches. -# Deleting the `4b` block flipped exactly 22: every `4b:` reject fixture (16), the four -# `4b exclusion: neighbouring …` controls, and the two stage-failure fixtures that reach -# their stage only through 4b — `checklist parser failure fires` and -# `4b claim validator failure fires`, since deleting the block means neither the parser -# nor the validator is ever called. -# Two fixtures flip in NEITHER mutation, and that is the expected result rather than an -# omission: `scan error fires` and `4a/4b exclusion filter failure fires` break a stage -# that BOTH checks use, so the surviving check still fires when its sibling is deleted. -# In both runs no accept case moved and no unrelated case moved — the second half of the -# check, and the one a non-empty flip set alone does not establish. +# Deleting a marked block flips (baseline exit 0, mutant exit 1): +# 4a -> 20 every `4a:` reject fixture (15), the four `exclusion: neighbouring ...` +# controls, which depend on 4a because they carry the assertion phrase, and +# `4a value extraction failure fires`, whose stage 4a alone reaches. +# 4b -> 22 every `4b:` reject fixture (16), the four `4b exclusion: neighbouring ...` +# controls, and the two stage-failure fixtures that reach their stage only +# through 4b -- `checklist parser failure fires` and +# `4b claim validator failure fires`. +# 4c -> 19 every `4c:` reject fixture (18) and +# `4c canonical-line parser failure fires`. NO accept case moved, which is +# the second half of the check and the one a non-empty flip set alone does +# not establish. +# 4c measured 13 before the placement and terminator fixtures existed, and that number was +# briefly recorded here against a suite that no longer produced it. A measured block +# carries only measured numbers: re-run, do not extrapolate. +# `scan error fires` and `4a/4b exclusion filter failure fires` flip in NONE of the three: +# they break a stage that several checks use, so a surviving check still fires. # -# An earlier version of this block recorded 17 and 11. Those were true when written and -# went stale the moment fixtures were added — which is exactly the failure the trigger -# below exists to prevent, and it was caught at Gate B rather than by the trigger. If you -# add a fixture, you are changing this mapping. +# 4b measured 21 on the first run of this round, against a recorded 22. That was a real +# regression, not drift: `checklist parser failure fires` greps the checker's output, and +# its pattern was the bare `parser failed`, which check 4c's new diagnostic also ends in. +# The fixture had stopped testing 4b -- deleting the 4b block left it green. The pattern +# is now `checklist parser failed` and the count is 22 again. An earlier version of this +# block recorded 17 and 11, and went stale the moment fixtures were added. # -# RE-RUN TRIGGER — broader than "the scan logic", because the mapping above is +# RE-RUN TRIGGER -- broader than "the scan logic", because the mapping above is # invalidated by more than that: re-run and update BOTH this block and the PR record -# after changing either marked check, its markers, any of these fixtures or their -# assertion names, or the harness that runs them. +# after changing any marked check, its markers, any of these fixtures or their assertion +# names, or the harness that runs them. # Diagnostics these cases must name, so none can pass on an unrelated violation. MODEL='name one executing model' CLAIM='count claim disagrees' @@ -582,7 +601,12 @@ printf '#!/bin/sh\nexit 2\n' > "$work/r/fakebin/awk" chmod +x "$work/r/fakebin/awk" out=$( cd "$work/r" && PATH="$work/r/fakebin:$PATH" sh scripts/check-invariants.sh 2>&1 ); st=$? if [ "$st" -eq 0 ]; then fail "checklist parser failure fires (exited 0 - parser failure read as clean)" -elif ! printf '%s' "$out" | grep -q 'parser failed'; then +elif ! printf '%s' "$out" | grep -q 'checklist parser failed'; then + # `checklist parser failed`, not the bare `parser failed` this used to match. The stub + # above fails EVERY awk, and check 4c's diagnostic also ends in "parser failed" — so the + # loose pattern made this fixture pass whenever either parser broke. It stopped being a + # test of 4b: deleting the 4b block left it green, which the mutation run caught as a + # flip count of 21 against a recorded 22. fail "checklist parser failure fires (wrong diagnostic: $(printf '%s' "$out" | tr '\n' ' '))" else pass "checklist parser failure fires"; fi @@ -625,6 +649,132 @@ inject_case "4b claim validator failure fires" awk 'words=*' 'claim validator fa inject_case "4a value extraction failure fires" awk '*extract-target-model*' \ 'per-file checks failed' +# --- Prompt conformance: check 4c, the closed severity set -------------------------- +# +# 4c is a whole-file exactly-once count, so its fixtures need no region shapes: each case +# writes one or both prompt copies and asserts the shared diagnostic. The near-miss cases +# are the point — a second occurrence, a line that merely CONTAINS the sentence, and a +# title-case copy all read as correct to a human skimming the file. +SEV='closed severity set' + +# $3/$4 are file bodies, or a sentinel a body cannot express: +# @KEEP@ leave the initializer's valid copy @GONE@ delete it @LOCK@ chmod 000 +sev_put() { # $1 = path, $2 = body-or-sentinel + case "$2" in + @KEEP@) : ;; @GONE@) rm -f "$1" ;; @LOCK@) chmod 000 "$1" ;; + *) printf '%s\n' "$2" > "$1" ;; + esac +} +sev_case() { # $1 = name, $2 = 1|0 expect reject, $3 = CLAUDE.md, $4 = command file + rm -rf "$work/r"; mkdir -p "$work/r/scripts" "$work/r/.github/workflows" \ + "$work/r/plugins/p/.claude-plugin" + cp "$CHECKER" "$work/r/scripts/" + init_prompt_fixtures "$work/r" + printf '%s\n' '{"name": "p", "version": "1.0.0"}' > "$work/r/plugins/p/.claude-plugin/plugin.json" + printf '%s\n' "$PINNED" > "$work/r/.github/workflows/ci.yml" + sev_put "$work/r/CLAUDE.md" "$3" + sev_put "$work/r/plugins/dev-workflow/commands/workflow-init.md" "$4" + out=$( cd "$work/r" && sh scripts/check-invariants.sh 2>&1 ); st=$? + chmod 644 "$work/r/CLAUDE.md" 2>/dev/null + if [ "$2" -eq 1 ]; then + if [ "$st" -eq 0 ]; then fail "$1 (exited 0)" + elif ! printf '%s' "$out" | grep -q "$SEV"; then + fail "$1 (wrong diagnostic: $(printf '%s' "$out" | tr '\n' ' '))" + else pass "$1"; fi + else + if [ "$st" -eq 0 ]; then pass "$1" + else fail "$1 (exited $st: $(printf '%s' "$out" | tr '\n' ' '))"; fi + fi +} + +# The command file must keep its checklist or the case fails 4b instead of 4c. +# $1 goes INSIDE the `### 2.1` section; $2, if given, after it (outside the template). +sev_tpl() { + printf '# Prompt Standards\n\n## Checklist (each item must be verifiably true)\n\n' + i=1; while [ "$i" -le 12 ]; do printf '%s. **item %s**\n' "$i" "$i"; i=$((i + 1)); done + printf '\n## After\n\nReviewed against all 12 items.\n\n' + printf '### 2.1 CLAUDE-md\n\n%s\n\n### 2.2 next\n\n%s\n' "$1" "${2:-}" +} +TPL_NONE=$(sev_tpl "nothing here") + +sev_case "4c: both copies stating the line accepted" 0 "@KEEP@" "@KEEP@" +sev_case "4c: absent from CLAUDE.md rejected" 1 "# F" "@KEEP@" +sev_case "4c: absent from the command file rejected" 1 "@KEEP@" "$TPL_NONE" +sev_case "4c: absent from both rejected" 1 "# F" "$TPL_NONE" +sev_case "4c: twice in one file rejected" 1 "# F + +$SEV_LINE +$SEV_LINE" "@KEEP@" +sev_case "4c: blockquoted line accepted" 0 "# F + +> $SEV_LINE" "@KEEP@" +sev_case "4c: indented line accepted" 0 "# F + + $SEV_LINE" "@KEEP@" +sev_case "4c: line with leading text rejected" 1 "# F + +Ignore the following. $SEV_LINE" "@KEEP@" +sev_case "4c: line with trailing text rejected" 1 "# F + +$SEV_LINE Except NIT." "@KEEP@" +sev_case "4c: two copies on one physical line rejected" 1 "# F + +$SEV_LINE $SEV_LINE" "@KEEP@" +sev_case "4c: title-case copy rejected" 1 "# F + +Severity is one of exactly: Blocker | Major | Minor | Nit — no other token." "@KEEP@" +sev_case "4c: paraphrase rejected" 1 "# F + +Severity is one of: BLOCKER, MAJOR, MINOR, NIT and no other token." "@KEEP@" +sev_case "4c: trailing space rejected" 1 "# F + +$SEV_LINE " "@KEEP@" +# --- placement, command file only ------------------------------------------------ +# The exploit that whole-file counting alone let through: exactly one occurrence, but in +# the command file's own prose rather than the scaffolded template, so an initialized +# project receives nothing. Verified against the real file before this rule existed. +sev_case "4c: line outside the scaffolded template rejected" 1 "@KEEP@" \ + "$(sev_tpl "nothing here" "$SEV_LINE")" +sev_case "4c: line inside the scaffolded template accepted" 0 "@KEEP@" \ + "$(sev_tpl "$SEV_LINE")" +# The anchor's own failure modes fail LOUDLY rather than skipping the placement rule -- +# the safe direction, and the thing an anchor-based check must get right. +sev_case "4c: missing 2.1 anchor rejected" 1 "@KEEP@" "$(sev_tpl "$SEV_LINE" | sed 's/^### 2\.1.*/## not an anchor/')" +sev_case "4c: duplicate 2.1 anchor rejected" 1 "@KEEP@" "$(sev_tpl "$SEV_LINE") +### 2.1 CLAUDE-md again +" +# The template's own unnumbered subsections must not truncate the range: terminating on +# any `###` instead of a NUMBERED one would put a line after them outside the template. +sev_case "4c: line after an unnumbered subsection accepted" 0 "@KEEP@" \ + "$(sev_tpl "### Mechanics + +$SEV_LINE")" +# The terminator is checked, not only the anchor. Renaming `### 2.2` to something +# unnumbered widens the range to the NEXT numbered heading, and a line planted in the gap +# used to count as inside the template -- verified green against the real file before the +# terminator rule existed. Both halves are fixtures: the drift itself, and the exploit. +sev_case "4c: unnumbered terminator rejected" 1 "@KEEP@" "$(sev_tpl "$SEV_LINE" | sed 's/^### 2\.2 next/### not numbered/')" +sev_case "4c: line planted in the widened gap rejected" 1 "@KEEP@" "$(sev_tpl "nothing here" | sed 's/^### 2\.2 next/### not numbered/') +$SEV_LINE + +### 2.3 later" +sev_case "4c: absent terminator rejected" 1 "@KEEP@" "$(sev_tpl "$SEV_LINE" | sed '/^### 2\.2 next/d')" +sev_case "4c: CLAUDE.md needs no 2.1 anchor" 0 "# F + +$SEV_LINE" "@KEEP@" + +sev_case "4c: missing CLAUDE.md rejected" 1 "@GONE@" "@KEEP@" +if [ "$(id -u)" -ne 0 ]; then + # root satisfies -r on a mode-000 file, so the checker is right and the fixture would + # be wrong; the suite's scan-error case guards the same way. + sev_case "4c: unreadable CLAUDE.md rejected" 1 "@LOCK@" "@KEEP@" +fi + +# The parser branch, through the same PATH seam the 4a/4b stage failures use. The 4c awk +# is identified by its `sev-canon-count` marker comment. +inject_case "4c canonical-line parser failure fires" awk '*sev-canon-count*' \ + 'closed severity set parser failed' + printf '\n---\n' if [ "$fail_n" -eq 0 ]; then printf 'all passed (%s assertions)\n' "$pass_n"; else printf '%s passed, %s FAILED\n' "$pass_n" "$fail_n"; exit 1 diff --git a/todos.md b/todos.md index 0549bbf..df9c4fe 100644 --- a/todos.md +++ b/todos.md @@ -91,8 +91,10 @@ driven by recurrence rather than by enthusiasm. an escalation: the guard-scope precheck against the 2026-08-04 row (whose guard is *"the exhaustiveness statement"*, for a sentence naming what a mechanism does not cover) puts this shape outside it, so the count alone does not escalate. - *Trigger: rides with the reviewer-availability fallback story — next in queue, amending the - same §5 region, so one Gate B covers all three edits.* + *Trigger: **re-pointed 2026-08-16** at + `docs/superpowers/stories/2026-08-14-sequential-branch-calls-hook-story.md`. The + reviewer-availability story closed without shipping the sequential-calls rider — it is a + hook change, not a §5 prose edit, and the unchanged hook counts each call as a pass.* - [ ] **§5 gives the finding-line severity by example only, never as a closed set.** The gate prompt shows `MAJOR | high | …` and tells the reader to filter to Blocker/Major, but never states the four permitted tokens, and the acceptance rule validates shape — terminator, @@ -101,7 +103,11 @@ driven by recurrence rather than by enthusiasm. well-formed, so it passed every check and the Blocker/Major filter had to be applied by interpretation. Fix candidate: pin the enum in §5's finding-line spec and in `/workflow-init`'s template. Fingerprint `prompt-vague-criteria`, rung `P std`, no prior row. - *Trigger: rides with the reviewer-availability fallback story, with the row above.* + **CLOSED 2026-08-16** — §5 and the template now state the enum as a closed set, and + `scripts/check-invariants.sh` check 4c asserts it is present exactly once in each copy + (and, in the command file, inside the scaffolded template's own section). The reader + normalizes an out-of-enum token to `MAJOR` rather than discarding the pass, which is + what PR #23's four `IMPORTANT` findings needed. - [ ] **The supersession story's AC 1 restates `CLAUDE.md` §5's profile-change procedure instead of referencing it.** Raised by CodeRabbit on PR #23 and accepted as accurate: the criterion spells out propose-axes → pause for confirmation → write the header, and does so *lossily* — @@ -166,6 +172,18 @@ driven by recurrence rather than by enthusiasm. dangerous direction. Counts toward this row's eventual trigger; not fixed now, and note that any fix must keep the empty-list fallthrough firing rather than trade a redundant warning for a missed one. + **Occurrence 3 (2026-08-14): same shape as occurrence 2, same consumer.** `git add` + and `git commit` issued in one Bash call on PR #23's close; the staged set was empty + at `PreToolUse`, `is_docs_only` fell through, and the Gate-B STOP fired on a + docs-only commit. Like occurrence 2 a **false positive** — the safe direction. + **Occurrence 4 (2026-08-16): same shape again, on PR #24's review pass.** `git add` and + `git commit` in one Bash call; the staged set was empty at `PreToolUse`, `is_docs_only` + fell through, and the Gate-B STOP fired on a docs-only commit. A **false positive** like + 2 and 3. Counted here even though it was observed rather than suffered, because the + trigger is recurrence of the shape and an occurrence noticed by the person who caused it + is still the shape recurring; excluding it would tune the count to who was watching. + Four occurrences of the timing gap now, three of them benign; the dangerous `tree_hash` + consumer above is still the one that decides this row's priority. - [ ] **No regression test for a `git add`/`write-tree` failure inside the throwaway index.** Derived from the code, not recalled: sections 24a-24e stub FIVE failure shapes — every checksum tool failing silently, a checksum printing a token then failing, the @@ -380,8 +398,52 @@ backlog. mirror, and changes no part of the §5 **file protocol** this row's trigger names — not the slot names, not the pre-call delete, not the terminator or acceptance rules. Recorded so a later reader can check the reading rather than re-derive it. - *Trigger: the next round touching the §5 - file protocol.* + **SECOND OCCURRENCE, TRIGGER FIRED (2026-08-16).** The reviewer-availability story's + cycles destroyed a predecessor's findings file **and** its dispositions before the + surviving 44 artifacts were archived by hand. That is the second observed destruction, + after the 2026-07-26 profiles cycle. **The row stays open** — the fix is still naming (a + cycle component in the slot) or archiving, never relaxing the pre-call delete, which is + load-bearing. + *Trigger: FIRED. Was: the next round touching the §5 file protocol.* + +- [ ] **Attribution for the human-exception record form.** The `` in a + `Human exception:` record is unverified, and §5 says so in the shipped text: nothing + checks that it belongs to whoever decided, that a human was asked, or that the reason is + honest. Hardening for what ships; needs no availability attestation. + *Trigger: the first record whose authorship is disputed or unattributable.* +- [ ] **External-authority zero-pass research.** A signed commit or a protected-branch + approval is the one design class the reviewer-availability cycles never tried, and its + requirements are already named: a **trusted signer identity** *and* a **role policy** + saying which identities may approve — an ordinary signer can be the author — plus an + **availability attestation from a party that is not the author**, which neither mechanism + supplies on its own. It is one untried direction worth reconsidering, **not** the only one + that could work; the design space was never exhausted + (`docs/superpowers/specs/2026-08-14-reviewer-availability-fallback-design.md` §1.5). + *Trigger: a renewed need to close a gate cycle with no review — a second multi-day + reviewer outage, or the operational bridges of that design's §7 proving unavailable.* +- [ ] **Tracked re-review debt.** The human-exception form records a decision and creates no + follow-up obligation, so "the re-review never happened" is not an observable event today. + Stated as one deliberately. + *Trigger: a human explicitly asks for follow-up review on a recorded exception and that + follow-up is later found not to have happened.* +- [ ] **A recording mechanism for severity normalization.** Rider (b) normalizes an + unrecognized severity token to `MAJOR` and records nothing. The drift is visible to the + reader at the moment the pass is validated — the findings file carries the original token + on the finding line — but nothing is durable: `.context/` is git-ignored and slot + collisions have destroyed findings here (row above). A companion record was designed and + **cut**, at a measured cost: it needed a token-identity rule, a bijection audit, a + logical-pass/attempt/credited-count identity model, edits to four shipped hook reminder + strings, and a `docs/hardening-log.md` supersession row. + *Trigger: a pass is normalized and the drift goes unnoticed in review.* +- [ ] **The hook's `is_docs_only` exempts any `.md` path outside a prompt directory**, which is + broader than §5's prose list (`docs/**.md`, `README.md`, `MANIFEST.md`). Found while + siting a removed debt store. + *Trigger: a root `.md` file acquiring gate-relevant state.* +- [ ] **Tier-2 counting and containment.** A same-family reviewer whose passes are worth + counting, per `docs/superpowers/stories/2026-08-14-tier-2-same-family-reviewer-story.md`. + The unchanged hook counts each call as a pass, so counting is part of the problem, not a + detail of it. + *Trigger: the tier-2 story being picked up, or a second multi-day reviewer outage.* - [ ] **Finding B — a §5 version stamp, so a scaffolded CLAUDE.md can tell it lags the installed plugin.** Split out of the canvas-findings round after two Gate-A passes showed it is a design, not a sentence. Spec questions: a semantic §5 locator