Skip to content

feat(fabrika): implement the governance contract's seven verbs (#5199) - #5351

Merged
usirin merged 2 commits into
mainfrom
usirin/governance-verbs-5199-B1A71218
Aug 10, 2026
Merged

feat(fabrika): implement the governance contract's seven verbs (#5199)#5351
usirin merged 2 commits into
mainfrom
usirin/governance-verbs-5199-B1A71218

Conversation

@usirin

@usirin usirin commented Aug 10, 2026

Copy link
Copy Markdown
Member

Fixes #5199

Builds packages/fabrika-cli/src/governance/ — all seven verbs
claude-plugins/fabrika/skills/governance/contract.md derives — plus the one shipped-surface change
the contract still lists as outstanding.

What the contract actually asked for, checked first-party against main

The issue body was written on 2026-08-09 and says "0 of 7 verbs and 0 of 3 surface changes". Two of
the three surface changes landed in the meantime, and the contract on main records it. Re-derived
against origin/main at 1baa7d02:

Piece State before this PR
the seven verbs absent — no packages/fabrika-cli/src/governance/ at all, so 0 of 7 shipped
change 1 — SHIP_NAMESPACES admits governance landed (#5206), plus 1b requiredWithFloor (#5036)
change 2 — the verdict-marker namespace class admits governance landed (#5206); its stated residual is the registry fixtures
change 3 — a registered governance-digest wire format absent — built here

So this PR adds seven verbs and one surface change, and re-derives none of the two that landed.

The verbs

Verb Answers
governance scope whether the diff derives the namespace, over which of the four roots, at the bound head, with self and the records in the diff
governance sweep the uncited live-accepted records whose domain a subject touches, ranked — subject from a bound commit (--record) or the corpus (--landed)
governance guards the anchored invariants the bound diff removes or modifies, plus the guard-bearing files it touches
governance base this skill's own text at a PR's merge base — the self fence's bytes
governance post the single sanctioned emit of the governance namespace verdict
governance digest the decision records that landed in a window, with each landing commit and its anchor delta
governance readout compose the ranked rows, upsert the durable artifact, read them back

Nothing already solved is re-derived. GOVERNANCE_ROOTS / touchesGovernanceRoot are imported
from review/classes.ts — one derivation, read by ship scope, ship gate's required-set floor and
governance scope alike. The ranking core (decisionBearingText, tokenize, the idf scoring,
RARITY_FLOOR) is imported from adr/sweep.ts; governance sweep owns only the subject
acquisition. bindHead comes from review/head.ts, the leak predicate from report/leaks.ts, and
normalizeForReadback from report/compose.ts.

The exit table

packages/fabrika-cli/src/governance/codes.ts imports every shared seat under an alias and restates
no numeral. GOVERNANCE_SEATS in exit-code-alignment.ts is SHARED_SEATS — every name and every
reading matches the base's.

Seat Source Reading
3 5 6 7 8 9 11 report/codes.ts EMPTY_STDIN, LEAKED_PATH, BARE_AT_PATH, NO_TARGETZERO_SCOPE, WRITE_UNKNOWN, READBACK_MISMATCH, PRECONDITION_UNKNOWN
10 triage/codes.ts OFF_VOCABULARY — imported from triage because that is where this group's reading is named
12 13 review/codes.ts STALE_HEAD, INCOMPLETE_SCAN — this group proves the same two facts
4 declared locally DELIBERATE_GAP — no verb here composes body sections, so the gap is registered rather than silently absent
14 declared locally NOT_HARNESS_TOUCHING

No seat was re-used and no VACATED seat re-seated. 14 is declared, deliberately not
imported: review/codes.ts seats its own 14 as ACL_DENIED, a different group's private band, so
an import beside 12 and 13 would take the wrong meaning silently. codes.unit.test.ts pins that
and pins the three-way distinction the group turns on — 1/127 never decided, 11 a precondition
read failed with nothing written, 8 a write attempted with an UNKNOWN outcome — and every proven
verdict at 3+ leaves stdout empty by construction (verb.ts's refuse hardcodes it).

Shared registration files — every edit an insertion

Three other lanes are on these files, so each edit is an append at the end of an existing array,
record or section. git diff --cached --numstat on every shared file:

13	1	claude-plugins/fabrika/docs/wire-formats.md
44	0	packages/fabrika-cli/README.md
12	0	packages/fabrika-cli/src/exit-code-alignment.ts
2	0	packages/fabrika-cli/src/exit-code-alignment.unit.test.ts
172	0	packages/fabrika-cli/src/io/git.ts
9	0	packages/fabrika-cli/src/io/issues.ts
2	0	packages/fabrika-cli/src/registry.ts
63	1	packages/fabrika-cli/src/wire/registry.ts

The two deleted lines are one edit, disclosed rather than hidden: governance was appended to the
existing verdict-marker row's producers array, which this group now genuinely produces, and
the wire-formats table row is that same change re-rendered. Both are in-place appends to a list — no
content removed. Every other shared file is strictly insertion-only. The doc region was regenerated
with fabrika wire index --write, never hand-merged; the README.md change adds a
## The governance group section ahead of ## The wire group and reflows nothing.

io/issues.ts gained a comments field on IssueRecord, read off value.comments rather than
through the existing destructure so the addition stays a pure insertion. It is what makes
governance readout's 13 seat provable — without a declared count there is no denominator, and a
completeness refusal over no denominator proves nothing.

Deviations

The Tier-M scan is reported below; this section states what it found and the three judgment calls
that are not the scan's to make.

Tier-M scan, one hit, a known false positive. The scan's bare xit( alternative matches inside
process.exit(outcome.code) in src/governance/command.ts's emit adapter. That line is
byte-identical to the same line in every sibling group's command.ts (spike, grill, map,
review, ship). No test is skipped, no suppression is added, and no assertion is removed anywhere
in this diff.

A behaviour the contract specifies that is UNREACHABLE as ordered, proven by execution. The
contract's change-2 residual asks for governance fixture rows on the verdict-marker registry row
so wire/conformance.ts drives a governance arm. A round-trip governance arm is not
representable: WireFixtures.roundTrip (src/wire/format.ts) is a single WireRoundTripFixture,
not a list, so a second round-trip fixture cannot be added without changing that type — and changing
it would touch every registered row. Proven by execution rather than asserted: adding a second
roundTrip key to the row is TS1117 (duplicate property), and replacing the existing
review-code round-trip would delete the coverage it stands for. This PR therefore lands the half
that IS representable — governance appended to producers, and a governance malformed fixture
whose namespace is not kebab-case — and leaves the round-trip arm to whoever decides whether
WireFixtures should carry a list. Surfaced on #5199 rather than patched over.

Two places the contract is under-determined; the conservative branch was taken in both, and both
are surfaced on #5199 rather than invented over.

  1. self under an ambiguous skill-root resolution. governance base states all three arms —
    one root, zero (7), more than one (11) — but governance scope's self flag is specified only
    as "true when any changed path is under the resolved skill root", with no arm for zero or several.
    Taken conservatively: self is true when a path is under any candidate root, so an ambiguous
    install flags the self fence rather than skipping it, and zero candidates leaves self false
    because there is nothing to be under. roots.unit.test.ts pins both.
  2. governance scope's 13 versus the review scope's exit-13 proof counts local-git paths against GitHub's declared count #5154 finding. The contract seats a short changed-file read
    on 13 explicitly ("received < declared count"). review scope's exit-13 proof counts local-git paths against GitHub's declared count #5154 established that git and GitHub
    legitimately disagree on that count — git pairs a rename into one --name-only path where GitHub
    counts two — which is why review scope reports the disagreement and never refuses on it. The
    contract wins here and the refusal is implemented as written, because it is the fail-closed
    direction (a rename-only PR refuses rather than deriving from a list it cannot prove complete);
    the tension is noted at the check site and on The governance skill specifies seven verbs and three shipped-surface changes, none of which exist #5199.

Two wording supersets, stated so a reviewer diffing against the contract is not surprised. The
12 refusal governance scope emits is bindHead's, which adds "the tree you scoped is not the one
under review" before the contract's "re-scope at <live> (ADR 0058)" — a superset of the specified
text, taken by importing the binding rather than re-deriving it. The 11 binding refusal is
re-phrased into each verb's own noun by governance/head.ts's withBindingNoun, which is unit-tested
in both directions so a change to bindHead's wording reds a test rather than silently passing the
imported noun through.

(repair round 1) The known defect this diff shipped undisclosed — now fixed, disclosed, and
pinned.
#5199 carried a warning from the #5206 lane that governance guards would read its own
SKILL.md prose as a twelfth anchor and wanted one exclusion line at implementation time. The
initial diff took neither remedy and said nothing here; the gate reproduced it by execution
(anchorsIn returned 12 over a file carrying 11 anchors). Fixed in src/governance/anchors.ts:
inline-code spans are blanked before the anchor regex runs, in anchorsIn (the anchors-in-reach
denominator) and in sightingOf (so scanAnchors cannot report a phantom modified when that
sentence is reworded). The blanking is length-preserving, so indices into the masked text are
indices into the original and the modified comparison still slices the real bytes — unchanged
behaviour for every real anchor. Executed proof: anchorsIn over
claude-plugins/fabrika/skills/governance/SKILL.md now returns 11 — lines 14, 27, 47, 77, 90,
97, 116, 149, 166, 214, 242 — with line 117 (the sentence documenting the tag) excluded. Why this
shape and not the obvious one:
a "an anchor must open its line" rule would under-count, because
real anchors sit after a list bullet (- <!-- anchor: H1 -->) and after a heading
(## Open questions <!-- anchor: OPEN-QUESTIONS -->); a corpus sweep found the only backticked
anchor tags are the two that document the pattern, and no real anchor inside backticks. Pinned by
four new unit tests, including one over the real skill file.

(repair round 1) Rebased onto latest origin/main, which had landed the handoff-pack wire
format (#5350).
Three textual conflicts, each "both sides appended", resolved by keeping both
sides: wire/registry.ts, the generated docs/wire-formats.md, and fabrika-cli/README.md.
fabrika wire index --write re-run afterwards reports index written 9 9 with zero diff. The
zero-deletion property is intact: the only lines removed against origin/main remain the one
disclosed verdict-marker producers line and its re-rendered doc row.

Checks

  • pnpm typecheck (workspace, turbo) — 31/31 green. The in-package tsgo -p tsconfig.json was run
    from packages/fabrika-cli, never from the repo root (Root tsconfig.json has no noEmit, so tsgo -p tsconfig.json writes output into the tree #5312).
  • pnpm vitest run in packages/fabrika-cli — 247 files, 3589 tests, all green (148 of them new).
  • pnpm lint:worktree — clean.
  • (repair round 1, re-run at the rebased head) in-package tsgo -p tsconfig.json clean;
    pnpm vitest run in packages/fabrika-cli — 257 files, 3694 tests, all green;
    pnpm lint:worktree clean; fabrika wire index --write zero diff.

@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

No preview deploy

  • No preview deploy for this PR — its diff touches no deploy-relevant path, so no preview stack was minted and e2e is not applicable. (966a842)
  • web — Stage pr-5351 torn down.

@usirin

usirin commented Aug 10, 2026

Copy link
Copy Markdown
Member Author

review-code: FAIL @ 27ea0fc — not merge-ready

Verified PR #5351 against the acceptance criteria of #5199, one at a time. All nine acceptance criteria PASS. One sub-gate row fails — deviation-disclosure, on an undisclosed known defect I reproduced by execution. The remedy is one exclusion plus its test, not a redesign.

Read the PR head (§HEAD): every file under review sourced from 27ea0fce via a per-run ref and a throwaway worktree, never the launched checkout’s working copy. §CP: not control plane on both axescp-classify classify against the live CONTROL_PLANE_RE returned not-control-plane [path-clear-no-content-source] over 38 files, and the ADR-0164 content axis is vacuous (0 .decisions/** files in the diff, so guard-content-probe has nothing to classify — recorded as vacuous, not as a second confirmation). So this verdict takes the SHA-bound form, not the advisory one.

Acceptance criteria

  • [PASS] all seven verbs in packages/fabrika-cli/src/governance/command.ts:274 Command.withSubcommands([scope, sweep, guards, base, post, digest, readout]), each declared with leafCommand at :64/:84/:133/:153/:174/:210/:245.
  • [PASS] each verb reuses the imported core rather than re-deriving it — sweep-verb.ts:18 imports {renderEntry, SweepCandidate, sweep} from ../adr/sweep.ts (no second lexical sweep anywhere in the group); roots.ts:16 / scope-verb.ts:17 / post-verb.ts:28 import GOVERNANCE_ROOTS + touchesGovernanceRoot from ../review/classes.ts; authored.ts:16 imports the leak predicate from ../report/leaks.ts; post-verb.ts:27 / readout-verb.ts:19 import normalizeForReadback from ../report/compose.ts.
  • [PASS] every read binds the commit first — governance/head.ts:18 imports bindHead from ../review/head.ts and wraps it as bindGovernanceHead; called before any artifact read in scope-verb.ts:70, sweep-verb.ts:148, guards-verb.ts:58, base-verb.ts:64, post-verb.ts:177 — the four artifact verbs the contract’s "commit binding runs before every read" section names, plus post.
  • [PASS] surface change 1 (SHIP_NAMESPACES admits governance) — already shipped; correctly not re-derived. Verified first-party against origin/main: review/classes.ts:161-164 already reads [...SHIP_CLASS_NAMES.map(...), "governance"], landed by ef3e090e (feat(fabrika-cli): let ship gate require the governance namespace #5206). This PR does not touch that file.
  • [PASS] surface change 2 (the verdict-marker namespace regex and NAMESPACE_PREFIXES) — already shipped; correctly not re-derived. wire/verdict-marker.ts:73 on origin/main is /^(review|check-epic-plan|governance)(-[a-z0-9]+)*$/ and :78 is ["review", "check-epic-plan", "governance"] — both widened together, same commit ef3e090e. This PR does not touch that file. Its stated residual (governance fixtures on the registry row) is addressed in its representable half — see judgment call 3 below.
  • [PASS] surface change 3 (a registered governance-digest wire format) — new schema module wire/governance-digest.ts plus the registry row at wire/registry.ts:287+ (key, purpose, module, producers, consumers, emit/read, round-trip + absent + five malformed fixtures, brand witnesses). The generated index is genuinely regenerated: I re-ran fabrika wire index --write inside the review worktree at this head and it reported index written 8 8 with zero resulting diff on claude-plugins/fabrika/docs/wire-formats.md, so the drift check stays green.
  • [PASS] governance/codes.ts imports every shared seat rather than restating a numeral — the file carries exactly one private numeric literal (NOT_HARNESS_TOUCHING = 14, graded below) plus the registered DELIBERATE_GAP = 4. OFF_VOCABULARY comes from ../triage/codes.ts, STALE_HEAD / INCOMPLETE_SCAN from ../review/codes.ts, the seven writing seats from ../report/codes.ts, each under an alias. All twelve export names the contract demands are present, so checkAlignment’s name-keyed check has every seat it looks for.
  • [PASS] registered in both files — exit-code-alignment.ts:197 GOVERNANCE_SEATS = SHARED_SEATS and :204 governance: GOVERNANCE_SEATS; exit-code-alignment.unit.test.ts:18 imports the module and :48 adds governance to the TABLES record. Also registered as a group in registry.ts:22/48.
  • [PASS] unit + CLI tests in the sibling shape — 13 *.unit.test.ts beside their modules plus governance.cli.test.ts, the shape review and ship ship. Ran in the review worktree at this head: the governance surface alone is 14 files / 148 tests, all green (matching the body’s claim exactly), and the whole package is 247 files / 3589 tests green. In-package typecheck (tsgo -p tsconfig.json, run from packages/fabrika-cli, never the repo root per Root tsconfig.json has no noEmit, so tsgo -p tsconfig.json writes output into the tree #5312) is clean.

Sub-gates

  • [FAIL] deviation-disclosure — one known defect, recorded on the linked issue for implementation time, left unfixed and undisclosed (§DEV class 3/4). The governance skill specifies seven verbs and three shipped-surface changes, none of which exist #5199’s comment of 2026-08-09T22:45:04Z records, verbatim: "governance guards will read its own SKILL.md prose as a twelfth anchor … Wants one exclusion line at implementation time (exclude the skill own documentation of the pattern, or make the quoted example non-matching)." The diff takes neither branch and the ## Deviations section does not mention it. Reproduced by execution at this head, not asserted: running anchorsIn (governance/anchors.ts:41) over claude-plugins/fabrika/skills/governance/SKILL.md returns 12 while the file carries 11 real line-start anchors — the twelfth is the prose at SKILL.md:117 documenting the pattern itself, matched by ANCHOR because the scan is textual over raw bytes. Two consequences, both landing on the self-editing case the fence exists for: inReach (guards-verb.ts:89-91) — the denominator whose whole job is to make "scanned nothing, found nothing" unrenderable as a pass — is inflated by one, and an edit to that documentation line makes scanAnchors report a phantom modified anchor. Remedy: take one of the two branches the note names, add the test, disclose it. §DEV’s rule is what makes this the row it is — undisclosed-and-detected is a [FAIL]. The section is otherwise honest, so there is no falsified None. aggravation here.
  • [PASS] comment-discipline — 4047 added lines scanned on comment-bearing files. The added comments are KEEP-category by the deslop-comments rubric: local invariants at their enforcement site (guards-verb.ts:89-91 the ADR-0092 denominator; base-verb.ts:139-140 proven-absence vs unreadable), a workaround with its forcing constraint (roots.ts:58-60 the lettered-variant id), incident pointers rather than re-derivations (digest-verb.ts:80-82 to Agents execute skills from the working tree, so a stale checkout silently applies withdrawn doctrine #4338/A stale working tree fails toward "does not exist" — recurrence of #308 across 4 seats #4163; post-verb.ts:219-221 to review post's upsert edits the OLDEST duplicate in a namespace #5048), and module docblocks that state the one non-obvious thing (anchors.ts on why the inventory lives in the guarded file). No narration, name-restatement or ADR-why re-derivation found.
  • glossary-freshness: not applicable — no new feature folder / public package / export (detector expressible here: 50 candidate surfaces on base). Row omitted.
  • containment marker on The governance skill specifies seven verbs and three shipped-surface changes, none of which exist #5199 reads none; session-caching gate: not applicable; unresolved-threads: not applicable — no review threads on this PR. Rows omitted.

The three judgment calls the body flags — graded

1. The local 14 NOT_HARNESS_TOUCHING declaration is CORRECT, and it is not the restated-numeral defect. This is not an exception the author argued for — it is what the contract orders. contract.md:179-186: "review/codes.ts seats its own 14 as ACL_DENIED; that is a different group’s private band and carries no cross-group uniqueness obligation (interface convention rule 3), so declare 14 locally and do not import it — an implementer who imports 14 from review alongside 12 and 13 gets the wrong meaning silently." The rule this repo enforces is import the shared meaning, never restate a shared numeral; 14 is not a shared meaning — it is two private bands colliding on one number, so importing it would have been the defect. codes.unit.test.ts pins exactly that, asserting NOT_HARNESS_TOUCHING === 14 and review.ACL_DENIED === 14 side by side so the collision is documented rather than discovered. Three-way split confirmed: 1/127 (never decided), 11 PRECONDITION_UNKNOWN (a precondition read failed, nothing written) and 8 WRITE_UNKNOWN (a write attempted, outcome UNKNOWN) are asserted pairwise-distinct; and every verdict at 3+ leaves stdout empty by constructionverb.ts:51-55 refuse hardcodes an empty stdout, and a grep confirms no governance module builds a non-zero outcome by hand.

2. The two deletions are ONE disclosed change — verified. wire/registry.ts -1/+63: the single removed line is the existing verdict-marker row’s producers: ["review", "check-epic-plan"], replaced in place by ["review", "check-epic-plan", "governance"]. wire-formats.md -1/+13: that same row re-rendered inside the generated block. Nothing removed on either. Confirmed by re-running fabrika wire index --write at this head — zero diff, so the doc row is exactly what the registry renders and was not hand-merged. Every other shared file (README.md, exit-code-alignment.ts + its test, io/git.ts, io/issues.ts, registry.ts) is strictly zero-deletion, which keeps the in-flight sibling lane on those files collision-free.

3. The TS1117 claim reproduces, and landing the partial arm was RIGHT. wire/format.ts:91-92 declares readonly roundTrip: WireRoundTripFixture — a single object, not a list — so a second roundTrip key on the verdict-marker row is a duplicate object-literal property. Reproduced with the package’s own tsgo: error TS1117: An object literal cannot have multiple properties with the same name. A governance round-trip arm is therefore unrepresentable without changing WireFixtures, which would touch every registered row; and replacing the existing review-code round-trip would delete the coverage it stands for. Landing the representable half — governance on producers plus a governance malformed fixture (registry.ts:92-95, "the governance namespace is not kebab-case") — and surfacing the rest is the right call: it is strictly additive, it loses no existing coverage, and the blocked half is a decision about a shared type belonging to whoever owns WireFixtures, not to this lane. This did not warrant blocking, and the disclosure is where it belongs.

The two surfaced clauses

(a) self under an ambiguous skill-root resolution — the gap is real, the branch taken is defensible, the resolution recorded UNKNOWN. Confirmed first-party: contract.md:365 gives scope’s self one clause only — "true when any changed path is under the resolved skill root" — while governance base (contract.md:716-719) states all three arms (one root; zero → 7; more than one → 11), and governance scope’s own exit table (contract.md:376-382) carries no arm for either. So the contract genuinely does not say what self is under a zero- or many-root install. roots.ts:71-73,87 takes "true under any candidate, false with zero", which flags the self fence under ambiguity rather than skipping it — the conservative direction for a fence whose purpose is to stop a self-editing PR being judged by its own new rules. The competing reading (mirror base and refuse on 11 for many) is also defensible; I am not ruling between them. UNKNOWN — unruled. It is pinned in roots.unit.test.ts and surfaced on #5199, which is the right disposition. No FAIL.

(b) governance scope’s 13 versus #5154 — implemented as the contract states; a real tension, recorded UNKNOWN, not blocking here. The contract is unambiguous (contract.md:382: "the changed-file enumeration is provably short (received < declared count)", with the verbatim stderr message the implementation emits), and scope-verb.ts:84-96 implements it with the tension noted at the check site. The tension is real and I verified both sides: #5154 is CLOSED and repaired, and review/scope-verb.ts:69-88 at this head now reports the git-vs-GitHub disagreement and refuses only on an empty local list, in its own words "reported, never refused on (#5154)". So this PR reintroduces, in a new verb, the exact strict inequality a sibling verb just removed — the known consequence being a false 13 on a rename-only governance-root PR, whose reviewing session then cannot derive the namespace it needs in order to post a verdict for it. UNKNOWN — genuinely unruled: the contract is this issue’s spec and says one thing, #5154’s repair says the other, and nothing has ruled which wins (the contract’s own tie-break clause names the CLI-interface doc, not a sibling issue). Implementing the spec and disclosing it is correct behaviour for this lane, so this is not a FAIL. Recommendation, non-binding: file the reconciliation as its own issue rather than leaving it only in a progress comment, so it does not die there.

Deviations section

Not None., and its Tier-M claim checks out. The canonical Tier-M scan at this head reports ## Deviations section present and 1 suppression/skip line(s), 0 removed-assertion line(s); the one hit is the added process.exit(outcome.code) in governance/command.ts:36, matched by the scan’s bare xit( alternative. Confirmed a false positive: the identical line appears in 19 sibling groups’ command.ts (adr, build, epic, grill, ledger, hook, plan, review-ui, map, ship, spike, report, triage, review, spend, wire, status, ui). No test is skipped, no suppression added, no assertion removed anywhere in the diff.

Run-evidence bundle: PRESENT for head 27ea0fce — producer run 31416082475, artifact 9073724320, manifest.commit == head, schemaVersion 1; checks 2/2 pass; tests 2424/2424 passed, 0 failed, 0 skipped.

The failing row above must be addressed before this PR can merge. The PR stays open and unmerged; #5199 stays open and assigned. Re-request review once it is satisfied.

On the scope re-derivation, since it was the thing most at risk on this ticket: the coder was right and both prior framings were wrong. Checked first-party against origin/main, not taken on anyone’s word — packages/fabrika-cli/src/governance/ did not exist at all (so all seven verbs were genuinely absent, and any "three verbs already shipped" framing is false), while surface changes 1 and 2 had already landed via ef3e090e (#5206, with 1b requiredWithFloor via #5036) — which the issue body’s "0 of 3 surface changes" missed, and which contract.md:230-236 records as landed. This PR rebuilt nothing that had already shipped and skipped nothing that had not.

Verdict-written: 2026-08-10T18:14:42Z

usirin added 2 commits August 10, 2026 11:20
Build `packages/fabrika-cli/src/governance/` — `scope`, `sweep`, `guards`,
`base`, `post`, `digest`, `readout` — plus the `governance-digest` wire format
the contract's third shipped-surface change specifies.

Nothing already solved is re-derived: the four-root list and its predicate come
from `review/classes.ts` (one derivation, read by `ship scope`, `ship gate`'s
required-set floor and `governance scope`), the ranking core from
`adr/sweep.ts`, the commit binding from `review/head.ts`, the leak predicate
from `report/leaks.ts` and the read-back normalizer from `report/compose.ts`.

`governance/codes.ts` imports every shared seat rather than restating a numeral
and declares only `14 NOT_HARNESS_TOUCHING` locally — `review`'s `14` is
`ACL_DENIED`, a different group's private band.
#5199)

The anchor scan is textual over raw bytes, so the governance skill's own sentence
describing the tag counted as a twelfth anchor of eleven — inflating the
anchors-in-reach denominator and, on a reword, reporting a phantom modified anchor.
Blank inline-code spans before matching, length-preserving so callers still slice
the real bytes. Position is not the discriminator: real anchors sit after a bullet
and after a heading too.
@usirin
usirin force-pushed the usirin/governance-verbs-5199-B1A71218 branch from 27ea0fc to 966a842 Compare August 10, 2026 18:25
@usirin

usirin commented Aug 10, 2026

Copy link
Copy Markdown
Member Author

review-code: PASS @ 966a842 — merge-ready

Re-gate of PR #5351 against #5199 at the repaired head. The prior verdict FAILed at 27ea0fce on a
single blocking row — deviation-disclosure, the undisclosed twelfth-anchor defect. That row is now
discharged, verified by execution over the live corpus, and it introduces no under-count.
The nine
acceptance criteria were graded PASS at 27ea0fce and are not re-litigated here; the four
previously-graded items the repair could have disturbed were re-verified and all four survived
unchanged.

Read the PR head (§HEAD): every file sourced from 966a8424 via a per-run ref and a throwaway
worktree, never the launched checkout. §CP, both axes: cp-classify classify against the live
CONTROL_PLANE_RE over 38 files returned not-control-plane [path-clear-no-content-source]; the
ADR-0164 content axis is vacuous (0 .decisions/** files in the diff, so guard-content-probe
has nothing to classify — recorded as vacuous, not as a second confirmation). Required-namespace
set from pipeline-cli class-probe classify --namespaces: review-code, one namespace (the one
.md in the diff sits under claude-plugins/, which the §CLASS docs carve-out excludes, and no path
is under a skills or agents root or apps/web/src/). So this verdict takes the SHA-bound form.

The blocking row, re-gated

[PASS] deviation-disclosure — the defect is fixed, disclosed, pinned, and provably introduces no
under-count.

The fix blanks inline-code spans before the anchor regex runs, in both call sites — anchorsIn
(the anchors-in-reach denominator) and sightingOf (which feeds scanAnchors, closing the
phantom-modified half). ANCHOR appears exactly twice in governance/anchors.ts and both are
masked, so there is no unmasked path left.

1. The 11, executed. Ran the shipped anchorsIn (not a re-implementation) over
claude-plugins/fabrika/skills/governance/SKILL.md at this head: 11. The excluded twelfth is
SKILL.md:117, the sentence documenting the tag. The eleven that remain are exactly the lines the
body claims — 14, 27, 47, 77, 90, 97, 116, 149, 166, 214, 242 — checked line by line.

2. No real anchor lost anywhere — corpus sweep, before and after. Ran the new masking over the
whole tree at this head, counting per file with and without it.

  • Governance-root corpus (the four roots GOVERNANCE_ROOTS declares — .decisions/, .claude/,
    .github/, claude-plugins/): 874 files, 22 carrying an anchor. 138 anchors before the mask,
    136 after.
    The delta is exactly 2, and both are the pattern's own documentation:
    governance/SKILL.md:117 and governance/contract.md:595. The coder's claim is confirmed, not
    merely plausible
    — in the corpus the verb actually scans, those two are the only backticked
    anchor tags, and no genuine anchor lives inside backticks.
  • Whole repo (3,693 tracked files, for a wider net): 191 down to 183. The extra 6 are all outside
    the governance roots and all documentation-of-the-pattern too: packages/fabrika-cli/README.md:550,
    governance/anchors.ts:4 (the module docblock), three lines in anchors.unit.test.ts (the new
    tests' own fixtures, deliberate) and one template-literal test fixture in
    digest-verb.unit.test.ts:34 — a TS source line, not a scanned artifact, and at runtime the string
    it builds carries no backticks and still counts.
  • No file's count went up. Worth stating because the mask can in principle manufacture an
    anchor (a backticked span sitting between the comment opener and the anchor: keyword does not
    match raw but does match masked). Zero instances in the corpus; noting it as an observation, not a
    defect.

3. Line-opening would NOT have sufficed — the rejection is right, and the sweep understates it.
Ran a leading-whitespace-then-tag rule over the same governance-root corpus against the shipped rule:
mask counts 136, line-open counts 114. A line-opening rule would silently drop 22 of 136
real anchors — 16% of the corpus.
Three distinct shapes, not one edge case:

  • after a list bullet — build-epic/SKILL.md:137,141,146,150,160,163,167 and
    check-epic-plan/NOTES.md:14,27,35,45,57,62,66, the two files the coder named;
  • after a heading — build-epic/SKILL.md:132 (the "Hypotheses under eval test" heading with the tag
    trailing it) and :155;
  • a third shape the coder did not name — end-of-sentence, front-door/SKILL.md:176, where the tag
    closes the sentence rather than opening the line, plus the bullet form at :114.

So the simpler rule trades a 2-anchor over-count for a 22-anchor under-count, on the very denominator
whose job is to make "scanned nothing, found nothing" unrenderable as a pass. The more complex rule
is not over-engineering; it is the only one of the two that is correct.
Ruling: line-opening would
have been a strictly worse defect.

4. Length-preservation holds, so the offsets are sound. Blanking with a repeated space is
length-preserving by construction, and checked empirically over 587,087 lines across 3,693 files:
zero mismatches
, per line and per whole file. That is what makes sightingOf correct: the regex
runs on the masked text, but the slice that produces rest runs on the original — same index,
same byte, so rest and the removed/added to modified comparison are semantically unchanged.
Probed it directly: a real anchor preceded by an inline-code span on the same line still reports
modified on a reword and removed on a deletion, and anchorsIn still counts it as 1. ANCHOR
carries no g flag, and a global regex's lastIndex is reset by the replace call, so neither regex
is stateful across calls.

5. The four new tests pin behaviour rather than restate it. scanAnchors returns an empty list
for a minus/plus pair that only rewords the backticked description (the phantom-modified half,
which a count-only test would miss); anchorsIn on a backticked tag is 0; anchorsIn on a
bullet-anchor plus a heading-anchor is 2 — this is the one that would red if someone "simplified"
to the line-opening rule, so the rejected alternative is now pinned, not just argued; and the fourth
reads the real governance/SKILL.md off disk and asserts 11, which makes the fix corpus-bound
rather than fixture-bound. Executed: the governance + wire + exit-code suites are 26 files / 452 tests
green; the whole package is 257 files / 3,694 tests green, matching the body's claim exactly.

One consequence worth surfacing, non-blocking and correct: governance/contract.md now reports 0
anchors where it reported 1, so isGuardBearing no longer classifies it guard-bearing. That is the
right answer — its single hit was the same documentation sentence, and the file carries no anchored
invariant. SKILL.md stays guard-bearing at 11.

The four items the repair could have disturbed — all survived

  • [PASS] scope re-derivation. Unchanged. Surface changes 1 and 2 are still on main and still
    correctly not re-derived: review/classes.ts:163 carries "governance" in SHIP_NAMESPACES and
    wire/verdict-marker.ts:73/:78 carry it in both the regex and NAMESPACE_PREFIXES. Neither file is
    in this PR's 38.
  • [PASS] the locally-declared 14 NOT_HARNESS_TOUCHING. governance/codes.ts:82, still
    contract-ordered at contract.md:179-186 ("declare 14 locally and do not import it"), still
    pinned beside review.ACL_DENIED === 14 at codes.unit.test.ts:34-36 so the two private bands
    colliding on one number is documented rather than discovered.
  • [PASS] the partial conformance arm. wire/registry.ts still lands the representable half:
    producers: ["review", "check-epic-plan", "governance"] plus the governance malformed fixture
    ("the governance namespace is not kebab-case"). The blocked round-trip half is still surfaced, not
    patched over.
  • [PASS] the deletion budget — exact, at the new merge-base. The merge-base is 33680e0e, which
    is origin/main itself, so the branch sits directly on the head that landed handoff-pack. Numstat
    across the whole PR: two deleted lines in total, and they are the two disclosed ones —
    wire/registry.ts's producers: ["review", "check-epic-plan"] (replaced in place) and its
    re-rendered row in docs/wire-formats.md. Every other shared file is strictly zero-deletion,
    including packages/fabrika-cli/README.md (+44/-0), src/registry.ts (+2/-0), io/git.ts (+172/-0)
    and io/issues.ts (+9/-0).

The rebase — nothing from handoff-pack dropped

All three conflicted files keep both sides. wire/registry.ts carries the handoff-pack row and
the new governance-digest row; docs/wire-formats.md renders both; src/registry.ts registers both
handoffCommand and governanceCommand; and the README's whole "The handoff group" section
(lines 478-491, all four verbs) is intact under a zero-deletion diff. Re-ran fabrika wire index --write myself inside the review worktree at this head: index written 9 9, and the worktree came
back clean afterwards
— zero diff, so the generated doc is exactly what the registry renders and the
drift check stays green.

Sub-gates

  • [PASS] deviation-disclosure — see above. ## Deviations is present and appended, not
    rewritten
    : two new (repair round 1) entries (the anchor fix with its executed proof, and the
    rebase), with every prior entry intact. The canonical Tier-M scan at this head reports the section
    present and 1 suppression/skip line(s), 0 removed-assertion line(s) — the same single false
    positive
    , the process.exit(outcome.code) line in governance/command.ts, matched by the scan's
    bare xit( alternative and byte-identical to the same line in 19 sibling groups' command.ts.
    Re-run and confirmed: no test skipped, no suppression added, no assertion removed.
  • [PASS] comment-discipline — the repair adds one docblock on maskInlineCode. KEEP-category by
    the deslop-comments rubric: it states the local invariant at its enforcement site (why
    length-preservation is load-bearing for the callers' offsets) and why the simpler rule was rejected,
    which is exactly the judgement a future reader would otherwise "simplify" away. Not a re-derivation
    of an ADR's why.
  • glossary-freshness: not applicable; session-caching gate: not applicable;
    unresolved-threads: not applicable — no review threads on this PR. Rows omitted.

Checks, executed at this head

  • In-package typecheck tsgo -p tsconfig.json, run from packages/fabrika-cli and never from the
    repo root (Root tsconfig.json has no noEmit, so tsgo -p tsconfig.json writes output into the tree #5312) — clean.
  • pnpm vitest run in packages/fabrika-cli257 files / 3,694 tests, all green.
  • biome check over src/governance — 28 files, clean.
  • fabrika wire index --write9 9, zero resulting diff.
  • CI at this head: 41 checks pass, 4 skipping, 0 failing — including produce run-evidence bundle,
    scan changed files for leaks and packages unit tests.

The two clauses previously recorded UNKNOWN — still UNKNOWN

Both were deliberately left untouched by the repair, and I am not ruling on either. Recorded again,
unchanged:

  1. self under an ambiguous skill-root resolution. The contract gives scope's self one clause
    and no arm for a zero- or many-root install, while governance base states all three. The branch
    taken (roots.ts:71-73,87 — true under any candidate, false under zero) is the conservative
    direction for a fence meant to stop a self-editing PR being judged by its own new rules, and the
    competing reading is also defensible. UNKNOWN — unruled. Pinned in roots.unit.test.ts and
    surfaced on The governance skill specifies seven verbs and three shipped-surface changes, none of which exist #5199. Not a FAIL.
  2. governance scope's 13 versus review scope's exit-13 proof counts local-git paths against GitHub's declared count #5154. The contract seats a short changed-file read on 13
    explicitly; review scope's exit-13 proof counts local-git paths against GitHub's declared count #5154's repair made review scope report the git-versus-GitHub disagreement and
    refuse only on an empty local list. This PR implements the contract as written and notes the tension
    at the check site and on The governance skill specifies seven verbs and three shipped-surface changes, none of which exist #5199. Nothing has ruled which wins. UNKNOWN — genuinely unruled.
    Implementing the spec and disclosing it is correct for this lane, so not a FAIL. Non-binding
    recommendation stands: file the reconciliation as its own issue so it does not die in a progress
    comment.

Coverage self-check: the one required namespace, review-code, carries a SHA-bound marker at
966a8424b0354ccdc7f7af54f6f7214798ce1ef7. Coverage complete.

The merge is not mine — ship-it is the consumer of this PASS.

Verdict-written: re-gate at 966a8424, superseding the 27ea0fce FAIL, which stands as the record of
that head.

@usirin
usirin added this pull request to the merge queue Aug 10, 2026
Merged via the queue into main with commit 5d6ee12 Aug 10, 2026
46 checks passed
@usirin
usirin deleted the usirin/governance-verbs-5199-B1A71218 branch August 10, 2026 18:51
@github-actions github-actions Bot mentioned this pull request Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The governance skill specifies seven verbs and three shipped-surface changes, none of which exist

1 participant