diff --git a/AGENTS.md b/AGENTS.md index db7d6a4..c5ec0f3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -47,10 +47,10 @@ docs/ architecture.md # layout + the two non-obvious design decisions coding-workflow.md # the methodology this plugin encodes getting-started.md # first story, end to end - prompt-standards.md # the 11-item checklist every prompt must pass + prompt-standards.md # the 12-item checklist every prompt must pass hardening-log.md # append-only findings ledger (union-merged) hardening-taxonomy.md # this project's fingerprint classes - pr-review-bots.md # which bot posts line findings vs. summary only + pr-review-bots.md # the Wait-for routing list + where each bot's findings appear source-files/ # the extraction seed this repo was built from ``` @@ -126,7 +126,7 @@ reader can judge whether it still holds. helpers, framework APIs — goes only in that project's `docs/hardening-taxonomy.md`, never into the `harden-finding` skill. Otherwise one project leaks into every other. -11. **Prompt changes pass `docs/prompt-standards.md`** — all 11 checklist items, for +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 nothing mechanical checks them. diff --git a/CLAUDE.md b/CLAUDE.md index ca54b0b..debd298 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -109,8 +109,20 @@ advisory — validate before applying; dismissed finding → one-line why. (args `instruction`, `whatWasImplemented`, `baseSha`; `reviewType: full` runs spec + quality in parallel). Skip ONLY trivial changes. Check against @AGENTS.md. Re-review after every fix — a fix changes the diff and the hook - invalidates the prior pass, which is where the 3 come from. Same coverage rule - as Gate A: put "report every finding with severity and confidence; say + invalidates the prior pass, which is where the 3 come from. + + **A fix that changes specified behaviour updates the spec in the same commit.** If a + Gate-B fix alters something the approved spec pins down — an ordering, a terminal + state, a contract — the spec is stale the moment you commit, and the next reader + trusts it. Update both, and let the re-review cover both. This is not hypothetical: + a Gate-B fix here reordered a precedence rule and added a terminal state, the spec + was left describing the old behaviour, and a PR bot found the disagreement after + merge-readiness. Neither gate caught it *in that run*: Gate A had already passed the + spec before the fix existed, and the Gate-B call was given only the diff. A reviewer + handed both artifacts could catch it — which is why this is a rule about what you + commit, not a claim about what the gates detect. + + Same coverage rule as Gate A: put "report every finding with severity and confidence; say `NO FINDINGS` if clean" in `additionalContext`, with the same one-line format. You filter to Blocker/Major, Codex never does. diff --git a/docs/hardening-log.md b/docs/hardening-log.md index 2202ac4..8543fc2 100644 --- a/docs/hardening-log.md +++ b/docs/hardening-log.md @@ -17,3 +17,5 @@ escape `\|`, one line), `source` (gate-a|gate-b|bot|manual), | 2026-07-18 | prompt-diagnostic-cause-unnamed | workflow-init preflight reported "restart the session" for every not-loaded codex, hiding a same-named server winning on scope precedence, which a restart can never fix | manual | major | P std | docs/prompt-standards.md item 10 | | 2026-07-18 | dependency-unpinned | floating action refs and ubuntu-latest in ci.yml and in the CI template workflow-init scaffolds | gate-b | major | 2 lint | scripts/check-invariants.sh + .test.sh — guards the tested line-oriented spellings of action refs (incl. quoted, docker tag/digest, multi-per-line), runner values incl. tested matrix forms, and npx `-y`/`--yes` forms; raises the floor but is not exhaustive parsing (e.g. `uses :` with a space, or a line-split value, still evades) | | 2026-07-18 | docs-drift | three docs claimed the plugin manifest declares hooks; it declares none — the 0.2.1 duplicate-hooks load failure | gate-b | major | 1 prose | AGENTS.md Don'ts, "Never state what the manifest declares without reading it" (new rule + grep recipe; invariant 6 already existed and constrains the manifest, not claims about it) | +| 2026-07-18 | unverified-enforcement-claim | six citable claims across one spec, its plan and its diff that something was enforced/caught/guaranteed where no mechanism did it — each caught by a gate, none by the author, one written into the same document that records the pattern | gate-a | major | P std | docs/prompt-standards.md item 11 (+ the same item in the workflow-init inline template). Mixed provenance: four from Gate A on the spec, one from Gate A on the plan, one from Gate B on the pre-merge diff; `source` records Gate A as the majority and the trigger | +| 2026-07-18 | docs-drift | a Gate-B fix reordered a precedence rule and added a terminal state in process-pr-review; the approved spec was never updated and disagreed until a PR bot found it | bot | major | P std | CLAUDE.md §5 Gate B, "a fix that changes specified behaviour updates the spec in the same commit" (+ the workflow-init inline template). Escalated from the 2026-07-18 `1 prose` row, whose AGENTS.md rule was scoped to manifest claims and could not reach spec-vs-implementation drift | diff --git a/docs/hardening-taxonomy.md b/docs/hardening-taxonomy.md index e689b1d..5745eff 100644 --- a/docs/hardening-taxonomy.md +++ b/docs/hardening-taxonomy.md @@ -23,7 +23,21 @@ the synonyms a future reader might search for instead. Aliases: misleading remedy, wrong-fix loop, undiagnosed state, symptom collision, "restart the session" for a cause a restart never fixes. -**Promotion candidate.** This class is stack-neutral, not project vocabulary, so it -belongs in the `harden-finding` base list rather than here. It lives here because the -skill says to mint into this file (the plugin ships the base classes, the project owns -its own). Move it up when the base taxonomy is next revised. +- `unverified-enforcement-claim` — a prompt, spec or doc asserts that something is + enforced, caught, guaranteed or prevented, where no mechanism does it. Aliases: + claimed guarantee, asserted enforcement, false backstop, "the check catches it", + security theatre, a rule described as automatic that is instruction-only. + + **Not the same as `docs-drift`**, though they overlap. `docs-drift` is two documents + (or a document and the code) disagreeing; grep it when the fix is to make them + agree. This class is a single claim that is untrue on its own terms even with + nothing to contradict it — "output validation catches a budget overrun" was false + because the output carries no call count, not because another file said otherwise. + The fix differs too: drift is detected by comparing the two artifacts and fixed by + updating whichever is stale; this one is fixed by verifying the mechanism exists + before the sentence is written. + +**Promotion candidate.** These classes are stack-neutral, not project vocabulary, so +they belong in the `harden-finding` base list rather than here. They live here because +the skill says to mint into this file (the plugin ships the base classes, the project +owns its own). Move them up when the base taxonomy is next revised. diff --git a/docs/pr-review-bots.md b/docs/pr-review-bots.md index 4296e51..6b91ec2 100644 --- a/docs/pr-review-bots.md +++ b/docs/pr-review-bots.md @@ -1,31 +1,49 @@ # PR review bots — dev-workflow-kit Which automated reviewers run on this repo, and what each one *actually produces*. -`/dev-workflow:process-pr-review` reads this file to decide **which bots to wait for** -(only those that post line-by-line findings) and which to treat as context only. +`/dev-workflow:process-pr-review` routes on the **Wait for** list below — that list is +the authority. This table is descriptive: it records where each bot's findings have +been *observed*, which may be `inconsistent`. -Getting this wrong is expensive in both directions: waiting on a bot that structurally -cannot post line comments hangs the loop, and reading a summary as a findings source -silently drops real findings. +Getting this wrong is expensive in both directions: waiting on a channel a bot never +uses hangs the loop, and treating a channel as context silently drops real findings. -| Bot | Enabled | Posts line-by-line findings | Notes (plan/tier limits, quirks) | +| Bot | Enabled | Where findings appear | Notes (plan/tier limits, completion signal, quirks) | |---|---|---|---| -| CodeRabbit | yes | **yes** | Observed on PR #1 (plan: Pro Plus, profile CHILL): posts real inline review comments on the diff, each with severity and a committable suggestion, plus a walkthrough summary comment. Read the inline comments — the walkthrough is not a findings source. | -| Greptile | yes | **no** | Observed on PR #1: posts a single PR-level summary comment and nothing inline. Its findings live *inside* that summary under a "Comments Outside Diff" section, so they are easy to miss — parse the summary body, do not wait for line comments that never come. Took ~10 min to post. | +| CodeRabbit | yes | **inline** | Observed on PR #1 (plan: Pro Plus, profile CHILL): posts real inline review comments on the diff, each with severity and a committable suggestion, plus a walkthrough summary comment. Read the inline comments — the walkthrough is not a findings source. | +| Greptile | yes | **inconsistent — see notes** | **PR #1:** a single PR-level summary and nothing inline; its two findings sat *inside* that summary under "Comments Outside Diff". **PR #4:** two inline P2 comments, both valid and both accepted. Same repo, same account, the same day roughly seven hours apart (PR #4's inline comments are timestamped 2026-07-18T19:12Z). Behaviour is not predictable from two observations, so read **both** the inline comments and the summary body. Re-evaluate after the next observation, and keep this as `inconsistent` until evidence explains a *stable rule* — one more data point may add a third behaviour rather than settle anything, and reclassifying on thin evidence is what produced the wrong entry the first time. Took ~5–10 min to post on both. | | Cursor Bugbot | no | n/a | Comments only to say it is disabled for this account. Ignore. | -**Wait for:** CodeRabbit (inline findings) and Greptile (its summary carries the -findings) — both post a check, so `gh pr checks` going non-pending is the signal. +**Wait for — this list is authoritative.** CodeRabbit (inline findings) and Greptile +(inline *and* summary — check both, see its row). + +**Completion signal, per bot.** CodeRabbit posts a status, so `gh pr checks` shows it. +Greptile's is unreliable: `gh pr checks` listed a "Greptile Review" entry for PR #4 +while it was open, but the check-runs and statuses API for that same head +(`bf875b1`) returns only `quality` and `CodeRabbit` — so do not treat `gh pr checks` +alone as proof Greptile has finished. Wait for its review or summary comment to +appear via `gh pr view --json comments,reviews`. + +`/dev-workflow:process-pr-review` routes on **this list**, not on the table's column. +The column is descriptive — it says where a bot's findings tend to appear, and may read +`inconsistent`, which is not a routing instruction. A bot named here is waited for and +both of its channels are read; a bot absent here is not waited for. **Context only:** _(none)_ — Cursor Bugbot is disabled, not context. A bot belongs under **Wait for** only once it has been *seen* producing findings here. Listing an unconfirmed bot there is the failure this file exists to prevent — `/dev-workflow:process-pr-review` would block on findings that never arrive. -**Note the trap Greptile sets:** it is *not* a line-comment bot, so nothing appears -in `gh api .../pulls/N/comments` — but it is also *not* summary-only in the -"context, no findings" sense. Both of its PR #1 findings were real and were accepted. -Treating it as context-only would have silently dropped them. +**Note the trap Greptile sets:** on PR #1 nothing appeared in +`gh api .../pulls/N/comments`, yet it was *not* summary-only in the "context, no +findings" sense — both findings were real and accepted. On PR #4 it posted inline +instead. Either way the findings were real, so the safe reading is: check both +channels and treat whatever it produces as a findings source. + +This row is deliberately left ambiguous rather than resolved. The file's job is to say +what is actually known, and what is known today is that the behaviour varied between +two observations. A confident-looking entry backed by one PR is what put "no" here in +the first place. **Revisit when:** a bot's plan/tier changes (a Free→Pro upgrade can turn a summary-only bot into a findings bot), or a bot is enabled/disabled. diff --git a/docs/prompt-standards.md b/docs/prompt-standards.md index f65f653..1c136b7 100644 --- a/docs/prompt-standards.md +++ b/docs/prompt-standards.md @@ -64,7 +64,28 @@ Living references (consult, don't copy — copies go stale): preflight reported "restart the session" for every not-loaded Codex, so a same-named server winning on scope precedence went undiagnosed through this repo's own initialization. -11. **Calibrated emphasis.** Reserve MUST/CRITICAL/ALL-CAPS for genuinely hard +11. **Enforcement claims name their mechanism.** Any sentence saying something is + enforced, caught, guaranteed or prevented names *what does it*, and the author + verified that mechanism exists before writing the sentence — by reading the code, + running the command, or checking the doc it relies on. Why: an unverified guarantee + is worse than an admitted gap, because a reader stops looking. This one is + load-bearing: six instances are citable from this repo's own history, each caught by a + review gate and none by the author — "a rogue write cannot be invisible" (refuted by + `todos.md`), "all four gaps are recorded in todos.md" (two were), "output validation + catches a budget overrun" (the output carries no call count), a precheck attestation + described as proving a check ran when it is caller-authored text (commit `e30568d`), + "an altered claim lands on the wrong thread" (the caller's equality check rejects + it), and prose claiming the gate hook matched a path its regex does not match. One + was written into the same document that records this pattern. + + When the mechanism turns out not to exist, say what actually happens instead — + "this is a rule the agent keeps; nothing counts for it". + + Worth knowing how that count was arrived at: it was first asserted as six without + checking, then "corrected" to five from memory, and only settled at six by reading + the commits. Both wrong numbers were themselves unverified claims — which is the + strongest available argument for this item. +12. **Calibrated emphasis.** Reserve MUST/CRITICAL/ALL-CAPS for genuinely hard rules; default to plain wording ("Use X when …"). Why: current models follow instructions more literally and overtrigger on aggressive language (documented in the best-practices page). Existing heavy emphasis (e.g. the diff --git a/docs/superpowers/plans/2026-07-18-finding-triage-agent.md b/docs/superpowers/plans/2026-07-18-finding-triage-agent.md index 0c7ced7..74c19a8 100644 --- a/docs/superpowers/plans/2026-07-18-finding-triage-agent.md +++ b/docs/superpowers/plans/2026-07-18-finding-triage-agent.md @@ -1,5 +1,10 @@ # finding-triage Agent Implementation Plan +> **Historical note (added 2026-07-18, after merge).** References here to an "11-item" +> checklist and to "item 11" (calibrated emphasis) describe `docs/prompt-standards.md` +> as it stood when this plan was approved. It now has 12 items, and item 11 is +> "enforcement claims name their mechanism". +> > **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 one read-only subagent, `finding-triage`, that judges whether a single PR-review defect claim is true of the code, and wire it into `process-pr-review`. diff --git a/docs/superpowers/specs/2026-07-18-subagent-definitions-design.md b/docs/superpowers/specs/2026-07-18-subagent-definitions-design.md index fa84167..a1633e8 100644 --- a/docs/superpowers/specs/2026-07-18-subagent-definitions-design.md +++ b/docs/superpowers/specs/2026-07-18-subagent-definitions-design.md @@ -1,5 +1,12 @@ # Subagent definition: finding-triage — Design +> **Historical note (added 2026-07-18, after merge).** This document and its plan +> reference an 11-item `docs/prompt-standards.md` and use "item 11" to mean *calibrated +> emphasis*. A twelfth item was added afterwards — "enforcement claims name their +> mechanism", now item 11 — so present-tense numbering here refers to the checklist as +> it stood when this was approved. The change is itself a hardening of a pattern this +> document's own review exposed. + **Date:** 2026-07-18 · **Status:** narrowed after Gate A pass 4, revised after pass 5; Gate A clean at pass 6 (SHIP) · **Version target:** 0.4.0 **Kill condition.** If Gate A on this narrowed spec does not converge — final pass diff --git a/plugins/dev-workflow/commands/process-pr-review.md b/plugins/dev-workflow/commands/process-pr-review.md index e69b4ec..c1113ec 100644 --- a/plugins/dev-workflow/commands/process-pr-review.md +++ b/plugins/dev-workflow/commands/process-pr-review.md @@ -10,11 +10,14 @@ Target model: Claude via Claude Code. ## Step 0 — Load this project's bot matrix -Read `docs/pr-review-bots.md`. It states which bots run on this repo, **which of -them actually post line-by-line findings**, and which only post a summary. This -distinction is the whole point of the file: waiting on a bot that structurally -cannot post line comments hangs the loop forever, and treating a summary as a -findings source silently drops real findings. +Read `docs/pr-review-bots.md`. Its **Wait for** list is the routing authority — the +table beside it is descriptive, recording *where* each bot's findings have been seen +and how you know it has finished. A bot's channel can vary between PRs, so a row may +honestly read `inconsistent`; that is an observation, never a routing instruction. + +Getting this wrong is expensive in both directions: waiting on a channel a bot never +uses hangs the loop forever, and treating a findings source as context silently drops +real findings. If `docs/pr-review-bots.md` is missing: do not guess the matrix. Run `gh pr view --json comments,reviews` to see which bots have actually posted on this @@ -32,10 +35,15 @@ the end. ## Step 2 — Wait for the findings bots -Wait only for the bots `docs/pr-review-bots.md` lists as **posting line-by-line -findings**. Once each of those has posted its review (even with zero comments), -begin. Summary-only bots are context, never a findings source — do not wait for line -comments from them. +Route on that file's **Wait for** list, which is authoritative. Wait for every bot +named there, and read **both** channels for each — inline review comments *and* the +PR-level summary body. Do not route on the table's column: it describes where a bot's +findings tend to appear and may read `inconsistent`, which is an observation, not an +instruction. + +A bot absent from **Wait for** is not waited for. A bot present there is a findings +source in whichever channel it used, and its row names the completion signal to watch +— which is not always a check: a bot can post findings without ever posting a status. ## Step 3 — Process diff --git a/plugins/dev-workflow/commands/workflow-init.md b/plugins/dev-workflow/commands/workflow-init.md index 1ca0fa2..432a9eb 100644 --- a/plugins/dev-workflow/commands/workflow-init.md +++ b/plugins/dev-workflow/commands/workflow-init.md @@ -291,8 +291,20 @@ advisory — validate before applying; dismissed finding → one-line why. (args `instruction`, `whatWasImplemented`, `baseSha`; `reviewType: full` runs spec + quality in parallel). Skip ONLY trivial changes. Check against @AGENTS.md. Re-review after every fix — a fix changes the diff and the hook - invalidates the prior pass, which is where the 3 come from. Same coverage rule - as Gate A: put "report every finding with severity and confidence; say + invalidates the prior pass, which is where the 3 come from. + + **A fix that changes specified behaviour updates the spec in the same commit.** If a + Gate-B fix alters something the approved spec pins down — an ordering, a terminal + state, a contract — the spec is stale the moment you commit, and the next reader + trusts it. Update both, and let the re-review cover both. This is not hypothetical: + a Gate-B fix here reordered a precedence rule and added a terminal state, the spec + was left describing the old behaviour, and a PR bot found the disagreement after + merge-readiness. Neither gate caught it *in that run*: Gate A had already passed the + spec before the fix existed, and the Gate-B call was given only the diff. A reviewer + handed both artifacts could catch it — which is why this is a rule about what you + commit, not a claim about what the gates detect. + + Same coverage rule as Gate A: put "report every finding with severity and confidence; say `NO FINDINGS` if clean" in `additionalContext`, with the same one-line format. **What counts as prose (the only Gate-B exemption).** Every staged path is @@ -468,7 +480,14 @@ Living references (consult, don't copy — copies go stale): own fix. Why: causes with an identical symptom but different fixes are the case the reader cannot resolve alone — offering only the most common one sends them round a loop that never terminates. -11. **Calibrated emphasis.** Reserve MUST/CRITICAL/ALL-CAPS for genuinely hard +11. **Enforcement claims name their mechanism.** Any sentence saying something is + enforced, caught, guaranteed or prevented names *what does it*, and the author + verified that mechanism exists before writing it — by reading the code, running the + command, or checking the doc it relies on. Why: an unverified guarantee is worse + than an admitted gap, because a reader stops looking. When the mechanism turns out + not to exist, say what actually happens instead ("this is a rule the agent keeps; + nothing counts for it"). +12. **Calibrated emphasis.** Reserve MUST/CRITICAL/ALL-CAPS for genuinely hard rules; default to plain wording ("Use X when …"). Why: current models follow instructions more literally and overtrigger on aggressive language (documented in the best-practices page). Existing heavy emphasis (e.g. @@ -517,27 +536,35 @@ revalidation entry in `todos.md`. ### 2.5 `docs/pr-review-bots.md` — which bot actually finds things The `/dev-workflow:process-pr-review` command reads this. Fill the table with the -user — ask which bots are enabled and, for each, **whether it posts line-by-line -review comments or only a summary**. Do not guess: the whole point of the file is -that a summary-only bot must never be waited on for line findings. +user — ask which bots are enabled and, for each, **where its findings appear**: inline +review comments, the PR summary body, or both. Do not guess, and do not assume the +answer is stable: a bot can post a summary on one PR and inline comments on the next, +so a column may honestly read `inconsistent`. Record what was observed and when. ````markdown # PR review bots — Which automated reviewers run on this repo, and what each one *actually produces*. -`/dev-workflow:process-pr-review` reads this file to decide **which bots to wait for** -(only those that post line-by-line findings) and which to treat as context only. +`/dev-workflow:process-pr-review` routes on the **Wait for** list below — that list is +authoritative. The table is descriptive: it records where each bot's findings have been +seen, which may be `inconsistent`. -Getting this wrong is expensive in both directions: waiting on a bot that structurally -cannot post line comments hangs the loop, and reading a summary as a findings source +Getting this wrong is expensive in both directions: waiting on a bot that never posts +in the channel you are watching hangs the loop, and treating a channel as context silently drops real findings. -| Bot | Enabled | Posts line-by-line findings | Notes (plan/tier limits, quirks) | +| Bot | Enabled | Where findings appear | Notes (plan/tier limits, completion signal, quirks) | |---|---|---|---| -| | yes/no | **yes** / no — summary only | | - -**Wait for:** -**Context only:** +| | yes/no | inline / summary / both / **inconsistent** | | + +**Wait for — this list is authoritative.** +**Context only:** **Revisit when:** a bot's plan/tier changes (a Free→Pro upgrade can turn a summary-only bot into a findings bot), or a bot is enabled/disabled.