Skip to content

fix(fabrika-cli): scope the exit-code alignment guard to the shipped verb registry (#5213) - #5295

Merged
usirin merged 1 commit into
mainfrom
usirin/eval-exit-code-alignment-5213-E0A29804
Aug 10, 2026
Merged

fix(fabrika-cli): scope the exit-code alignment guard to the shipped verb registry (#5213)#5295
usirin merged 1 commit into
mainfrom
usirin/eval-exit-code-alignment-5213-E0A29804

Conversation

@usirin

@usirin usirin commented Aug 10, 2026

Copy link
Copy Markdown
Member

Fixes #5213

What changed

Two defects, one root. The root was the guard's scope.

The guard can now see a group that ships no table. codeTableGroupsIn finds directories holding a codes.ts, and the coverage test used that as its scope. A group with no table was therefore not in the set being checked, so it could sit in no registry and the assertion stayed green — over the group with the loosest exit discipline in the package. Scope now comes from registry.ts: a group is checked because the CLI ships it. coverageGaps reports four gaps and the test reds on each:

  • unclassified — a group shipped by the registry, or carrying a table on disk, that no registry here classifies. This is the blindness, pinned.
  • unshipped — a registration for a group the CLI no longer ships.
  • untabledWithTable / tableMissing — the untabled record gone stale in either direction.

It throws ZeroCoverageScope rather than reporting no gaps when either side of the scan is empty (ADR 0092) — a registry that failed to load would otherwise report zero groups and therefore zero gaps.

UNTABLED_GROUPS records adr and spend as the two remaining registered groups with no table, each with the reason it is still unchecked and a tracking issue. That is what lets the guard treat any other untabled group as a failure instead of silence.

eval gets a table and its refusals get seated on it. packages/fabrika-cli/src/eval/codes.ts imports MALFORMED_DOCUMENT and ZERO_SCOPE from the base rather than restating the numerals (the review-ui discipline), and adds INTEGRITY_VIOLATION = 12 and RUNS_NOT_EXECUTED = 13 on its own account. eval is registered in ALIGNED_GROUPS.

All 21 process.exit sites in eval/command.ts are re-seated. GATE_FAIL_EXIT_CODE = 1 is gone:

  • 6 → MALFORMED_DOCUMENT (a named JSON artifact read in full that does not conform)
  • 2 → ZERO_SCOPE (an eval set that decodes and carries zero cases)
  • 1 → INTEGRITY_VIOLATION (keeps: decodes, breaks its own integrity rules — its own seat because no decoder change helps)
  • 1 → RUNS_NOT_EXECUTED (run: the suite completed, some planned runs did not execute)
  • 11 → FAILED from verb.ts (six flag-value usage errors, and five catch-arms — check, report, cases, run, keeps — where the read failed before the verb could judge anything) — correctly still 1, which is what the convention reserves it for

The regression test fails against pre-fix code

Verified by removing eval: EVAL_SEATS from ALIGNED_GROUPS and re-running:

× classifies each as the base, aligned, deliberately unaligned, or recorded untabled
  AssertionError: expected [ 'eval' ] to deeply equal []

That is the acceptance criterion's own check. The old assertion could not produce it: with no eval/codes.ts on disk, eval was not in onDisk, so onDisk == registered held and the suite was green. Three further tests pin the property directly rather than the instance — a shipped group with no table and no registration is reported, a table on disk that no registry classifies is reported, and an empty scan throws instead of returning no gaps.

Verification

  • pnpm typecheck — 31/31 tasks pass
  • packages/fabrika-cli suite — 195 files, 2825 tests pass
  • pnpm lint:worktree — clean

Deviations

  • adr and spend are recorded, not fixed. Both allocate proven codes per-verb rather than from a group table, and adr already collides with itself (relate-verb.ts seats NO_SUBJECT = 3, sweep-verb.ts seats it 4). The issue's triage note scoped them out; the change makes them visible in UNTABLED_GROUPS with a stated reason. Follow-up filed as fabrika adr and spend allocate exit codes per-verb instead of from a group table #5294. Naming them "untabled" is an admission of a tracked gap, and the guard asserts each entry carries a reason and that none of them actually ships a table.
  • eval/command.ts still calls process.exit directly rather than returning VerbOutcomes through verb.ts's answer/refuse. The issue notes eval imports nothing from verb.ts; this imports FAILED and seats every refusal on a named constant, which is what the acceptance criteria ask for. Converting the five verbs to pure VerbOutcome-returning cores with a thin adapter is a larger refactor and is not in this diff.
  • No ADR. This applies existing decisions (verb.ts's reserved-code convention, ADR 0092's zero-scope rule) rather than making a new one.

…verb registry (#5213)

The coverage check scanned directories holding a codes.ts, so a group with no
table was invisible to it. eval was that group: no table, every refusal on 1, and
registered in neither ALIGNED_GROUPS nor UNALIGNED_GROUPS — the loosest exit
discipline in the package sitting under a guard that reported green over it.

Scope now comes from registry.ts, so a group is checked because the CLI ships it.
coverageGaps reds on any group it cannot classify, from either side, and throws
rather than reporting an all-clear from an empty scan (ADR 0092). UNTABLED_GROUPS
records adr and spend as tracked gaps (#5294) instead of leaving them silent.

eval gets a codes.ts: MALFORMED_DOCUMENT and ZERO_SCOPE imported from the base,
INTEGRITY_VIOLATION and RUNS_NOT_EXECUTED on its own. All 21 process.exit sites
are re-seated — proven outcomes onto the table, usage errors and failed reads onto
verb.ts's FAILED.
@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. (a6f7d48)
  • web — Stage pr-5295 torn down.

@usirin

usirin commented Aug 10, 2026

Copy link
Copy Markdown
Member Author

review-code: PASS @ a6f7d48 — merge-ready

Verified against issue #5213's acceptance criteria, one criterion at a time, from the PR head
materialized read-only into a throwaway worktree (per-run ref; no working tree switched).

Routing. pipeline-cli class-probe classify --namespaces at this head → has-code → the single
required namespace is review-code. No apps/web/src/** path, so review-design is neither
required nor dispatched. Coverage self-check: 1/1 required namespace carries a SHA-bound marker.

§CP, both axes — not control plane. cp-classify classify
not-control-plane [path-clear-no-content-source]: no path matches the live CONTROL_PLANE_RE
(packages/fabrika-cli/** matches no packages/ §CP branch in live CODEOWNERS), and the diff
carries no .decisions/** file, so ADR 0164's content clause has nothing to decide.
This PR is on the auto-merge lane: this PASS binds and ships with no human approval.

Acceptance criteria

  • [PASS] eval/codes.ts exists and seats proven outcomes at 3+, following a sibling's shape —
    MALFORMED_DOCUMENT and ZERO_SCOPE are imported from report/codes.ts
    (BAD_SECTIONS = 4, NO_TARGET = 7, read at head), not re-typed numerals — the review-ui
    discipline, and the reason the alignment checker can verify them at all. INTEGRITY_VIOLATION = 12
    and RUNS_NOT_EXECUTED = 13 are its own, and the base-collision check for eval passes.
  • [PASS] Every proven-outcome process.exit re-seated off 1; failure-to-run sites stay on 1
    all 21 sites read individually at head; GATE_FAIL_EXIT_CODE is gone (grep at head returns
    nothing). See the seat audit below.
  • [PASS] eval appears in exactly one registry — ALIGNED_GROUPS.eval = EVAL_SEATS, absent from
    UNALIGNED_GROUPS and UNTABLED_GROUPS; the test at lines 119-126 asserts that disjointness.
  • [PASS] eval is in TABLES and the coverage test passes with it registered — the alignment test
    file runs 44/44 green at this head.
  • [PASS] Removing the registration makes the coverage test red — independently reproduced, not
    taken on report
    (below).
  • [PASS] pnpm typecheck and the fabrika-cli suite pass — independently re-run at this head:
    tsgo -p tsconfig.json clean (direct, uncached), suite 195 files / 2825 tests passed.
    Matches the PR body's numbers. CI at this head is green on every required check, including
    lint / format / typecheck, packages unit tests and produce run-evidence bundle.

The central claim: the guard can now SEE — verified as a class, not as an instance

This is guard blindness — the green was affirmative and wrong — so it was gated on whether the
class is closed, not on whether eval happens to be fixed. Four experiments, all run in the
disposable head worktree, all reverted (git status --porcelain clean afterwards).

  1. The blindness is real (pre-fix world reconstructed). Restored base exit-code-alignment.ts
    and its test from origin/main, deleted eval/codes.ts to recreate the pre-fix tree, ran the
    base guard: 34/34 passed — green over an unregistered, untabled group. The old assertion
    expect([...onDisk].sort()).toEqual([...registered].sort()) scoped itself to directories holding
    a codes.ts, so eval was absent from both sides and the equality held. Confirmed at source,
    not inferred.

  2. The reported regression reproduces exactly. Removed eval: EVAL_SEATS from ALIGNED_GROUPS
    at head and re-ran:
    AssertionError: expected [ 'eval' ] to deeply equal [] at exit-code-alignment.unit.test.ts:63.
    Byte-for-byte the failure the PR body claims. The author's argument for why the old assertion
    could not produce it is correct, and (1) proves it rather than arguing it.

  3. The next instance of the same blindness is caught (the class). Constructed a hypothetical
    new
    group — appended {name: "gecit"} to registeredGroups in registry.ts, shipping no
    codes.ts and registered nowhere — and re-ran:
    AssertionError: expected [ 'gecit' ] to deeply equal []. The guard reds on a brand-new
    untabled registry group.
    Scope genuinely comes from the shipped registry now: coverageGaps
    takes registeredGroups.map((g) => g.name) and reports unclassified over
    union(registered, onDisk) − classified, so a group is checked because the CLI ships it. The fix
    cures the class, not just eval.

  4. The obvious escape hatch is closed too. Registering that same table-less gecit in
    ALIGNED_GROUPS does not buy silence — 4 tests red (tableMissing reports it, and the
    per-group describe.each fails "has a module to check"). The only quiet path left is an explicit
    UNTABLED_GROUPS entry, which the test forces to carry a reason and forbids from also shipping a
    table — an admission with a name on it, which is the point.

ZeroCoverageScope (ADR 0092). Throws on an empty scan on either side —
registered.length === 0 and onDisk.length === 0 each throw before any gap is computed, so a
registry that failed to load reports a throw rather than "zero groups, therefore zero gaps". Pinned
by the test at lines 99-106 in both directions. coverageGaps reds on all five named conditions:
unclassified from the registry side (exp. 3) and from the on-disk side (test at line 93),
unshipped, untabledWithTable, tableMissing (exp. 4).

The 21 re-seated exit sites — audited individually, and a correction to the PR body

Every site read at head and judged proven-outcome vs failure-to-invoke. The seating is correct at
all 21
. But the PR body's tally is wrong, and the record should say so.

code PR body claims actually at head
MALFORMED_DOCUMENT (4) 8 6
ZERO_SCOPE (7) 2 2
INTEGRITY_VIOLATION (12) 1 1
RUNS_NOT_EXECUTED (13) 1 1
FAILED (1) 9 — "six usage errors, three read-failed catch arms" 11 — six usage errors, FIVE read-failed catch arms

Total is 21 either way; the split is not. The five read-failed catch arms are check
(ManifestUnreadable), report (RowsUnreadable), cases and run (EvalSetUnreadable), and
keeps (KeepsUnreadable) — the body names three. All five are genuinely the fail-closed unknown
case, not a proven failure mislabelled
: each is reached only by Effect.mapError off a
fs.readFileString that produced no bytes, so the verb never got to judge anything — exactly what
verb.ts reserves 1 for, and what the issue's own triage note blesses. Two of them are worth
naming because they are the least obvious and still check out: run's arm also catches a
--json-schema file that could not be read, and keeps's also catches the provenance ledger at a
path derived by default — both are still "no bytes in hand", not a proven fact.

The six FAILED sites that are not catch arms are all flag-value usage errors (--baseline-stage,
--baseline-surface x3, --stage, --arms) — usage errors are what 1 is reserved for, so they
are right where they belong.

Consumers of the old codes: none found. Nothing outside the package scripts fabrika eval exit
codes — no shell script, no workflow, no test asserts an exit status from these verbs; eval/README.md
documents only "non-zero", never a specific code. So no caller's contract is silently broken by the
re-seating. This is why the finding below is not blocking.

Finding (non-blocking, but please fix the body before merge). The mapping table in ## What changed is a stated-as-verified count that does not match the diff, and the PR body becomes the
squash commit message — a future reader auditing "which sites stayed on 1" will find eleven where
the record promises nine. The code needs no change; editing the PR body does not move the SHA, so
this verdict stays bound. Correcting it is a body edit, not a repair round.

Deviations (§DEV)

  • [PASS] deviation-disclosure — ## Deviations is a literal heading, and all three entries are
    true and unsoftened, checked at source:
    • adr / spend recorded not fixed, with the self-collision claim verified:
      adr/relate-verb.ts:15 seats NO_SUBJECT = 3 and adr/sweep-verb.ts:17 seats it 4.
      spend likewise allocates per-verb (read-verb.ts 3/4/5, rollup-verb.ts 3/4/5/6). Follow-up
      fabrika adr and spend allocate exit codes per-verb instead of from a group table #5294 exists and is open, and carries the collision. The disclosure understates nothing.
    • eval/command.ts still exits directly rather than returning VerbOutcomes — true; it imports
      only FAILED from verb.ts and every site is a bare process.exit.
    • No ADR — true; the diff carries no .decisions/** file.
  • Nothing undisclosed that this gate could see. (The tally correction above is a body-accuracy
    finding, not a §DEV class — it discloses no departure, it miscounts one.)

Other sub-gates

  • glossary-freshness: not applicable — no new feature folder / public package / public export
    (detector expressible here: 50 candidate surfaces on base).
  • containment: no marker on fabrika eval sits outside the exit-code alignment discipline and the guard cannot see it #5213 — no-op, contributes no row.
  • session-caching staleness: 0 candidate lines.
  • unresolved review threads: scan ran, none unaccounted.
  • comment discipline: the added comments are load-bearing — each states a why with no other home
    (why MALFORMED_DOCUMENT is deliberately not 1, why INTEGRITY_VIOLATION earns its own seat,
    why scope moved to the registry, why UNTABLED_GROUPS is an admission rather than an exemption).
    No separators, no name-restaters, no ADR re-derivation. Clean.

Notes, not gates

  • Sibling lane fix(fabrika): prove push containment on the force path, never on the lease alone #5289 touches packages/fabrika-cli/src/build/** and claude-plugins/fabrika/**
    no file overlap with this PR, and it does not touch exit-code-alignment.ts or its test. No
    textual conflict risk. If fix(fabrika): prove push containment on the force path, never on the lease alone #5289 adds a private build code colliding with the base, the existing
    alignment test catches it in whichever PR lands second; that is the guard doing its job, not a
    conflict this PR creates.
  • capture and delegate (named in the issue's triage note as table-less) are not in
    registeredGroups, so the registry-scoped guard does not see them — correct by design: a group the
    CLI does not ship has no exit surface to discipline.
  • Minor: expect(reason).not.toEqual("") would accept a whitespace-only UNTABLED_GROUPS reason.
    Not worth a round-trip; noting it only so it is on the record.

review-code does not merge. This is a merge-ready signal, nothing more. ship-it is the
authorized merge step; merging auto-closes #5213 via Fixes #5213. The issue is left as-is.

Verdict-written: 2026-08-10T06:14:14Z

@usirin
usirin added this pull request to the merge queue Aug 10, 2026
Merged via the queue into main with commit 830b041 Aug 10, 2026
46 checks passed
@usirin
usirin deleted the usirin/eval-exit-code-alignment-5213-E0A29804 branch August 10, 2026 06:27
@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 eval sits outside the exit-code alignment discipline and the guard cannot see it

1 participant