feat(fabrika): implement the governance contract's seven verbs (#5199) - #5351
Conversation
No preview deploy
|
|
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 — Read the PR head (§HEAD): every file under review sourced from Acceptance criteria
Sub-gates
The three judgment calls the body flags — graded1. The local 2. The two deletions are ONE disclosed change — verified. 3. The The two surfaced clauses(a) (b) Deviations sectionNot Run-evidence bundle: PRESENT for head 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 Verdict-written: 2026-08-10T18:14:42Z |
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.
27ea0fc to
966a842
Compare
|
review-code: PASS @ 966a842 — merge-ready Re-gate of PR #5351 against #5199 at the repaired head. The prior verdict FAILed at Read the PR head (§HEAD): every file sourced from The blocking row, re-gated[PASS] deviation-disclosure — the defect is fixed, disclosed, pinned, and provably introduces no The fix blanks inline-code spans before the anchor regex runs, in both call sites — 1. The 11, executed. Ran the shipped 2. No real anchor lost anywhere — corpus sweep, before and after. Ran the new masking over the
3. Line-opening would NOT have sufficed — the rejection is right, and the sweep understates it.
So the simpler rule trades a 2-anchor over-count for a 22-anchor under-count, on the very denominator 4. Length-preservation holds, so the offsets are sound. Blanking with a repeated space is 5. The four new tests pin behaviour rather than restate it. One consequence worth surfacing, non-blocking and correct: The four items the repair could have disturbed — all survived
The rebase — nothing from
|
Fixes #5199
Builds
packages/fabrika-cli/src/governance/— all seven verbsclaude-plugins/fabrika/skills/governance/contract.mdderives — plus the one shipped-surface changethe contract still lists as outstanding.
What the contract actually asked for, checked first-party against
mainThe 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
mainrecords it. Re-derivedagainst
origin/mainat1baa7d02:packages/fabrika-cli/src/governance/at all, so 0 of 7 shippedSHIP_NAMESPACESadmitsgovernancerequiredWithFloor(#5036)verdict-markernamespace class admitsgovernancegovernance-digestwire formatSo this PR adds seven verbs and one surface change, and re-derives none of the two that landed.
The verbs
governance scopeselfand the records in the diffgovernance sweepacceptedrecords whose domain a subject touches, ranked — subject from a bound commit (--record) or the corpus (--landed)governance guardsgovernance basegovernance postgovernancenamespace verdictgovernance digestgovernance readoutNothing already solved is re-derived.
GOVERNANCE_ROOTS/touchesGovernanceRootare importedfrom
review/classes.ts— one derivation, read byship scope,ship gate's required-set floor andgovernance scopealike. The ranking core (decisionBearingText,tokenize, the idf scoring,RARITY_FLOOR) is imported fromadr/sweep.ts;governance sweepowns only the subjectacquisition.
bindHeadcomes fromreview/head.ts, the leak predicate fromreport/leaks.ts, andnormalizeForReadbackfromreport/compose.ts.The exit table
packages/fabrika-cli/src/governance/codes.tsimports every shared seat under an alias and restatesno numeral.
GOVERNANCE_SEATSinexit-code-alignment.tsisSHARED_SEATS— every name and everyreading matches the base's.
35678911report/codes.tsEMPTY_STDIN,LEAKED_PATH,BARE_AT_PATH,NO_TARGET→ZERO_SCOPE,WRITE_UNKNOWN,READBACK_MISMATCH,PRECONDITION_UNKNOWN10triage/codes.tsOFF_VOCABULARY— imported fromtriagebecause that is where this group's reading is named1213review/codes.tsSTALE_HEAD,INCOMPLETE_SCAN— this group proves the same two facts4DELIBERATE_GAP— no verb here composes body sections, so the gap is registered rather than silently absent14NOT_HARNESS_TOUCHINGNo seat was re-used and no VACATED seat re-seated.
14is declared, deliberately notimported:
review/codes.tsseats its own14asACL_DENIED, a different group's private band, soan import beside
12and13would take the wrong meaning silently.codes.unit.test.tspins thatand pins the three-way distinction the group turns on —
1/127never decided,11a preconditionread failed with nothing written,
8a write attempted with an UNKNOWN outcome — and every provenverdict at
3+ leaves stdout empty by construction (verb.ts'srefusehardcodes 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 --numstaton every shared file:The two deleted lines are one edit, disclosed rather than hidden:
governancewas appended to theexisting
verdict-markerrow'sproducersarray, which this group now genuinely produces, andthe 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; theREADME.mdchange adds a## The governance groupsection ahead of## The wire groupand reflows nothing.io/issues.tsgained acommentsfield onIssueRecord, read offvalue.commentsrather thanthrough the existing destructure so the addition stays a pure insertion. It is what makes
governance readout's13seat provable — without a declared count there is no denominator, and acompleteness 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 insideprocess.exit(outcome.code)insrc/governance/command.ts's emit adapter. That line isbyte-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 anywherein this diff.
A behaviour the contract specifies that is UNREACHABLE as ordered, proven by execution. The
contract's change-2 residual asks for
governancefixture rows on theverdict-markerregistry rowso
wire/conformance.tsdrives a governance arm. A round-trip governance arm is notrepresentable:
WireFixtures.roundTrip(src/wire/format.ts) is a singleWireRoundTripFixture,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
roundTripkey to the row isTS1117(duplicate property), and replacing the existingreview-coderound-trip would delete the coverage it stands for. This PR therefore lands the halfthat IS representable —
governanceappended toproducers, and a governance malformed fixturewhose namespace is not kebab-case — and leaves the round-trip arm to whoever decides whether
WireFixturesshould 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.
selfunder an ambiguous skill-root resolution.governance basestates all three arms —one root, zero (
7), more than one (11) — butgovernance scope'sselfflag is specified onlyas "true when any changed path is under the resolved skill root", with no arm for zero or several.
Taken conservatively:
selfis true when a path is under any candidate root, so an ambiguousinstall flags the self fence rather than skipping it, and zero candidates leaves
selffalsebecause there is nothing to be under.
roots.unit.test.tspins both.governance scope's13versus 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 readon
13explicitly ("received < declared count"). review scope's exit-13 proof counts local-git paths against GitHub's declared count #5154 established that git and GitHublegitimately disagree on that count — git pairs a rename into one
--name-onlypath where GitHubcounts two — which is why
review scopereports the disagreement and never refuses on it. Thecontract 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
12refusalgovernance scopeemits isbindHead's, which adds "the tree you scoped is not the oneunder review" before the contract's "re-scope at
<live>(ADR 0058)" — a superset of the specifiedtext, taken by importing the binding rather than re-deriving it. The
11binding refusal isre-phrased into each verb's own noun by
governance/head.ts'swithBindingNoun, which is unit-testedin both directions so a change to
bindHead's wording reds a test rather than silently passing theimported 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 guardswould read its ownSKILL.mdprose as a twelfth anchor and wanted one exclusion line at implementation time. Theinitial diff took neither remedy and said nothing here; the gate reproduced it by execution
(
anchorsInreturned 12 over a file carrying 11 anchors). Fixed insrc/governance/anchors.ts:inline-code spans are blanked before the anchor regex runs, in
anchorsIn(theanchors-in-reachdenominator) and in
sightingOf(soscanAnchorscannot report a phantommodifiedwhen thatsentence is reworded). The blanking is length-preserving, so indices into the masked text are
indices into the original and the
modifiedcomparison still slices the real bytes — unchangedbehaviour for every real anchor. Executed proof:
anchorsInoverclaude-plugins/fabrika/skills/governance/SKILL.mdnow 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 backtickedanchor 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 thehandoff-packwireformat (#5350). Three textual conflicts, each "both sides appended", resolved by keeping both
sides:
wire/registry.ts, the generateddocs/wire-formats.md, andfabrika-cli/README.md.fabrika wire index --writere-run afterwards reportsindex written 9 9with zero diff. Thezero-deletion property is intact: the only lines removed against
origin/mainremain the onedisclosed
verdict-markerproducersline and its re-rendered doc row.Checks
pnpm typecheck(workspace, turbo) — 31/31 green. The in-packagetsgo -p tsconfig.jsonwas runfrom
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 runinpackages/fabrika-cli— 247 files, 3589 tests, all green (148 of them new).pnpm lint:worktree— clean.tsgo -p tsconfig.jsonclean;pnpm vitest runinpackages/fabrika-cli— 257 files, 3694 tests, all green;pnpm lint:worktreeclean;fabrika wire index --writezero diff.