Skip to content

fix(fabrika): give the adr and spend verb groups one exit table each - #5309

Merged
usirin merged 4 commits into
mainfrom
usirin/adr-spend-group-codes-5294-D44EBC62
Aug 10, 2026
Merged

fix(fabrika): give the adr and spend verb groups one exit table each#5309
usirin merged 4 commits into
mainfrom
usirin/adr-spend-group-codes-5294-D44EBC62

Conversation

@usirin

@usirin usirin commented Aug 10, 2026

Copy link
Copy Markdown
Member

The adr verbs each invented their own exit numbers, and they disagreed: NO_SUBJECT meant exit 3 from adr supersede and exit 4 from adr sweep, while exit 3 meant four different things depending on which verb you ran. spend had the same per-verb habit without the collision. Both groups now read their codes from one table per group, so a caller can look at $? and know what happened without first asking which verb produced it.

Fixes #5294

What changed

  • New packages/fabrika-cli/src/adr/codes.ts and packages/fabrika-cli/src/spend/codes.ts. Every verb in each group imports from its table; no verb file declares a numeric exit constant any more.
  • Both groups join ALIGNED_GROUPS in packages/fabrika-cli/src/exit-code-alignment.ts, each sharing two seats with the report base — NO_TARGET (7) for "the thing you named is not there" and PRECONDITION_UNKNOWN (11) for "the read that would have proven it failed". UNTABLED_GROUPS is now empty; the registry stays so a future untabled group is recorded rather than silent.
  • adr new's BAD_ARGUMENT moved to exit 1 and the constant is gone (see below).
  • Docs and help strings re-stated: the adr contract's two tables per verb — the five Exit status tables and the five Errors tables (fifteen rows, re-seated in repair round 1) — plus a new group table, both command.ts help strings, the index.ts docblock, and four other surfaces that cited the old numbers.

The final tables

adrpackages/fabrika-cli/src/adr/codes.ts

Code Name Meaning
7 NO_SUBJECT the record the caller named is not there (shared with report's NO_TARGET)
11 DIR_UNREADABLE the record directory could not be read, so the outcome is UNKNOWN (shared with report's PRECONDITION_UNKNOWN)
12 ALREADY_EXISTS the target path already exists — refused, never overwritten
13 NO_BY --by has no record under --dir
14 NO_STATUS_LINE no single rewritable status: line
15 MULTI_LINE_DIFF the rewrite would have touched another line — nothing written
16 ALREADY_SUPERSEDED already superseded by …
17 BASE_UNFETCHABLE --base could not be fetched
18 IN_FLIGHT_UNKNOWN the open pull requests could not be enumerated

spendpackages/fabrika-cli/src/spend/codes.ts

Code Name Meaning
7 INPUT_ABSENT no transcript / no ledger at that path — a proven absence
11 INPUT_UNREADABLE the input could not be read, or its absence could not be established — UNKNOWN
12 NOTHING_MEASURED read in full, nothing billed / no rows
13 WINDOW_SELECTED_NO_ROWS rows exist, this window selects none

A proven result and an UNKNOWN never share a code in either table: INPUT_ABSENT (7) and NOTHING_MEASURED (12) are proven, INPUT_UNREADABLE (11) is the read that could not execute.

5 stayed vacated

Neither table seats 5, and the reason is stated once in adr/codes.ts rather than re-derived per verb: it meant "the record directory was read and is empty — refusing" until #5254 and #5297 made that state an answer, so a new meaning there would hand a caller pinned to the old reading a wrong answer under a familiar number. Because the adr group's own band starts at 12, the seat is structurally out of reach, and adr/codes.unit.test.ts asserts it stays empty.

The adr new BAD_ARGUMENT question, settled

BAD_ARGUMENT was a usage error sitting on a proven code, and it exits 1 now. The deciding evidence is inside the group: adr resolve already refuses a non-four-digit id with FAILED (exit 1). Seating the same fact — a malformed <id> — on 4 under adr new and on 1 under adr resolve is the one-fact-two-numbers defect this issue is about, at the reserved end of the table. packages/fabrika-cli/src/verb.ts reserves 1 for exactly this, and 3+ for outcomes a verb PROVED; a bad flag value is not an outcome.

Who consumes these numbers — checked before re-seating

Searched origin/main for anything that branches on a value from fabrika adr or fabrika spend:

  • No shell script, no .github workflow, no pipeline-cli tool invokes either group.
  • claude-plugins/fabrika/skills/adr/SKILL.md states only "a non-zero exit is UNKNOWN" — value-agnostic. Its one numeric mention (exit 6) was prose and is updated.
  • The in-package tests are the real consumers and import by name, so they carry automatically. The two that pinned literals are updated deliberately: adr/relate-verb.unit.test.ts's [3, 4, 5, 6, 7] tuple and the >= 3 / distinctness assertions in spend/read-verb.unit.test.ts and spend/rollup-verb.unit.test.ts.
  • Prose consumers updated: claude-plugins/fabrika/skills/adr/contract.mdboth its table kinds, Exit status and Errors; round 1 shipped only the Exit status tables and the review caught the fifteen stale Errors rows — plus claude-plugins/fabrika/docs/cli-interface-convention.md, claude-plugins/fabrika/skills/triage/contract.md, packages/fabrika-cli/README.md (including its spend "never a zero" paragraph, found in the repair round's re-grep), both command.ts help strings.

Nothing outside the package branches on a specific value, so this re-seat cannot silently mis-branch a caller; it fails loudly in tests, which is the good failure.

The regression test fails against pre-fix code

verbLocalCodesIn (new, in exit-code-alignment.ts) reads verb sources, because a module namespace cannot tell an import from a declaration — and that difference is the whole defect. Run against the pre-fix adr verb modules restored from origin/main it reports fifteen declarations across five files, including NO_SUBJECT twice on two numbers; run against this diff it reports none. Both group tests assert it is empty, and both refuse to pass over an empty table (ADR 0092).

The guard genuinely sees both groups

adr and spend are in ALIGNED_GROUPS with real seat maps, not parked in UNTABLED_GROUPS. coverageGaps over the shipped registry reports {unclassified: [], unshipped: [], untabledWithTable: [], tableMissing: []}, and the describe.each(ALIGNED_GROUPS) block in exit-code-alignment.unit.test.ts now runs the drift and collision checks over both tables.

Tooling, re-run at the repair-round head and deliberately uncached: pnpm exec tsgo -p packages/fabrika-cli/tsconfig.json exits 0, and pnpm --filter @kampus/fabrika-cli exec vitest run is 201 files / 2912 tests green, resolving inside this worktree. pnpm lint:worktree reports no diagnostic on any file this PR touches (its remaining output is pre-existing, on untouched generated .js).

Deviations

  • Class: narrowed/widened the suggested fix-shape. Said: give each group a codes.ts and re-seat the per-verb constants. Did: also merged adr sweep's CORPUS_UNREADABLE into DIR_UNREADABLE, and replaced spend's seven per-verb names (TRANSCRIPT_ABSENT/LEDGER_ABSENT, TRANSCRIPT_UNREADABLE/LEDGER_UNREADABLE, NO_BILLED_TURNS/LEDGER_HOLDS_NO_ROWS) with four group names. Why: a table whose point is "one meaning per number" reads badly with two names for one fact, and triage's note observed that spend's two verbs were already structurally parallel. Disposition: no action needed — the stderr line still names the transcript or the ledger, so nothing a caller reads was lost.

  • Class: changed a test that asserted the old shape. Said: nothing about it. Did: rewrote adr/relate-verb.unit.test.ts's "the exit codes are the contract's" test from a literal [3, 4, 5, 6, 7] tuple to a >= 3 + mutual-distinctness assertion, since the tuple pinned exactly the per-verb numbering being removed. Why: pinning literals in a per-verb file re-creates the coupling the group table exists to end. Disposition: no action needed — the stronger group-wide assertions live in the new adr/codes.unit.test.ts.

  • Class: touched a file another lane also edits. Said: keep any edit to exit-code-alignment.ts minimal. Did: three edits — the two registry entries (ADR_SEATS, SPEND_SEATS, both added to ALIGNED_GROUPS), emptying UNTABLED_GROUPS with a docblock explaining why the export stays, and one new 12-line export verbLocalCodesIn used by both new group tests. Why: the source-level check has no other honest home, and duplicating it in two group test files would be worse. Disposition: for the reviewer to judge — the alignment machinery itself is otherwise untouched.

  • Class: edited files the acceptance criteria did not list. Said: update adr/contract.md, both command.ts help strings, and the index.ts docblock. Did: also claude-plugins/fabrika/docs/cli-interface-convention.md, claude-plugins/fabrika/skills/triage/contract.md, claude-plugins/fabrika/skills/adr/SKILL.md and packages/fabrika-cli/README.md. Why: each cited adr's per-verb allocation or a specific old number as a live fact, so leaving them would ship a published contradiction — exactly the risk triage flagged. Disposition: no action needed. Where those docs used adr as the standing example of the per-verb shape, they now point at report dedup (fabrika report — the alignment base — seats two exit codes per-verb, colliding with its own group table #5296), which is still live.

  • Class: resolved a flagged prerequisite rather than escalating. Said: if the index.ts "the contract assigns each verb its own codes" claim reads as a real prior ruling, stop and escalate. Did: read it as a description of the status quo and rewrote it. Why: the actual ruling lives in cli-interface-convention.md, which says both shapes are correct and a group picks by whether its verbs share refusal meanings. That ruling is untouched — adr and spend simply exercise the choice it grants. Disposition: no action needed; flagging it here so a reviewer can disagree.

  • Class: left a sibling defect alone. Said: do not absorb fabrika report — the alignment base — seats two exit codes per-verb, colliding with its own group table #5296. Did: left report/dedup-verb.ts's out-of-table codes exactly as they are, and scoped the new source-level check to the two groups this issue owns rather than every tabled group. Why: a repo-wide version of that check would red on report today. Disposition: #5296 owns it.

  • No ADR. This re-seats numbers inside an existing, already-ruled discipline; it establishes no new rule. Pre-assigned 0267 is unused.

  • (repair round 1) Class: edited files the acceptance criteria did not list. Said: fix the fifteen stale Errors rows in adr/contract.md and move verbLocalCodesIn. Did: also re-seated packages/fabrika-cli/README.md's spend "I could not measure it is never a zero" paragraph, which still published 3 / 4 / 5 for INPUT_ABSENT / INPUT_UNREADABLE / NOTHING_MEASURED. Why: the verdict asked for a repo-wide re-grep for surviving old literals, and this is what it turned up — the same published-contradiction risk, one group over. Disposition: no action needed.

  • (repair round 1) Class: narrowed/widened the suggested fix-shape. Said (non-blocking): verbLocalCodesIn's .sort() sits inside the flatMap, and the scan covers *-verb.ts only. Did: moved the .sort() outside the flatMap so the whole result is ordered, and recorded the *-verb.ts scope as a stated limit in the docblock rather than widening the scan. Why: the ordering fix is one line and makes the docblock's stable-list claim true; widening the scan is a behaviour change with its own false-positive question (a non-exit-code UPPER = <digit> constant), which does not belong in a repair round. Disposition: for the reviewer to judge — the scope limit is now documented rather than fixed.

  • (repair round 1) Class: edited a line the verdict did not name. Said: nothing about it. Did: adr/contract.md's adr resolve and adr sweep sections each said "5 is vacated here for the same reason it is under adr next", but the adr next section carries no such note — the reason lives once in the group-level table above. Both now point there. Why: a cross-reference to a note that does not exist sends a reader looking for the one thing this PR is trying to make findable. Disposition: no action needed.

  • (repair round 3) Class: edited lines the verdict did not name. Said: re-seat README.md L556-559 and rewrite L235's adr clause. Did: also rewrote the identical "adr allocates per verb" claim in packages/fabrika-cli/src/triage/codes.ts and claude-plugins/fabrika/skills/review/contract.md, which a repo-wide re-grep at the PR head found still false. Why: fixing only the named line would have produced a round 4 on the identical shape — the exact failure rounds 1 and 2 each hit by stopping at the first hit in a file. Disposition: no action needed.

  • (repair round 3) Class: narrowed the suggested fix-shape. Said: rewrite the adr clause "the way the other two copies were rewritten". Did: in src/triage/codes.ts the clause collapses to a #5296 pointer instead of restating the queue unreadable / search index unreadable gloss a fourth time. Why: CLAUDE.md's comments convention — a why is stated at its one load-bearing site and a docblock that re-derives it collapses to a pointer. Disposition: for the reviewer to judge; the prose surfaces (README, review/contract.md) keep the full gloss, only the code docblock points.

  • (repair round 3) Class: left a flagged item alone deliberately. Said (recorded UNKNOWN, not blocking): docs/cli-interface-convention.md L120 enumerates 6 of the 16 shipped codes.ts files. Did: nothing — not made exhaustive, no "no flaw here" wording, no restructuring. Why: that is open, unruled v1-surface coverage criterion enforced inconsistently: grilling passed, graduate failed #5290 (type:decision, status:triaged); resolving it here in either direction would decide it by precedent. Disposition: #5290 owns it.

  • (repair round 3) Class: pushed a head the prior verdict is not bound to. Said: nothing about it. Did: rebased onto latest origin/main before fixing (clean, no conflicts) per the repair step, so the push is --force-with-lease and the head moved past 229b0c62. Why: the repair step freshens the base to surface a textual conflict at code-time. Disposition: no action needed — the re-review re-binds the verdict to the new head.

  • (repair round 4) Class: narrowed the suggested fix-shape — by inverting the sweep method. Said: sweep by number, not by file. Did exactly that, and it is the only reason this round found the sixth site: the sweep enumerated every occurrence of the vacated 3/4/5/6 and the re-seated 7/11/12/13/15/17/18 across the whole checked-out worktree — tables, fenced examples, prose, .ts docblocks, .sh, .yml — and judged each hit against the codes.ts files at head, rather than scoping to a file list or a file-name pattern. src/adr/resolve.ts had never been opened in any prior round because every earlier sweep scoped to *-verb.ts. GitHub search/code was deliberately not used: it indexes the default branch only and cannot see text living solely on this branch. Disposition: no action needed.

  • (repair round 4) Class: left a flagged item alone deliberately. Said: docs/cli-interface-convention.md L120 is not a defect (true as written, non-exhaustive by design, outside this issue's acceptance criteria), and criterion 10 is discharged by silent omission (founder ruling on v1-surface coverage criterion enforced inconsistently: grilling passed, graduate failed #5290). Did: nothing — the line is untouched and no "not applicable" or "no flaw here" wording was added anywhere. Disposition: #5290 owns the enumeration question.

  • (repair round 4) Class: left a by-number sweep hit alone deliberately. Said: nothing about it. Did: the sweep surfaced docs/cli-interface-convention.md L367-378, where the worked block seats a refusal on 3 and a usage error on 1. Left untouched. Why: that block documents decisions next-id, a hypothetical illustration verb the same file explicitly labels "Illustration only. It is not a commissioned verb, and it does not pre-commit the /adr contract" (L340-343) — it is not the adr group and its 3 is not an adr seat. Re-seating it would make the illustration cite numbers from a table it is documented as not belonging to. Disposition: recorded so a reviewer re-running the by-number sweep sees the hit was judged, not missed.

  • (repair round 4) Class: pushed a head the prior verdict is not bound to. Said: nothing about it. Did: rebased onto latest origin/main before fixing (clean, no conflicts) per the repair step, so the push is --force-with-lease and the head moved past 718bed60. Why: the repair step freshens the base to surface a textual conflict at code-time. Disposition: no action needed — the re-review re-binds the verdict to the new head. The suite at the new base is 201 files / 2917 tests green (2912 before the rebase; the five added came in with main).

@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. (9c75658)
  • web — Stage pr-5309 torn down.

@usirin

usirin commented Aug 10, 2026

Copy link
Copy Markdown
Member Author

review-skill: FAIL @ c88195e

Verified against issue #5294 at the pinned head, read-only via a per-run ref. Class set from class-probe classify --namespaces at this head: review-code, review-skill (30 files; no apps/web/src/ path, so no review-design). Not control-plane on both axes (cp-classify: path-clear-no-content-source), so a PASS here auto-ships with no human approval — this verdict binds.

Blocking: the adr contract still publishes the pre-fix per-verb codes

claude-plugins/fabrika/skills/adr/contract.md carries two kinds of table per verb: an Exit status table and an Errors table (Message (stderr) | Code | Kind). The diff rewrote every Exit status table and touched zero Errors rows — git diff origin/main..HEAD on that file matches no line of the form | <n> | refusal|usage error |. All five Errors tables are therefore stale:

line message documented actual (from source)
136 adr next: cannot fetch <ref> 3 17 BASE_UNFETCHABLE
137-138 adr next: cannot enumerate open pull requests 4 18 IN_FLIGHT_UNKNOWN
139 adr next: cannot read <dir> at <ref> 6 11 DIR_UNREADABLE
284 adr new: <path> already exists 3 12 ALREADY_EXISTS
285-286 adr new: bad id / bad slug 4 1 (the deliberate BAD_ARGUMENT move)
379 adr resolve: cannot fetch <ref> 3 17
380 adr resolve: cannot enumerate open pull requests 4 18
381 adr resolve: cannot read <dir> at <ref> 6 11
488 adr supersede: no record for id <id> 3 7 NO_SUBJECT
489 adr supersede: no record for --by id 4 13 NO_BY
490 adr supersede: no single frontmatter status: line 5 14 NO_STATUS_LINE
491 adr supersede: rewrite would have changed <n> line(s) 6 15 MULTI_LINE_DIFF
492 adr amend-in-part: already "superseded by ..." 7 16 ALREADY_SUPERSEDED
655 adr sweep: cannot read <dir> 3 11 DIR_UNREADABLE
656 adr sweep: no readable ADR for --new 4 7 NO_SUBJECT

Actuals read from relate-verb.ts, sweep-verb.ts, new-verb.ts, next-verb.ts, resolve-verb.ts at this head — each imports its constants from the new adr/codes.ts, so the code is right and only the contract is wrong.

Three reasons this blocks rather than nits:

  1. The contract now contradicts itself on one page, in the exact shape fabrika adr and spend allocate exit codes per-verb instead of from a group table #5294 exists to kill. In the adr supersede section the Exit status table (line 478) says 7 = "<id> has no record under --dir"; the Errors table ten lines later (line 492) says 7 = "already superseded by ...". One number, two meanings, in one verb section of the artifact whose whole purpose in this PR is to establish one meaning per number.
  2. It re-publishes the vacated 5. Line 490 documents exit 5 as a live adr supersede refusal. 5 is genuinely gone from the code — adr/codes.unit.test.ts asserts allocatedCodes(codes).has(5) === false, the private band starts at 12, and I found no reachable path to it — but the SKILL contract is what an agent reads to interpret $?, and it still says adr supersede exits 5. That is the seat fix(fabrika): a readable-but-empty .decisions/ mints 0001 instead of exiting 5 #5297 deliberately cleared so a readable-but-empty .decisions/ can mint 0001. Requirement holds in code and fails in the contract.
  3. It contradicts a fix this same PR made. SKILL.md line 25 was correctly updated exit 6 to exit 11 for "a directory it could not read at all". contract.md line 139 documents the same refusal as 6. The PR fixed the prose mention and missed the table.

The PR body states "Prose consumers updated: claude-plugins/fabrika/skills/adr/contract.md" and "the adr contract's four per-verb tables plus a new group table". That is true of the Exit status tables and not true of the Errors tables. This is the consumer the re-seat missed — the machine-facing spec, not a shell script.

Fix: re-seat all fifteen Errors rows above, then re-run the review.

Verified sound (non-blocking)

  • SKILL.md 6 to 11 is correct.
  • triage/contract.md, docs/cli-interface-convention.md re-point adr to report dedup as the standing per-verb example; no stale number survives in either.
  • No skill, script, workflow or pipeline-cli tool branches on a numeric status from fabrika adr / fabrika spend — I grepped claude-plugins/ and .github/ for $? -eq <n> / exitCode === <n> patterns and found no adr/spend-related hit. The author's consumer claim holds everywhere except the contract tables above.
  • All six ## Deviations entries are literal and TRUE at source (verified individually).

@usirin

usirin commented Aug 10, 2026

Copy link
Copy Markdown
Member Author

review-code: FAIL @ c88195e

Verified against issue #5294 at the pinned head, read-only via a per-run ref plus an isolated throwaway worktree. Not control-plane on both axes, so a PASS auto-ships with no human approval — this verdict binds. Sibling verdict: review-skill: FAIL (the adr contract still publishes the pre-fix codes).

The exit-code work itself is correct and well-proven. One defect introduced by this diff blocks.

Blocking: the new export was spliced between a docblock and the function it documents

In packages/fabrika-cli/src/exit-code-alignment.ts, verbLocalCodesIn was inserted directly after codeTableGroupsIn's docblock rather than before or after the pair. The file now reads:

/**
 * The `<group>/codes.ts` tables that actually exist on disk - one of {@link coverageGaps}'s two
 * inputs, and on its own never the scan's scope ... (#5213). Paths resolve physically ...
 */
/**
 * The exit-code constants a group's verb modules declare **themselves** ...
 */
export const verbLocalCodesIn = (groupDir: string) => { ... };

export const codeTableGroupsIn = (srcDir: string) => { ... };

Two consequences: codeTableGroupsIn is now undocumented, and the orphaned block is actively misleading — it describes reading codes.ts files, coverageGaps' inputs, and symlink resolution, none of which is true of the function it now sits above. That docblock is #5295's contribution, and this is the one place the review brief asked me to confirm nothing from #5295/#5299 was superseded. Everything else from those two PRs survives intact (see below); this one paragraph does not.

Fix: move the verbLocalCodesIn definition and its own docblock either above codeTableGroupsIn's docblock or below codeTableGroupsIn's body.

Also worth fixing while in there (non-blocking)

  • .sort() is inside the flatMap, so it sorts each file's matches, not the result. readdirSync order is not guaranteed, so a non-empty verbLocalCodesIn return is not deterministically ordered. Harmless today because both consumers assert toEqual([]), but the docblock presents the return as a stable <file>: <NAME> list and the PR body quotes an ordered fifteen-row result.
  • Scope is *-verb.ts only. A code seated in a non-verb module in the same group (say adr/next.ts) is invisible to the check. The docblock is honest about scanning "verb modules", so this is a stated limit rather than a bug — noting it because the regex is shape-based (export const UPPER = <digit>), so it would also false-positive on a non-exit-code constant like ID_WIDTH = 4 if such a const ever moved into a verb module.

Verified at source — all four brief items

  1. No missed code consumer. No verb module in adr/ or spend/ declares a numeric exit constant any more (grep for export const UPPER = <digit> returns only the two codes.ts files plus unrelated domain constants ID_WIDTH, RARITY_FLOOR, DEFAULT_LIMIT, LEDGER_ROW_VERSION). No script, workflow or pipeline-cli tool branches on a numeric status from either group. BAD_ARGUMENT to 1 is sound and self-consistent: adr resolve already refused a malformed id with FAILED (1), so the old 4 put one fact on two numbers; nothing distinguishes a bad flag from a generic failure because per verb.ts a bad flag is not a proven outcome. The only missed consumer is the adr contract's Errors tables — see the review-skill verdict.
  2. 5 is unreachable. Absent from both tables; the private band starts at 12; adr/codes.unit.test.ts genuinely asserts expect(allocatedCodes(codes).has(5)).toBe(false); and no remaining reference to ZERO_SCOPE or a literal 5 exit exists in either group's source. Both new tests also carry an ADR-0092 non-vacuity guard (allocatedCodes(codes).size > 0).
  3. The guard still sees both groups, and still reds on a new one. adr and spend are in ALIGNED_GROUPS with real seat maps ({NO_SUBJECT: NO_TARGET, DIR_UNREADABLE: PRECONDITION_UNKNOWN} and {INPUT_ABSENT: NO_TARGET, INPUT_UNREADABLE: PRECONDITION_UNKNOWN}), so describe.each(ALIGNED_GROUPS) runs the drift and collision checks over both. Emptying UNTABLED_GROUPS makes the guard stricter, not quieter: with the exemption registry empty, any classified group missing a table falls into tableMissing. I constructed the next instance — coverageGaps over the live 16-group registry plus a hypothetical unregistered group shipping no codes.ts returns {"unclassified":["gecit"],...}, which reds expect(gaps().unclassified).toEqual([]). Baseline over the real registry is four empty arrays, as claimed.
  4. The regression genuinely fails pre-fix. Reproduced: verbLocalCodesIn against the five adr verb modules restored from origin/main reports exactly 15 declarations across 5 files, with NO_SUBJECT appearing twice (relate-verb.ts and sweep-verb.ts). Against this head both adr/ and spend/ return []. Author's claim confirmed to the number.

Shared-file check. origin/main has moved to 83804e02, but the only commit since the merge-base touches packages/fabrika-cli/src/eval/README.md, so there is no drift on exit-code-alignment.ts. #5295 (830b0417) and #5299 (c3c58af0) are both ancestors of this head. The diff against origin/main on that file is exactly the three disclosed edits — two registry entries, UNTABLED_GROUPS emptied, one new export — with the alignment machinery untouched. #5295's ghost coverage tests and the ZeroCoverageScope throws all survive and pass; only the docblock placement above is damaged.

Scope ruling (asked for explicitly). The CORPUS_UNREADABLE merge and the spend seven-to-four rename are in scope, not a refactor riding a bug fix. The acceptance criterion is one table per group where a code means one thing across the group; keeping CORPUS_UNREADABLE and DIR_UNREADABLE as two names for one seat, or seven spend names for four facts, produces a table that fails the group's own seats one meaning on each number assertion. The renames are forced by the criterion rather than adjacent to it. The counter-reading is real but weaker: no behaviour changes, no caller reads the names, and the stderr line still names the transcript or the ledger, so nothing observable was lost. Correctly disclosed either way.

Tooling. Ran in my own throwaway worktree, not via worktree-typecheck.sh: pnpm exec tsgo -p tsconfig.json exits 0, and pnpm vitest run is 201 files / 2912 tests green.

@usirin

usirin commented Aug 10, 2026

Copy link
Copy Markdown
Member Author

review-code: FAIL @ 229b0c6

Re-gate after repair round 1. Verified against the PR head, read from a per-run ref, not a checkout.
Classification: non-§CP on both axes — no changed path matches the live CONTROL_PLANE_RE, and
pipeline-cli cp-classify classify returns not-control-plane [path-clear-no-content-source]
(proven ordinary). So this is the auto-merge lane and this verdict is the only gate.

Required namespaces from class-probe classify --namespaces at this head: review-code,
review-skill. No review-design (no apps/web/src/ path).

The re-derivation, done against source rather than against the diff

Ground truth, read from packages/fabrika-cli/src/adr/codes.ts, src/spend/codes.ts and the
src/report/codes.ts seats they import:

  • adr: NO_SUBJECT 7, DIR_UNREADABLE 11, ALREADY_EXISTS 12, NO_BY 13, NO_STATUS_LINE 14,
    MULTI_LINE_DIFF 15, ALREADY_SUPERSEDED 16, BASE_UNFETCHABLE 17, IN_FLIGHT_UNKNOWN 18.
  • spend: INPUT_ABSENT 7, INPUT_UNREADABLE 11, NOTHING_MEASURED 12, WINDOW_SELECTED_NO_ROWS 13.

Then the reachable-code set per verb, read from each *-verb.ts refusal site rather than from the
tables — so a row that changed to a different wrong number would still be caught.

What passes. The repair's headline claims hold:

  • All fifteen Errors rows across the five verb tables in skills/adr/contract.md re-seat correctly,
    and each matches the Exit status table above it. Both table kinds are now consistent; the second
    kind is no longer missed.
  • The group matrix at contract.md L66-78 is truth-accurate cell by cell — 7 marked only for
    supersede/sweep, 11 only for next/resolve/sweep, 12 only for new, 13-16 only for supersede,
    17/18 only for next/resolve. That matches the imports in each verb file exactly.
  • 7 no longer carries two meanings inside adr supersede.
  • 5 survives only as a vacated-seat note (contract.md L82, L373, L649) — not published as a live
    adr code anywhere.
  • No verb file declares a numeric exit constant; every one imports from its group table.
  • verbLocalCodesIn now sits below codeTableGroupsIn, which is reunited with its docblock, and the
    .sort() is outside the flatMap so the whole result is ordered. The *-verb.ts scope is stated
    as a limit in the docblock.
  • Both command.ts help strings, docs/cli-interface-convention.md and skills/triage/contract.md
    state the re-seated numbers.
  • exit-code-alignment.unit.test.ts + both new codes.unit.test.ts files: 63 tests green at this
    head. CI at this head is green — 46 check runs declared, 46 read back paginated, zero failures.

Blocking — packages/fabrika-cli/README.md still publishes the pre-fix numbers

The repair round's repo-wide re-grep caught the spend read paragraph in this file and fixed it
(L493-495 → 7/11/12, correct). It missed the spend rollup paragraph sixty lines further down
in the same file. README.md L556-559 reads:

Four refusals, none of them a zero. 3 is no ledger at that path (nothing recorded yet), 4
is a ledger that could not be read (the spend is UNKNOWN), 5 is one read in full that yielded no
rows at all, and 6 is a ledger that does hold rows where the given window selects none

All four numbers are wrong. Against src/spend/codes.ts and the refusal sites in
src/spend/rollup-verb.ts they are 7 (INPUT_ABSENT), 11 (INPUT_UNREADABLE), 12
(NOTHING_MEASURED) and 13 (WINDOW_SELECTED_NO_ROWS).

This is not merely stale. spend now shares the report base's seats, so 3 and 4 are live codes
in that aligned table with other meanings (EMPTY_STDIN, BAD_SECTIONS). A reader taking this
paragraph at face value gets four wrong meanings, two of which collide with real seats — which is the
exact defect class #5294 exists to close, reintroduced in prose one section below where it was fixed.
The spend rollup help string in src/spend/command.ts states 7/11/12/13 correctly, so the
package ships two contradictory published contracts for the same verb.

Second miss in the same file: README.md L235 still asserts

That alignment does not extend repo-wide: adr allocates per verb, and report dedup's own
3/4 mean something else again.

adr no longer allocates per verb — this PR is what changed that. skills/triage/contract.md and
docs/cli-interface-convention.md both had the same sentence and both were correctly rewritten to
point at report dedup (#5296) instead; this third copy was not. Triage flagged this precise risk:
"the fix must therefore update the contract doc, the index.ts docblock and both command.ts help
strings in the same PR, or the package ships two contradictory published contracts."

Both spots fail acceptance criterion 5. The README.md diff on this PR touches only two hunks
(L180, L490-495), so neither line was reviewed and re-seated.

Not decided — resolves to UNKNOWN

docs/cli-interface-convention.md L120 now reads "report, triage, review, adr, spend and
wire each ship a <group>/codes.ts". Sixteen groups ship one on disk (adr build epic eval hook ledger plan report review review-ui ship spend status triage ui wire), so the enumeration is a
subset. The list was already partial before this PR (it named four); this PR added two without making
it exhaustive.

Whether a write-up that enumerates surfaces must account for every one, or may omit those with
nothing to say, is exactly the question open and unruled on #5290 (type:decision,
status:triaged, no analysis). I am not deciding it, in either direction, and it is not part of
the FAIL above — the FAIL stands on the README.md findings alone and is independent of how #5290
lands. Recording it as UNKNOWN so it is not silently resolved by precedent here.

To clear this gate

Re-seat README.md L556-559 to 7/11/12/13, and rewrite L235's adr clause the way the other
two copies were rewritten. Nothing else in this PR is blocking.

Verdict-written: 2026-08-10T08:00:33Z

@usirin

usirin commented Aug 10, 2026

Copy link
Copy Markdown
Member Author

review-skill: PASS @ 229b0c6

Re-gate after repair round 1. Verified against the PR head, read from a per-run ref, not a checkout.
Classification: non-§CP on both axes — no changed path matches the live CONTROL_PLANE_RE
(claude-plugins/fabrika/** is not claude-plugins/kampus-pipeline/**, and no CODEOWNERS row
covers it), and pipeline-cli cp-classify classify returns
not-control-plane [path-clear-no-content-source]. So this is the auto-merge lane.

Required namespaces from class-probe classify --namespaces at this head: review-code,
review-skill. This verdict covers the claude-plugins/fabrika/skills/** half only.

Skill surfaces verified

Three skill files change here, and all three are truth-accurate against
packages/fabrika-cli/src/adr/codes.ts, src/spend/codes.ts and the reachable-code set read from
each *-verb.ts refusal site — re-derived from source, not checked against the diff.

skills/adr/contract.md — the file the previous round failed on. It carries two table kinds per
verb, and round 1 re-seated only the Exit status kind. Both kinds are now consistent:

Verb Exit status rows Errors rows Matches source
adr next 0, 1, 11, 17, 18 17, 18, 18, 11, 1 yes
adr new 0, 1, 12 12, 1, 1, 1 yes
adr resolve 0, 1, 11, 17, 18 17, 18, 11, 1, 1 yes
adr supersede / amend-in-part 0, 1, 7, 13, 14, 15, 16 7, 13, 14, 15, 16 yes
adr sweep 0, 1, 7, 11 11, 7 yes

All fifteen Errors rows re-seat correctly, and no row landed on a different wrong number. The
prose exit references outside the tables agree too — L153 (11), L155 (18), L419 (17), L468
(15), L521 (13), L667/668 (11, 7).

The new group table at L66-78 is accurate cell by cell against each verb's imports: 7 marked only
for supersede/sweep, 11 only for next/resolve/sweep, 12 only for new, 13-16 only for
supersede, 17/18 only for next/resolve. 7 no longer carries two meanings inside
adr supersede. 5 appears only as a vacated-seat note (L82, L373, L649) and is not published as a
live code anywhere in the file — the cross-references at L373 and L649 now point at the group-level
note that actually exists, rather than at an adr next note that never did.

skills/adr/SKILL.md — the single numeric mention, adr next's unreadable-directory refusal,
moves 6 to 11. Correct: DIR_UNREADABLE is report's PRECONDITION_UNKNOWN = 11. The skill's
standing rule is value-agnostic ("a non-zero exit is UNKNOWN"), so nothing else here depends on a
number.

skills/triage/contract.md — correctly drops the two claims that adr allocates per verb and
that it does not participate in the report alignment. Both are now false, and the rewrite points at
report dedup (#5296), which is still live. The report dedup 3/4 example it keeps is accurate.

No skill-hygiene issue: no home/absolute/sibling path, links resolve (CI's dead-internal-link and
skill-frontmatter jobs are green at this head), and the frontmatter is untouched.

Not decided — resolves to UNKNOWN

docs/cli-interface-convention.md L120 (a docs/ file, outside this namespace, raised here only for
visibility) now reads "report, triage, review, adr, spend and wire each ship a
<group>/codes.ts", while sixteen groups ship one on disk. The list was already partial before this
PR; this PR added two entries without making it exhaustive.

Whether a write-up that enumerates surfaces must account for every one, or may omit those with
nothing to say, is open and unruled on #5290 (type:decision, status:triaged, no analysis). I am
not deciding it in either direction, and it is not a factor in this PASS. Recording it as UNKNOWN
so it is not silently settled by precedent here.

Verdict

The skill surfaces are correct and internally consistent, and the round-1 defect is fully discharged.
review-skill passes.

This PR is not mergeable yetreview-code is FAIL at this head over two surviving pre-fix code
sets in packages/fabrika-cli/README.md (L556-559 and L235). See that verdict; nothing in it touches
skills/**.

Verdict-written: 2026-08-10T08:01:45Z

@usirin
usirin force-pushed the usirin/adr-spend-group-codes-5294-D44EBC62 branch from 229b0c6 to 718bed6 Compare August 10, 2026 08:15
@usirin

usirin commented Aug 10, 2026

Copy link
Copy Markdown
Member Author

review-code: FAIL @ 718bed6

Re-gate after repair round 3. Verified PR #5309 against the acceptance criteria of #5294, one at a time, sourcing every file from the PR head (per-run ref + throwaway worktree; the launched tree was never switched). Ground truth for every literal below was re-derived from the codes.ts files at this head, not from the diff:

  • src/report/codes.tsNO_TARGET = 7, PRECONDITION_UNKNOWN = 11
  • src/adr/codes.tsNO_SUBJECT = 7, DIR_UNREADABLE = 11, ALREADY_EXISTS = 12, NO_BY = 13, NO_STATUS_LINE = 14, MULTI_LINE_DIFF = 15, ALREADY_SUPERSEDED = 16, BASE_UNFETCHABLE = 17, IN_FLIGHT_UNKNOWN = 18
  • src/spend/codes.tsINPUT_ABSENT = 7, INPUT_UNREADABLE = 11, NOTHING_MEASURED = 12, WINDOW_SELECTED_NO_ROWS = 13

Per-criterion

  • [PASS] Both codes.ts exist; no verb file declares a numeric exit constant. src/adr/codes.ts and src/spend/codes.ts are present, each importing its two shared seats from ../report/codes.ts rather than re-typing a numeral. verbLocalCodesIn (src/exit-code-alignment.ts:347) reads the verb sources and both group tests assert it is empty.
  • [PASS] One number = one meaning, one name = one number, within each group. Read cell by cell off both tables above: adr seats 7/11/12–18 with no repeat; spend seats 7/11/12/13 with no repeat. NO_SUBJECT no longer splits across relate/sweep.
  • [PASS] Both groups out of UNTABLED_GROUPS and in exactly one registry. UNTABLED_GROUPS is {} (src/exit-code-alignment.ts:171), with a docblock stating why the export survives. ALIGNED_GROUPS carries adr: ADR_SEATS and spend: SPEND_SEATS, each with a real seat map.
  • [PASS] exit-code-alignment.unit.test.ts passes; coverageGaps clean. Full package suite run in the review worktree at this head: 201 files / 2912 tests passed.
  • [FAIL] The published surfaces state the re-seated numbers. The tables and help strings do — src/adr/command.ts (11/17/18, 12+1, 11/17/18, 7/13/14/15/16), src/spend/command.ts (7/11/12 and 7/11/12/13), src/index.ts no longer asserts per-verb allocation, README.md L493-497 and L556-560 both correct. But six published sites still carry the pre-fabrika adr and spend allocate exit codes per-verb instead of from a group table #5294 numbering — five of them in claude-plugins/fabrika/skills/adr/contract.md (gated in review-skill, listed there), and one on the code surface:
    • packages/fabrika-cli/src/adr/resolve.ts:7 — the module docblock states "a failed read exits 3 or 4 with nothing on stdout." At this head adr resolve's failed reads are 11 / 17 / 18; 3 and 4 are not seats in adr/codes.ts at all. This is the group's own source publishing the numbering the PR removed, in the exact docblock whose point is that a failed read is never the absent answer.
  • [PASS] The adr new BAD_ARGUMENT question is settled and reasoned in the PR body. Moved to 1, with the in-group evidence (adr resolve already refuses a non-four-digit id at 1) stated. src/adr/new-verb.ts:31 carries the one-line rationale; src/adr/codes.ts L18-21 states it once for the group.
  • [PASS] Landed after fabrika eval sits outside the exit-code alignment discipline and the guard cannot see it #5213 / PR fix(fabrika-cli): scope the exit-code alignment guard to the shipped verb registry (#5213) #5295. PR fix(fabrika-cli): scope the exit-code alignment guard to the shipped verb registry (#5213) #5295 merged 2026-08-10T06:27:14Z; fabrika eval sits outside the exit-code alignment discipline and the guard cannot see it #5213 closed as completed. This head is rebased onto origin/main past both.

Standing sub-gates

  • comment-discipline — one added comment is a defect on its own terms: src/adr/resolve.ts:7 is a docblock stating a false fact about this head. Rubber-stamped as MIGRATE/CORRECT, not CUT — the sentence is load-bearing (it is the "absent is never a failed read" invariant at its site), it is simply wrong now. The rest of the added comments in this diff earn their place (the two codes.ts docblocks state the vacated-5 reason once each rather than per verb, which is the convention).
  • unresolved-threads — not applicable: no review threads on this PR (reviewThreads read ran and returned none).
  • glossary-freshness — not applicable: no new feature folder / public package / public export.
  • flag-gating — not applicable: the linked issue carries no **Containment:** flag marker.
  • session-caching — not applicable: no new session-caching path.
  • deviation-disclosure — PASS as far as this gate can see: ## Deviations is present and itemised per round, including round 3's three entries and the rebase.
  • CI at head — read paginated, --slurp, flattened count checked against declared: 46 declared / 46 flattened, all success except four legitimately skipped (integration, e2e, deploy, lint workflow YAML). Green.

§CP

not-control-plane [path-clear-no-content-source] — 32 files scanned, 0 .decisions/** ADRs content-probed; claude-plugins/fabrika/** and packages/fabrika-cli/** match no CODEOWNERS row and the live CONTROL_PLANE_RE anchors gate-critical skills at claude-plugins/kampus-pipeline/skills/. So this verdict is binding and SHA-bound, and a PASS here would auto-ship with no human in the loop. That is precisely why the residual sites below block.

Why this is round 4 and not a nit

Rounds 1, 2 and 3 each failed the same way: a second stale instance inside a file the round had already edited. Round 3 fixed that for README.md, src/triage/codes.ts and skills/review/contract.md — all three re-verified correct at this head. The worked examples in adr/contract.md were never swept in any round (all four $ echo $? blocks still print the old code), and src/adr/resolve.ts was never opened at all because it is not a *-verb.ts file. Both are the same class, one layer down. The remedy is a sweep by number, not by file: grep every 3/4/5/6 on an adr/spend surface and check each against the table.

review-code does not merge. ship-it is the authorised merge step.

Verdict-written: 2026-08-10T08:34:14Z

@usirin

usirin commented Aug 10, 2026

Copy link
Copy Markdown
Member Author

review-skill: FAIL @ 718bed6

Re-gate after repair round 3. Behavioral-artifact class: claude-plugins/fabrika/skills/** + claude-plugins/fabrika/docs/**. Every file read from the PR head via a per-run ref, never the launched checkout.

Ground truth re-derived from packages/fabrika-cli/src/adr/codes.ts at this head (not from the diff): NO_SUBJECT = 7, DIR_UNREADABLE = 11, ALREADY_EXISTS = 12, NO_BY = 13, NO_STATUS_LINE = 14, MULTI_LINE_DIFF = 15, ALREADY_SUPERSEDED = 16, BASE_UNFETCHABLE = 17, IN_FLIGHT_UNKNOWN = 18; a malformed <id>/<slug> is a usage error at 1.

What is correct at this head

  • claude-plugins/fabrika/skills/adr/contract.md — the group matrix (L67-78). Checked cell by cell against adr/codes.ts: 0/1/7/11/12/13/14/15/16/17/18, and each verb column marks only the codes that verb can reach. Correct.
  • All five per-verb Exit status tables (L126-130, L276-278, L367-371, L476-482, L644-647) and all five Errors tables (L136-140, L284-287, L379-383, L488-492, L655-656) — 15 re-seated Errors rows re-verified, every row matches the table.
  • 5 survives only as a vacated-seat note (L82-85, L373, L649) and is a live code nowhere.
  • claude-plugins/fabrika/skills/adr/SKILL.md:25exit 11 for an unreadable .decisions/. Correct.
  • claude-plugins/fabrika/skills/review/contract.md:112-113 — the round-3 rewrite. wire's 38 re-verified against src/wire/codes.ts (ABSENT = 3UNUSABLE_FIELDS = 8) and report dedup's 3/4 against src/report/dedup-verb.ts (QUEUE_UNREADABLE = 3, SEARCH_UNREADABLE = 4). Both correct; the false "adr allocates per verb" claim is gone.
  • claude-plugins/fabrika/docs/cli-interface-convention.md:115-123 — the per-verb shape now reads "shipped nowhere", with report dedup (fabrika report — the alignment base — seats two exit codes per-verb, colliding with its own group table #5296) named as the residue, and adr/spend moved into the per-group list. Correct.
  • claude-plugins/fabrika/skills/triage/contract.md — no surviving adr/spend exit-number claim.

The blocker — five stale sites, all in claude-plugins/fabrika/skills/adr/contract.md

Every one contradicts a table in the same document, so the file publishes two different contracts for the same refusal. Four are worked examples, which is the part a reader copies:

Line What it publishes Ground truth
L183 adr next in-flight enumeration failure → $? = 4 IN_FLIGHT_UNKNOWN = 18 (this file's own L130 / L137)
L303 adr new target path exists → $? = 3 ALREADY_EXISTS = 12 (L278 / L284)
L412 adr resolve --base unfetchable → $? = 3 BASE_UNFETCHABLE = 17 (L370 / L379)
L515 adr supersede no --by record → $? = 4 NO_BY = 13 (L479 / L489)
L525 prose: "Exit 6 is that rule made mechanical rather than remembered" (the one-line-diff immutability rule) MULTI_LINE_DIFF = 15 — and this file already says 15 at L468 and L491. 6 is not a seat in the adr table at all.

This is the same failure shape as rounds 1, 2 and 3: a second stale instance inside a file the round had already edited. Rounds 1 and 2 swept this file's tables; nobody swept its fenced examples or its Grounding prose. Sweep by number, not by file — grep every 3/4/5/6 on an adr/spend surface and check each against codes.ts.

There is a sixth site on the code surface (packages/fabrika-cli/src/adr/resolve.ts:7), gated separately in the review-code verdict on this PR.

Criterion 10 (#5290) — applied, not reported UNKNOWN

claude-plugins/fabrika/docs/cli-interface-convention.md:120 enumerates six groups that ship a <group>/codes.ts where more do. I read the founder ruling on #5290 first-party (issue comment, 2026-08-10T07:53:39Z): silent omission discharges the criterion — no explicit "not applicable" line is owed. Leaving L120 untouched is therefore sanctioned, and I record it as not a defect, not as UNKNOWN. Independently: the sentence is true as written (those six groups do each ship a table), it does not claim to be exhaustive, and it is outside #5294's acceptance criteria. Nothing here blocks.

Skill-hygiene

  • No frontmatter, trigger-phrase, or cross-reference change in this diff — the edits are body prose and tables only.
  • Every in-repo link checked in the touched sections resolves (CI's dead-internal-link job is green at this head).
  • No home-directory, machine-local, or sibling-repo path in any touched line.
  • validate skill frontmatter and lint skill corpus … both success at this head.

§CP

not-control-plane [path-clear-no-content-source]claude-plugins/fabrika/** matches no CODEOWNERS row, and the live CONTROL_PLANE_RE anchors the gate-critical skills at claude-plugins/kampus-pipeline/skills/. Binding, SHA-bound verdict; no human approval gate stands behind it.

review-skill does not merge. ship-it is the authorised merge step.

Verdict-written: 2026-08-10T08:34:24Z

usirin and others added 4 commits August 10, 2026 01:38
The adr group seated NO_SUBJECT on 3 in relate-verb and on 4 in sweep-verb,
and exit 3 carried four meanings across its five verbs. spend never collided
but allocated the same distinctions per verb. Both groups now allocate from a
single <group>/codes.ts, share the base's target and precondition seats, and
keep their own codes at 12 and up. 5 stays vacated in adr (#5254, #5297).
…e a docblock

The round-1 diff rewrote every Exit status table in adr/contract.md and
touched none of the five Errors tables, so fifteen rows still published the
pre-fix per-verb numbers - including a live 5 on a seat #5297 vacated, and a
7 that meant two different things ten lines apart. Re-seats all fifteen, plus
the spend paragraph in the package README that a repo-wide re-grep turned up.

verbLocalCodesIn had been spliced between codeTableGroupsIn's docblock and its
function; it moves below, and its .sort() moves outside the flatMap so the
whole result is ordered rather than each file's share of it.

Refs #5294
…false per-verb adr claim

Round 3. README L556-559 published 3/4/5/6 for `spend rollup`; the group's seats
are 7/11/12/13, and 3/4 are live seats with other meanings in the shared `report`
base, so the package shipped two contradictory contracts for one verb.

The "`adr` allocates per verb" assertion was false in three places at this head,
not one: README L235, src/triage/codes.ts and skills/review/contract.md. All
three now point at `report dedup` (#5296) as the surviving cross-group
difference; the codes.ts docblock collapses to that pointer rather than
re-narrating the why a fourth time.
Rounds 1-3 swept this corpus by file and by phrase; each round left a
second stale instance inside a file it had already edited. This round
swept by NUMBER instead — every occurrence of the vacated 3/4/5/6 and
the re-seated 7/11/12/13/15/17/18 across the whole worktree, in every
context (tables, fenced examples, prose, .ts docblocks), judged against
the codes.ts files at head.

Six sites, all contradicting a table in their own file:

- adr/contract.md L183: `adr next` in-flight enumeration failure
  printed 4 -> IN_FLIGHT_UNKNOWN = 18
- adr/contract.md L303: `adr new` path exists printed 3 -> ALREADY_EXISTS = 12
- adr/contract.md L412: `adr resolve` base unfetchable printed 3 ->
  BASE_UNFETCHABLE = 17
- adr/contract.md L515: `adr supersede` no --by record printed 4 -> NO_BY = 13
- adr/contract.md L525: Grounding prose said "Exit 6"; 6 is not an adr seat
  at all -> MULTI_LINE_DIFF = 15, which the same file already states at
  L468 and L491
- src/adr/resolve.ts:7: the docblock said a failed read exits 3 or 4. The
  verb imports DIR_UNREADABLE / BASE_UNFETCHABLE / IN_FLIGHT_UNKNOWN, so
  its failed reads are 11 / 17 / 18. This file was never opened in any
  prior round because every sweep scoped to *-verb.ts.

Four of the five doc sites are worked examples — the part a reader copies.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@usirin

usirin commented Aug 10, 2026

Copy link
Copy Markdown
Member Author

review-code: PASS @ 9c75658 — merge-ready

Verified PR #5309 against the acceptance criteria of #5294, one at a time, at head 9c75658d16a06a9533a7ded5f5bf83d99c03a380 (round-4 re-gate; the prior verdicts on c88195eb / 229b0c62 / 718bed60 are stale by rebase).

Ground truth re-derived from the codes.ts files at this head, not from the PR body:

  • adr (packages/fabrika-cli/src/adr/codes.ts) — NO_SUBJECT=7, DIR_UNREADABLE=11 (both imported from ../report/codes.ts), ALREADY_EXISTS=12, NO_BY=13, NO_STATUS_LINE=14, MULTI_LINE_DIFF=15, ALREADY_SUPERSEDED=16, BASE_UNFETCHABLE=17, IN_FLIGHT_UNKNOWN=18.

  • spend (packages/fabrika-cli/src/spend/codes.ts) — INPUT_ABSENT=7, INPUT_UNREADABLE=11, NOTHING_MEASURED=12, WINDOW_SELECTED_NO_ROWS=13.

  • report (untouched base, src/report/codes.ts) — 3=EMPTY_STDIN, 4=BAD_SECTIONS, 5=LEAKED_PATH, 6=BARE_AT_PATH, 7=NO_TARGET, 8=WRITE_UNKNOWN, 9=READBACK_MISMATCH, 10=CLASSIFIED, 11=PRECONDITION_UNKNOWN.

  • [PASS] adr/codes.ts and spend/codes.ts exist, each the single table its group's verbs import; no verb file declares a numeric exit constant — a scan for export const <NAME> = <digit> across src/adr/ and src/spend/ at head returns numeric declarations from codes.ts only; the other three hits (next.ts:ID_WIDTH, sweep.ts:RARITY_FLOOR/DEFAULT_LIMIT, spend/ledger.ts:LEDGER_ROW_VERSION) are not exit codes. Every *-verb.ts in both groups imports its codes from ./codes.ts.

  • [PASS] One number = one meaning, one name = one number, within each group — adr seats {7,11,12,13,14,15,16,17,18}, spend seats {7,11,12,13}; both sets are collision-free and codes.unit.test.ts in each group pins it (the allocatedCodes sharing check plus a non-empty-table assert, ADR 0092).

  • [PASS] Both groups out of UNTABLED_GROUPS and into exactly one registry — exit-code-alignment.ts:171 UNTABLED_GROUPS = {}; ALIGNED_GROUPS carries adr: ADR_SEATS and spend: SPEND_SEATS, each a real two-seat map (NO_TARGET / PRECONDITION_UNKNOWN) with its reasoning in the docblock. Neither is in UNALIGNED_GROUPS.

  • [PASS] exit-code-alignment.unit.test.ts passes with coverageGaps clean — full package suite run at this head in an isolated head checkout: 201 files / 2917 tests passed, 0 failed. CI at head is green: 46 declared / 46 flattened check runs, 42 success + 4 skipped, 0 failure, 0 pending.

  • [PASS] Published numbers re-stated everywhere — checked truth-shaped against the tables above, not diff-shaped: adr/contract.md's group matrix (L66-78) cell by cell; its five per-verb Exit status tables (next 11/17/18; new 12 plus usage-1; resolve 11/17/18; supersede 7/13/14/15/16; sweep 7/11) and five Errors tables; its prose at L153/L155/L419/L468/L521/L525/L667; its four fenced echo $? examples (L183=18, L303=12, L412=17, L515=13). adr/command.ts and spend/command.ts help strings match seat for seat. packages/fabrika-cli/README.mdadr L180 (15), spend read L493-495 (7/11/12), spend rollup L556-558 (7/11/12/13). index.ts docblock no longer asserts per-verb allocation. adr/SKILL.md:25 now reads exit 11. src/adr/resolve.ts:7 docblock now reads "exits 11, 17 or 18".

  • [PASS] The adr new BAD_ARGUMENT question is settled and reasoned in the PR body — it moved to exit 1 (usage error) and the constant is gone. The argument is grounded: adr resolve already refuses a non-four-digit id on 1, verb.ts reserves 1 for usage errors and 3+ for proven outcomes, so seating the same fact on 4 was the one-fact-two-numbers defect at the reserved end of the table. codes.ts:18-21 carries the same reasoning at its site.

  • [PASS] Landed after fabrika eval sits outside the exit-code alignment discipline and the guard cannot see it #5213 / PR fix(fabrika-cli): scope the exit-code alignment guard to the shipped verb registry (#5213) #5295UNTABLED_GROUPS and coverageGaps are present on the merge base (introduced by 830b0417, PR fix(fabrika-cli): scope the exit-code alignment guard to the shipped verb registry (#5213) #5295); this diff empties the registry rather than introducing it.

Independent by-number sweep for a seventh site — run fresh at the head, not taken from the PR's report. Scope: the whole tree at head (excluding node_modules, dist, .turbo), searching (a) every file naming any adr/spend verb, (b) every echo $? block under claude-plugins/ and packages/fabrika-cli/, (c) the retired constant names (CORPUS_UNREADABLE, TRANSCRIPT_ABSENT, TRANSCRIPT_UNREADABLE, NO_BILLED_TURNS, LEDGER_ABSENT, LEDGER_UNREADABLE, LEDGER_HOLDS_NO_ROWS, BAD_ARGUMENT), and (d) two reversed proximity patterns pairing "exit <n>" with adr/spend/decision-record/transcript/ledger/corpus wording. Result: no seventh site. The only surviving mentions of the retired names are the two historical clauses inside the new adr/codes.ts docblock (L5, L20), which describe the pre-fix state and are correct as history. Every other repo-wide hit belongs to a different verb group's own table (governance, plan, epic, review, build, ship, front-door) or to packages/pipeline-cli/'s separate v1 tools (ADR 0238: fabrika reimplements, never calls v1).

Check on the six "correct, not stale" dismissals — each independently re-derived, since a wrongly-dismissed hit is indistinguishable from a missed one:

  • report's seats 3/4/5/6 in front-door/contract.md (L240, L254-257, L1011-1013, L1024-1027) and governance/contract.md (L133, L146-149, L862-864, L1070-1072) are those groups' own report-aligned seats — EMPTY_STDIN / BAD_SECTIONS (a registered DELIBERATE_GAP) / LEAKED_PATH / BARE_AT_PATH — verified against src/report/codes.ts at head. Correctly dismissed.
  • packages/fabrika-cli/README.md's 3/4/5/6 rows (L200-201, L233-240, L267-268, L384-385, L463-464) are the report / triage / review / wire / hook sections, not adr / spend. Correctly dismissed.
  • The vacated-seat notes at adr/contract.md L82 (group-level), L373 and L649 (adr resolve, adr sweep, both pointing at the group-level note) are consistent with codes.ts:13-16 and with adr/codes.unit.test.ts's has(5) === false assertion. Correctly dismissed.
  • The decisions next-id block at docs/cli-interface-convention.md L367-403 documents an uncommissioned illustration verb the same file explicitly labels "Illustration only… does not pre-commit the /adr contract" (L340-343). It is not an adr seat. Correctly dismissed.

docs/cli-interface-convention.md L120's non-exhaustive enumeration is not reported as a defect and is not UNKNOWN: the founder ruled directly (2026-08-10T07:53:39Z, issue-comment 5237387187 on #5290) that silent omission discharges criterion 10 and that no "no flaw here" line is owed. I read that ruling myself. Round 4 correctly added no such wording anywhere.

Sub-gates:

  • comment-discipline — PASS. 311 added comment-bearing lines scanned; the added docblocks in adr/codes.ts, spend/codes.ts, the two codes.unit.test.ts files and verbLocalCodesIn each state a load-bearing why at its enforcement site (why 5 stays vacated, why the shared seats are imported rather than re-typed, why the source-level read exists, and the *-verb.ts scope limit as a stated limit). KEEP under the deslop-comments rubric; no narration, restatement or ADR re-derivation.
  • unresolved-threads — not applicable; the review-thread read ran and returned no unresolved thread.
  • deviation-disclosure — PASS. The body's ## Deviations section discloses every departure across all four rounds, including the two sweep hits it judged rather than fixed.
  • glossary-freshness — the detector fired on one new public export (+export * as AdrCodes from "./adr/codes.ts" in index.ts) with .glossary/TERMS.md untouched. Judged, not omitted: not a defect here. The export ships no new domain concept — "a verb group's one exit table" is already established repo-wide (16 <group>/codes.ts files), already named in docs/cli-interface-convention.md and the package README, and enforced by exit-code-alignment.ts; AdrCodes is a namespace alias over that existing concept, made importable so the new group test can read it. @kampus/fabrika-cli is not a new package, and the immediately preceding table-adding PR (fix(fabrika-cli): scope the exit-code alignment guard to the shipped verb registry (#5213) #5295, src/eval/codes.ts, merged) touched no glossary either. The gate's target — one concept drifting into four names (health.ts flagshipBound is misnamed — a flag eval defaulting false, not a binding check #864) — is not engaged. Recorded explicitly so the judgement is falsifiable rather than silent.

§CP: not-control-plane, both axes — the canonical classifier at this head over all 33 changed files returns CONTROL_PLANE_TOUCHED='', GUARD_TOUCHING='', CP_FILES_N=33, ADR_N=0, exit 0. Path axis clear (no .claude/**, no .github/**, no gate-critical kampus-pipeline skill — the diff is claude-plugins/fabrika/** plus packages/fabrika-cli/**); content axis clear (zero .decisions/** files to probe). So this verdict uses the SHA-bound marker form, and it auto-ships.

Run-evidence bundle: UNKNOWN, not absent. The materialize-head helper aborted at this head (iso-preflight.sh:51: WORKTREE_ROOT: unbound variable, exit 1), so the head was materialized by the §HEAD fallback instead — a per-run ref asserted equal to 9c75658d16a06a9533a7ded5f5bf83d99c03a380 plus an isolated throwaway checkout — and the bundle lookup was not run. An unread lookup is never "no bundle". Verified instead from the diff, the full in-tree unit run (201 files / 2917 tests green — the fail-closed full project, never a feature-scoped subset) and the paginated check-run enumeration at head (46 declared, 46 flattened, 0 failures).

Read the PR head (§HEAD): all files under review sourced from 9c75658d16a06a9533a7ded5f5bf83d99c03a380 via the per-run ref and its throwaway checkout, never the launched checkout's working copy.

All criteria pass. This PR is merge-ready. review-code does not mergeship-it is the authorized merge step; merging will auto-close #5294 via Fixes #5294.

Verdict-written: 2026-08-10T09:08:20Z

@usirin

usirin commented Aug 10, 2026

Copy link
Copy Markdown
Member Author

review-skill: PASS @ 9c75658 — merge-ready

Verified PR #5309's skills class against #5294's acceptance criteria plus the skill-rigor checklist, at head 9c75658d16a06a9533a7ded5f5bf83d99c03a380 (round-4 re-gate; the verdicts on c88195eb / 229b0c62 / 718bed60 are stale by rebase).

Class routing: pipeline-cli class-probe classify --namespaces over the PR's 33 changed files at head names two namespaces — review-code and review-skill — and no review-design (no apps/web/src/** path). Both are gated in this one pass; the review-code marker at this same head is its own separate comment.

Skill-class surface in the diff (4 files, all claude-plugins/fabrika/skills/**): adr/SKILL.md, adr/contract.md, review/contract.md, triage/contract.md.

Acceptance criteria touching this class (the same conjunctive list, judged on the skill surface):

  • [PASS] AC5 — the published contract states the re-seated numbers. adr/contract.md now carries one group exit table (L66-78) plus five per-verb Exit status tables and five Errors tables, and every cell matches packages/fabrika-cli/src/adr/codes.ts at head (NO_SUBJECT=7, DIR_UNREADABLE=11, then 12-18). Checked truth-shaped, cell by cell, against the shipped table — not against the diff. The four fenced echo $? examples read 18 / 12 / 17 / 13, each matching the refusal its example produces.
  • [PASS] AC5 — adr/SKILL.md:25 now reads "a directory it could not read at all refuses (exit 11)"; 11 is DIR_UNREADABLE at head. It was exit 6, which is not an adr seat at all under the new table.
  • [PASS] AC2 as published — the group table is internally collision-free and matches the code: one number, one meaning, across all six verbs.

Skill-rigor checklist (run regardless of AC):

  • [PASS] 1. Behavioral correctness. Traced the changed instructions as an executing agent would. adr/SKILL.md's refusal line now names the code the binary actually returns, so an agent diagnosing a failed adr next reads the true state instead of a number the group no longer seats. adr/contract.md's per-verb tables list only codes that verb can reach and none re-seats a number, which is what the group table claims. The three prose surfaces that used adr as the standing example of per-verb allocation now say the opposite of what they said, and the opposite is what is true at head.
  • [PASS] 2. Trigger / description quality. No frontmatter changed. The adr/SKILL.md diff is exactly one line (its refusal-code sentence); review/contract.md and triage/contract.md are contract bodies with no trigger surface. Nothing widens or narrows any skill's firing set, so no sibling lane is shadowed.
  • [PASS] 3. Cross-skill conflict / shadowing. The edit changes a claim three sibling contracts publish about a fourth group, so both sides of that seam had to move together — and they did. review/contract.md and triage/contract.md drop "adr allocates per verb" and repoint at report dedup, and that replacement claim is true at head: packages/fabrika-cli/src/report/dedup-verb.ts:21,23 seat QUEUE_UNREADABLE = 3 and SEARCH_UNREADABLE = 4 outside report/codes.ts, tracked by fabrika report — the alignment base — seats two exit codes per-verb, colliding with its own group table #5296. packages/fabrika-cli/src/triage/codes.ts:15-16 carries the same pointer, so the code docblock and the two prose contracts agree. No skill is left asserting the retired claim: a repo-wide re-grep at head found no surviving "adr allocates per verb" text anywhere.
  • [PASS] 4. Gate-invariant preservation. Nothing in the §CP set is in the diff's reach — the canonical classifier at this head returns CONTROL_PLANE_TOUCHED='' and GUARD_TOUCHING='' over all 33 files, and no path touches .claude/**, .github/**, or any of the six gate-critical claude-plugins/kampus-pipeline/skills/** surfaces. The diff is claude-plugins/fabrika/** plus packages/fabrika-cli/**. No marker matcher, SHA-binding rule, conjunctive bar, author gate or control-plane refusal appears in it. The check ran and had nothing to weaken.
  • [PASS] 5. Contract implementation ticket (ADR 0248). In scope by file list — the diff changes three claude-plugins/fabrika/skills/*/contract.md. The check is satisfied on the facts rather than by exemption: these contracts specify no unbuilt verb. Every adr verb they describe is already shipped in packages/fabrika-cli/src/adr/, and the implementation for the change these contract edits describe lands in this same PR, tracked by the open issue named in the body (Fixes #5294) — an implementation ticket, not an authoring brief. ADR 0248's failure mode — a merged spec whose verbs do not exist and which has no route into the build pool — is structurally unreachable here.

Independent by-number sweep over the skill surface — run fresh at head, not taken from the PR's report: every echo $? block under claude-plugins/, every file naming an adr or spend verb, and the retired constant names. No seventh site. The remaining 3/4/5/6 citations in front-door/contract.md and governance/contract.md are those groups' own report-aligned seats (EMPTY_STDIN / a registered DELIBERATE_GAP / LEAKED_PATH / BARE_AT_PATH), verified against src/report/codes.ts; governance/evals/fixtures/eval-4.md's exit 11 is governance sweep, not adr sweep; and docs/cli-interface-convention.md's decisions next-id block is the file's own explicitly-labelled "Illustration only" verb (L340-343), not an adr seat. Each was judged, not skipped.

Criterion 10 is not reported as UNKNOWN: the founder ruled directly (2026-08-10T07:53:39Z, issue-comment 5237387187 on #5290) that silent omission discharges it. I read the ruling myself. docs/cli-interface-convention.md L120's non-exhaustive enumeration is therefore not a defect, and this round correctly added no "not applicable" wording anywhere.

deviation-disclosure (§DEV): PASS — the body's ## Deviations section is present and discloses every departure across all four rounds, including the two by-number sweep hits round 4 judged rather than fixed and the deliberate no-op on the #5290 enumeration question.

§CP: not-control-plane, both axes (path-clear and content-clear, ADR_N=0), so this is a binding SHA-bound marker rather than an advisory, and a PASS auto-ships.

Read the PR head (§HEAD): every file under review sourced from 9c75658d16a06a9533a7ded5f5bf83d99c03a380 via a per-run ref asserted equal to that SHA and an isolated throwaway checkout — never the launched checkout's working copy.

All acceptance criteria and all five rigor checks pass. This PR is merge-ready on the skills class. review-skill does not mergeship-it is the authorized merge step, and it requires the current-head PASS in the review-code namespace too (posted separately at this same head).

Verdict-written: 2026-08-10T09:10:04Z

@usirin
usirin added this pull request to the merge queue Aug 10, 2026
Merged via the queue into main with commit a100d93 Aug 10, 2026
46 checks passed
@usirin
usirin deleted the usirin/adr-spend-group-codes-5294-D44EBC62 branch August 10, 2026 09:21
@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.

fabrika adr and spend allocate exit codes per-verb instead of from a group table

1 participant