From 5be1b99a3c5d5cb0d99bb766619dbe2fce7a8b0b Mon Sep 17 00:00:00 2001 From: Bren Pike Date: Thu, 24 Sep 2026 08:59:46 -0600 Subject: [PATCH 1/7] refactor(plugin): state rules without history or tracker references --- CLAUDE.md | 2 +- plugin/agents/overlord.md | 16 ++++++---------- plugin/governance/remediation-doctrine.md | 6 +++--- plugin/references/brood-ledger-model.md | 12 ++++++------ plugin/references/github-pr-review-graphql.md | 12 ++++++------ plugin/references/run-ledger-schema.md | 18 +++++++++--------- plugin/skills/github-review-loop/SKILL.md | 4 ++-- plugin/skills/next-wave/SKILL.md | 4 ++-- plugin/skills/next-wave/scripts/next-wave.sh | 2 +- plugin/skills/record-state-result/SKILL.md | 4 ++-- plugin/skills/spawn-brood/SKILL.md | 2 +- plugin/workflows/pr-feedback-remediation.json | 2 +- plugin/workflows/standard-delivery.json | 4 ++-- 13 files changed, 42 insertions(+), 46 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 252ed01e..1219a7c5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -116,7 +116,7 @@ The local Codex review model is operator-overridable via `HIVEMIND_LOCAL_REVIEW_ The default post-PR watch (see Branching / PR workflow above) can be switched off standing-wide via `HIVEMIND_SKIP_PR_WATCH`, set in the `env` block of `.claude/settings.json` (committed) or `.claude/settings.local.json` (gitignored, per-account). Unset/empty → the overlord watches normally per its default-watch rule (zero behavior change). Set (checked by presence) → the overlord never watches, short-circuiting the per-run `request.raw` read entirely. Because the committed settings file is in-repo, the key inherits into brood worktrees. Reference: ADR-0029. -The post-merge decision report is opt-in via `HIVEMIND_ENABLE_DECISION_REPORT`, set in the `env` block of `.claude/settings.json` (committed) or `.claude/settings.local.json` (gitignored, per-account). Unset/empty → the report is off: the deferred-report scan renders nothing and makes no GitHub call, but still touches the zero-byte `.decision-report-done` marker for awaiting runs, so enabling it later only reports runs that finish after it is enabled. Set (checked by presence) → the overlord surfaces the post-merge decision report as before. The decision journal is written either way. Because the committed settings file is in-repo, the key inherits into brood worktrees. Reference: ADR-0030. +The post-merge decision report is opt-in via `HIVEMIND_ENABLE_DECISION_REPORT`, set in the `env` block of `.claude/settings.json` (committed) or `.claude/settings.local.json` (gitignored, per-account). Unset/empty → the report is off: the deferred-report scan renders nothing and makes no GitHub call, but still touches the zero-byte `.decision-report-done` marker for awaiting runs, so enabling it later only reports runs that finish after it is enabled. Set (checked by presence) → the overlord surfaces the post-merge decision report. The decision journal is written either way. Because the committed settings file is in-repo, the key inherits into brood worktrees. Reference: ADR-0030. ## Brood execution diff --git a/plugin/agents/overlord.md b/plugin/agents/overlord.md index ab8b987c..7ca6ad92 100644 --- a/plugin/agents/overlord.md +++ b/plugin/agents/overlord.md @@ -32,7 +32,7 @@ These are mechanical hard stops. They hold in every workflow state, in the Refle ## Reflex (Ledger-Skip) -A Reflex is the trivial fast path: it skips the router AND the run ledger. A task is a Reflex only when ALL hold — one owner, one known file, trivial change, branch classification clear, no version impact, no review remediation, no brood. For a Reflex, drive the short delivery tail by intent exactly as today: delegate the single change `with exact file scope`, checkpoint via `hivemind:molt`, validate, open the PR. The same default watch rule applies to the Reflex tail: after opening the PR, run `hivemind:github-review-loop` under the canonical predicate stated once under `## Review Remediation Posture` (Default post-PR watch) — including its `HIVEMIND_SKIP_PR_WATCH` short-circuit, which applies here too. The ONE difference is the signal: the in-session request text rather than `request.raw`, because a Reflex has no run ledger; with no ledger to write, the citation obligation is met by naming the relied-upon span in the tail's own report instead of in `event.outputs`. The predicate itself is NOT restated here, so the two sites cannot drift. If any condition is uncertain, it is NOT a Reflex — it enters the state machine. +A Reflex is the trivial fast path: it skips the router AND the run ledger. A task is a Reflex only when ALL hold — one owner, one known file, trivial change, branch classification clear, no version impact, no review remediation, no brood. For a Reflex, drive the short delivery tail by intent: delegate the single change `with exact file scope`, checkpoint via `hivemind:molt`, validate, open the PR. The same default watch rule applies to the Reflex tail: after opening the PR, run `hivemind:github-review-loop` under the canonical predicate stated once under `## Review Remediation Posture` (Default post-PR watch) — including its `HIVEMIND_SKIP_PR_WATCH` short-circuit, which applies here too. The ONE difference is the signal: the in-session request text rather than `request.raw`, because a Reflex has no run ledger; with no ledger to write, the citation obligation is met by naming the relied-upon span in the tail's own report instead of in `event.outputs`. The predicate itself is NOT restated here, so the two sites cannot drift. If any condition is uncertain, it is NOT a Reflex — it enters the state machine. Everything that is not a Reflex enters the workflow state machine. @@ -60,9 +60,7 @@ At `implement_step`, the overlord dispatches the WHOLE wave — every step-id in **Epoch-scoped done-set survives replans.** The done-set `hivemind:next-wave` uses to compute readiness is scoped to the CURRENT plan epoch (`.plan.epoch`), maintained entirely by the `hivemind:record-state-result` engine — the overlord passes/derives nothing extra for it (per `${CLAUDE_PLUGIN_ROOT}/references/run-ledger-schema.md`, recording ANY cerebrate planning-state result bumps the epoch and every appended event is stamped with `plan_epoch` automatically). Consequently, after a `needs_replan → plan` re-plan, a fresh plan generation MAY SAFELY REUSE positional `STEP-NNN` step-ids: a prior generation's `completed_steps` credit was stamped under the prior epoch and will NOT skip the new generation's same-id step, because `hivemind:next-wave` scopes its done-set read to the CURRENT epoch only. No manual unique-id or prefix convention for step-ids across replans is needed. -**Wave-of-one degrades to today's serial behavior.** A linear or dependency-chained plan yields waves of exactly one step — i.e., precisely the prior serial one-step-at-a-time loop. The wave model is a strict SUPERSET: it changes nothing for a fully chained plan and only adds parallelism where the plan's `depends_on` graph leaves steps independent. - -**File-disjointness + parallel safety.** Because the engine guarantees wave members have disjoint file scopes, parallel wave delegations NEVER write the same file. Each parallel wave delegation of size greater than one carries `wave_scopes` = the union of ITS siblings' declared scopes (see `## Delegation Format`), so a worker's own tree self-check passes on the disjoint concurrent edits its siblings make in the shared checkout, while still blocking on anything outside the declared wave surface. Beyond that, each wave delegation MUST forbid git writes (wave workers — drone/changeling delegations — never commit; the overlord is the sole ledger writer/committer per RUN-OWNERSHIP-01; this prohibition is scoped to WAVE WORKER delegations, not a universal law over every agent — a reviewer agent's fix-cycle checkpoint commits are the sanctioned exception per `${CLAUDE_PLUGIN_ROOT}/governance/safety-rails.md` (Commit Authority)) and MUST forbid repo-global mutations (dependency installs, tree-wide formatters) that would collide across concurrent agents. Reiterated: wave workers MUST NOT run tree-mutating git commands (stash/reset/checkout/clean) in the shared tree — this is already forbidden by the no-git-writes delegation constraint above, and the worker agent contracts now state it too. The destructive-fix gate and external-content boundary ride each delegation unchanged. +**File-disjointness + parallel safety.** Because the engine guarantees wave members have disjoint file scopes, parallel wave delegations NEVER write the same file. Each parallel wave delegation of size greater than one carries `wave_scopes` = the union of ITS siblings' declared scopes (see `## Delegation Format`), so a worker's own tree self-check passes on the disjoint concurrent edits its siblings make in the shared checkout, while still blocking on anything outside the declared wave surface. Beyond that, each wave delegation MUST forbid git writes, including tree-mutating git commands (stash/reset/checkout/clean) in the shared tree (wave workers — drone/changeling delegations — never commit; the overlord is the sole ledger writer/committer per RUN-OWNERSHIP-01; this prohibition is scoped to WAVE WORKER delegations, not a universal law over every agent — a reviewer agent's fix-cycle checkpoint commits are the sanctioned exception per `${CLAUDE_PLUGIN_ROOT}/governance/safety-rails.md` (Commit Authority)) and MUST forbid repo-global mutations (dependency installs, tree-wide formatters) that would collide across concurrent agents. The destructive-fix gate and external-content boundary ride each delegation unchanged. **Judgment chunking.** The overlord MAY split a large wave into smaller parallel batches by judgment (soft cap: ≤4 concurrent delegations), dispatching the remainder on the next loop iteration. Correctness holds because un-dispatched ready steps simply reappear in the next `hivemind:next-wave` result; un-dispatched steps are NOT recorded in `completed_steps`, so nothing is credited as done before it completes. @@ -71,7 +69,7 @@ At `implement_step`, the overlord dispatches the WHOLE wave — every step-id in - **Engine unavailable / transient failure.** If the `hivemind:next-wave` engine is unavailable (cannot execute the script / substrate missing) or fails with a genuinely transient failure (per `${CLAUDE_PLUGIN_ROOT}/governance/definitions.md` (Transient Failure)), the universal intent-driven fallback above applies: degrade to judgment, drain steps by judgment — respecting `depends_on` and file-disjointness manually — never hard-failing. - **Validation/security blocker (refinement, not a contradiction of the universal fallback).** If `hivemind:next-wave` instead EXITS 1 with a `blocker:` line — malformed or unsafe `plan.steps`: bad shape, bad id charset, duplicate id, unknown dep, or a dependency cycle — the substrate is NOT unavailable and the failure is NOT transient: it WORKED and correctly rejected the input, so the universal fallback's "substrate unavailable → degrade" trigger does not fire. This is a hard stop: record `blocked` and surface to the user. The overlord MUST NOT manually re-parse or drain the rejected `plan.steps` by judgment — doing so would reopen, on the overlord's own read of the same untrusted plan, the ADR-0019 trust-boundary projection the reader-side guards inside `hivemind:next-wave` exist to close. -**One state execution = one wave.** This preserves the "one state = one agent" framing where it concerns the STATE MACHINE: a single `implement_step` state execution IS one wave, and a wave is N concurrent delegations WITHIN that one agent-state execution — not a new state type. Where earlier prose describes the `agent` state as spawning "the named agent" (singular), read it as the state execution dispatching a wave of N concurrent delegations of bioforms picked by intent from `allowed_agents`. +**One state execution = one wave.** A single `implement_step` state execution IS one wave, and a wave is N concurrent delegations of bioforms picked by intent from `allowed_agents` WITHIN that one agent-state execution — not a new state type. **Persist the PR identity when recording the `open_pr` state result.** `hivemind:open-plan-pr` RETURNS the opened PR as routing YAML (`url` + `head_ref_oid`); that routing data is NOT persisted unless the overlord forwards it. When recording the `open_pr` state result via `hivemind:record-state-result`, the overlord MUST pass the PR identity into the call's free-form `outputs` object — `outputs: { pr: , head_ref_oid: }` — using `pr` for the PR URL `hivemind:open-plan-pr` returned. This is the SAME sanctioned `event.outputs` write-path the `recurrence_origin` marker and the `decisions[]` journal already ride (per `${CLAUDE_PLUGIN_ROOT}/references/run-ledger-schema.md` (Event shape)); it is a free-form output, not a new ledger field. Without this write the recorded `open_pr` event's `event.outputs` defaults to `{}`, so the deferred post-merge decision-report trigger below could never derive the run's PR and would never fire for a standard-delivery run. (This persists the PR for standard-delivery runs; a `pr-feedback-remediation` run has no `open_pr` state and persists its PR identically into the `pr_branch_preflight` event's `event.outputs.pr`/`head_ref_oid` instead, per the `pr_branch_preflight` Safety Rail above — so the deferred report below derives the run's PR from `event.outputs.pr` of EITHER event.) @@ -96,8 +94,6 @@ Intent-driven execution is the universal fallback for the whole machine. Wheneve - **Version skew (ledger PRESENT, valid JSON, `workflow_version` mismatched):** the engine-writable case. Read the ledger for facts, invoke `hivemind:mark-intent-fallback` (run_id + the current state string + a summary, NO `close_status`) to atomically set `run.mode: intent_fallback` and append a fallback event, suspend transition gating, keep appending events as an append-only observability log, and finish by judgment. - **Torn / missing / unresolvable ledger (no readable ledger to write to — file absent, invalid JSON, or `state.current` unrecoverable):** start-fresh-by-judgment. `hivemind:mark-intent-fallback` HARD-BLOCKS here (the engine requires the ledger to exist and parse as JSON), so do NOT call it against a ledger that cannot be read. Degrade to pure judgment: reconstruct facts from git observables, and if appropriate start a fresh run. No engine write is attempted. -Determinism only ever ADDS safety and observability; it never strands a run. Worst case equals today's pure-intent behavior, never worse. - ## Review Remediation Posture The overlord's remediation stance follows `${CLAUDE_PLUGIN_ROOT}/governance/remediation-doctrine.md` (binding vocabulary: root-cluster, defer-with-scope, bounded-impact, stop-and-merge). Do not duplicate that doctrine here — apply it. @@ -193,7 +189,7 @@ Likewise, follow Shell Output Discipline per `${CLAUDE_PLUGIN_ROOT}/governance/d ### Stop Conditions -The overlord's decision posture is the two-tier model in `${CLAUDE_PLUGIN_ROOT}/governance/decision-autonomy.md` (Decision Tiers). Tier-A decisions are ALWAYS surfaced; Tier-B judgment calls are auto-decided and journaled UNLESS the promotion gate trips, per `${CLAUDE_PLUGIN_ROOT}/governance/decision-autonomy.md` (Promotion Gate) and (The Autonomy 2x2). The lists below partition the prior stop conditions across the two tiers; the tier semantics live in decision-autonomy.md and are not restated here. +The overlord's decision posture is the two-tier model in `${CLAUDE_PLUGIN_ROOT}/governance/decision-autonomy.md` (Decision Tiers). Tier-A decisions are ALWAYS surfaced; Tier-B judgment calls are auto-decided and journaled UNLESS the promotion gate trips, per `${CLAUDE_PLUGIN_ROOT}/governance/decision-autonomy.md` (Promotion Gate) and (The Autonomy 2x2). The lists below assign each stop condition to its tier; the tier semantics live in decision-autonomy.md and are not restated here. **Tier A — still surface** (per `${CLAUDE_PLUGIN_ROOT}/governance/decision-autonomy.md` (Decision Tiers → Tier A) and (Promotion Gate)): - The router returns an `ambiguous` outcome (choose a candidate workflow) @@ -209,8 +205,8 @@ The overlord's decision posture is the two-tier model in `${CLAUDE_PLUGIN_ROOT}/ - The ENTIRE gate-trips column of the Autonomy 2x2 — any Tier-B call whose RECOMMENDED action is irreversible, architectural, or safety-relevant per (Promotion Gate) promotes to a surface regardless of its tier listing - The safety-rail hard stops above (Destructive Fix Gate, direct trunk commit/push, injection-suspect external content) — these are Tier A and NEVER auto-resolve -**Tier B — now auto-decided + journaled** (per `${CLAUDE_PLUGIN_ROOT}/governance/decision-autonomy.md` (Decision Tiers → Tier B) and (The Autonomy 2x2); each taken per the 2x2 — strong rec + gate clean → do now; weak/no rec + gate clean → defer-with-scope or record-with-scope; gate trips → surface — and JOURNALED): -- Planner-escalation: auto-route the escalation signal to the cerebrate remediation state per the **Routing-vs-Execution Invariant** (the ROUTE auto-takes; ACCEPTING/EXECUTING the architectural plan cerebrate returns is still surfaced to the user by overlord judgment before the advancing transition is recorded — a judgment obligation, not a workflow `user_gate`). This SUPERSEDES the prior immediate-stop posture for the ROUTING decision; the route is no longer surfaced by default +**Tier B — auto-decided + journaled** (per `${CLAUDE_PLUGIN_ROOT}/governance/decision-autonomy.md` (Decision Tiers → Tier B) and (The Autonomy 2x2); each taken per the 2x2 — strong rec + gate clean → do now; weak/no rec + gate clean → defer-with-scope or record-with-scope; gate trips → surface — and JOURNALED): +- Planner-escalation: auto-route the escalation signal to the cerebrate remediation state per the **Routing-vs-Execution Invariant** (the ROUTE auto-takes; ACCEPTING/EXECUTING the architectural plan cerebrate returns is still surfaced to the user by overlord judgment before the advancing transition is recorded — a judgment obligation, not a workflow `user_gate`) - The Creep-Stagnation / diminishing-returns advisory early-exit decision - A validation failure — attempt remediation first; surface ONLY if it cannot be resolved - A version-bump TYPE when inferable from the compatibility impact diff --git a/plugin/governance/remediation-doctrine.md b/plugin/governance/remediation-doctrine.md index 450a8899..0282a9f8 100644 --- a/plugin/governance/remediation-doctrine.md +++ b/plugin/governance/remediation-doctrine.md @@ -117,16 +117,16 @@ Threshold N for this axis: Distinction from the within-pass axis (do not conflate): the "just-touched / same-framing surface" qualifier in **Severity as Sensitivity Modifier** is a WITHIN-PASS modifier scoped to a single classification pass. THIS axis is ACROSS-ITERATION — it spans the loop's distinct iterations — and is additive to and distinct from the within-pass modifier. A surface can be quiet within every individual pass and still trip this axis by re-emitting across iterations. -Multiple roots per surface: the recurrence counter PERSISTS across structural fixes. Closing root #1 with an accepted structural fix does NOT reset the counter. A surface that has already yielded one root is held to a LOWER threshold for the next — having needed a structural fix once is evidence the surface is structurally hot, so the next recurrence trips sooner. +Multiple roots per surface: the recurrence counter PERSISTS across structural fixes. Closing the FIRST root with an accepted structural fix does NOT reset the counter. A surface that has already yielded one root is held to a LOWER threshold for the next — having needed a structural fix once is evidence the surface is structurally hot, so the next recurrence trips sooner. ## Bounded-Tail vs Recurring-Class Disambiguation -The **Stop-and-Merge** section reserves "every push spawns only a fresh bounded tail, never a new defect class" as a merge precondition. That bounded-tail clause now applies ONLY to MATURE surfaces. The disambiguation: +The **Stop-and-Merge** section reserves "every push spawns only a fresh bounded tail, never a new defect class" as a merge precondition. That bounded-tail clause applies ONLY to MATURE surfaces. The disambiguation: - **Young surface + recurring findings** (the surface was introduced or heavily modified in this PR/initiative): this is NOT a bounded tail. A young surface that keeps emitting findings is a design smell, so it escalates to a root-cause ZOOM-OUT (question the key/primitive per the **Closed-by-Construction Acceptance Test**), never to merge-advisory. This is the escalation path of **Cross-Iteration Same-Surface Recurrence**. - **Mature / legacy surface + bounded tail**: this remains a merge-advisory candidate per **Stop-and-Merge**. A genuine mature-surface bounded tail — a hardened legacy surface whose remaining findings are a converging tail with a structural home — must STILL reach `merge_advised`. The young-surface escalation rule does not gate it. -Regression guard: do not let the young-surface escalation swallow the mature-surface merge path. The two are disjoint by Gate B of **Cross-Iteration Same-Surface Recurrence** — youth is the discriminator. A mature surface failing Gate B routes to merge-advisory exactly as before this section existed. +Regression guard: do not let the young-surface escalation swallow the mature-surface merge path. The two are disjoint by Gate B of **Cross-Iteration Same-Surface Recurrence** — youth is the discriminator. A mature surface failing Gate B routes to merge-advisory. ## Post-Fix Young-Tail Reroute Synthesis diff --git a/plugin/references/brood-ledger-model.md b/plugin/references/brood-ledger-model.md index ce461831..8f8651d2 100644 --- a/plugin/references/brood-ledger-model.md +++ b/plugin/references/brood-ledger-model.md @@ -52,7 +52,7 @@ The manifest is JSON (`manifest_version: 4`, integer), written to a temp file un } ``` -What changed from `manifest_version: 3`: top-level `brood_id` is now the generated GUID (was a timestamp) and `created_at` is added; the per-strain `branch` is DERIVED (`strain//`, the spawn-time scratch ref) and is now display/context only on the read side; `worktree_path` is RETAINED and is the read side's exact-match worktree LOOKUP KEY (it is matched as a string against git's worktree-path set, never consumed as a path anchor — #270; the read side gates it through the path-SELECTOR value-class, which PERMITS a `..` directory-name substring — validity is git-set membership — while still rejecting framing/command-sub/leading-dash/empty, so a legitimate worktree under a `..`-bearing dir name is located rather than false-rejected); and `run.suggested_ledger` is DROPPED — the read side derives the ledger path from git ground truth, so recording it was redundant manifest-path trust. `run.suggested_id` is KEPT as the lineage reconciliation key. +Field semantics: top-level `brood_id` is the generated GUID `brood-` and `created_at` is the UTC instant the manifest was written; the per-strain `branch` is DERIVED (`strain//`, the spawn-time scratch ref) and is display/context only on the read side; `worktree_path` is the read side's exact-match worktree LOOKUP KEY (it is matched as a string against git's worktree-path set, never consumed as a path anchor; the read side gates it through the path-SELECTOR value-class, which PERMITS a `..` directory-name substring — validity is git-set membership — while still rejecting framing/command-sub/leading-dash/empty, so a legitimate worktree under a `..`-bearing dir name is located rather than false-rejected); and the manifest records NO ledger path — the read side derives it from git ground truth, so recording it would be redundant manifest-path trust. `run.suggested_id` is the lineage reconciliation key. Field derivation (emitted by `spawn-brood.sh` via `jq -nc` per strain then `jq -s` to fold the array, all untrusted values bound as `--arg`): @@ -76,9 +76,9 @@ None of these fields point at a ledger the hatchery creates — they are pointer where only `` is manifest-sourced (gated as a strict single-component identifier — no slash). The full containment chain is `CHECKOUT_ROOT ⊇ git-worktree ⊇ ledger`: a git-reported worktree outside the checkout fails closed, and the ledger leaf must sit beneath that worktree. A `worktree_path` that matches no live worktree selects nothing (fail-closed → `MISSING`). A tampered manifest path can no longer redirect the bounded reader, because no manifest path is consumed as an anchor (ADR-0021, ADR-0019 amendment). -**Worktree PATH is the lookup key; `branch` is display/context only (#270).** The lookup was re-keyed from the per-strain `branch` onto the worktree `worktree_path`, gated through the path-SELECTOR value-class (PERMITS a `..` directory-name substring since the value is matched against git's set and never traversed; still rejects framing/command-sub/leading-dash/empty). The worktree PATH is STABLE across a child branch switch — a brood child boots on the spawn-time scratch ref `strain//` and then derives its own compliant `/-` working branch via `hivemind:create-working-branch` (off `base`), switching the worktree HEAD — and the child opens its PR from THAT derived branch. Branch-keying broke the moment the child switched HEAD off the scratch ref, because the manifest still recorded the scratch branch while git reported the new one; path-keying is immune since the worktree path never moves. The manifest `branch` field is STILL recorded and emitted as the display `branch` column, but it is no longer the selector. Two consequences of path-keying: a **detached-HEAD** worktree (which carries no `branch refs/heads/...` line and was DROPPED by branch-keying) is now located, since every porcelain record has a `worktree` line; and the prior **duplicate-branch → `MALFORMED`** case is GONE — git guarantees worktree-path uniqueness, so a path key cannot collide. A `bare` repo record is excluded from the selectable set. `manifest_version` stays 4; both `worktree_path` and `branch` fields stay recorded (the back-compat note in the manifest section already covers that existing v4 manifests carry `worktree_path`, so no migration is needed). +**Worktree PATH is the lookup key; `branch` is display/context only.** The lookup keys on the per-strain `worktree_path`, gated through the path-SELECTOR value-class (PERMITS a `..` directory-name substring since the value is matched against git's set and never traversed; still rejects framing/command-sub/leading-dash/empty). The worktree PATH is STABLE across a child branch switch — a brood child boots on the spawn-time scratch ref `strain//` and then derives its own compliant `/-` working branch via `hivemind:create-working-branch` (off `base`), switching the worktree HEAD — and the child opens its PR from THAT derived branch, leaving the manifest `branch` stale. Path-keying is immune to that switch because the worktree path never moves. The manifest `branch` field is recorded and emitted as the display `branch` column, but it is not the selector. Two consequences of path-keying: a **detached-HEAD** worktree (which carries no `branch refs/heads/...` line) is located, since every porcelain record has a `worktree` line; and a path key cannot collide — git guarantees worktree-path uniqueness. A `bare` repo record is excluded from the selectable set. `manifest_version` is 4; both `worktree_path` and `branch` fields are recorded. -**The PR probe keys on the worktree's git-reported LIVE branch, not the display `branch` (#270).** Re-keying the worktree LOOKUP onto `worktree_path` made the manifest `branch` column display-only, but the PR probe still needs a branch to query (`gh pr list --head `). It keys that probe on the worktree's git-reported LIVE branch — the `branch refs/heads/` line of the SAME porcelain record the path lookup selected, captured in parallel with the path set and emitted as a new last projector field (`live_branch`, index 10). This closes the post-switch PR-head drift the lookup re-key left open: after a child derives its compliant working branch and opens its PR from it, the manifest `branch` is the stale scratch ref, so `gh pr list --head ` finds nothing and the strain falsely derives `failed (session ended, no PR)`. Keying on the live branch follows the worktree HEAD onto the derived branch and finds the real PR. The live branch is gated through the same identifier value-class as the display branch (it becomes a `gh --head` argument) and is GROUND-TRUTH metadata only — never a path, never a confinement input; the git path remains the sole confinement anchor. A **detached-HEAD** worktree emits no `branch refs/heads/...` line, so its `live_branch` resolves to `MISSING` → the collector's sentinel gate SKIPS the PR probe → on a dead session it derives `failed (session ended, no PR)`. That is ACCEPTED fail-closed, not a regression: a worktree that never switched onto a derived branch genuinely has no PR head to probe. +**The PR probe keys on the worktree's git-reported LIVE branch, not the display `branch`.** The worktree LOOKUP keys on `worktree_path`, which makes the manifest `branch` column display-only, but the PR probe still needs a branch to query (`gh pr list --head `). It keys that probe on the worktree's git-reported LIVE branch — the `branch refs/heads/` line of the SAME porcelain record the path lookup selected, captured in parallel with the path set and emitted as the last projector field (`live_branch`, index 10). This closes post-switch PR-head drift: after a child derives its compliant working branch and opens its PR from it, the manifest `branch` is the stale scratch ref, so `gh pr list --head ` finds nothing and the strain falsely derives `failed (session ended, no PR)`. Keying on the live branch follows the worktree HEAD onto the derived branch and finds the real PR. The live branch is gated through the same identifier value-class as the display branch (it becomes a `gh --head` argument) and is GROUND-TRUTH metadata only — never a path, never a confinement input; the git path remains the sole confinement anchor. A **detached-HEAD** worktree emits no `branch refs/heads/...` line, so its `live_branch` resolves to `MISSING` → the collector's sentinel gate SKIPS the PR probe → on a dead session it derives `failed (session ended, no PR)`. That is ACCEPTED fail-closed, not a regression: a worktree that never switched onto a derived branch genuinely has no PR head to probe. ## Injected child-task metadata @@ -125,7 +125,7 @@ The hatchery monitors a brood by reading only. It never mutates child ledgers, a `brood-status` derives each strain's status from **external observables + the manifest's static fields + the child run ledger (informational)**. The entire collection loop — multi-brood discovery, per-strain external-observable probing (tmux/branch/PR), child-ledger workflow-state projection, status derivation, and aggregation — lives in committed shell (ADR-0020): a THIN executable entrypoint `${CLAUDE_PLUGIN_ROOT}/skills/brood-status/scripts/brood-status-collect.sh` plus a PURE source-safe library `_shared/brood-status-derive.sh` (status-derivation rule table + bucket classification + per-brood/global aggregation — no I/O). The entrypoint internally calls the committed discovery script (`brood-discover.sh`) and the PURE single-manifest projector (`brood-status-project.sh`), runs the impure tmux/branch/PR probes itself, derives status via the pure lib, and emits ONE JSON document (schema `brood-status-collect/1`). The **navigator** (SKILL.md) is reduced to: run the entrypoint, render markdown from its JSON, write the human summary — it no longer runs any loop, probe, or derivation in prose. This closes the path-splice **structurally**: the entrypoint invokes the projector with inert shell variables (`"$manifest"`, `"$root"`), so untrusted discovered manifest paths AND the operator-controlled checkout root NEVER cross into LLM-authored command source (per security-policy.md / ADR-0019: double-quoting does not neutralize `$(...)`/backtick/`${}` in command SOURCE) — both the brood-id residual and the checkout-root residual close by construction. -Reading child-ledger workflow-state is **LIVE**, implemented in the PURE projector `${CLAUDE_PLUGIN_ROOT}/skills/brood-status/scripts/brood-status-project.sh`. The projector sources four single-responsibility libs: `_shared/allowlist.sh` (floor-at-input value-class gate), `_shared/manifest-json.sh` (jq-based JSON field extraction), `_shared/ledger-project.sh` (jq scalar projection + validation), and `_shared/containment.sh` (path confinement). It projects exactly two scalars per strain: `run.status` (validated against the exact enum `running|complete|blocked|cancelled`) and `state.current` (validated against `^[a-z0-9_]+$`, length ≤ 64). Values that are absent yield the fixed token `MISSING`; values that are present but out-of-allowlist or unparseable yield `MALFORMED` — raw bytes are never emitted; every display cell is output-encoded at the emit boundary (escape `|`, strip C0/DEL). The ledger path is derived from **git ground truth**: the strain's REAL worktree comes from `git worktree list --porcelain` keyed by EXACT-MATCH on the manifest `worktree_path` (lookup key only, never consumed as a path; #270 — gated through the path-SELECTOR value-class, which PERMITS a `..` directory-name substring (git-set membership is the validation) while still rejecting framing/command-sub/leading-dash/empty), and the ledger is `/.hivemind/runs//state.json` — confined by the `CHECKOUT_ROOT ⊇ git-worktree ⊇ ledger` chain. The manifest `branch` is display/context only and is NOT the selector; a `worktree_path` matching no live worktree is rejected (fail-closed → `MISSING`) and never read. The same porcelain parse also captures each worktree's git-reported LIVE branch (`branch refs/heads/`) into a parallel set keyed by the same path, emitted as the projector's last field (`live_branch`); the collector keys its `gh pr list --head` PR probe on THAT live branch — not the display `branch` — so the probe follows the worktree HEAD onto a child's derived branch and finds the PR it opened there (#270). A detached-HEAD worktree has no live branch → `live_branch` is `MISSING` → the PR probe is skipped (fail-closed). This projection is **informational only**: it populates the `Strain State (claimed)` / `Strain Status (claimed)` display columns but never overrides the observable-derived `Hatchery Status (observed)` column (external observables remain ground truth — ADR-0007). +Reading child-ledger workflow-state is **LIVE**, implemented in the PURE projector `${CLAUDE_PLUGIN_ROOT}/skills/brood-status/scripts/brood-status-project.sh`. The projector sources four single-responsibility libs: `_shared/allowlist.sh` (floor-at-input value-class gate), `_shared/manifest-json.sh` (jq-based JSON field extraction), `_shared/ledger-project.sh` (jq scalar projection + validation), and `_shared/containment.sh` (path confinement). It projects exactly two scalars per strain: `run.status` (validated against the exact enum `running|complete|blocked|cancelled`) and `state.current` (validated against `^[a-z0-9_]+$`, length ≤ 64). Values that are absent yield the fixed token `MISSING`; values that are present but out-of-allowlist or unparseable yield `MALFORMED` — raw bytes are never emitted; every display cell is output-encoded at the emit boundary (escape `|`, strip C0/DEL). The ledger path is derived from **git ground truth**: the strain's REAL worktree comes from `git worktree list --porcelain` keyed by EXACT-MATCH on the manifest `worktree_path` (lookup key only, never consumed as a path — gated through the path-SELECTOR value-class, which PERMITS a `..` directory-name substring (git-set membership is the validation) while still rejecting framing/command-sub/leading-dash/empty), and the ledger is `/.hivemind/runs//state.json` — confined by the `CHECKOUT_ROOT ⊇ git-worktree ⊇ ledger` chain. The manifest `branch` is display/context only and is NOT the selector; a `worktree_path` matching no live worktree is rejected (fail-closed → `MISSING`) and never read. The same porcelain parse also captures each worktree's git-reported LIVE branch (`branch refs/heads/`) into a parallel set keyed by the same path, emitted as the projector's last field (`live_branch`); the collector keys its `gh pr list --head` PR probe on THAT live branch — not the display `branch` — so the probe follows the worktree HEAD onto a child's derived branch and finds the PR it opened there. A detached-HEAD worktree has no live branch → `live_branch` is `MISSING` → the PR probe is skipped (fail-closed). This projection is **informational only**: it populates the `Strain State (claimed)` / `Strain Status (claimed)` display columns but never overrides the observable-derived `Hatchery Status (observed)` column (external observables remain ground truth — ADR-0007). The hatchery may read: @@ -144,7 +144,7 @@ When deriving a strain's status, prefer sources in this order: ```text 1. external observables: tmux session, branch existence, PR state (probed on the worktree's git - LIVE branch — `live_branch`, field 10 — not the display-only manifest `branch`; #270) + LIVE branch — `live_branch`, field 10 — not the display-only manifest `branch`) 2. manifest static fields 3. child run ledger: run.status / state.current (informational — never overrides tier 1 or 2 Status; MAY DEMOTE an alive child OUT of `running` to `starting`, but never promotes / never hides a dead session) @@ -172,7 +172,7 @@ Two independent mechanisms address a child that was injected but never started i `starting (session alive, workflow not yet started)` is a DISTINCT, TRANSIENT (non-terminal) status — non-`running`, non-`complete`, and distinct from `failed`. It buckets into `blocked/failed` for the per-brood summary line so the three-bucket count keeps summing to total (it has made no forward progress, so it is counted against completion, never silently dropped). -The `PR` column above reflects the probe on the worktree's git LIVE branch (`live_branch`), so a child that switched onto its derived branch and opened a PR there reads `merged`/`open` on the dead branch (→ `complete`/`blocked`) instead of falsely falling to `failed (session ended, no PR)` (#270). A **detached-HEAD** worktree has no live branch → `live_branch` is `MISSING` → the PR probe is SKIPPED (sentinel gate) → its `PR` column is `none`, so a detached-HEAD DEAD session derives `failed (session ended, no PR)`. That is ACCEPTED fail-closed (a worktree that never switched to a derived branch genuinely has no PR head to probe), no worse than before and never a fabricated/garbage probe. +The `PR` column above reflects the probe on the worktree's git LIVE branch (`live_branch`), so a child that switched onto its derived branch and opened a PR there reads `merged`/`open` on the dead branch (→ `complete`/`blocked`) instead of falsely falling to `failed (session ended, no PR)`. A **detached-HEAD** worktree has no live branch → `live_branch` is `MISSING` → the PR probe is SKIPPED (sentinel gate) → its `PR` column is `none`, so a detached-HEAD DEAD session derives `failed (session ended, no PR)`. That is ACCEPTED fail-closed (a worktree that never switched to a derived branch genuinely has no PR head to probe), no worse than before and never a fabricated/garbage probe. This is tier-3 child-ledger evidence applied within its informational-only contract: it DEMOTES an alive-but-unstarted child away from `running`, but it NEVER promotes a strain to `complete` and NEVER hides a dead session (the started-evidence gate touches only the alive branch; the dead branch derives `complete`/`blocked`/`failed` from observables regardless of ledger content). A `MALFORMED` `state.current` is fail-closed — it does NOT count as started-evidence, so a corrupt ledger demotes to `starting`, never up to `running`. ADR-0007's invariant that the ledger never OVERRIDES observable status is preserved: an alive session is still observably alive; the ledger only refines whether that alive session reads as `running` or `starting`. diff --git a/plugin/references/github-pr-review-graphql.md b/plugin/references/github-pr-review-graphql.md index 35210fb5..734dd11b 100644 --- a/plugin/references/github-pr-review-graphql.md +++ b/plugin/references/github-pr-review-graphql.md @@ -15,8 +15,8 @@ Resolvable pull request review threads are GraphQL objects. Do not try to resolv - [Detection Filtering](#detection-filtering) — filters to apply before yielding any result as actionable feedback - [Reply to Review Thread](#reply-to-review-thread) — mutation to post a reply to an existing review thread - [Resolve Review Thread](#resolve-review-thread) — mutation to mark a review thread as resolved -- [Surface-to-Delivery Contract](#surface-to-delivery-contract) — canonical mapping of feedback surface to mutation(s) used (issue #218) -- [Reaction Marker](#reaction-marker) — self-authored `EYES` reaction that marks a fixed non-thread surface handled (issue #265) +- [Surface-to-Delivery Contract](#surface-to-delivery-contract) — canonical mapping of feedback surface to mutation(s) used +- [Reaction Marker](#reaction-marker) — self-authored `EYES` reaction that marks a fixed non-thread surface handled - [Author Filtering](#author-filtering) — rules for scoping feedback to specific reviewer identities - [Codex Approval Detection](#codex-approval-detection) — paginated 👍 reaction lookup that signals Codex approval @@ -28,7 +28,7 @@ Sanctioned exception — canonical fix-history classification: the github-review ## Pagination Requirement -Page all connections via `-F after="CURSOR"` using `endCursor` from `pageInfo`. Omit `-F after` on first page. Nested connections (e.g., thread comments) require per-item queries with the item's `id`. This requirement governs the reviewer's deep body-level fetch (reviews, review threads, thread comments, top-level comments) and the Codex approval reactions lookup — NOT the `github-review-loop` thin poll, which is deliberately coarse (scalar `totalCount`s only, no connection walking) per plan D5; citing this section to justify adding cursor walks to the poll is out of scope. +Page all connections via `-F after="CURSOR"` using `endCursor` from `pageInfo`. Omit `-F after` on first page. Nested connections (e.g., thread comments) require per-item queries with the item's `id`. This requirement governs the reviewer's deep body-level fetch (reviews, review threads, thread comments, top-level comments) and the Codex approval reactions lookup — NOT the `github-review-loop` thin poll, which is exempt by design: it reads scalar `totalCount`s only and walks no connections. ## Fetch Reviews @@ -228,11 +228,11 @@ Maps each feedback surface to the mutation(s) used to mark a fixed surface handl | Surface | Mutation(s) | Notes | |---------|-------------|-------| | `thread` | `addPullRequestReviewThreadReply` then (conditional) `resolveReviewThread` | Reply targets the thread node id (`PRRT_...`). Resolve fires only when the thread is unresolved after reply. Executed by `reply-resolve.sh`. | -| `toplevel` | `addReaction` (`EYES`) on the IssueComment node | A self-authored `EYES` (👀) reaction is added to the reviewer's top-level IssueComment node as the handled marker. NOT reply-targeted, NOT thread-resolvable (top-level PR comments have no thread node). `reply-resolve.sh` is NOT invoked for this surface — its `toplevel` silent no-op is UNCHANGED; posting `addPullRequestReviewThreadReply` against a non-thread node was the #218 defect. | +| `toplevel` | `addReaction` (`EYES`) on the IssueComment node | A self-authored `EYES` (👀) reaction is added to the reviewer's top-level IssueComment node as the handled marker. NOT reply-targeted, NOT thread-resolvable (top-level PR comments have no thread node). `reply-resolve.sh` is NOT invoked for this surface — it is a silent no-op for `toplevel`; posting `addPullRequestReviewThreadReply` against a non-thread node fails, because a top-level IssueComment has no review-thread node to target. | | `review` | `addReaction` (`EYES`) on the PullRequestReview node | Same as `toplevel`: a self-authored `EYES` (👀) reaction is added to the reviewer's PullRequestReview summary node as the handled marker. Review-summary nodes have no thread node, so they are NOT reply-targeted and NOT thread-resolvable. `reply-resolve.sh` is NOT invoked — its `review` silent no-op is UNCHANGED. | | unmapped / unknown | fail-closed | Any surface value not in the table above causes the script to exit with an error rather than fall through silently. | -The former `Addresses: ` body line that was appended to replies is removed from the live path — thread replies carry only the fix summary, not a back-reference URL. The handled marker for non-thread surfaces is the `EYES` reaction described below; ZERO new PR comments are posted. +Thread replies carry only the fix summary: no `Addresses: ` back-reference line is appended on the emit path. The handled marker for non-thread surfaces is the `EYES` reaction described below; ZERO new PR comments are posted. ## Reaction Marker @@ -269,7 +269,7 @@ A surface is handled when its `reactionGroups` contains an entry with `content = The `EYES` marker here is OUR self-authored reaction on a per-COMMENT / per-REVIEW node. It is a DISJOINT subject from the 👀 reaction described in [Codex Approval Detection](#codex-approval-detection), which is Codex's reaction on the PR OBJECT meaning "still running". The two share an emoji but never the same subject: -- **This marker (#265):** `EYES` reaction on an `IssueComment` / `PullRequestReview` node, authored by our viewer, detected via `reactionGroups { content viewerHasReacted }` on that node → surface handled. +- **This marker:** `EYES` reaction on an `IssueComment` / `PullRequestReview` node, authored by our viewer, detected via `reactionGroups { content viewerHasReacted }` on that node → surface handled. - **Codex "still running" (existing):** `eyes` reaction on the PR object, authored by Codex, detected via the REST reactions endpoint → never approval. A reader must not conflate them: per-node viewer-scoped handled marker vs PR-object Codex-authored progress signal. diff --git a/plugin/references/run-ledger-schema.md b/plugin/references/run-ledger-schema.md index f32d6a90..08dd2315 100644 --- a/plugin/references/run-ledger-schema.md +++ b/plugin/references/run-ledger-schema.md @@ -131,27 +131,27 @@ Append-only blocker log. `plan_epoch` is a TOP-LEVEL, ENGINE-WRITTEN event field — distinct from the free-form, caller-supplied `outputs` object below. `record-state-result` stamps it on EVERY event it appends, recording the `.plan.epoch` value the ledger held at append time. Because it is engine-written rather than caller-supplied, it cannot be forged or omitted by a caller the way anything under `outputs` can. -`event.outputs` is free-form and recorded verbatim. NO schema change and NO new REQUIRED field is implied by the convention that follows — `event.outputs` stays free-form/optional. +`event.outputs` is free-form and recorded verbatim. This holds for every convention below: each is additive and free-form — NO schema change and NO new REQUIRED field — and `event.outputs` stays free-form/optional. **Convention (proactive-recurrence-origin marker):** on a `root-cluster-suspected` transition, `event.outputs` MAY carry the named origin-marker key (`recurrence_origin`). Its presence distinguishes a proactively-derived zoom-out from a reviewer-returned one. The key's name, values, and absence semantics are defined SOLELY in `${CLAUDE_PLUGIN_ROOT}/governance/remediation-doctrine.md (### Proactive Zoom-Out Ledger Marker)` — that subsection is the single source; this note does not restate them. -**Convention (open_pr PR identity):** on an `open_pr` transition, `event.outputs` MAY carry the opened PR's identity — `pr` (the PR URL) and `head_ref_oid` (the PR head SHA) — recorded verbatim like `recurrence_origin`. The overlord forwards these from the routing YAML `hivemind:open-plan-pr` returns (`url` → `pr`, `head_ref_oid`). The deferred post-merge decision report derives the run's PR from `event.outputs.pr` — that report is CONFIG-GATED (opt-in via `HIVEMIND_ENABLE_DECISION_REPORT`, OFF by default), so recording these keys does NOT imply a report will fire; the trigger and policy single source is `${CLAUDE_PLUGIN_ROOT}/governance/decision-autonomy.md (## Post-Merge Decision Report Trigger)`. This is additive and free-form — NO schema change and NO new required field. +**Convention (open_pr PR identity):** on an `open_pr` transition, `event.outputs` MAY carry the opened PR's identity — `pr` (the PR URL) and `head_ref_oid` (the PR head SHA). The overlord forwards these from the routing YAML `hivemind:open-plan-pr` returns (`url` → `pr`, `head_ref_oid`). The deferred post-merge decision report derives the run's PR from `event.outputs.pr` — that report is CONFIG-GATED (opt-in via `HIVEMIND_ENABLE_DECISION_REPORT`, OFF by default), so recording these keys does NOT imply a report will fire; the trigger and policy single source is `${CLAUDE_PLUGIN_ROOT}/governance/decision-autonomy.md (## Post-Merge Decision Report Trigger)`. -**Convention (pr-feedback-remediation PR identity):** a `pr-feedback-remediation` run has no `open_pr` state, so on a `pr_branch_preflight` (and/or `intake`) transition `event.outputs` MAY carry the resolved PR's `pr` (URL) and `head_ref_oid` (head SHA), recorded verbatim like the `open_pr` keys above. The overlord records these from the PR it resolves and checks out at `pr_branch_preflight`. The deferred post-merge decision report derives the run's PR from `event.outputs.pr` of EITHER event and is CONFIG-GATED the same way (opt-in via `HIVEMIND_ENABLE_DECISION_REPORT`, OFF by default), single-sourced to `${CLAUDE_PLUGIN_ROOT}/governance/decision-autonomy.md (## Post-Merge Decision Report Trigger)`. This is additive and free-form — NO schema change, NO new required field, and a key/path DISTINCT from `decisions[]`, `recurrence_origin`, and `plan.steps`. +**Convention (pr-feedback-remediation PR identity):** a `pr-feedback-remediation` run has no `open_pr` state, so on a `pr_branch_preflight` (and/or `intake`) transition `event.outputs` MAY carry the resolved PR's `pr` (URL) and `head_ref_oid` (head SHA). The overlord records these from the PR it resolves and checks out at `pr_branch_preflight`. The deferred post-merge decision report derives the run's PR from `event.outputs.pr` of EITHER event and is CONFIG-GATED the same way (opt-in via `HIVEMIND_ENABLE_DECISION_REPORT`, OFF by default), single-sourced to `${CLAUDE_PLUGIN_ROOT}/governance/decision-autonomy.md (## Post-Merge Decision Report Trigger)`. These keys/paths are DISTINCT from `decisions[]`, `recurrence_origin`, and `plan.steps`. -**Convention (decision-journal array):** `event.outputs` MAY carry an OPTIONAL, free-form `decisions[]` array, recorded verbatim like `recurrence_origin`. Each entry carries the fields `ts`, `state`, `situation`, `options`, `tradeoffs`, `rec_strength`, `gate`, `disposition`, `decision`, `rationale`, and `reversible`. The semantics single source — the autonomy posture, the 2x2, the promotion gate, the disposition vocabulary, and these fields — is `${CLAUDE_PLUGIN_ROOT}/governance/decision-autonomy.md (## Decision Journal)`; this note does not restate the 2x2 or promotion-gate mechanics. `decisions[]`, `recurrence_origin`, the `pr` / `head_ref_oid` open_pr keys, and the `plan.steps` plan-steps writers are DISTINCT keys/paths on or around `event.outputs` and do not collide. +**Convention (decision-journal array):** `event.outputs` MAY carry an OPTIONAL, free-form `decisions[]` array. Each entry carries the fields `ts`, `state`, `situation`, `options`, `tradeoffs`, `rec_strength`, `gate`, `disposition`, `decision`, `rationale`, and `reversible`. The semantics single source — the autonomy posture, the 2x2, the promotion gate, the disposition vocabulary, and these fields — is `${CLAUDE_PLUGIN_ROOT}/governance/decision-autonomy.md (## Decision Journal)`; this note does not restate the 2x2 or promotion-gate mechanics. `decisions[]`, `recurrence_origin`, the `pr` / `head_ref_oid` open_pr keys, and the `plan.steps` plan-steps writers are DISTINCT keys/paths on or around `event.outputs` and do not collide. -**Convention (completed_steps wave marker):** `event.outputs` MAY carry an OPTIONAL, free-form `completed_steps` array, recorded verbatim like `recurrence_origin` — this is the SAME sanctioned free-form `event.outputs` write-path already used by `decisions[]` and `recurrence_origin`, NOT a new ledger schema field and NOT a `facts.*` mutation. `completed_steps` is a JSON array of plan-step id strings (e.g. `["STEP-001","STEP-004"]`), recorded when the overlord records an `implement_step` state result for a completed WAVE. A single-step wave records a `completed_steps` array of length 1 — a strict subset of the multi-step case, not a distinct shape. +**Convention (completed_steps wave marker):** `event.outputs` MAY carry an OPTIONAL, free-form `completed_steps` array — this is the SAME sanctioned free-form `event.outputs` write-path already used by `decisions[]` and `recurrence_origin`, NOT a `facts.*` mutation. `completed_steps` is a JSON array of plan-step id strings (e.g. `["STEP-001","STEP-004"]`), recorded when the overlord records an `implement_step` state result for a completed WAVE. A single-step wave records a `completed_steps` array of length 1 — a strict subset of the multi-step case, not a distinct shape. This convention is producer-authorized at the write boundary: `record-state-result` honors `outputs.completed_steps` ONLY when the recording state is a wave-producing agent state — `states..type == "agent"` AND `states.` declares `allowed_agents` AND `hivemind:cerebrate` is NOT a member of that `allowed_agents` set, derived from the packaged workflow definition — any other recording state is rejected (blocker, ledger byte-unchanged), symmetric to the plan-write authorization guard that restricts `plan.steps` persistence to cerebrate planning states. Exactly TWO engines append events to the ledger: `record-state-result`, which enforces the guard above, and `mark-intent-fallback`, which strips `completed_steps` from its fallback event outputs unconditionally; `init-run-ledger` only creates the ledger (`events: []`) and appends nothing. Because every event appender enforces this producer-authorization discipline, an unauthorized `completed_steps` credit for the current epoch is UNREPRESENTABLE in the ledger — so `next-wave.sh` reads `events[].outputs.completed_steps` unconditionally and remains a pure ledger reader, with no workflow-def knowledge of its own. Done-ness of a plan step is DERIVED from the union of `events[].outputs.completed_steps`, SCOPED TO THE CURRENT PLAN EPOCH ONLY — i.e. events whose top-level `plan_epoch` equals the ledger's current `.plan.epoch` (`//0` for pre-epoch ledgers, preserving identical behavior to the prior unscoped-union reading) — NOT from `plan.steps[].status`, which stays planner-emitted `pending` and is inert for execution purposes, and NOT from an unscoped union across the entire event log. The epoch scope exists because positional `STEP-NNN` ids are reused across plan generations: a `needs_replan` transition replaces `plan.steps` in the same append-only ledger, and an unscoped union would let a prior generation's `completed_steps` credit satisfy a new generation's same-id step, silently skipping it. Keying done-ness by `plan_epoch` makes that cross-generation collision unrepresentable. This split exists because the `record-state-result` engine forbids non-cerebrate plan writes (see [workflow-state-machine.md](${CLAUDE_PLUGIN_ROOT}/references/workflow-state-machine.md) `(### agent)`): only a cerebrate-agent state may persist `plan.steps`, so step done-ness cannot live there without violating that authorization guard. Recording done-ness in events instead leaves the plan-write authorization guard untouched. `completed_steps`, `decisions[]`, `recurrence_origin`, the `pr` / `head_ref_oid` open_pr keys, and the `plan.steps` plan-steps writers are DISTINCT keys/paths on or around `event.outputs` and do not collide. `plan_epoch` is a separate, TOP-LEVEL, engine-written event field — not a free-form `outputs` key, and not part of this distinct-keys-under-`outputs` set. -**Non-change clarifications (so a future reader does not "fix" a non-bug):** +**Ledger-shape invariants:** -- NO `schema_version` bump is implied by the decision-journal convention — `decisions[]` is a free-form `event.outputs` key, not a required ledger field. -- NO new `run.status` value is introduced — the enum stays `running | complete | blocked | cancelled`. The post-merge report's "awaiting" condition is DERIVED at Resume-On-Start (a PR exists, `event.outputs.decisions[]` carries ≥1 `did-now`/`deferred`/`recorded` entry, and the zero-byte `.decision-report-done` marker is absent), NOT stored. That derived predicate is IDENTICAL whether or not the report is enabled — when `HIVEMIND_ENABLE_DECISION_REPORT` is unset or empty the same awaiting set is derived from the same local ledger reads, with no PR-state check and no report rendered, per `${CLAUDE_PLUGIN_ROOT}/governance/decision-autonomy.md (## Post-Merge Decision Report Trigger)`. -- NO `artifacts.decision_report` ledger marker is used — the post-merge report is CHAT-ONLY (rendered and surfaced to the user, never written to disk). Its idempotency token is the EXISTENCE of a zero-byte `.decision-report-done` marker `touch`ed in the run dir — NOT a `decision-report.md` content file (which no longer exists), and not a ledger field. The marker means "this run will produce no further decision report" — it is written on BOTH paths: already reported, OR suppressed while the report was off. Marker shape is unchanged by that wider meaning: still zero-byte, still the sole idempotency token, still no ledger marker and no schema change. +- `schema_version` is NOT bumped by the decision-journal convention — `decisions[]` is a free-form `event.outputs` key, not a required ledger field. +- The `run.status` enum is exactly `running | complete | blocked | cancelled`. The post-merge report's "awaiting" condition is DERIVED at Resume-On-Start (a PR exists, `event.outputs.decisions[]` carries ≥1 `did-now`/`deferred`/`recorded` entry, and the zero-byte `.decision-report-done` marker is absent), NOT stored. That derived predicate is IDENTICAL whether or not the report is enabled — when `HIVEMIND_ENABLE_DECISION_REPORT` is unset or empty the same awaiting set is derived from the same local ledger reads, with no PR-state check and no report rendered, per `${CLAUDE_PLUGIN_ROOT}/governance/decision-autonomy.md (## Post-Merge Decision Report Trigger)`. +- The post-merge report uses no `artifacts.decision_report` ledger marker — it is CHAT-ONLY (rendered and surfaced to the user, never written to disk). Its sole idempotency token is the EXISTENCE of a zero-byte `.decision-report-done` marker `touch`ed in the run dir — not a content file, and not a ledger field. The marker means "this run will produce no further decision report" and is written on BOTH paths: already reported, OR suppressed while the report was off. It is zero-byte and implies no schema change. ## Blocker shape diff --git a/plugin/skills/github-review-loop/SKILL.md b/plugin/skills/github-review-loop/SKILL.md index 759321d3..9873d520 100644 --- a/plugin/skills/github-review-loop/SKILL.md +++ b/plugin/skills/github-review-loop/SKILL.md @@ -71,8 +71,8 @@ BARE token (`sed -n 's/^BASELINE=//p' | head -1`). A `SNAPSHOT_ERROR` line or non-zero exit → RETRY ONCE; a second failure is terminal `blocked` (same posture as `PREFLIGHT_ERROR`). NEVER arm the Monitor with an empty or absent seed — the poll rejects it as `POLL_ERROR` regardless, so failing here is the honest path. -Capturing BEFORE cycle 0 IS the fix for the #324 blind window: a seed taken after -cycle 0 re-opens it. +Capture BEFORE cycle 0: a seed taken after cycle 0 opens a blind window, in +which feedback that arrived during cycle 0 is never seen by the watch. **3. Cycle 0.** Dispatch `hivemind:github-reviewer` fix mode (see Dispatch contract) over pre-existing PR feedback before arming the Monitor. NEVER prefiltered. Handle diff --git a/plugin/skills/next-wave/SKILL.md b/plugin/skills/next-wave/SKILL.md index 009fdedd..4c71dffb 100644 --- a/plugin/skills/next-wave/SKILL.md +++ b/plugin/skills/next-wave/SKILL.md @@ -12,8 +12,8 @@ shell: bash Compute the next dispatchable WAVE of independent plan steps for the intra-run parallel-wave implement loop. The wave is the maximal, plan-order, file-disjoint subset of the READY steps — the steps whose dependencies are already done and whose file scopes do -not collide — so the overlord session can fan them out concurrently while degrading to today's -serial behavior when the plan graph forces it. The deterministic engine is the committed, +not collide — so the overlord session can fan them out concurrently, and the wave runs serially +when the plan graph forces it. The deterministic engine is the committed, READ-ONLY script `${CLAUDE_PLUGIN_ROOT}/skills/next-wave/scripts/next-wave.sh`; this body is a thin navigator that runs the script once and interprets its routing. The engine mutates NOTHING — it reads the ledger, derives the wave, and prints a routing decision. diff --git a/plugin/skills/next-wave/scripts/next-wave.sh b/plugin/skills/next-wave/scripts/next-wave.sh index 517123ab..4736965e 100755 --- a/plugin/skills/next-wave/scripts/next-wave.sh +++ b/plugin/skills/next-wave/scripts/next-wave.sh @@ -4,7 +4,7 @@ # # Computes the READY SET and the next dispatchable WAVE of plan steps for the intra-run # parallel-wave implement loop. This is the engine that lets the overlord fan out -# independent plan steps concurrently while preserving today's serial behavior when the +# independent plan steps concurrently while running serially when the # plan graph forces it. It reads the run ledger and PRINTS a routing decision; it mutates # NOTHING (no ledger write, no temp file, no atomic rename) — a pure read -> derive -> emit # engine. It sits in the same committed-script engine-op family as record-state-result.sh / diff --git a/plugin/skills/record-state-result/SKILL.md b/plugin/skills/record-state-result/SKILL.md index d21b9f06..c9489e8b 100644 --- a/plugin/skills/record-state-result/SKILL.md +++ b/plugin/skills/record-state-result/SKILL.md @@ -57,8 +57,8 @@ cerebrate's YAML plan `steps` into a JSON array and includes `plan_steps` (and o `.plan.path`). When those keys are ABSENT (missing or null), `.plan.*` is left UNTOUCHED — never clobbered to `[]`. -`init-run-ledger`'s `plan_steps` remains a writer ONLY for the child/resume SEED path -(default `[]`); it is no longer the primary live writer (see that skill's §A Plan-Steps Seam). +`init-run-ledger`'s `plan_steps` is a writer ONLY for the child/resume SEED path +(default `[]`); the primary live writer is here (see that skill's §A Plan-Steps Seam). The `outputs` field here is the event's free-form `outputs` object — it is NOT a plan-steps writer; use `plan_steps` for that. diff --git a/plugin/skills/spawn-brood/SKILL.md b/plugin/skills/spawn-brood/SKILL.md index 32a6604f..91c83788 100644 --- a/plugin/skills/spawn-brood/SKILL.md +++ b/plugin/skills/spawn-brood/SKILL.md @@ -72,7 +72,7 @@ After: non-failed observable rendered by `hivemind:brood-status` as such; only `failed` is the error state in brood-status derivation. Verification reads the child run-ledger `state.current` as ground truth — capture-pane - is not used (architectural direction established in #213/#248). Whether a + is not used. Whether a child actually completed turn-start is also observed independently by `hivemind:brood-status` from run-ledger ground truth (`state.current` present => `running`, absent => `starting`), not only by spawn-brood. diff --git a/plugin/workflows/pr-feedback-remediation.json b/plugin/workflows/pr-feedback-remediation.json index 23ee6001..5d5dc350 100644 --- a/plugin/workflows/pr-feedback-remediation.json +++ b/plugin/workflows/pr-feedback-remediation.json @@ -82,7 +82,7 @@ }, "implement_step": { "type": "agent", - "description": "Overlord dispatches the whole ready wave of remediation steps as parallel agents (mixed drone/changeling from the static allowed set) in one batch, awaits all, then aggregates results with blocked > needs_replan > complete precedence. A wave of one step degrades to today's serial behavior.", + "description": "Overlord dispatches the whole ready wave of remediation steps as parallel agents (mixed drone/changeling from the static allowed set) in one batch, awaits all, then aggregates results with blocked > needs_replan > complete precedence. A wave of one step runs serially.", "allowed_agents": ["hivemind:drone", "hivemind:changeling"], "transitions": { "complete": "checkpoint", diff --git a/plugin/workflows/standard-delivery.json b/plugin/workflows/standard-delivery.json index 69dc187c..cbea644e 100644 --- a/plugin/workflows/standard-delivery.json +++ b/plugin/workflows/standard-delivery.json @@ -66,7 +66,7 @@ }, "implement_step": { "type": "agent", - "description": "Overlord dispatches the whole ready wave as parallel agents (mixed drone/changeling from the static allowed set) in one batch, awaits all, then aggregates results with blocked > needs_replan > complete precedence. A wave of one step degrades to today's serial behavior.", + "description": "Overlord dispatches the whole ready wave as parallel agents (mixed drone/changeling from the static allowed set) in one batch, awaits all, then aggregates results with blocked > needs_replan > complete precedence. A wave of one step runs serially.", "allowed_agents": ["hivemind:drone", "hivemind:changeling"], "transitions": { "complete": "checkpoint", @@ -223,7 +223,7 @@ }, "implement_step_postpr": { "type": "agent", - "description": "Overlord dispatches the whole ready wave as parallel agents (mixed drone/changeling from the static allowed set) in one batch, awaits all, then aggregates results with blocked > needs_replan > complete precedence. A wave of one step degrades to today's serial behavior.", + "description": "Overlord dispatches the whole ready wave as parallel agents (mixed drone/changeling from the static allowed set) in one batch, awaits all, then aggregates results with blocked > needs_replan > complete precedence. A wave of one step runs serially.", "allowed_agents": ["hivemind:drone", "hivemind:changeling"], "transitions": { "complete": "checkpoint_postpr", From 463f4f05bff7629182353f4d587f438920c2a6d0 Mon Sep 17 00:00:00 2001 From: Bren Pike Date: Thu, 24 Sep 2026 09:10:15 -0600 Subject: [PATCH 2/7] test(policy): fail on tracker references in plugin runtime prose --- tests/policy/safety-tracker-ref-guard.json | 14 +++ tools/policy_check.sh | 100 +++++++++++++++++++++ 2 files changed, 114 insertions(+) create mode 100644 tests/policy/safety-tracker-ref-guard.json diff --git a/tests/policy/safety-tracker-ref-guard.json b/tests/policy/safety-tracker-ref-guard.json new file mode 100644 index 00000000..ecaf1a3b --- /dev/null +++ b/tests/policy/safety-tracker-ref-guard.json @@ -0,0 +1,14 @@ +{ + "rule": "check15-tracker-ref-guard-present", + "description": "P3 consumer-assertion: CHECK 15 (No tracker references in plugin runtime prose) in tools/policy_check.sh is the CI guard that makes P19 (doctrine anchors on durable records, not tracker IDs) real rather than decoration, per P17. This fixture pins the two literals that prove the guard still exists and still reports through the shared finding machinery: the `=== CHECK 15:` section header (the block is present and still announces itself) and `add_finding 'CHECK15'` (it still emits CHECK15 findings, so --strict and the allowlist still apply to it). superset mode fails if either literal is removed; extras elsewhere in the file are ignored. Residual, stated plainly: this pin claims presence only. It cannot prove the regex still detects anything, and it is blind to a weakened pattern -- narrowing CHECK15_TRACKER_PATTERN, or narrowing the discovery globs, leaves both pinned literals intact and this fixture green.", + "set_check": { + "extract_regex": "(=== CHECK 15:|add_finding 'CHECK15')", + "expected_set": [ + "=== CHECK 15:", + "add_finding 'CHECK15'" + ], + "files": [ + { "path": "tools/policy_check.sh", "mode": "superset" } + ] + } +} diff --git a/tools/policy_check.sh b/tools/policy_check.sh index 47a028e7..d91cd1a8 100644 --- a/tools/policy_check.sh +++ b/tools/policy_check.sh @@ -1516,6 +1516,106 @@ fi mark_time 'CHECK14' +# ── CHECK 15: No tracker references in plugin runtime prose ──────────────── +# +# WHAT IT GUARANTEES. docs/engineering-principles.md P19 (doctrine anchors on +# durable records, not tracker IDs) forbids a bare `#NNN` in runtime doctrine: +# the ticket closes, is renumbered in meaning, or is superseded, and the prose +# silently rots. P17 says a mechanizable rule left to reviewer vigilance is +# decoration, so P19 is only real once a CI guard asserts it -- this is that +# guard. +# +# SCOPE. plugin/**/*.md plus plugin/workflows/*.json: the runtime-loaded +# instruction payload an agent reads as its own context. Committed shell under +# plugin/ is deliberately OUT of scope -- its comments are developer-facing and +# are never loaded into an agent's context, so a tracker ID there stales a +# maintainer note rather than doctrine. +# +# SCAN SHAPE. One awk pass per file strips a trailing CR (prose in this repo is +# stored CRLF) and skips YAML frontmatter (line 1 `---` opens, the next `---` +# closes), which is structured metadata rather than prose. Fenced code blocks +# and indented lines are deliberately NOT skipped: real tracker references here +# have lived inside a ```text fence and in an indented continuation line, so +# skipping either construct would let the class ship unseen. +# +# PATTERN CLAUSES, each earning its place: +# * a digit is REQUIRED after `#` -> ATX headings (`# `, `## `) and a +# `#!` shebang can never match. +# * no preceding word character or `/` -> a cross-repo citation such as +# `cli/cli#12258` names its repo and stays legal, while a bare `#12258` +# is caught. +# * `{1,5}` digits AND no trailing alphanumeric -> hex colors `#123456` and +# `#1a2b3c` can never match. +# * anchors: a `(#section)` fragment starts with a letter, and the `](#...)` +# strip below additionally covers digit-leading slugs. +# * the backtick strip exempts inline-code placeholders such as the literal +# issue-number placeholder in plugin/skills/prd-to-issues/SKILL.md. +# +# Discovery FAILS CLOSED: zero discovered prose files is an ERROR, not a pass, +# so a moved or renamed payload tree cannot silently disarm this check. +# +# RESIDUAL, stated plainly: a fenced example that legitimately needs a literal +# `#123` must move into inline code or be allowlisted like any other finding. +echo '' +echo '=== CHECK 15: No tracker references in plugin runtime prose ===' + +CHECK15_TRACKER_PATTERN='(^|[^A-Za-z0-9_/])#[0-9]{1,5}([^0-9A-Za-z_]|$)' + +check15_found=false +check15_file_count=0 + +# scan_file_for_tracker_refs FILE +# Emits a CHECK15 finding per prose line carrying a bare tracker reference. +scan_file_for_tracker_refs() { + local prose_file="$1" + local line_num textline residual token + while IFS=$'\t' read -r line_num textline; do + # Cheap gate: the overwhelming majority of prose lines carry no `#` at + # all, so the sed/grep pipeline below is only paid for candidates. + case "$textline" in + *'#'*) ;; + *) continue ;; + esac + residual="$(printf '%s' "$textline" | sed -E 's/`[^`]*`//g; s/\]\(#[^)]*\)//g')" + if ! printf '%s' "$residual" | grep -qE "$CHECK15_TRACKER_PATTERN"; then + continue + fi + token="$(printf '%s' "$residual" | grep -oE "$CHECK15_TRACKER_PATTERN" | head -n1)" + check15_found=true + add_finding 'CHECK15' "$prose_file" "$line_num" \ + "tracker reference '${token}' in plugin runtime prose -- cite a durable anchor (ADR, named invariant, or a present-tense description of the rule), never an issue or PR number" + done < <(awk ' + { sub(/\r$/, "") } + NR == 1 && $0 == "---" { in_frontmatter = 1; next } + in_frontmatter && $0 == "---" { in_frontmatter = 0; next } + in_frontmatter { next } + { print NR "\t" $0 } + ' "$prose_file") +} + +while IFS= read -r -d '' prose_file; do + check15_file_count=$((check15_file_count + 1)) + scan_file_for_tracker_refs "$prose_file" +done < <( + find "$PLUGIN_ROOT" -name '*.md' -type f -print0 + find "$PLUGIN_ROOT/workflows" -maxdepth 1 -name '*.json' -type f -print0 +) + +if [[ "$check15_file_count" -eq 0 ]]; then + check15_found=true + add_finding 'CHECK15' "$PLUGIN_ROOT" 0 \ + "Tracker-reference discovery found ZERO plugin runtime prose files -- no plugin/**/*.md and no plugin/workflows/*.json were discovered, which disarms this check; restore the payload tree or retire the check deliberately" +fi + +if [[ "$check15_found" == false ]]; then + echo "[PASS] Check 15: No tracker references in $check15_file_count plugin runtime prose files" + CHECKS_PASSED=$((CHECKS_PASSED + 1)) +else + CHECKS_FAILED=$((CHECKS_FAILED + 1)) +fi + +mark_time 'CHECK15' + # ── SAFETY REGRESSION TESTS ──────────────────────────────────────────────── echo '' From 2af2a7cd94d2d1bcc54a31ac5e6de533b6e7e40c Mon Sep 17 00:00:00 2001 From: Bren Pike Date: Thu, 24 Sep 2026 09:11:13 -0600 Subject: [PATCH 3/7] chore(release): bump version to 4.0.2 --- CHANGELOG.md | 10 ++++++++++ plugin/.claude-plugin/plugin.json | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b980638..9c29f54f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +## [4.0.2] - 2026-09-24 + +### Added + +- `tools/policy_check.sh` CHECK 15: fails on GitHub tracker references (bare `#NNN`) in plugin runtime prose (`plugin/**/*.md` and `plugin/workflows/*.json`), with fixture `tests/policy/safety-tracker-ref-guard.json`. Headings, shebangs, hex colors, in-page anchors, inline-code placeholders, and `owner/repo#N` citations are exempt. + +### Changed + +- Runtime prose in `agents/overlord.md`, `governance/remediation-doctrine.md`, `references/run-ledger-schema.md`, `references/brood-ledger-model.md`, `references/github-pr-review-graphql.md`, the `github-review-loop`, `spawn-brood`, `record-state-result`, and `next-wave` skills, and the `standard-delivery` and `pr-feedback-remediation` workflow descriptions now states each rule in present tense: issue and PR numbers, "no longer / today's / as before / now" framing, and change-history narration are removed. No rule or constraint changed. + ## [4.0.1] - 2026-09-24 ### Fixed diff --git a/plugin/.claude-plugin/plugin.json b/plugin/.claude-plugin/plugin.json index 539dc88c..92f46e4d 100644 --- a/plugin/.claude-plugin/plugin.json +++ b/plugin/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "hivemind", - "version": "4.0.1", + "version": "4.0.2", "description": "Claude Code plugin providing a structured multi-agent framework with overlord, cerebrate, drone, changeling, local-reviewer, and github-reviewer agents plus workflow skills for git branching, commits, PRs, and code review remediation.", "author": { "name": "brenpike" From 9eb6133c16cbbd63065f1cfabeaf804ba854c023 Mon Sep 17 00:00:00 2001 From: Bren Pike Date: Thu, 24 Sep 2026 09:30:44 -0600 Subject: [PATCH 4/7] fix(policy): harden CHECK 15 tracker guard detection and discovery --- tests/policy/safety-tracker-ref-guard.json | 2 +- tools/policy_check.sh | 120 +++++++++++++++++---- 2 files changed, 103 insertions(+), 19 deletions(-) diff --git a/tests/policy/safety-tracker-ref-guard.json b/tests/policy/safety-tracker-ref-guard.json index ecaf1a3b..ba739519 100644 --- a/tests/policy/safety-tracker-ref-guard.json +++ b/tests/policy/safety-tracker-ref-guard.json @@ -1,6 +1,6 @@ { "rule": "check15-tracker-ref-guard-present", - "description": "P3 consumer-assertion: CHECK 15 (No tracker references in plugin runtime prose) in tools/policy_check.sh is the CI guard that makes P19 (doctrine anchors on durable records, not tracker IDs) real rather than decoration, per P17. This fixture pins the two literals that prove the guard still exists and still reports through the shared finding machinery: the `=== CHECK 15:` section header (the block is present and still announces itself) and `add_finding 'CHECK15'` (it still emits CHECK15 findings, so --strict and the allowlist still apply to it). superset mode fails if either literal is removed; extras elsewhere in the file are ignored. Residual, stated plainly: this pin claims presence only. It cannot prove the regex still detects anything, and it is blind to a weakened pattern -- narrowing CHECK15_TRACKER_PATTERN, or narrowing the discovery globs, leaves both pinned literals intact and this fixture green.", + "description": "P3 consumer-assertion: CHECK 15 (No tracker references in plugin runtime prose) in tools/policy_check.sh is the CI guard that makes P19 (doctrine anchors on durable records, not tracker IDs) real rather than decoration, per P17. This fixture pins the two literals that prove the guard still exists and still reports through the shared finding machinery: the `=== CHECK 15:` section header (the block is present and still announces itself) and `add_finding 'CHECK15'` (it still emits CHECK15 findings, so --strict and the allowlist still apply to it). superset mode fails if either literal is removed; extras elsewhere in the file are ignored. Residual, stated plainly: this pin claims presence only. It cannot prove the regex still detects anything, and it is blind to a weakened pattern -- narrowing CHECK15_TRACKER_PATTERN, or narrowing the discovery globs, leaves both pinned literals intact and this fixture green. That uncovered half is not left open: no fixture in this directory can witness detection semantics (pin-authoring contract rule 7), so it is covered instead by the structural checks inside tools/policy_check.sh itself -- the CHECK 15 detection canary, which asserts one positive and one negative case per documented pattern clause through the pure tracker_ref_token predicate, and the per-arm zero-discovery assertions, which fail closed on each discovery arm separately.", "set_check": { "extract_regex": "(=== CHECK 15:|add_finding 'CHECK15')", "expected_set": [ diff --git a/tools/policy_check.sh b/tools/policy_check.sh index d91cd1a8..bbfd1edb 100644 --- a/tools/policy_check.sh +++ b/tools/policy_check.sh @@ -1544,43 +1544,73 @@ mark_time 'CHECK14' # * no preceding word character or `/` -> a cross-repo citation such as # `cli/cli#12258` names its repo and stays legal, while a bare `#12258` # is caught. -# * `{1,5}` digits AND no trailing alphanumeric -> hex colors `#123456` and -# `#1a2b3c` can never match. +# * no trailing alphanumeric -> a letter-bearing hex colour such as +# `#1a2b3c` can never match. The digit run itself is UNBOUNDED, so a tracker +# id of any length is in reach; the cost is that a pure-digit colour literal +# (`#123456`) is flagged too. That direction is deliberate: a false positive +# here is loud and allowlistable, a false negative is silent forever. # * anchors: a `(#section)` fragment starts with a letter, and the `](#...)` # strip below additionally covers digit-leading slugs. # * the backtick strip exempts inline-code placeholders such as the literal # issue-number placeholder in plugin/skills/prd-to-issues/SKILL.md. # -# Discovery FAILS CLOSED: zero discovered prose files is an ERROR, not a pass, -# so a moved or renamed payload tree cannot silently disarm this check. +# Discovery FAILS CLOSED PER ARM: each of the two discovery arms (`plugin/**/*.md` +# and `plugin/workflows/*.json`) carries its OWN zero-file assertion. An aggregate +# count cannot carry this guarantee -- a missing, renamed, or unreadable workflows +# tree yields zero JSON files while the markdown arm keeps the aggregate nonzero, +# so half the stated scope would vanish with the check still green. +# +# DETECTION CANARY: the safety fixture for this guard pins presence only (the +# section banner and `add_finding 'CHECK15'`), so a narrowed pattern or a dropped +# exemption leaves it green. The canary below asserts the DETECTION semantics +# directly, one case per pattern clause above, in both directions. # # RESIDUAL, stated plainly: a fenced example that legitimately needs a literal # `#123` must move into inline code or be allowlisted like any other finding. echo '' echo '=== CHECK 15: No tracker references in plugin runtime prose ===' -CHECK15_TRACKER_PATTERN='(^|[^A-Za-z0-9_/])#[0-9]{1,5}([^0-9A-Za-z_]|$)' +CHECK15_TRACKER_PATTERN='(^|[^A-Za-z0-9_/])#[0-9]+([^0-9A-Za-z_]|$)' check15_found=false check15_file_count=0 +# tracker_ref_token TEXTLINE +# Echoes the first bare tracker reference in TEXTLINE, or nothing when the line +# carries none. PURE -- no findings, no globals, always exit 0 -- so the +# detection canary below can assert the guard's semantics directly rather than +# merely asserting that the guard exists. +tracker_ref_token() { + local textline="$1" residual + # Cheap gate: the overwhelming majority of prose lines carry no `#` at all, + # so the sed/grep pipeline below is only paid for candidates. + case "$textline" in + *'#'*) ;; + *) return 0 ;; + esac + residual="$(printf '%s' "$textline" | sed -E 's/`[^`]*`//g; s/\]\(#[^)]*\)//g')" + # `|| true` is load-bearing under `set -euo pipefail`: `head -n1` can close + # the pipe before `grep` finishes, and the resulting SIGPIPE status would + # otherwise abort the whole run on a line that merely has no match. + printf '%s' "$residual" | grep -oE "$CHECK15_TRACKER_PATTERN" | head -n1 || true +} + # scan_file_for_tracker_refs FILE # Emits a CHECK15 finding per prose line carrying a bare tracker reference. scan_file_for_tracker_refs() { local prose_file="$1" - local line_num textline residual token + local line_num textline token while IFS=$'\t' read -r line_num textline; do - # Cheap gate: the overwhelming majority of prose lines carry no `#` at - # all, so the sed/grep pipeline below is only paid for candidates. + # Same cheap gate as tracker_ref_token, hoisted so a line with no `#` + # never pays the command-substitution fork. case "$textline" in *'#'*) ;; *) continue ;; esac - residual="$(printf '%s' "$textline" | sed -E 's/`[^`]*`//g; s/\]\(#[^)]*\)//g')" - if ! printf '%s' "$residual" | grep -qE "$CHECK15_TRACKER_PATTERN"; then + token="$(tracker_ref_token "$textline")" + if [[ -z "$token" ]]; then continue fi - token="$(printf '%s' "$residual" | grep -oE "$CHECK15_TRACKER_PATTERN" | head -n1)" check15_found=true add_finding 'CHECK15' "$prose_file" "$line_num" \ "tracker reference '${token}' in plugin runtime prose -- cite a durable anchor (ADR, named invariant, or a present-tense description of the rule), never an issue or PR number" @@ -1593,20 +1623,74 @@ scan_file_for_tracker_refs() { ' "$prose_file") } +check15_md_count=0 while IFS= read -r -d '' prose_file; do - check15_file_count=$((check15_file_count + 1)) + check15_md_count=$((check15_md_count + 1)) scan_file_for_tracker_refs "$prose_file" -done < <( - find "$PLUGIN_ROOT" -name '*.md' -type f -print0 - find "$PLUGIN_ROOT/workflows" -maxdepth 1 -name '*.json' -type f -print0 -) +done < <(find "$PLUGIN_ROOT" -name '*.md' -type f -print0 2>/dev/null) -if [[ "$check15_file_count" -eq 0 ]]; then +check15_json_count=0 +while IFS= read -r -d '' prose_file; do + check15_json_count=$((check15_json_count + 1)) + scan_file_for_tracker_refs "$prose_file" +done < <(find "$PLUGIN_ROOT/workflows" -maxdepth 1 -name '*.json' -type f -print0 2>/dev/null) + +check15_file_count=$((check15_md_count + check15_json_count)) + +# Per-arm fail-closed. Each arm's tree is non-empty by construction, so a zero +# count means that arm was moved, renamed, or is unreadable -- and a per-arm +# assertion is the only shape that catches it: an aggregate count stays nonzero +# while one arm silently contributes nothing. +if [[ "$check15_md_count" -eq 0 ]]; then check15_found=true add_finding 'CHECK15' "$PLUGIN_ROOT" 0 \ - "Tracker-reference discovery found ZERO plugin runtime prose files -- no plugin/**/*.md and no plugin/workflows/*.json were discovered, which disarms this check; restore the payload tree or retire the check deliberately" + "Tracker-reference discovery found ZERO plugin/**/*.md runtime prose files, which disarms half this check; restore the payload tree or retire the check deliberately" fi +if [[ "$check15_json_count" -eq 0 ]]; then + check15_found=true + add_finding 'CHECK15' "$PLUGIN_ROOT/workflows" 0 \ + "Tracker-reference discovery found ZERO plugin/workflows/*.json runtime definitions, which disarms half this check; restore the workflow-definition tree or retire the check deliberately" +fi + +# ── CHECK 15 DETECTION CANARY ────────────────────────────────────────────── +# One case per documented pattern clause, in BOTH directions. A narrowed +# pattern, a dropped strip, or a lost exemption turns this run red where the +# presence-pinning safety fixture would stay green. +check15_expect_hit() { + if [[ -z "$(tracker_ref_token "$1")" ]]; then + check15_found=true + add_finding 'CHECK15' 'tools/policy_check.sh' 0 \ + "detection canary: no tracker reference detected in \"$1\" -- CHECK15_TRACKER_PATTERN has been narrowed and the guard no longer catches the class it claims to ban" + fi +} + +check15_expect_miss() { + local hit + hit="$(tracker_ref_token "$1")" + if [[ -n "$hit" ]]; then + check15_found=true + add_finding 'CHECK15' 'tools/policy_check.sh' 0 \ + "detection canary: exempt construct \"$1\" was flagged as tracker reference '${hit}' -- an exemption clause has been dropped and the guard now fires on legal prose" + fi +} + +check15_expect_hit 'see #123 for the rationale' +check15_expect_hit 'tracked as #7.' +check15_expect_hit '(#42) covers the remainder' +check15_expect_hit '#5 is the earliest' +check15_expect_hit 'superseded by #123456' +check15_expect_hit 'superseded by #1000000' + +check15_expect_miss '# Heading' +check15_expect_miss '## Subheading' +check15_expect_miss '#!/usr/bin/env bash' +check15_expect_miss 'cross-repo citation cli/cli#12258 names its repo' +check15_expect_miss 'see [the anchor](#section-2) above' +check15_expect_miss 'the literal placeholder `#123` inside inline code' +check15_expect_miss 'the colour #1a2b3c is letter-bearing' +check15_expect_miss 'no hash here at all' + if [[ "$check15_found" == false ]]; then echo "[PASS] Check 15: No tracker references in $check15_file_count plugin runtime prose files" CHECKS_PASSED=$((CHECKS_PASSED + 1)) From 80f42469eed0eee5f2fb7bfeedc2ce03418040f2 Mon Sep 17 00:00:00 2001 From: Bren Pike Date: Thu, 24 Sep 2026 10:38:53 -0600 Subject: [PATCH 5/7] test(policy): add CHECK 15 scanner canary fixtures --- CHANGELOG.md | 2 +- .../fixtures/tracker-ref-scan-canary.md | 36 +++++++++++++++++++ .../tracker-ref-unclosed-frontmatter.md | 10 ++++++ 3 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 tests/policy/fixtures/tracker-ref-scan-canary.md create mode 100644 tests/policy/fixtures/tracker-ref-unclosed-frontmatter.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c29f54f..6c1cf08c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- `tools/policy_check.sh` CHECK 15: fails on GitHub tracker references (bare `#NNN`) in plugin runtime prose (`plugin/**/*.md` and `plugin/workflows/*.json`), with fixture `tests/policy/safety-tracker-ref-guard.json`. Headings, shebangs, hex colors, in-page anchors, inline-code placeholders, and `owner/repo#N` citations are exempt. +- `tools/policy_check.sh` CHECK 15: fails on GitHub tracker references (bare `#NNN`) in plugin runtime prose (`plugin/**/*.md` and `plugin/workflows/*.json`), with fixture `tests/policy/safety-tracker-ref-guard.json`. Every line is scanned, including YAML frontmatter, fenced blocks, and indented lines, and one finding per line lists every tracker reference on that line. The check carries a scanner-level canary over committed fixtures. Headings, shebangs, hex colors, in-page anchors, inline-code placeholders, and `owner/repo#N` citations are exempt. ### Changed diff --git a/tests/policy/fixtures/tracker-ref-scan-canary.md b/tests/policy/fixtures/tracker-ref-scan-canary.md new file mode 100644 index 00000000..41bcb908 --- /dev/null +++ b/tests/policy/fixtures/tracker-ref-scan-canary.md @@ -0,0 +1,36 @@ +--- +name: tracker-ref-scan-canary +description: frontmatter cites #11 here +--- + + + +# tracker-ref scan canary + +Body prose cites #21 as a tracker id. + +```text +Fenced text cites #31 and must still be scanned. +``` + +- A list item whose continuation line is indented four spaces. + Indented continuation cites #41 and must still be scanned. + +Adjacent refs #12 #34 and later #56. + +## Heading 2 + +```bash +#!/usr/bin/env bash +``` + +Letter-bearing colour #1a2b3c stays legal. + +See [the section](#section-2) for details. + +Inline placeholder `#123` stays legal. + +Upstream cli/cli#12258 citation stays legal. diff --git a/tests/policy/fixtures/tracker-ref-unclosed-frontmatter.md b/tests/policy/fixtures/tracker-ref-unclosed-frontmatter.md new file mode 100644 index 00000000..ffd4a608 --- /dev/null +++ b/tests/policy/fixtures/tracker-ref-unclosed-frontmatter.md @@ -0,0 +1,10 @@ +--- +name: tracker-ref-unclosed-frontmatter + + + +unclosed frontmatter must not hide #77 From f7d944a774bc6902a718b24e2a7427dac988b7cc Mon Sep 17 00:00:00 2001 From: Bren Pike Date: Thu, 24 Sep 2026 10:50:32 -0600 Subject: [PATCH 6/7] fix(policy): scan every line and report every tracker ref in CHECK 15 --- tools/policy_check.sh | 226 ++++++++++++++++++++++++++++++------------ 1 file changed, 164 insertions(+), 62 deletions(-) diff --git a/tools/policy_check.sh b/tools/policy_check.sh index bbfd1edb..5beb82c1 100644 --- a/tools/policy_check.sh +++ b/tools/policy_check.sh @@ -1531,28 +1531,55 @@ mark_time 'CHECK14' # are never loaded into an agent's context, so a tracker ID there stales a # maintainer note rather than doctrine. # -# SCAN SHAPE. One awk pass per file strips a trailing CR (prose in this repo is -# stored CRLF) and skips YAML frontmatter (line 1 `---` opens, the next `---` -# closes), which is structured metadata rather than prose. Fenced code blocks -# and indented lines are deliberately NOT skipped: real tracker references here -# have lived inside a ```text fence and in an indented continuation line, so -# skipping either construct would let the class ship unseen. +# SCAN SHAPE. The awk program is `{ print NR "\t" $0 }` and nothing else: zero +# state, zero regions, one record per physical line. NO line is exempt by +# POSITION -- YAML frontmatter, fenced code blocks, and indented continuation +# lines are all scanned. Exemption is by PATTERN only. Two reasons, both +# load-bearing. First, a region skipper is a state machine whose failure mode is +# silent: one unclosed region opener swallows the entire remaining file body and +# the check still reports PASS over prose it never read, so that whole class of +# failure is DELETED here rather than guarded. Second, frontmatter in this +# payload is not inert metadata -- it is runtime-loaded agent context (a skill's +# name and description are read before its body), so it carries doctrine and +# earns the same rule as the body. # -# PATTERN CLAUSES, each earning its place: -# * a digit is REQUIRED after `#` -> ATX headings (`# `, `## `) and a -# `#!` shebang can never match. -# * no preceding word character or `/` -> a cross-repo citation such as -# `cli/cli#12258` names its repo and stays legal, while a bare `#12258` -# is caught. -# * no trailing alphanumeric -> a letter-bearing hex colour such as -# `#1a2b3c` can never match. The digit run itself is UNBOUNDED, so a tracker -# id of any length is in reach; the cost is that a pure-digit colour literal -# (`#123456`) is flagged too. That direction is deliberate: a false positive -# here is loud and allowlistable, a false negative is silent forever. -# * anchors: a `(#section)` fragment starts with a letter, and the `](#...)` -# strip below additionally covers digit-leading slugs. -# * the backtick strip exempts inline-code placeholders such as the literal -# issue-number placeholder in plugin/skills/prd-to-issues/SKILL.md. +# PATTERN CLAUSES are NORMALIZE-THEN-MATCH: every legal construct is deleted +# from a working copy of the line first, and whatever survives is matched by the +# bare token pattern `#[0-9]+`. Each sed pass, and what it earns: +# * `s/\r$//` -- CR strip. Prose here may be stored CRLF +# depending on autocrlf, and stripping the trailing CR makes every record +# and every token byte-identical to the LF case. +# * `s/`[^`]*`//g` -- inline code. Exempts literal +# placeholders such as the issue-number placeholder in +# plugin/skills/prd-to-issues/SKILL.md. +# * `s/\]\(#[^)]*\)//g` -- Markdown in-page anchors `](#...)`, +# including digit-leading slugs a letter-first rule would miss. +# * `s@([A-Za-z0-9_/])#@\1@g` -- a `#` glued to a word character or `/`, +# which is the cross-repo citation form `cli/cli#12258`: it names its repo, +# so it does not rot, and stays legal. +# * `s/#([0-9]+)([A-Za-z_])/\1\2/g` -- a letter-bearing hex colour such as +# `#1a2b3c`. The trailing class MUST be `[A-Za-z_]` and MUST NOT admit +# digits: with `[A-Za-z0-9_]` the greedy `[0-9]+` run backtracks one digit to +# feed the trailing class, so the `#` is stripped from EVERY multi-digit +# reference (`see #123` yields no token) and the guard goes silently blind. +# ATX headings (`# `, `## `) and a `#!` shebang need no pass of their own: the +# matcher requires a digit immediately after the `#`. +# +# CARDINALITY. The matcher consumes no surrounding context, so every reference +# on a line is reported, not just the first. A context-consuming matcher eats +# the separator between neighbours and misses the second (`x #12 #34` reports +# only `#12`), and that is not cosmetic here: the allowlist keys on (rule, path, +# line), so a finding that is not line-COMPLETE lets an allowlist entry absolve +# a reference no reviewer ever saw. +# +# WITNESSES, two layers, because they fail independently: +# * the DETECTION CANARY asserts the pure predicate `tracker_ref_tokens` over +# literal lines -- one case per pattern clause above, in both directions, +# and by EXACT token string so a dropped neighbour is caught. +# * the SCANNER CANARY asserts `scan_prose_file_refs` over committed fixtures. +# It is the only layer that can witness the file-level traversal: the awk +# record shape, the line numbering, and the absence of any region skipping +# (including an unclosed-frontmatter fixture that no predicate test reaches). # # Discovery FAILS CLOSED PER ARM: each of the two discovery arms (`plugin/**/*.md` # and `plugin/workflows/*.json`) carries its OWN zero-file assertion. An aggregate @@ -1560,67 +1587,84 @@ mark_time 'CHECK14' # tree yields zero JSON files while the markdown arm keeps the aggregate nonzero, # so half the stated scope would vanish with the check still green. # -# DETECTION CANARY: the safety fixture for this guard pins presence only (the -# section banner and `add_finding 'CHECK15'`), so a narrowed pattern or a dropped -# exemption leaves it green. The canary below asserts the DETECTION semantics -# directly, one case per pattern clause above, in both directions. -# -# RESIDUAL, stated plainly: a fenced example that legitimately needs a literal -# `#123` must move into inline code or be allowlisted like any other finding. +# RESIDUALS, stated plainly: +# * allowlist granularity is the LINE, not the token. An entry added for one +# reference on a line would also absolve a reference added to that same line +# later. Zero CHECK15 allowlist entries exist today, so nothing is absolved +# in practice; closing this for real needs a token-level key in the shared +# preload_allowlist/test_allowlisted machinery, which every check shares. +# * bare anchor TEXT such as `(#2-slug)` written outside a `](...)` link is +# reported as a reference. Move it into a real link or into inline code. +# * a pure-digit hex colour (`#123456`) is reported as a tracker reference. +# Deliberate: an unbounded digit run keeps a tracker id of any length in +# reach, and a loud allowlistable false positive beats a silent permanent +# false negative. +# * the scanner canary pins fixture LINE NUMBERS in this file. The fixtures say +# so in-file; edit fixture and canary together. echo '' echo '=== CHECK 15: No tracker references in plugin runtime prose ===' -CHECK15_TRACKER_PATTERN='(^|[^A-Za-z0-9_/])#[0-9]+([^0-9A-Za-z_]|$)' +CHECK15_TRACKER_PATTERN='#[0-9]+' +CHECK15_LEGAL_CONTEXT_SED='s/\r$//; s/`[^`]*`//g; s/\]\(#[^)]*\)//g; s@([A-Za-z0-9_/])#@\1@g; s/#([0-9]+)([A-Za-z_])/\1\2/g' check15_found=false check15_file_count=0 -# tracker_ref_token TEXTLINE -# Echoes the first bare tracker reference in TEXTLINE, or nothing when the line -# carries none. PURE -- no findings, no globals, always exit 0 -- so the -# detection canary below can assert the guard's semantics directly rather than -# merely asserting that the guard exists. -tracker_ref_token() { - local textline="$1" residual +# tracker_ref_tokens TEXTLINE +# Prints EVERY bare tracker reference in TEXTLINE, space-separated in source +# order, or nothing when the line carries none. PURE -- no findings, no globals, +# always exit 0 -- so the detection canary below can assert the guard's +# semantics directly rather than merely asserting that the guard exists. +tracker_ref_tokens() { + local textline="$1" normalized tokens # Cheap gate: the overwhelming majority of prose lines carry no `#` at all, # so the sed/grep pipeline below is only paid for candidates. case "$textline" in *'#'*) ;; *) return 0 ;; esac - residual="$(printf '%s' "$textline" | sed -E 's/`[^`]*`//g; s/\]\(#[^)]*\)//g')" - # `|| true` is load-bearing under `set -euo pipefail`: `head -n1` can close - # the pipe before `grep` finishes, and the resulting SIGPIPE status would - # otherwise abort the whole run on a line that merely has no match. - printf '%s' "$residual" | grep -oE "$CHECK15_TRACKER_PATTERN" | head -n1 || true + normalized="$(printf '%s' "$textline" | sed -E "$CHECK15_LEGAL_CONTEXT_SED")" + # `|| true` is load-bearing under `set -euo pipefail`: `grep -o` exits 1 when + # the normalized line carries no token, which is the common case for a line + # whose every `#` was legal, and that status would otherwise abort the run. + tokens="$(printf '%s' "$normalized" | grep -oE "$CHECK15_TRACKER_PATTERN" | tr '\n' ' ' || true)" + printf '%s' "${tokens% }" } -# scan_file_for_tracker_refs FILE -# Emits a CHECK15 finding per prose line carrying a bare tracker reference. -scan_file_for_tracker_refs() { +# scan_prose_file_refs FILE +# Prints one record per offending line, `LINENOtok1 tok2 ...`, in file +# order. PURE -- no findings, no globals, always exit 0 -- so the scanner canary +# below can assert the file-level traversal (record shape, line numbering, and +# the absence of any region skipping) over committed fixtures. +scan_prose_file_refs() { local prose_file="$1" - local line_num textline token + local line_num textline tokens while IFS=$'\t' read -r line_num textline; do - # Same cheap gate as tracker_ref_token, hoisted so a line with no `#` + # Same cheap gate as tracker_ref_tokens, hoisted so a line with no `#` # never pays the command-substitution fork. case "$textline" in *'#'*) ;; *) continue ;; esac - token="$(tracker_ref_token "$textline")" - if [[ -z "$token" ]]; then + tokens="$(tracker_ref_tokens "$textline")" + if [[ -z "$tokens" ]]; then continue fi + printf '%s\t%s\n' "$line_num" "$tokens" + done < <(awk '{ print NR "\t" $0 }' "$prose_file") +} + +# scan_file_for_tracker_refs FILE +# Thin reporting wrapper: turns each record from scan_prose_file_refs into a +# CHECK15 finding. Carries no detection logic of its own. +scan_file_for_tracker_refs() { + local prose_file="$1" + local line_num tokens + while IFS=$'\t' read -r line_num tokens; do check15_found=true add_finding 'CHECK15' "$prose_file" "$line_num" \ - "tracker reference '${token}' in plugin runtime prose -- cite a durable anchor (ADR, named invariant, or a present-tense description of the rule), never an issue or PR number" - done < <(awk ' - { sub(/\r$/, "") } - NR == 1 && $0 == "---" { in_frontmatter = 1; next } - in_frontmatter && $0 == "---" { in_frontmatter = 0; next } - in_frontmatter { next } - { print NR "\t" $0 } - ' "$prose_file") + "tracker reference(s) ${tokens} in plugin runtime prose -- cite a durable anchor (ADR, named invariant, or a present-tense description of the rule), never an issue or PR number; every reference on the line is listed because an allowlist entry covers the whole line, not one token" + done < <(scan_prose_file_refs "$prose_file") } check15_md_count=0 @@ -1654,24 +1698,39 @@ if [[ "$check15_json_count" -eq 0 ]]; then fi # ── CHECK 15 DETECTION CANARY ────────────────────────────────────────────── -# One case per documented pattern clause, in BOTH directions. A narrowed -# pattern, a dropped strip, or a lost exemption turns this run red where the +# One case per documented pattern clause, in BOTH directions, plus exact-token +# cases that pin CARDINALITY. A narrowed pattern, a dropped normalization pass, +# or a matcher that stops after the first token turns this run red where the # presence-pinning safety fixture would stay green. check15_expect_hit() { - if [[ -z "$(tracker_ref_token "$1")" ]]; then + if [[ -z "$(tracker_ref_tokens "$1")" ]]; then check15_found=true add_finding 'CHECK15' 'tools/policy_check.sh' 0 \ - "detection canary: no tracker reference detected in \"$1\" -- CHECK15_TRACKER_PATTERN has been narrowed and the guard no longer catches the class it claims to ban" + "detection canary: no tracker reference detected in \"$1\" -- CHECK15_TRACKER_PATTERN has been narrowed, or a CHECK15_LEGAL_CONTEXT_SED pass now eats a real reference, and the guard no longer catches the class it claims to ban" fi } check15_expect_miss() { local hit - hit="$(tracker_ref_token "$1")" + hit="$(tracker_ref_tokens "$1")" if [[ -n "$hit" ]]; then check15_found=true add_finding 'CHECK15' 'tools/policy_check.sh' 0 \ - "detection canary: exempt construct \"$1\" was flagged as tracker reference '${hit}' -- an exemption clause has been dropped and the guard now fires on legal prose" + "detection canary: exempt construct \"$1\" was flagged as tracker reference '${hit}' -- a CHECK15_LEGAL_CONTEXT_SED normalization pass has been dropped and the guard now fires on legal prose" + fi +} + +# check15_expect_tokens LINE EXPECTED +# Exact-string assertion on the FULL token list. expect_hit only proves that +# something was found; only this shape catches a matcher that reports the first +# reference on a line and silently drops its neighbours. +check15_expect_tokens() { + local got + got="$(tracker_ref_tokens "$1")" + if [[ "$got" != "$2" ]]; then + check15_found=true + add_finding 'CHECK15' 'tools/policy_check.sh' 0 \ + "detection canary: line \"$1\" yielded tokens '${got}' but expected '${2}' -- CHECK15_TRACKER_PATTERN or CHECK15_LEGAL_CONTEXT_SED no longer reports every reference on a line, and a line-keyed allowlist entry would then absolve the references it drops" fi } @@ -1691,6 +1750,49 @@ check15_expect_miss 'the literal placeholder `#123` inside inline code' check15_expect_miss 'the colour #1a2b3c is letter-bearing' check15_expect_miss 'no hash here at all' +check15_expect_tokens 'see #123 for context' '#123' +check15_expect_tokens 'tracked as #7.' '#7' +check15_expect_tokens '(#42) noted' '#42' +check15_expect_tokens '#5 first' '#5' +check15_expect_tokens 'superseded by #123456' '#123456' +check15_expect_tokens 'superseded by #1000000' '#1000000' +check15_expect_tokens 'x #12 #34' '#12 #34' +check15_expect_tokens '#12 #34' '#12 #34' +check15_expect_tokens 'a #12, #34.' '#12 #34' +check15_expect_tokens "$(printf 'tracked as #7.\r')" '#7' + +# ── CHECK 15 SCANNER CANARY ──────────────────────────────────────────────── +# The detection canary above witnesses the PREDICATE. This layer witnesses the +# FILE-LEVEL TRAVERSAL -- awk record shape, line numbering, and the absence of +# any region skipping -- by running scan_prose_file_refs over committed fixtures +# whose expected records (including their LINE NUMBERS) are pinned right here. +# A reintroduced frontmatter skip, an off-by-one in the record, or a deleted +# fixture reference turns this red; no predicate test can see any of those. +check15_expect_scan() { + local fixture_rel="$1" expected="$2" + local fixture_path got got_flat expected_flat + fixture_path="$REPO_ROOT/$fixture_rel" + if [[ ! -f "$fixture_path" ]]; then + check15_found=true + add_finding 'CHECK15' 'tools/policy_check.sh' 0 \ + "scanner canary: fixture ${fixture_rel} is missing -- the file-level traversal has no witness, so a reintroduced region skip or an off-by-one line number could ship unseen; restore the fixture rather than deleting the assertion" + return 0 + fi + got="$(scan_prose_file_refs "$fixture_path")" + if [[ "$got" != "$expected" ]]; then + got_flat="${got//$'\n'/ | }" + expected_flat="${expected//$'\n'/ | }" + check15_found=true + add_finding 'CHECK15' 'tools/policy_check.sh' 0 \ + "scanner canary: scanning ${fixture_rel} produced records [${got_flat}] but expected [${expected_flat}] -- the file-level traversal changed shape (a skipped region, a renumbered line, or a dropped token); fix the scanner, or update fixture and pinned records together if the fixture moved" + fi +} + +check15_expect_scan 'tests/policy/fixtures/tracker-ref-scan-canary.md' \ + $'3\t#11\n13\t#21\n16\t#31\n20\t#41\n22\t#12 #34 #56' +check15_expect_scan 'tests/policy/fixtures/tracker-ref-unclosed-frontmatter.md' \ + $'10\t#77' + if [[ "$check15_found" == false ]]; then echo "[PASS] Check 15: No tracker references in $check15_file_count plugin runtime prose files" CHECKS_PASSED=$((CHECKS_PASSED + 1)) From aaf56ea64f70d6713de5c242037779743c020429 Mon Sep 17 00:00:00 2001 From: Bren Pike Date: Thu, 24 Sep 2026 10:55:24 -0600 Subject: [PATCH 7/7] test(policy): pin the CHECK 15 scanner canary and describe its witnesses --- tests/policy/safety-tracker-ref-guard.json | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/policy/safety-tracker-ref-guard.json b/tests/policy/safety-tracker-ref-guard.json index ba739519..7ffe2bf0 100644 --- a/tests/policy/safety-tracker-ref-guard.json +++ b/tests/policy/safety-tracker-ref-guard.json @@ -1,10 +1,11 @@ { "rule": "check15-tracker-ref-guard-present", - "description": "P3 consumer-assertion: CHECK 15 (No tracker references in plugin runtime prose) in tools/policy_check.sh is the CI guard that makes P19 (doctrine anchors on durable records, not tracker IDs) real rather than decoration, per P17. This fixture pins the two literals that prove the guard still exists and still reports through the shared finding machinery: the `=== CHECK 15:` section header (the block is present and still announces itself) and `add_finding 'CHECK15'` (it still emits CHECK15 findings, so --strict and the allowlist still apply to it). superset mode fails if either literal is removed; extras elsewhere in the file are ignored. Residual, stated plainly: this pin claims presence only. It cannot prove the regex still detects anything, and it is blind to a weakened pattern -- narrowing CHECK15_TRACKER_PATTERN, or narrowing the discovery globs, leaves both pinned literals intact and this fixture green. That uncovered half is not left open: no fixture in this directory can witness detection semantics (pin-authoring contract rule 7), so it is covered instead by the structural checks inside tools/policy_check.sh itself -- the CHECK 15 detection canary, which asserts one positive and one negative case per documented pattern clause through the pure tracker_ref_token predicate, and the per-arm zero-discovery assertions, which fail closed on each discovery arm separately.", + "description": "P3 consumer-assertion: CHECK 15 (No tracker references in plugin runtime prose) in tools/policy_check.sh is the CI guard that makes P19 (doctrine anchors on durable records, not tracker IDs) real rather than decoration, per P17. This fixture pins three literals that prove the guard still exists, still emits CHECK15 findings through the shared finding machinery, and still carries its scanner canary: the `=== CHECK 15:` section header (the block is present and still announces itself), `add_finding 'CHECK15'` (it still reports through the shared finding machinery, so --strict and the allowlist still apply to it), and the `CHECK 15 SCANNER CANARY` section banner (the scanner-canary block has not been deleted alongside its assertions). superset mode fails if any literal is removed; extras elsewhere in the file are ignored. Structural coverage this pin does not itself carry, but which exists inside tools/policy_check.sh: a predicate canary asserting exact token sets through `tracker_ref_tokens`, a scanner canary asserting exact per-file records -- including line numbers -- through `scan_prose_file_refs` over the committed fixtures tests/policy/fixtures/tracker-ref-scan-canary.md and tests/policy/fixtures/tracker-ref-unclosed-frontmatter.md, and per-arm zero-discovery assertions that fail closed on each of the two discovery arms separately. Residuals, stated plainly: a presence pin can never prove the pattern still detects anything -- that is exactly what the in-check canaries carry, and this fixture only proves the canary section still exists, not that its assertions still run correctly. It is blind to a weakened pattern: narrowing CHECK15_TRACKER_PATTERN, or narrowing the discovery globs, leaves all three pinned literals intact and this fixture green. Allowlist granularity is a separate residual: an entry still covers its whole line, so a reference added later to an already-allowlisted line would be hidden by it; zero CHECK15 allowlist entries exist today, so nothing is absolved in practice.", "set_check": { - "extract_regex": "(=== CHECK 15:|add_finding 'CHECK15')", + "extract_regex": "(=== CHECK 15:|CHECK 15 SCANNER CANARY|add_finding 'CHECK15')", "expected_set": [ "=== CHECK 15:", + "CHECK 15 SCANNER CANARY", "add_finding 'CHECK15'" ], "files": [