diff --git a/CLAUDE.md b/CLAUDE.md index 663ce62..c620b13 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -118,6 +118,27 @@ the delete. Run one pass at a time: the slot name has no invocation-unique compo two concurrent calls on one slot race. Passes are sequential by construction, so that is a stated limitation, not a guarded one. +**Optional companions, from field practice.** Two files may sit beside a findings file. +Both are advisory human notes: neither is ever the findings file, neither participates in +pass validation, and either may be deleted or rebuilt. The findings file plus its +terminator remain the only hard requirement, and a zero-finding pass needs no companion. + +- `-dispositions.md` — one line per finding: verdict + reason. It makes a dismissal + durable, so "we looked at that and why" outlives the session rather than the chat. +- a **cycle-stable** resume note when a cycle is interrupted — `gate-a-spec-resume.md`, + `gate-a-plan-resume.md`, `gate-b-resume.md`. Cycle-stable, not pass-named: a note keyed + to the interrupted pass number is exactly the file a resuming agent will not look for + once the counter moves or an incomplete pass is discounted. Gate A runs separate spec + and plan loops, so those are two cycles; Gate B is one cycle with one note even under + `reviewType: full`, because the per-branch findings files race only since Codex's two + reviewers write them — the resume note is written by the outer agent, sequentially, and + splitting it would create two records able to disagree about one shared recovery budget. + Whoever runs the cycle writes it when useful, replaces it as the cycle moves, and + deletes it once the cycle closes. Nothing depends on it existing. + +(Field practice, infinite-portfolio-canvas: 7 dispositions files and a Gate-A resume note +had been invented per-session there before the protocol knew about them.) + **Accept a pass only when** the file exists and is readable; its last line is exactly `END OF FINDINGS ( total)`; it contains exactly `` finding lines *and nothing else* (or the single line `NO FINDINGS` when `` is 0 — "n valid lines somewhere in diff --git a/docs/hardening-log.md b/docs/hardening-log.md index 1c57175..6a0e7b7 100644 --- a/docs/hardening-log.md +++ b/docs/hardening-log.md @@ -28,3 +28,4 @@ escape `\|`, one line), `source` (gate-a|gate-b|bot|manual), | 2026-07-25 | unverified-enforcement-claim | docs/sparring-briefing.md asserted "This is a prompt artifact and follows docs/prompt-standards.md" while giving its target model as "any capable chat model" — checklist item 1 requires a NAMED executing model plus the author having checked that model's prompting page, and a model class satisfies neither half (there is no single page to check), so the document contradicted its own conformance claim | bot | major | pending | todos.md § Tooling revalidation, "Prompt-standards conformance checker — resolves two `pending` ledger rows (2026-07-25)". THIRD occurrence, after 2026-07-18 `P std` (prompt-standards item 11) and 2026-07-19 `1 prose` (the "never describe what a gate proves" rule, whose own ref states it is human-run, not CI-run, and "raises the floor; it does not close the class"). The class has now cycled P std → 1 prose → recurrence without ever reaching a deterministic rung, and this instance IS tool-decidable — a file claiming to follow prompt-standards must carry a `Target model:` line — which is why the escalation is mechanical rather than a fourth prose rule. The FIX landed in 4f31df2 (PR #12); the HARDENING is what is pending, deferred to the canvas-findings round on the same scope reasoning as the sibling row. Resolve by appending a rung-2 row referencing this one, never by editing it | | 2026-07-26 | unverified-enforcement-claim | third occurrence, resolved: a prompt asserting it follows docs/prompt-standards.md while naming no executing model — the PR #12 sparring-briefing said `Target model: any capable chat model` | bot | major | 2 lint | scripts/check-invariants.sh check 4a + fixtures in .test.sh. **Resolves the 2026-07-25 `pending` row** (same fingerprint, appended not edited). GUARDED SPELLING, exactly: in a `*.md` file containing the literal `prompt artifact and follows`, the column-zero `^Target model:` line must appear exactly once, its value must BEGIN with a tested token (`Claude\|Codex\|GPT`, portable ERE boundary — token-anywhere is not enough, since the real defect line contained "Claude" as provenance), and it must name exactly one DISTINCT token. STILL INSTRUCTION-BACKED: a bare `Target model: Claude` naming no execution surface passes; a value whose prose is wrong in a way no token test sees passes; a column-zero declaration inside a fenced block counts (no fence awareness in POSIX grep); an unknown future model fails until the token list is extended. BLIND SPOT, stated because it is structural: this ledger is EXCLUDED from the check it records — a ledger that quotes defects self-rejects the checks that detect them, and scanning it would fail the repo forever on rows like this one. OVER-ESCALATION WARNING: `harden-finding` compares fingerprints only, so a later in-class defect OUTSIDE the guarded spelling will be proposed for a stronger rung than anything justifies; that is a proposal a human validates, and this row names the guard so they can judge it. The skill's scope-blind recurrence rule is parked in todos.md | | 2026-07-26 | docs-drift | third occurrence, resolved: a prose count contradicting the checklist it counts — PR #12's `docs/prompt-standards.md` said "all ten items" while its own checklist ran 1-12 | bot | minor | 2 lint | scripts/check-invariants.sh check 4b + fixtures in .test.sh. **Resolves the 2026-07-25 `pending` row** (same fingerprint, appended not edited). GUARDED SPELLING, exactly: two-stage claim recognition over `*.md` — stage 1 matches `all ( checklist)? items` with outer word boundaries, stage 2 requires canonical decimal, so `all 012 items` fires as malformed rather than being invisible; every recognised claim must equal N, where N comes from exactly one `^## Checklist([[:space:]].*)?$` heading whose body holds labels forming the contiguous canonical sequence 1..N, in BOTH docs/prompt-standards.md and the workflow-init template, which must agree. Word forms are in scope BECAUSE the motivating occurrence was one — a digit-only check would have sailed past the very defect this row claims to harden. STILL INSTRUCTION-BACKED: counts above twenty in word form, ordinals, hyphenated compounds (`all twenty-one items`), split-line claims, and any count of something other than this checklist. Same ledger blind spot and over-escalation warning as the row above | +| 2026-07-26 | session-bound-context-not-durable | Gate A/B dispositions and interrupted-cycle state lived only in chat history, so a dismissal's reasoning and a cycle's resume point died with the session; the field had independently invented per-pass companion files (7 dispositions files and a Gate-A resume note in infinite-portfolio-canvas) because the shipped protocol knew only about the findings file | manual | minor | P std | CLAUDE.md §5 "Optional companions, from field practice" + the same block in the workflow-init inline template: `-dispositions.md` and a cycle-stable resume note (`gate-a-spec-resume.md`, `gate-a-plan-resume.md`, `gate-b-resume.md`). MECHANISM, stated exactly: this is a paragraph of prompt text and **nothing enforces it** — the companions are optional, no step requires one to exist, none participates in pass validation, and a cycle that writes none behaves exactly as before. It recommends durable notes; it does not make context durable. Reading `P std` as more than that would be `unverified-enforcement-claim`, the class resolved at rung 2 in #13. BLIND SPOT inherited from #13: checks 4a/4b do not scan docs/hardening-log.md, so nothing mechanical reads this row either. Gate A rejected `truncated-tool-output-read-as-complete` for this: nothing is truncated, the content was never written | diff --git a/docs/hardening-taxonomy.md b/docs/hardening-taxonomy.md index 00bba1c..0ea317a 100644 --- a/docs/hardening-taxonomy.md +++ b/docs/hardening-taxonomy.md @@ -72,6 +72,16 @@ the synonyms a future reader might search for instead. (Defined here retroactively: the 2026-07-20 ledger row used this class before any definition existed. Recorded now so the recurrence grep has something to land on.) +- `session-bound-context-not-durable` — reasoning that has to outlive a session is left in + chat history, so it is gone at the next session, model change or tool switch. Aliases: + lost rationale, why-did-we-dismiss-that, undocumented decision, no resume state, + context died with the session, "it was in the chat". + + **Not `docs-drift`.** Drift is two artifacts *disagreeing*; this is one artifact never + existing. The fix differs too: drift is repaired by updating whichever is stale, this by + writing the thing down at all. Also not `truncated-tool-output-read-as-complete` — + nothing here is truncated or misread as complete; the content was simply never durable. + **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 diff --git a/docs/pr-review-bots.md b/docs/pr-review-bots.md index af1b127..9c9cefe 100644 --- a/docs/pr-review-bots.md +++ b/docs/pr-review-bots.md @@ -10,21 +10,79 @@ uses hangs the loop, and treating a channel as context silently drops real findi | Bot | Enabled | Where findings appear | Notes (plan/tier limits, completion signal, quirks) | |---|---|---|---| -| 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. | +| 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. **Its status check can pass while the comment says "Review rate limited" — see the completion-signal note: a green check does not prove the final head was reviewed.** | | Greptile | yes | **summary always; inline usually** | Four PRs observed (#1, #2, #4, #5): a PR-level **summary comment every time**, with findings sometimes only inside it under "Comments Outside Diff". **Inline** comments on #2 (1), #4 (2), #5 (1) but **none on #1** — so inline is usual, not guaranteed. Read both channels; the summary is the one that has never been missing. **Completion signal: none you can block on.** `gh pr checks` displayed a "Greptile Review" entry for #4 and #5, but the check-runs and statuses APIs return no Greptile entry for any of those heads — the two tools disagree, so neither proves it has finished. Posts within ~4–11 min. | | Cursor Bugbot | no | n/a | Comments only to say it is disabled for this account. Ignore. | **Routing — these lists are authoritative.** -- **Wait for (block on it):** CodeRabbit. It has a status check, so `gh pr checks` - going non-pending is proof it finished. +- **Wait for (block on it):** CodeRabbit. It has a status check, so `gh pr checks` going + non-pending is proof the *check* finished — which is what you block on. That is **not** + the same as proof the final head was reviewed; verify that separately before merging + (below). - **Process opportunistically (never block):** Greptile. Read whatever it has posted when the CodeRabbit-gated pass begins, in both channels. If it posts later, handle it as a follow-up. - **Ignore:** Cursor Bugbot — disabled for this account, and it says so itself. -**Completion signal, per bot.** CodeRabbit posts a status, so `gh pr checks` shows it -and you can block on it. +**Completion signal, per bot.** CodeRabbit posts a status, so `gh pr checks` shows it and +you can block on it. **Two different things, and conflating them merges unreviewed heads.** + +- *The check stopped pending* — the blocking signal. Block on this. +- *The final head was reviewed* — a separate verification. Observed on #12 and #13: the + check passed while the issue comment read "Review rate limited", and on #13 the only + CodeRabbit **review record** carried `commit_id` `eed589c` while the merged head was + `92de0d2`. The head that merged was never reviewed, and the green check said nothing + about it. + +Verify the second before merging — a deterministic boolean, so it can gate rather than be +eyeballed: + +```sh +head=$(gh pr view --json headRefOid --jq .headRefOid) # the LIVE head, not local HEAD +gh api --paginate repos///pulls//reviews | jq -s " + [ .[][] + | select(.user.login==\"coderabbitai[bot]\") + | select(.commit_id==\"$head\") + | select((.body // \"\") | test(\"rate limit\"; \"i\") | not) + | select(.state==\"COMMENTED\" or .state==\"APPROVED\" or .state==\"CHANGES_REQUESTED\") + ] | length" | grep -qv '^0$' +``` + +**`--slurp` cannot be used here, and an earlier draft of this file said it could.** +`gh api --slurp` is rejected outright when combined with `--jq` +("the `--slurp` option is not supported with `--jq` or `--template`"), so that command +failed with a usage error rather than returning a boolean. Pipe the paginated raw pages +to `jq -s` instead — `--paginate` emits one JSON array per page, `-s` wraps them, which is +why the filter iterates `.[][]`. **Verified against #13, and the two stages report differently — say which you mean:** the +`jq` stage prints the count, `0` for the merged head `92de0d2` and `1` for `eed589c`, the +commit actually reviewed; the full pipeline prints nothing and communicates through its +**exit status**, `1` for the merged head and `0` for the reviewed commit. That inversion +is deliberate — exit 0 means "a qualifying review exists", so the pipeline can gate a +merge directly. Documenting +the unrun form broke this repo's own rule against documenting a command nobody ran. + +**Four observations now, and the fourth is the one that mattered.** #12 and #13 both +*merged* heads that were never reviewed — the miss was only found afterwards. On **#14** +the check said `pass` while the comment said "Review rate limited", and this query +returned `0` for head `787dd9a` **before** the merge: the re-trigger produced nothing, and +the PR merged on an explicit human decision with the exception recorded, the unreviewed +delta being a one-word prose correction the reviewer had itself requested. That is the +intended shape — the check is a signal you block on, this query is what tells you whether +a review actually happened, and when they disagree a human decides. + +`--paginate` matters: without it only page one is read, so a qualifying review can sit on +page two and be read as absent. `jq -s` is what slurps the pages — `gh api --slurp` cannot +do it here, being rejected outright when combined with `--jq`. `DISMISSED` is excluded — a dismissed review is not +a review of that head. If no qualifying record exists, re-trigger once; if it is still +absent, **merge only on an explicit human decision**, recording that the head went +unreviewed. + +What was actually measured, stated exactly: the rate-limit warning appeared in the **issue +comment**, while the review record was an earlier completed review of an earlier commit. +So the demonstrated failure is a *missing* review for the final head — which the +`commit_id` comparison catches. No rate-limited *review record* has been observed; the +body filter is bounded defensive filtering, not a check against something seen. **Greptile has no signal you can block on**, across four PRs: `gh pr checks` displayed a "Greptile Review" entry for #4 and #5, while the check-runs and statuses APIs return no diff --git a/docs/superpowers/plans/2026-07-26-cycle-close-nudge-and-companions.md b/docs/superpowers/plans/2026-07-26-cycle-close-nudge-and-companions.md new file mode 100644 index 0000000..4f5928b --- /dev/null +++ b/docs/superpowers/plans/2026-07-26-cycle-close-nudge-and-companions.md @@ -0,0 +1,344 @@ +# Plan — PR 2: companion files, the deferred template sync, and the release + +Layer 2 of the canvas field-findings round. PR 1 (merged, #13) took finding D — the two +mechanical checks. This PR takes **C** (optional companion files), the deferred downstream-neutral template +sync, **one** taxonomy class, **one** ledger row, **five** `todos.md` edits, the +`pr-review-bots.md` caveat, and the **0.5.1 → 0.6.0** release. **Finding A was cut after five Gate-A passes** (see below) and +becomes a story alongside B. **It edits `plugins/**`, so invariant 12 requires a version bump: +0.5.1 → 0.6.0.** + +Finding B (a §5 version stamp) stays split out; this PR only records its story. + +Each task is **Scope → Rule → Files → Verify.** + +--- + +## Constraints PR 1 now imposes on this PR + +Checks 4a and 4b are live on main and scan `plugins/dev-workflow/commands/workflow-init.md`, +which this PR edits. Measured before starting: + +| | now | must stay | +|---|---|---| +| column-zero `Target model:` lines in `workflow-init.md` | 1 | exactly 1 | +| template checklist items | 12 | equal to the repo copy (12) | +| count claims in that file | 0 | any added must equal 12 | + +The §5 template lives inside a fenced block in that file. A `Target model:` line added at +**column zero** inside that fence counts toward 4a's cardinality — the check has no fence +awareness, by documented design. So template text must not introduce one. + +--- + +## Finding A — CUT from this PR, now a designed story + +**A drew a Major on all five Gate-A passes and is removed rather than narrowed again.** +The final version said: note every accepted actionable finding fixed this cycle, close the +cycle, then run `harden-finding` once per finding, and it claimed that scope matched +`process-pr-review` step 5 exactly. It does not. Step 5 **checks** every accepted +actionable finding against the ledger but **invokes** the skill only "if one matches an +existing class, or a new class is clearly warranted" — so the reminder would have +instructed agents to harden one-offs the settled PR path deliberately declines, while +asserting parity it did not have. + +That was the fifth consecutive pass concentrating fire on one paragraph, and the +accumulated findings say the same thing from five directions: a reminder that depends on +volatile same-session memory, covering a scope it keeps mis-stating, is not a solution to +"the only mandated ledger check lives on a path some projects never walk". A is a design — +identity, dedup, consumption semantics, and a scope rule that matches step 5 rather than +approximating it — and it joins B as a story (Task 5). + +**This PR therefore ships the full remaining scope:** C, the downstream-neutral template +sync, one taxonomy class, one ledger row, five `todos.md` edits, the `pr-review-bots.md` +caveat, and the 0.6.0 release. Nothing here depends on A — the companions are +independently useful, and the sync and bump were always separate obligations. + +--- + +## Task 1 — C: recommended companion files + +**Scope.** The field evolved per-pass companions the §5 protocol does not know about, so +dispositions and resume context live only in chat history. Canvas: 7 `*-dispositions.md`, +3 `*-decisions.md`, 1 Gate-A resume note. + +**Rule.** Extend the §5 file protocol with **optional** companions: +- `-dispositions.md` — one line per finding: verdict + reason. +- a resume note — **cycle-stable**, not pass-slot-named. A note bound to the interrupted + pass number is exactly the file a resuming agent does not know to look for once the + counter moves or an incomplete pass is discounted, which would reproduce finding C. + **One name per independent cycle:** `gate-a-spec-resume.md`, `gate-a-plan-resume.md`, + `gate-b-resume.md`. Gate A genuinely runs separate spec and plan loops, so those are two + cycles. Gate B is **one** cycle and gets **one** note, even though `full` runs two + reviewer branches: the findings files need one path per branch because *Codex's two + reviewers* write them concurrently, but the resume note is written by the outer agent, + sequentially. Splitting it per branch would create two records able to disagree about a + single shared recovery budget and a validation rule that already spans both files. +- **Lifecycle, kept as light as the rest of C.** Whoever runs the cycle writes it when + useful, replaces it as the cycle moves, and deletes it once the cycle closes. No + mandated fields, no write-ahead requirement, and nothing depends on it existing. + +**These stay OPTIONAL, and nothing in this PR depends on them.** They are advisory human +notes: no step requires one to exist, none participates in pass validation, and a cycle +that never writes one behaves exactly as it does today. An earlier draft made a resume +note load-bearing while still calling it optional; that dependency is gone along with the +feature that created it. + +The text must state they are advisory human notes, are **never** the findings file, +**never** participate in pass validation, and may be deleted or rebuilt. The findings file +plus terminator stay the only hard requirement. A zero-finding pass needs no companion. + +**Credit placement.** The field-practice credit (infinite-portfolio-canvas) goes in this +repo's `CLAUDE.md` and the CHANGELOG, **not** into the scaffolded template: downstream +readers do not need this repo's provenance to follow the rule, and inline template tokens +are governed by prompt-standards item 8. + +**Files.** This repo's `CLAUDE.md` §5; the §5 template in `workflow-init.md`; +`plugins/dev-workflow/CHANGELOG.md` (the credit lives there, so the task that settles the +credit owns the file — otherwise an implementer satisfies every per-task Files list and +still drops it). + +**Verify.** No sentence can be read as adding a validation requirement; the scaffolded +template contains no `infinite-portfolio-canvas` reference; the CHANGELOG entry does. + +--- + +## Task 2 — The deferred template sync, downstream-neutral + +**Scope.** PR #12 deferred adding the "ad-hoc task briefs are prompts too" paragraph to +the scaffolded template. This round is its named vehicle. + +**Rule — NOT verbatim.** Gate B on PR 1 established the repo paragraph is not portable: it +links `docs/sparring-briefing.md`, which `/workflow-init` does not scaffold, and asserts +"two field incidents" in this repo's voice. Copying it verbatim would ship a broken link +and a foreign claim into every initialized project. Write a **downstream-neutral variant** +with **no unscaffolded path reference and no incident count**. Verify **semantic parity**, +not byte equality. + +**Both halves of the principle must survive, or the variant becomes a false process +claim.** The repo paragraph says two things: an ad-hoc brief is a prompt and should carry +this checklist's habits (success criteria, stop conditions, verified claims), **and** +nobody reviews a brief against all 12 items — which is exactly why those habits have to +live in how briefs are written. Keeping only the first half would tell every initialized +project that each brief is formally reviewed, inventing a process no one runs. + +**Files.** `plugins/dev-workflow/commands/workflow-init.md`. + +**Verify.** The template paragraph contains no `sparring-briefing` reference and no +incident count; `sh scripts/check-invariants.sh` still exits 0 (the constraint table above). + +--- + +## Task 3 — Taxonomy: mint one class + +**Rule.** Add ONE class to `docs/hardening-taxonomy.md` (never the skill — invariant 10), +with alias hints: +*(`mandatory-step-anchored-to-optional-path` is NOT minted here. A is cut, so nothing in +this PR hardens it; the class is minted by the story that implements A, or the ledger +gains a class no row uses.)* +- `session-bound-context-not-durable` — reasoning that must outlive a session is left in + chat history. Gate A rejected `truncated-tool-output-read-as-complete` for C: nothing is + truncated there, and reusing it would corrupt that class's recurrence count. + +**Verify.** Before adding the class, run `harden-finding`'s own minting precondition — +grep **both** lists (the skill's base taxonomy *and* `docs/hardening-taxonomy.md`) for the +closest match, aliases included — and record the near matches compared. "It does not +duplicate an existing class" is not checkable without naming that comparison, and a +near-duplicate fingerprint silently splits a recurrence group in two. +**Re-read immediately before writing.** `harden-finding` requires a fresh read of the +ledger right before appending, and the same applies to the taxonomy: another branch or +agent can add the class or the row between the initial grep and the write, producing a +duplicate class or a duplicate first-occurrence row in an append-only file. Re-grep both +taxonomy lists before minting and re-run the anchored column-2 grep immediately before the +append; if a match appeared meanwhile, reconcile with it instead of appending. + +--- + +## Task 4 — Ledger: one appended row + +| fingerprint | source | severity | rung | ref | +|---|---|---|---|---| +| `session-bound-context-not-durable` | manual | minor | P std | the optional-companion section, both copies | + +**Rule.** The row must name its actual mechanism — a paragraph of prompt text — and state +plainly that **no checker or hook enforces compliance**. The companion convention can be +skipped entirely and nothing notices, so a bare `P std` row would read as though it made +session context durable rather than merely recommending durable notes; a row claiming more +than that is `unverified-enforcement-claim`, the class this repo resolved at rung 2 in #13. +The row also notes the blind spot inherited from PR 1: checks 4a/4b do not scan +`docs/hardening-log.md`, so nothing mechanical reads it at all. + +**Verify.** Anchored column-2 grep; no row edited. + +--- + +## Task 5 — todos.md: five edits + +**Entry e resolves the row this PR discharges.** `todos.md` still carries PR #12's +unchecked deferral — add the ad-hoc-brief paragraph to the scaffolded template "when +`commands/workflow-init.md` is next touched", naming this round as its vehicle. Task 2 +*is* that work, so the row must be **marked resolved in the same PR**, noting that the +scaffolded text is a downstream-neutral variant rather than the verbatim paragraph the row +anticipated. Leaving it unchecked would advertise finished work as pending and keep a +now-false "upcoming vehicle" pointing at a round that already happened. + +**Placement, named — otherwise two implementers place these differently and one of them +creates active work.** Entries **a**, **c** and **d** are **new unchecked, trigger-gated +rows appended under the existing `## Tooling revalidation` heading**, matching the +trigger-gated form already used there (`*Trigger: …*`). Entry **b** does **not** add a +row: it **mutates the existing P2+P6 row in place**, adding the calibration point and +marking its trigger FIRED. Nothing goes under `## Now` — none of these is active work. + +**Verify.** Five edits: three appended rows, the P2+P6 row mutated, and PR #12's +template-sync row marked resolved; `## Now` unchanged; and +before appending, grep `todos.md` for each story's subject so a parked concern is not +duplicated under a second phrasing. + + +a. **Finding-B story** — spec questions: a semantic §5 locator (init may append the + section renumbered); per-state merge semantics (invariant 9 forbids silent overwrite, + and "re-run init to sync" promises what the command cannot give); stamp cardinality; and + a binding real on **every** push path, since the version-bump coupling proposed was + false — invariant 12's checker is `pull_request`-only. A stamp is a **wire format**, so + shipping a provisional one creates legacy on write. Record that canvas is being + re-synced manually right after this round, so the story carries no false schedule + pressure. +b. **Canvas calibration point** on the P2 row: 51 Gate-A pass files across 2 stories + (spec 14 + plan 14 + replan 4 + amend 12 + a3-spec 7), trigger **FIRED**. Edit only — + P2+P6 and P5-light are not implemented here. +c. **`harden-finding`'s recurrence rule is scope-blind.** Rungs guard scopes; the skill + compares only fingerprints; a ledger-prose workaround is unenforceable because agents + follow the skill, not the row. Sketched fix — before proposing escalation on a + same-fingerprint recurrence, read the prior row's stated guard: + - **outside** the guard → the prior mechanism never claimed this shape, so its rung did + not fail. Select the fitting rung independently; **do not escalate**. + - **inside** the guard → the mechanism was supposed to catch this and did not. That is a + regression: diagnose and repair or strengthen *that* mechanism. + **An earlier draft of this entry had those two branches inverted** — it said an + out-of-guard occurrence should escalate, which is precisely the over-escalation the + 2026-07-26 ledger rows warn about, so the parked "fix" would have entrenched the bug it + was filed against. Caught at Gate A. Trigger: the first human rejection of an + over-escalation the rows predicted, or the next round touching the skill. +d. **Finding A — a route from a fixed finding to the ledger for projects that never open + PRs.** Cut from this PR after drawing a Major on all five Gate-A passes; the accumulated + findings ARE its opening evidence, so the story starts from a real spec rather than a + blank page: + - it cannot rest on same-session memory — a compaction, interruption or handoff loses + the fixed-finding set and **nothing detects the loss** (pass 4, M4); + - its scope must **match `process-pr-review` step 5 exactly**: check every accepted + actionable fixed finding, but invoke `harden-finding` only when a class matches or a + new one is clearly warranted. Every draft that approximated this got the parity claim + wrong (pass 4 M5, pass 5 M1); + - a durable handoff needs real design — identity, dedup, consumption semantics — which + is why it was refused as a mid-round addition (pass 3, M4); + - it mints `mandatory-step-anchored-to-optional-path` when it lands; minting it earlier + would leave a class no ledger row uses. + *Trigger: the next round that touches §5, or a project reporting an empty ledger across + cycles that fixed findings.* + +--- + +## Task 6 — `docs/pr-review-bots.md`: the CodeRabbit caveat + +**Rule.** Record what both #12 and #13 showed: CodeRabbit's status check can pass while +the comment reads **"Review rate limited"**, so a green check does **not** prove the final +head was reviewed. + +**This is not only a table note, and calling it one would leave a contradiction in the +document.** The file currently says non-pending status *is* proof the bot finished, which +the new evidence contradicts. The edit must separate two things that were conflated and +update **all three sites together** — the table row, the **Wait for** entry, and the +completion-signal paragraph: + +- **"the check stopped pending"** — still the blocking signal, so CodeRabbit stays under + *Wait for*. That is what you may block on. +- **"the final head was reviewed"** — a *separate* verification, and it must name the + exact object or it just replaces one false proof with another. Issue comments carry no + reviewed commit, and an inline comment's `commit_id` proves a comment, not a completed + review. Use the **pull-request review** record, which does expose `commit_id`: + ```sh + head=$(gh pr view --json headRefOid --jq .headRefOid) # the LIVE head, not local HEAD + gh api --paginate repos///pulls//reviews | jq -s " + [ .[][] + | select(.user.login==\"coderabbitai[bot]\") + | select(.commit_id==\"$head\") + | select((.body // \"\") | test(\"rate limit\"; \"i\") | not) + | select(.state==\"COMMENTED\" or .state==\"APPROVED\" or .state==\"CHANGES_REQUESTED\") + ] | length" | grep -qv '^0$' + ``` + **The pagination is in the command above, not a note beside it.** Without `--paginate` + the query reads only the first page, so on a long-lived PR the qualifying final-head + review can sit on page two and be read as absent — sending a correctly-reviewed head + down the retrigger/human-override path. + + **`gh api --slurp` cannot be used with `--jq`** — it is rejected outright ("the + `--slurp` option is not supported with `--jq` or `--template`"), so an earlier draft's + command failed with a usage error instead of returning a boolean. Pipe the paginated raw + pages to `jq -s` instead; `--paginate` emits one array per page and `-s` wraps them, + which is why the filter iterates `.[][]`. **Verified against #13:** `0` for the merged + head `92de0d2`, `1` for `eed589c`, the commit actually reviewed. Documenting the unrun + form violated this repo's own rule against documenting a command nobody ran — run it. + That is a deterministic boolean: it exits non-zero when **no** qualifying record exists, + so it can gate a merge instead of being eyeballed. `DISMISSED` is excluded — a dismissed + review is not a review of that head. Several qualifying records are fine; one is enough. + + **What was actually measured, stated exactly, because the defensive filter is not the + observation.** On #12 and #13 the rate-limit warning appeared in CodeRabbit's **issue + comment**, while the **pull-request review record** was an earlier, completed review of + an earlier commit. The failure those PRs demonstrate is therefore a *missing* review for + the final head — which the `commit_id` comparison catches — **not** a rate-limited review + record carrying a stale commit id. No such record has been observed. The body filter is + retained as **bounded defensive filtering** in case one exists, and must not be described + as validating the warning that was seen; claiming otherwise would be this repository's + recurring habit of promoting a defensive check into an observation. + + **The match must be case-insensitive, and the reason is not the one first given.** Gate A + flagged this filter as case-sensitive and claimed `Review rate limited` would escape it. + Measured: it does **not** escape — the capital R is in `Review`, and `rate limited` + matches that string fine. But `⚠️ Rate limited` **does** escape, because there the + capital is in `Rate`. So the fix is right and its stated reason was wrong; the pattern is + now `test("rate limit"; "i")`, matching either spelling and the shorter stem. And the head must come from the **live PR** immediately before + merging; comparing against local `HEAD` or a stale checkout reproduces the false-proof + class this task exists to close. A record whose body says the review was rate + limited does not count. + + **Measured on #13, which is why this is a rule and not a worry.** Its only CodeRabbit + review record carries `commit_id = eed589c`, while the merged head was `92de0d2` — the + one-token awk fix pushed after that review. The check was green and the PR merged; the + final head was never reviewed, and the command above shows it in one line. `#12` settled + the same way. + +When they disagree, the bounded path is: re-trigger once, and if it is still +rate-limited, **merge only on an explicit human decision**, recording that the head went +unreviewed. Never let a green check stand in for a review that did not happen — that is +`unverified-enforcement-claim` applied to a bot. + +--- + +## Task 7 — Release + +**Rule.** Bump `plugins/dev-workflow/.claude-plugin/plugin.json` 0.5.1 → **0.6.0** (minor: +the scaffolded §5 template gains behaviour). CHANGELOG entry naming C's +companions **and** the template synchronization — and **not** A, which this PR does not +ship; naming it would publish behaviour 0.6.0 deliberately does not contain — omitting either would under-report a +user-visible template change in the same release. + +**Verify.** `sh scripts/check-version-bump.sh main` ok. + +--- + +## Invariants touched + +**2** (directional), **5**, **6**, **9** (constrains the deferred B story and any +`/workflow-init` edit), **10** (taxonomy stays project-local), **11** (every changed +prompt passes all 12 checklist items — `CLAUDE.md`, `workflow-init.md` and its inline +templates are prompts, and a self-review is required before Gate B), **12** (0.6.0). +**Hooks are not touched** — if any task pulls toward `codex-gate.sh`, stop and surface. + +## Success criteria + +1. **Prompt-standards self-review (invariant 11)** over every changed prompt — required + here, unlike PR 1, because this PR edits prompts rather than shell. +2. The **exact** quality command from `AGENTS.md § Commands`, green — including the two + checks merged in #13, which now police the file this PR edits. +3. `sh scripts/check-version-bump.sh main` green **after** the work is committed. +4. Gate B full: `plugins/**` and `CLAUDE.md` are the product, so no prose exemption applies. diff --git a/plugins/dev-workflow/.claude-plugin/plugin.json b/plugins/dev-workflow/.claude-plugin/plugin.json index 9ac4a2a..eeacb31 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.5.1", + "version": "0.6.0", "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 057cf66..c583731 100644 --- a/plugins/dev-workflow/CHANGELOG.md +++ b/plugins/dev-workflow/CHANGELOG.md @@ -22,6 +22,27 @@ 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.6.0 + +- **Optional companion files beside a gate findings file.** The §5 protocol knew only + about the findings file, so dispositions and interrupted-cycle state lived in chat + history and died with the session. The scaffolded template now describes two advisory + companions: `-dispositions.md` (one line per finding: verdict + reason) and a + cycle-stable resume note — `gate-a-spec-resume.md`, `gate-a-plan-resume.md`, + `gate-b-resume.md`. Cycle-stable rather than pass-named, because a note keyed to the + interrupted pass is exactly the file a resuming agent will not look for once the counter + moves. Both are optional and non-validating: the findings file plus its terminator remain + the only hard requirement, and nothing enforces the companions. Field practice from + infinite-portfolio-canvas, which had invented 7 dispositions files and a Gate-A resume + note per-session before the protocol knew about them. +- **The scaffolded prompt-standards template now covers ad-hoc task briefs.** A brief + handed to the coding agent for one task is a prompt with the same failure modes, held to + the checklist in spirit — success criteria, stop conditions, verified claims — while + nobody reviews a brief against all 12 items, which is why those habits must live in how + briefs are written. Deferred since 0.5.0 and deliberately **not** a verbatim copy of this + repo's paragraph: that one links a file `/workflow-init` never scaffolds and asserts this + repo's own incident count, so the template carries a downstream-neutral variant. + ## 0.5.1 - **Gate findings go to a file, not the MCP response.** Long finding lists came back cut diff --git a/plugins/dev-workflow/commands/workflow-init.md b/plugins/dev-workflow/commands/workflow-init.md index e43ffdf..b71c272 100644 --- a/plugins/dev-workflow/commands/workflow-init.md +++ b/plugins/dev-workflow/commands/workflow-init.md @@ -299,6 +299,25 @@ the delete. Run one pass at a time: the slot name has no invocation-unique compo two concurrent calls on one slot race. Passes are sequential by construction, so that is a stated limitation, not a guarded one. +**Optional companions, from field practice.** Two files may sit beside a findings file. +Both are advisory human notes: neither is ever the findings file, neither participates in +pass validation, and either may be deleted or rebuilt. The findings file plus its +terminator remain the only hard requirement, and a zero-finding pass needs no companion. + +- `-dispositions.md` — one line per finding: verdict + reason. It makes a dismissal + durable, so "we looked at that and why" outlives the session rather than the chat. +- a **cycle-stable** resume note when a cycle is interrupted — `gate-a-spec-resume.md`, + `gate-a-plan-resume.md`, `gate-b-resume.md`. Cycle-stable, not pass-named: a note keyed + to the interrupted pass number is exactly the file a resuming agent will not look for + once the counter moves or an incomplete pass is discounted. Gate A runs separate spec + and plan loops, so those are two cycles; Gate B is one cycle with one note even under + `reviewType: full`, because the per-branch findings files race only since Codex's two + reviewers write them — the resume note is written by the outer agent, sequentially, and + splitting it would create two records able to disagree about one shared recovery budget. + Whoever runs the cycle writes it when useful, replaces it as the cycle moves, and + deletes it once the cycle closes. Nothing depends on it existing. + + **Accept a pass only when** the file exists and is readable; its last line is exactly `END OF FINDINGS ( total)`; it contains exactly `` finding lines *and nothing else* (or the single line `NO FINDINGS` when `` is 0 — "n valid lines somewhere in @@ -521,6 +540,12 @@ Skills, gate prompts (CLAUDE.md §5), hook messages, slash commands, agent defin templates are prompts. When authoring or changing one, it must pass the checklist below — Gate A reviews prompt specs against these criteria via AGENTS.md. +**Ad-hoc task briefs are prompts too.** A brief handed to the coding agent for a single +task steers the same model with the same failure modes as anything above. Briefs are held +to this checklist **in spirit** — success criteria, stop conditions, verified claims — +but nobody reviews a brief against all 12 items, which is exactly why those habits have +to live in how briefs are written rather than in a review step. + Living references (consult, don't copy — copies go stale): - Anthropic prompting best practices: https://platform.claude.com/docs/en/build-with-claude/prompt-engineering/claude-prompting-best-practices diff --git a/todos.md b/todos.md index 54ab35e..f6053d0 100644 --- a/todos.md +++ b/todos.md @@ -134,9 +134,11 @@ backlog. - [ ] **P2 + P6 — risk/security profiles, and security sections in the intake, spec and gate templates.** One story: the profile is what the sections key off, so shipping the sections without it just adds a heading nobody knows how to fill in. - *Trigger: the first real intake in a product project* — the first time a story - exists whose risk profile is a real answer rather than a guess about what product - projects might need. + *Trigger: the first real intake in a product project* — **FIRED.** + Calibration point (infinite-portfolio-canvas, 2026-07-26): 51 Gate-A pass files + across 2 stories — spec 14, plan 14, replan 4, amend 12, a3-spec 7 — in a product + project on a long-lived branch. That is a real risk profile to key sections off, + not a guess about what product projects might need. - [ ] **P5 light — stable AC-/SEC-IDs in the story and plan templates.** Identifiers that survive from story to plan to review, so an acceptance criterion can be cited instead of re-described. *Trigger: rides with P2* — the IDs exist to label what P2's @@ -182,17 +184,12 @@ backlog. model change on either side of it (sparring model or coding agent) can shift what its conventions should say. Concretely pending: the switch of the coding agent to the new Opus generation fires this row. -- [ ] When `commands/workflow-init.md` is next touched for other reasons, add the - "ad-hoc task briefs are prompts too" paragraph (docs/prompt-standards.md, - repo copy) to the scaffolded prompt-standards template as well — one - sentence of drift between repo copy and template, recorded here instead of - forcing a plugin release for it (same reasoning as the PR #10 scope - decision). **Resolution vehicle: the upcoming canvas-findings hardening - round**, which touches `commands/workflow-init.md` for template changes and - carries a version bump regardless — the one-sentence sync rides there at no - extra release cost. Re-raised by CodeRabbit on PR #12 (Major) and kept - deferred there on the same reasoning; the scaffolded copy carries no false - claim, only one paragraph less. +- [x] **Ad-hoc-brief paragraph synced into the scaffolded template.** Done in the + canvas-findings round's PR 2, the vehicle this row named. Not the verbatim repo + paragraph, which turned out to be unportable: it links `docs/sparring-briefing.md` + (never scaffolded) and asserts this repo's own incident count. The template carries + a downstream-neutral variant preserving both halves of the principle — briefs carry + the checklist's habits, and nobody reviews a brief against all 12 items. - [x] **Prompt-standards conformance checker — resolved the two `pending` ledger rows (2026-07-25).** Landed as checks 4a and 4b in `scripts/check-invariants.sh`, with fixtures in its regression suite, in the canvas-findings round's **PR 1**. (No @@ -210,6 +207,43 @@ backlog. editable prose should reword instead.) And the ledger itself had to be **excluded** from both checks, because a ledger that quotes defects self-rejects the checks that detect them. +- [ ] **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 + (`/workflow-init` may append the section renumbered, so "no §5 heading" can misread + a valid section and append a duplicate); per-state merge semantics (invariant 9 + forbids a silent overwrite, and "re-run init to sync" promises what the command + cannot give); stamp cardinality (absent, duplicate, malformed); and a binding real + on **every** push path — the version-bump coupling first proposed was false, since + invariant 12's checker is `pull_request`-only. A stamp is a **wire format**: + shipping a provisional one writes legacy into every scaffolded file. *The one + known-stale instance (canvas) is being re-synced by hand, so this carries no + schedule pressure.* *Trigger: the next round that touches the §5 template.* +- [ ] **`harden-finding`'s recurrence rule is scope-blind.** Rungs guard *scopes*; the + skill compares only *fingerprints*; a ledger-prose workaround is unenforceable + because agents follow the skill, not the row. Sketched fix — before proposing + escalation on a same-fingerprint recurrence, read the prior row's stated guard: + **outside** it the prior mechanism never claimed that shape, so its rung did not + fail — pick the fitting rung, do **not** escalate; **inside** it, the mechanism was + meant to catch this and did not, so that is a regression to repair or strengthen. + (An earlier draft had those branches inverted, which would have entrenched the bug + it was filed against; Gate A caught it.) *Trigger: the first human rejection of an + over-escalation the 2026-07-26 rows predicted, or the next round touching the skill.* +- [ ] **Finding A — a route from a fixed finding to the ledger for projects that never + open PRs.** The only mandated ledger check lives in `process-pr-review` step 5, so a + no-PR project never reaches it: canvas has 51 Gate-A pass files and **0** ledger + rows. Cut from the canvas-findings round after drawing a Major on all five Gate-A + passes; those findings are the story's opening evidence rather than a blank page: + it cannot rest on same-session memory, because a compaction, interruption or handoff + loses the fixed-finding set and **nothing detects the loss**; its scope must match + `process-pr-review` step 5 *exactly* — check every accepted actionable fixed + finding, but invoke `harden-finding` only when a class matches or a new one is + clearly warranted, which every approximating draft got wrong; and a durable handoff + needs real design (identity, dedup, consumption semantics), which is why it was + refused as a mid-round addition. It mints + `mandatory-step-anchored-to-optional-path` when it lands — minting it earlier would + leave a class no row uses. *Trigger: the next round that touches §5, or a project + reporting an empty ledger across cycles that fixed findings.* - [ ] **Escalation trigger for the invariant checker — read this before patching it.** The checker asserts only the spellings its fixtures cover. Adding one more regex arm per newly-discovered spelling is *not* the ladder working; it is the same