Skip to content

feat(fabrika): implement the handoff contract's four verbs (#5025) - #5350

Merged
usirin merged 2 commits into
mainfrom
usirin/handoff-verbs-5025-CEE6894B
Aug 10, 2026
Merged

feat(fabrika): implement the handoff contract's four verbs (#5025)#5350
usirin merged 2 commits into
mainfrom
usirin/handoff-verbs-5025-CEE6894B

Conversation

@usirin

@usirin usirin commented Aug 10, 2026

Copy link
Copy Markdown
Member

Fixes #5025

fabrika handoff <capture|take|read|claim>, built against the landed
claude-plugins/fabrika/skills/handoff/contract.md
(PR #5293). The contract is the spec; where it proved under-determined I took the conservative branch
and surfaced the clause on #5025 rather than inventing over it.

What landed

  • packages/fabrika-cli/src/handoff/ — a pure core per verb plus a thin <verb>-verb.ts entry,
    each with a *.unit.test.ts beside it, assembled by handoff/command.ts. Same shape as map
    (feat(fabrika-cli): implement the wayfinding contract's map verbs #5342) and grill (feat(fabrika): implement the grilling contract's five verbs in packages/fabrika-cli (#5023) #5341). One handoff.cli.test.ts for the two facts no in-process test can
    establish: that the group is reachable by its registration alone, and that a refusal really leaves
    stdout empty across the process boundary.
  • handoff/codes.ts — the exit table. The eight seats shared with report/codes.ts are
    re-exported under aliases, never restated as numerals; 10 is held as DELIBERATE_GAP
    (no verb accepts a label flag, writes a label, or composes a title); 1215 are this group's own.
  • wire/handoff-pack.ts — the pack document as a registered wire format, with its
    src/wire/registry.ts row (fixtures + brands) and its ### handoff-pack narrative section.
  • io/issues.ts / io/pulls.ts — two appended reads: the repository's default branch, and the
    pull requests on a branch. No second path to either resource is opened.

The exit table

Code Meaning Source
0 the answer is on stdout verb.ts
1 / 2 / 127 usage error / no implementation resolved / never ran reserved by the interface convention
3 EMPTY_STDIN stdin was read and held nothing re-exported from report/codes.ts
4 BAD_SECTIONS a section is missing, out of order or empty, or content sits outside the closed set re-exported (stated widening)
5 LEAKED_PATH the composed document carries a machine-local path re-exported (unconditional here — no --redact)
6 BARE_AT_PATH a section is a bare @ path reference re-exported
7 NO_TARGET proven: the issue does not exist re-exported
8 WRITE_UNKNOWN a write was attempted and its outcome is UNKNOWN re-exported
9 READBACK_MISMATCH the write landed and the read-back differs re-exported
10 VACATED — held as DELIBERATE_GAP, deleted from HANDOFF_SEATS on the UI_SEATS precedent re-exported
11 PRECONDITION_UNKNOWN a precondition READ FAILED and nothing was written re-exported
12 WORK_UNREACHABLE proven: the work is unreachable by a successor and the loss was not declared this group's
13 NO_PACK proven: the issue carries no sealed pack to claim this group's
14 PACK_MALFORMED proven: a sealed pack exists and does not parse, or its digest disagrees this group's
15 PACK_CLAIMED proven: another nonce holds the latest pack's claim this group's

Every proven verdict sits at 3+ with empty stdout (refuse hardcodes that), and every refusal is
asserted in a unit test by exit code and by the bytes on each channel. 1 / 127 mean the call
never decided; 11 means a read failed before any write; 8 means a write was attempted and its
outcome is UNKNOWN. The three are three seats with three remedies.

Shared registration files — insertion-only

A concurrent lane (#5024, the prototyping group) is on the same files, so every edit here is a
pure insertion with zero deleted lines. git diff --numstat on the base, per file:

File +
packages/fabrika-cli/src/registry.ts 2 0
packages/fabrika-cli/src/exit-code-alignment.ts 14 0
packages/fabrika-cli/src/exit-code-alignment.unit.test.ts 2 0
packages/fabrika-cli/src/wire/registry.ts 60 0
claude-plugins/fabrika/docs/wire-formats.md 17 0
packages/fabrika-cli/README.md 40 0
packages/fabrika-cli/src/io/issues.ts 15 0
packages/fabrika-cli/src/io/pulls.ts 54 0

The wire-formats generated region was regenerated with fabrika wire index --write, never
hand-merged. The README gained a ## The handoff group section and nothing else was reflowed.
HANDOFF_SEATS is authored in exit-code-alignment.ts as an alias of GRILL_SEATS, which the two
groups reach independently: both name 7 NO_TARGET and both hold 10 empty.

Verification

  • pnpm typecheck in-package (packages/fabrika-cli) — clean.
  • pnpm vitest run in-package — 235 files, 3393 tests, all passing (80 of them this group's).
  • pnpm lint:worktree — clean.

Deviations

dev-tier-m.sh reported one suppression/skip line and zero removed-assertion lines.

  • The one hit is a known false positive, disclosed rather than suppressed. The scan's bare
    xit\( alternative matches inside process.exit( in
    packages/fabrika-cli/src/handoff/command.ts's emit adapter. Those bytes are byte-identical to the
    same adapter in every sibling group (map/command.ts, grill/command.ts, report/command.ts …) —
    it is the module-private emit the contract says each group declares for itself. No test is
    skipped, no lint rule is suppressed, and no @ts-expect-error / biome-ignore appears anywhere in
    this diff.

  • (repair round 1) Rebased onto main after the sibling spike lane (feat(fabrika): implement the prototyping contract's spike verbs (#5024) #5349) landed; nothing
    else changed.
    The four shared files collided textually only in packages/fabrika-cli/README.md;
    registry.ts, exit-code-alignment.ts and exit-code-alignment.unit.test.ts auto-merged. The
    README conflict was resolved additivelymain's ## The spike group section kept exactly
    where it sits, this branch's ## The handoff group section placed after it, no prose reflowed and
    no line dropped. The insertion-only table above still holds on the rebased diff
    (git diff --numstat origin/main HEAD reports 0 deletions in every file). The wire-formats
    generated region was regenerated with fabrika wire index --write and produced zero diff.
    Re-verified after the rebase: in-package pnpm typecheck clean, pnpm lint:worktree clean, and
    the in-package suite now 243 files / 3530 tests passing (the growth is the spike group arriving
    on the base, not new tests here). dev-tier-m.sh re-run on the rebased diff reports the same
    one suppression/skip line — the process.exit( false positive already disclosed above — and zero
    removed-assertion lines, so no new deviation fired this round.

  • (repair round 2) Acted on a review-code FAIL bound to the pre-rebase head. The verdict was
    written @ 401d0538; the round-1 rebase had already moved the head to 4903796, so the
    SHA-binding read stale and repair mode's own idempotency check resolved "nothing to repair". The
    finding was acted on anyway, because the rebase touched no line of
    packages/fabrika-cli/src/handoff/take-verb.ts and both bare 4 literals were re-read
    first-party at 4903796 before the edit. Disclosed here rather than quietly relying on the
    dispatch brief.

  • (repair round 2) The claim "re-exported under aliases, never restated as numerals" is now true
    at call sites, not only in handoff/codes.ts.
    As written above, that sentence describes the
    seat tablehandoff/codes.ts re-exports the eight shared seats and restates none of them —
    and it was accurate about that file the whole time. It read, fairly, as a claim about the group
    as a whole, and take-verb.ts had two call sites passing a bare 4 where the imported
    BAD_SECTIONS belonged. Those two are fixed (2 insertions / 2 deletions, one file, no
    behaviour change, no message change, no test change), so the sentence now holds on both readings.
    The earlier text is left standing rather than reworded: exit-code-alignment checks seat maps
    against exported constants and never inspects call sites, so the gap the reviewer found is real
    and worth keeping legible in the log.

Four contract clauses proved under-determined. Each is surfaced on #5025, and each was resolved
by the conservative branch rather than by an invented answer:

  1. git.base.branch's default has no source as written. The contract derives it from "the
    repository's default branch from the getIssue repo payload", but repos/{owner}/{repo}/issues/{n}
    returns no repository object — the field does not exist. Taken: a dedicated repos/<repo> read,
    whose failure is 11, never a guessed main.
  2. 6 BARE_AT_PATH is unreachable as ordered. The section check runs before the leak scan, and
    the shipped isBareAtReference tests only a body's first token — so a stdin that is a bare
    @path fails 4 first, and a composed document always opens with the marker. Taken: the
    predicate is applied per asserted section, which is the one reading that keeps the seat
    reachable and its remedy distinct.
  3. What happens to rows 14–19 when packedBranch is gone. The contract states rows 3–10 report
    live: null / moved and says nothing about the board rows. Taken: they are still compared — the
    pull request is selected by the packed branch's head ref and stays readable after a local branch
    is deleted, and dropping it would hide the one place a successor can still see what happened.
  4. A newest pack that is both malformed and unauthorized. The contract says a malformed pack is
    14 and never a disregarded row, and that an unauthorized pack is one. Taken: parse first, so
    the 14 refusal wins — the fail-closed direction.

One known absence, met as recorded: src/eval/corpus.ts's STAGES has no stage a handoff eval
entry could decode (#5241, owned by #4649's harness), so the skill-conventions §8 gate-3 leg is
blocked, not skipped. Nothing here widens SHIP_NAMESPACES or the verdict-marker namespace gates.

🤖 Generated with Claude Code

@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. (35225fc)
  • web — Stage pr-5350 torn down.

`fabrika handoff <capture|take|read|claim>` — the fabrika half of the
continuity story, built against the landed
`claude-plugins/fabrika/skills/handoff/contract.md`.

- `handoff/` holds a pure core per verb plus a thin `<verb>-verb.ts`
  entry, each with a `*.unit.test.ts` beside it, assembled by
  `handoff/command.ts` and registered in `src/registry.ts`.
- `handoff/codes.ts` re-exports the eight seats it shares with
  `report/codes.ts` under aliases, holds `10` as a deliberate gap, and
  allocates `12`-`15` for facts about a pack.
- The pack document lands as the registered wire format `handoff-pack`,
  whose closed section set is the injection defence a coordination
  artifact needs.
- Two shared reads are added to `io/`: the repository's default branch
  and the pull requests on a branch.

Fixes #5025
@usirin

usirin commented Aug 10, 2026

Copy link
Copy Markdown
Member Author

review-code: FAIL @ 401d053

Gated against issue #5025 and the landed claude-plugins/fabrika/skills/handoff/contract.md, read
first-party at this head rather than from the PR description. Verified read-only against a per-run
ref; nothing in this review mutated a working tree.

Required namespacespipeline-cli class-probe classify --namespaces over the 31 changed
files: has-code only, so review-code is the one required gate. No claude-plugins/*/skills/
or agents/ path, and the single claude-plugins/ file (docs/wire-formats.md) is carved out of
the docs surface, so review-skill and review-doc are not required and emit nothing.

Control-plane classification, both axes. Path axis: cp-classify classify returns
not-control-plane (path-clear-no-content-source, exit 3). Content axis (ADR 0164): VACUOUS
the diff carries no .decisions/** file, so guard-content-probe has nothing to classify. That is
one clear axis and one that does not apply, not two confirmations. Ordinary lane, so this verdict
takes the SHA-bound marker form.


One blocking defect

Two bare 4 numerals where the re-exported seat belongs

packages/fabrika-cli/src/handoff/take-verb.ts seats exit 4 three times. One uses the imported
constant; two restate the numeral:

  • line 121 — the ## Unsure is empty refusal: refuse(4, ...)
  • line 127 — the content-outside-the-closed-set refusal: refuse(4, ...)
  • line 132 — the shape refusal: refuse(BAD_SECTIONS, ...) (correct)

BAD_SECTIONS is already imported at line 32 of the same file and used 5 lines below the second
hit, so this is a slip rather than a design choice. It is the only such site in the group: every
other seat in take-verb.ts, read-verb.ts, claim-verb.ts, capture-verb.ts and guards.ts
goes through a named constant.

Why it blocks rather than rides as a nit. The contract states the rule normatively and states
its purpose in the same sentence — "3-11 are imported from src/report/codes.ts, not restated
as numerals
, so a drift is unrepresentable rather than merely detectable". Two bare numerals make
that drift representable: were report/codes.ts's BAD_SECTIONS to move, one verb would emit two
different codes for three flavours of the same fact. exit-code-alignment does not cover this — it
checks the seat map against a group's exported constants, never against call sites — so nothing else
in the suite catches it. The PR description also affirmatively claims the opposite ("re-exported
under aliases, never restated as numerals"), so the disclosure does not cover it either.

Repair: swap the two literals for BAD_SECTIONS. Two lines, no behaviour change, no test
churn. Re-request on the new head and this flips.

Everything else below passed.


The six load-bearing judgments

1. 10 treated as VACATED — CORRECT, and the precedent is real

Both halves are contract-ordered, not invented: the shared matrix seats 10 as
DELIBERATE_GAP - held empty, and the registration-burden list says in as many words that the gap
seat is DELETED from the seat map "on the UI_SEATS precedent". The PR does exactly both -
handoff/codes.ts exports DELIBERATE_GAP = REPORT_CLASSIFIED, and HANDOFF_SEATS omits the key.

The precedent is real and shipped three times over in src/exit-code-alignment.ts: UI_SEATS
destructures EMPTY_STDIN out of BUILD_SEATS; MAP_SEATS destructures OFF_VOCABULARY and
ZERO_SCOPE out; GRILL_SEATS is written without the classification seat. Keying the gap instead
would look up a name the base does not carry and red exit-code-alignment.unit.test.ts.

Vacating is right here rather than seating something. The base's 10 CLASSIFIED fires when a
title or a label carries a type or priority classification. I read the group's whole flag surface in
handoff/command.ts: --repo, --issue, --base, --nonce, --declare-unreachable. No label
flag, no verb writes a label, no verb composes a title - so the condition is genuinely unreachable,
not merely unexercised. Seating a new meaning on 10 would be the wrong move for a group that
aligns to the base at all: an aligned group re-using an occupied seat for a different fact is the
collision the alignment table exists to catch, and the group already has clean space at 12-15.

One thing I checked and cleared: HANDOFF_SEATS is an alias (= GRILL_SEATS), not a fresh
8-key literal, where the contract says "authored in that same file". It is declared in that file and
the coupling fails closed: a grill-side change to the map would no longer match handoff's own
codes.ts exports, yielding a SeatDrift and a red suite. Safe, and the docblock states the shared
derivation rather than leaving it implicit. Not a defect.

2. Both widenings - CORRECT, and both are the contract's, not the coder's

4 BAD_SECTIONS widened to "content outside the closed section set". Contract-mandated verbatim
at the THE-FOUR-WIDENING anchor, which also rules that the seat keeps the base's name and number
(explicitly contrasting review-ui, which renames its seat for a comparable widening). The
implementation matches: asserted.ts returns an Outside problem for both a stray line before the
first heading and any heading outside the four, and codes.ts declares the second meaning on the
export rather than carrying it silently - which is the specific thing the contract asks for, since
"an imported constant quietly carrying a second meaning is the drift the import exists to stop".
The trigger set grows and nothing is removed, so it moves fail-closed. Sound.

5 LEAKED_PATH unconditional. Also contract-stated: the base seat reads "...and --redact was
not given", and the contract rules that since no handoff verb offers --redact, 5 fires on any
machine-local path unconditionally - "the condition narrows; the meaning does not drift". Verified
both halves: no --redact flag exists anywhere in handoff/command.ts, and guards.ts's leakFree
has no redact branch - any scanBody hit refuses. This is the strictly safe direction for a verb
whose entire output is posted to a public issue, and it is the right call to make it unconditional
rather than to add a --redact the contract never specified.

Both widenings are declared at the export site, which is what keeps them auditable. No objection.

3. The 6 BARE_AT_PATH reachability fix - claim VERIFIED BY EXECUTION, fix is the right branch

I did not take the claim on report. I ran the shipped modules at this head directly.

Leg A - a bare-@ stdin never reaches a leak scan. The contract orders the section check (step
2) before the leak scan (step 6), so the interesting input dies first:

parseAsserted("@notes/handoff.md")
  -> {"_tag":"Problem","problem":{"_tag":"Outside","heading":"@notes/handoff.md"}}     => exit 4

Leg B - the composed document can never fire the predicate. isBareAtReference tests
body.trim().split(/\s/)[0] - the first whitespace-delimited token and nothing else. Composing a
pack whose four sections are all bare @ references:

first token of composed document   -> "<!--"        (the pack marker, always)
isBareAtReference(composed document) -> false

Leg C - and the asserted half as a whole is no better, because it always opens with ## Intent:

parseAsserted(all-four-sections-are-@paths) -> "Asserted"     (parses clean)
isBareAtReference(whole asserted half)      -> false          (first token is "##")
per-section isBareAtReference               -> intent=true established=true nextAct=true unsure=true

So the claim is true, and Leg C is the teeth of it. Under the contract's literal ordering, seat
6 is not merely hard to reach - it is unreachable on both inputs the contract points the scan at,
and a document with @notes/handoff.md under every heading posts cleanly. That is #3086's exact byte
pattern reaching a public artifact, which is the incident seat 6 was allocated for.

Grading the fix: the right branch, not an over-reach into spec territory. Four reasons.

  1. It restores a normative seat the contract enumerates in its shared matrix, its take exit
    table, and its Errors table. Leaving 6 structurally dead would have shipped a spec'd refusal
    that can never fire - worse than the deviation, and silently so.
  2. It moves only the 6 predicate. The 5 machine-local scan still runs last over the whole
    composed document, so the contract's stated reason for the ordering ("the proven half does not
    exist until step 4") is preserved intact. The change is surgical, not a re-ordering of the check
    list.
  3. It is fail-closed: the per-section predicate strictly adds refusals and removes none. No input
    that previously sealed now fails except the one the seat exists to catch.
  4. The apparent narrowing is not a dropped requirement. The Errors table has a
    <asserted half|derived ground state> variant, and under this fix only the asserted half can fire
    6. But isBareAtReference is first-token-only by design - it asks "did the body ever
    arrive at all". The derived ground is a JSON object the verb composes itself, so "the derived
    ground state is a bare @ reference" is not expressible by that primitive under any ordering.
    That branch of the message row is a contract artefact, not a capability the fix removed.

The coder also did the right thing on the boundary: it did not edit contract.md (out of scope
per #5025) and surfaced the clause on the issue instead. Correct handling.

One cosmetic follow-on, not blocking: guards.ts's leakFree still runs isBareAtReference as
its first branch, and at both of its call sites in take-verb.ts that branch is now provably dead
(Legs B and C above). It is harmless and fail-closed, but a reader will assume it is the live 6
path when the live one is the per-section check 90 lines earlier. Worth a comment or a removal
whenever this is next touched.

4. git.base.branch has no source - API claim CONFIRMED, resolution CORRECT

The contract's field-9 derivation is "--base, defaulting to the repository's default branch from
the getIssue repo payload". I read the live payload rather than trusting either side:

GET repos/{owner}/{repo}/issues/5025   ->   keys include repository_url
                                            keys do NOT include repository

There is no repository object on the single-issue endpoint - only a repository_url string,
from which no default branch is derivable without a second read. So the contract's stated source
does not exist, and the clause is genuinely under-determined. The claim is true.

The resolution is right. io/issues.ts adds a dedicated repoDefaultBranch reading
repos/{repo} for .default_branch, and both capture and take seat its failure at 11 with
"nothing was written". Two things make this the correct branch rather than a convenient one:

  • It refuses instead of guessing main. A guessed base is a digested and compared field, so a
    wrong guess would seal a pack asserting a base it never read, and the successor's drift check would
    then fire or not fire on the guess. That is exactly the "renders an unreadable source as a
    plausible healthy value" failure ADR 0092 and the contract's A-FAILED-READ-IS-NEVER-A-VALUE
    anchor both forbid.
  • It also refuses on a zero-length answer, not just a non-zero exit: "gh api exited 0 but named
    no default branch" is its own failure. That is the class-probe classify reads 0 files under parallel invocation, silently defaulting to has-code #4060 shape closed properly.

Surfaced on #5025 rather than patched into the contract. Correct handling.

5. The exit table - the three-way split HOLDS; one defect, already named above

  • Eight shared seats re-exported, never re-typed. handoff/codes.ts imports all eight from
    report/codes.ts under aliases (EMPTY_STDIN, BAD_SECTIONS, LEAKED_PATH, BARE_AT_PATH,
    NO_TARGET, WRITE_UNKNOWN, READBACK_MISMATCH, PRECONDITION_UNKNOWN), plus
    DELIBERATE_GAP = REPORT_CLASSIFIED for the vacated seat. Own seats 12-15 are integer literals
    at their point of allocation, which is correct - they are allocations, not re-exports.
  • Bare numerals where a re-export belongs: two, both 4, both in take-verb.ts. This is the
    blocking defect above. Every other seat across all four verbs plus guards.ts uses a named
    constant; I checked each site.
  • The three-way split holds, structurally rather than by convention.
    • 1 / 127 - never decided. 1 is FAILED from verb.ts, used only for the nonce-grammar
      usage error and an unreadable stdin; 127 is the interface convention's unresolved-binary seat.
    • 11 PRECONDITION_UNKNOWN - a precondition read failed, nothing written. Seated on the default-
      branch read, the ground derivation, the comment paging and the ACL read. Every one of those sites
      is upstream of the only two createComment calls in the group, so "nothing was written" is a
      property of the control flow, not a claim in a message.
    • 8 WRITE_UNKNOWN - a write was attempted, outcome UNKNOWN. Seated only on a failed
      createComment, in take and claim. 9 correctly sits beside it for the write that landed
      and read back differently, in both verbs.
  • Every proven verdict is >= 3 with empty stdout - by construction, not by discipline.
    verb.ts's refuse hardcodes stdout: "" and answer hardcodes code 0, so a non-zero exit
    carrying a payload is not constructible. 12-15 all route through refuse.
  • read's none at 0 versus claim's 13 is implemented as the contract splits it, and each
    verb's header states the reasoning at the point a reader meets it. read returns the none token
    on stdout at exit 0; claim refuses 13. Correct - a booting successor is not handed empty
    stdout on the ordinary case.

6. The blocked eval leg - treating it as BLOCKED is correct, and it does NOT hold up this PR

Verified the premise first-party at this head:

src/eval/corpus.ts:32   export const STAGES = ["triage", "build", "review", "ship-it"] as const;

No ideation stage, so there is no key under which a handoff eval entry could decode. The
skill-conventions §8 gate-3 leg is unrunnable for a structural reason in a file this ticket is
explicitly scoped out of.

Blocked, not skipped, is the right framing and it is the contract's own: the contract pre-records
this absence and says in as many words that the leg "is understood to be blocked, not skipped".
A skip would be this lane declining a leg it could run; this is a leg with no landing surface. The
distinction matters because a skip silently shrinks the gate while a block leaves a visible edge -
and the PR describes it as "one known absence, met as recorded", which is the honest form.

It should not hold up this PR. The gap is corpus-wide across the whole quintet, the harness is
owned by #4649, and #5241 is a live type:decision / status:triaged ticket. Blocking here would
convert a corpus-level open decision into a per-child stall for four sibling groups, and this lane
could only clear it by editing a file #5025 puts out of scope.

#5241 recorded UNKNOWN. Whether the corpus should gain an ideation stage, or the quintet should
declare entries some other way, is not this lane's to answer and this review does not answer it.


Insertion-only discipline (vs. the concurrent sibling lane)

Zero deletions, verified two independent ways. The GitHub files API over all 31 changed files,
and git diff --numstat origin/main...HEAD in a clean throwaway checkout of this head. Both agree:
every one of the 31 files is +N / -0. That includes all eight shared registration files the
sibling lane also appends to:

Shared file + -
packages/fabrika-cli/src/registry.ts 2 0
packages/fabrika-cli/src/exit-code-alignment.ts 14 0
packages/fabrika-cli/src/exit-code-alignment.unit.test.ts 2 0
packages/fabrika-cli/src/wire/registry.ts 60 0
claude-plugins/fabrika/docs/wire-formats.md 17 0
packages/fabrika-cli/README.md 40 0
packages/fabrika-cli/src/io/issues.ts 15 0
packages/fabrika-cli/src/io/pulls.ts 54 0

No deletion anywhere to name. If the sibling lane lands first, this rebases cheaply.

Wire-formats region re-generated independently. I ran fabrika wire index --write myself at this
head:

wire index: judged claude-plugins/fabrika/docs/wire-formats.md (12352 bytes) against 8 registered formats.
index	written	8	8

...and git status --porcelain came back empty. The committed region reproduces byte-for-byte
from the registry - it was generated, not hand-merged, so a sibling row landing first regenerates
cleanly rather than conflicting on hand-written prose.

## Deviations - the one disclosed hit re-run and confirmed

Ran the Tier-M scan against this PR:

deviation-disclosure: ## Deviations section present
deviation-disclosure: Tier-M scan - 1 suppression/skip line(s), 0 removed-assertion line(s)
DEV_SUPPRESS_LINE=781:+		process.exit(outcome.code);

Exactly the disclosed count and exactly the disclosed line. False positive confirmed: the scan's
bare xit\( alternative matches inside process.exit(. Byte-identity claim confirmed - that
same line appears at column 2 in the emit adapter of 17 other groups (ui, ledger, grill, adr,
review-ui, wire, plan, triage, status, map, epic, review, spend, report, build,
ship, hook). It is the module-private emit the contract says each group declares for itself.

Independently grepped the diff for @ts-expect-error, @ts-ignore, biome-ignore, eslint-disable,
.skip( and .only(: none. The ## Deviations section is honest, and correctly not None.

Acceptance criteria, one at a time

Re-ran everything in-package (never tsgo -p tsconfig.json from the repo root, per #5312).

  • Every contract verb implemented, and no verb it does not specify - PASS. capture, take,
    read, claim, exactly four, matching the contract's Verb inventory. No drift verb, no filing
    verb, no retirement verb, no push verb - each of which the contract's "Considered and deliberately
    not derived" list rules out by name.
  • Group shape - PASS. src/handoff/ holds a pure core per verb plus a thin <verb>-verb.ts
    entry, each with a *.unit.test.ts beside it, assembled by handoff/command.ts, plus one
    handoff.cli.test.ts for the two facts no in-process test can establish.
  • Registered, discoverable by registration alone - PASS. Two inserted lines in src/registry.ts
    and nothing else; no hand-maintained parallel list.
  • Verbs are pure functions of their dependencies - PASS. Every run* returns a VerbOutcome;
    the only process.exit and the only stream writes are in command.ts's emit adapter. Every
    refusal is asserted in-process by code and by the bytes on each channel.
  • Exit codes from verb.ts, proven outcomes at 3+, refusals empty on stdout - PASS structurally
    (refuse hardcodes empty stdout), with the two bare 4 numerals as the one blocking defect.
  • Each verb's header states whether empty is a fact or a failed read - PASS. capture: "There is
    no empty answer... board.pull being null is a fact". read: zero packs is a fact at exit
    0. take and claim state theirs likewise. No verb renders an unreadable source as healthy - I
    traced the default-branch read, the ground derivation, the comment paging and the ACL read, and all
    four seat 11.
  • All shared I/O through src/io/, no second path to a resource - PASS. The two additions
    (repoDefaultBranch, pullsForBranch) are appended to the existing io/issues.ts and
    io/pulls.ts rather than opened as a new path, and both reuse the module's own
    execCapture / pagedJson / parseJson seams.
  • ADR 0238 - no tether to v1 - PASS. Grepped every added line for pipeline-cli,
    kampus-pipeline and packages/pipeline-cli: zero hits. No import, no subprocess, no wrapper.
  • No wrapper verb, no second answer to a gated question - PASS. handoff is absent from
    SHIP_NAMESPACES, emits no verdict marker, and neither verdict-marker.ts's NAMESPACE regex nor
    its NAMESPACE_PREFIXES gate is widened - I confirmed no diff to that file. Nothing here can gate
    a merge, which is what keeps an interrupted session from becoming a blocked one.
  • Extends the shared surface rather than minting a parallel copy - PASS, and this is the cleanest
    part of the diff. HANDOFF_SEATS reuses GRILL_SEATS; the wire format takes a row in the existing
    registry; the two io/ reads are appended.
  • Under-determined clauses surfaced, not invented over - PASS. Four clauses named in the PR body,
    each resolved on the conservative branch, each surfaced on Implement the handoff contract's verbs in packages/fabrika-cli/ #5025. Neither SKILL.md nor
    contract.md is touched by this diff.
  • pnpm typecheck / pnpm lint green, new tests pass - PASS, re-run at this head:
    • pnpm typecheck (in packages/fabrika-cli) - clean, exit 0.
    • pnpm vitest run - 235 files, 3393 tests, all passing, matching the PR's stated numbers.
    • pnpm vitest run src/handoff - 9 files, 80 tests, so the "80 new tests" claim is exact.
  • Repo-relative paths only; no operator name or email - PASS. Grepped every added line for
    the absolute-home, home-rooted and tilde-relative path roots: zero hits.

One extra check worth recording

The contract prints the digest pre-image literally so a reader can compute the digest rather than
trust it (ADR 0247). I took it up - fed the contract's 19 printed pre-image lines to the shipped
bodyDigest:

lines: 19
bodyDigest(pre-image) = f9d0814b89b4
contract printed      = f9d0814b89b4

Exact match, and ground.unit.test.ts asserts preImage(WORKED) against the same transcribed
example, so the pre-image is pinned as a comparison rather than a claim. Related: COMPARED_FIELDS
is derived from DIGESTED_FIELDS by filter rather than hand-listed, which makes the 16-of-19
relationship structural - a good call the contract asked for but did not prescribe.

Verdict

FAIL at 401d0538dc91f66dd1a741a219e74f0ec4c4e9b9 on one mechanical defect: the two bare 4
numerals in packages/fabrika-cli/src/handoff/take-verb.ts (lines 121 and 127), where the imported
BAD_SECTIONS belongs and is already in scope.

This is otherwise a strong implementation. All four graded judgments I was asked to grade hardest -
the vacated 10, the two widenings, the 6 BARE_AT_PATH reachability fix, and the git.base.branch
resolution - are correct, and three of them are correct in the harder way: the coder found real
defects in the contract, took the fail-closed branch, surfaced them on #5025, and did not patch the
spec in code. The reachability finding in particular is a genuine catch that a less careful lane would
have shipped as a dead seat. Insertion-only discipline is clean, the generated region reproduces
byte-for-byte, and the deviation disclosure is accurate.

Swap the two literals and re-request; nothing else is outstanding from this gate.

@usirin
usirin force-pushed the usirin/handoff-verbs-5025-CEE6894B branch from 401d053 to 4903796 Compare August 10, 2026 17:33
…5025)

The three `take` refusals that share exit 4 now all name the imported
constant. Two restated the numeral, which makes a drift in
`report/codes.ts` representable — `exit-code-alignment` checks seat maps
against exported constants, never call sites, so nothing else catches it.

No behaviour change: same code, same messages, same tests.

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

usirin commented Aug 10, 2026

Copy link
Copy Markdown
Member Author

review-code: PASS @ 35225fc — merge-ready

Re-gate of #5350 against issue #5025 at the repaired head. Read the PR head (§HEAD): every file
under review was sourced from 35225fce through a per-run ref and a throwaway tree, never from the
launched checkout. This review mutated no working state, and no existing comment was edited or
deleted.

Required namespacespipeline-cli class-probe classify --namespaces over the 31 changed
files: 31 changed file(s) → has-code, one namespace, review-code. No skills/ or agents/
path, and the single claude-plugins/ file (docs/wire-formats.md) is carved out of the docs
surface, so review-doc / review-skill / review-design are not required and emit nothing.

Control-plane, both axes. Path axis: cp-classify classifynot-control-plane
(path-clear-no-content-source). Content axis (ADR 0164): VACUOUS — the diff carries no
.decisions/** file, so guard-content-probe has nothing to classify. One clear axis and one that
does not apply — not two confirmations. Ordinary lane, so this verdict takes the SHA-bound marker
form.


The blocking defect is discharged

The prior FAIL (@ 401d0538) rested on exactly one finding: two bare 4 numerals in
packages/fabrika-cli/src/handoff/take-verb.ts where the imported BAD_SECTIONS belongs. Verified
first-party at 35225fce:

  • All three BAD_SECTIONS seats now pass the imported constant — the ## Unsure-empty refusal
    (line 121), the content-outside-the-closed-set refusal (line 127), and the shape refusal (line
    132). BAD_SECTIONS was already in the import list at line 32; the repair did not touch it.
  • No bare numeral remains anywhere in the file. A refuse\([[:space:]]*[0-9] scan over the head
    blob returns zero matches — not just for 4, for any integer literal in a refuse seat.

The repair delta is exactly what was reported, and nothing else

The two-commit stat from 4903796e867f9a3695f9751bda984ee0979ba8eb to
35225fce9cf5fb2a6ca38b4ae6a3350791926a81:

 packages/fabrika-cli/src/handoff/take-verb.ts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

I read the full patch, not only the stat. The whole delta is two hunk lines — a bare 4, argument
replaced by BAD_SECTIONS, — at lines 121 and 127. No message change, no test change, no second
file, no opportunistic edit riding along. Confirmed: 1 file / +2 / −2 is the entire repair round.

The five judgments confirmed to survive the rebase + repair — all unchanged

Re-verified at 35225fce, not carried forward from the prior verdict.

  1. The vacated 10 / DELIBERATE_GAP — intact both halves. handoff/codes.ts still exports
    DELIBERATE_GAP = REPORT_CLASSIFIED with the module docblock stating why the seat is
    unreachable; HANDOFF_SEATS (an alias of GRILL_SEATS, whose literal omits the classification
    key) still carries eight keys with no DELIBERATE_GAP among them, per the UI_SEATS
    precedent.
  2. The 4 widening — intact. BAD_SECTIONS's docblock in handoff/codes.ts still declares the
    second clause ("content outside the closed set") at the export site, keeping the base's name and
    number and growing the trigger set fail-closed.
  3. The 5 widening — intact. LEAKED_PATH's docblock still states the unconditional firing and
    its reason (no handoff verb offers --redact); no --redact flag exists in
    handoff/command.ts.
  4. The per-section 6 BARE_AT_PATH predicate — intact and unmoved.
    Object.entries(asserted.value).find(([, body]) => isBareAtReference(body)) still runs per
    section, ahead of the composed-document leak scan, with the load-bearing comment explaining why
    testing either whole would make the seat unreachable (PR-create leaked a literal @/tmp body-file reference into a PR body (local-path leak) #3086).
  5. The dedicated git.base.branch read failing to 11 — intact on both legs. take-verb.ts
    seats a repoDefaultBranch failure at PRECONDITION_UNKNOWN with "nothing was written", and
    ground.ts's resolveCommit(options.base) failure propagates to the same seat.
    report/codes.ts still pins PRECONDITION_UNKNOWN = 11.

Insertion-only discipline — zero deletions, still holding

The sibling lane (#5351) is appending to several of the same files, so this matters more after a
rebase than before. Verified independently of the PR description: the GitHub files API over all 31
changed files sums [.[].deletions] | add to 0. Every file is +N / −0, including all eight
shared registration files (src/registry.ts, src/exit-code-alignment.ts,
src/exit-code-alignment.unit.test.ts, src/wire/registry.ts,
claude-plugins/fabrika/docs/wire-formats.md, README.md, src/io/issues.ts, src/io/pulls.ts).
Nothing was deleted anywhere in this PR.

## Deviations — appended, not rewritten; Tier-M re-run and confirmed

The section now carries two (repair round 2) entries appended below the round-1 and original
entries, all of which are still standing verbatim. Nothing was rewritten.

Tier-M scan re-run at this head:

deviation-disclosure: ## Deviations section present
deviation-disclosure: Tier-M scan — 1 suppression/skip line(s), 0 removed-assertion line(s)
DEV_SUPPRESS_LINE=781:+		process.exit(outcome.code);

Exactly the disclosed count and exactly the disclosed line — the scan's bare xit\( alternative
matching inside process.exit( in the module-private emit adapter. Same single false positive as
before the rebase, confirmed.
Zero removed-assertion lines. Independently scanned the added lines
for @ts-expect-error / @ts-ignore / biome-ignore / eslint-disable / .skip( / .only(:
none.

Verification re-run at this head

Never tsgo -p tsconfig.json from the repo root (#5312) — in-package throughout.

  • pnpm typecheck in packages/fabrika-cli — clean, exit 0.
  • pnpm vitest run in-package — 243 files / 3530 tests, all passing. This is the full
    in-package project, not a feature-scoped subset: the run-evidence bundle is non-present, and the
    degrade path requires the whole surface so a cross-cutting contract test cannot slip past
    (ADR 0092).
  • pnpm biome check over packages/fabrika-cli/src and claude-plugins/fabrika/docs — 611
    files checked, clean.

Run-evidence bundle: PENDING for head 35225fce — producer run is in_progress, not completed (queried head_sha=35225fce, 1 run-evidence run(s) at head, producer run 31416246429). PENDING is not ABSENT: the producer has not published for this head yet. — verified from diff + local run

unresolved-threads: the read ran and found no unresolved review thread on this PR.


The process judgment: proceeding was RIGHT

The coder found the FAIL verdict already stale — bound to @ 401d0538 while the round-1 rebase had
moved the head to 4903796 — so pipeline-cli verdict read resolved CODE_FAIL=0 and repair
mode's own rule said "nothing to repair, stop." It did not stop. That was the correct call, and
narrowly so.
Three things make it correct rather than an agent talking itself past a gate:

  1. It did not act on the stale verdict's authority — it re-derived the finding first-party. The
    distinguishing move is that it re-read take-verb.ts at the live head through the contents API
    and observed both bare 4s still present. The stale marker was the pointer; the live read was
    the evidence. An agent that had merely trusted a stale FAIL and edited would have been wrong
    even if the edit happened to be right.
  2. The signal it overrode was a mechanical SHA mismatch, not a substantive "this is fine."
    CODE_FAIL=0 here meant "no FAIL marker exists at this head" — a statement about marker
    bookkeeping after a rebase. It did not mean "the defect is gone." Treating the two as the
    same fact is the actual error, and the coder avoided it.
  3. It disclosed the override in the body rather than relying on the dispatch brief. The
    (repair round 2) entry names the stale binding, names why it proceeded, and names that it
    re-read at 4903796 before editing. A departure that is written down is auditable; that is
    exactly what §DEV exists for, and it is why I can grade this at all rather than reconstruct it.

The counterweight is real and belongs on the record: "ignore the idempotency check when you
believe you know better" is a dangerous general rule
, one short step from an agent reasoning its
way past a genuine gate. What keeps this instance safe is that the override resolved toward more
work
, not less — it re-verified rather than waved through, and the failure mode of being wrong was
a redundant no-op edit, not an ungated merge. The stopping alternative would have cost a full
re-gate cycle to re-emit a byte-identical finding, which is pure waste.

The durable lesson belongs in the repair-mode rule rather than in this PR: a CODE_FAIL=0 that
follows a rebase is UNKNOWN, not "clean."
The correct discharge is what happened here — re-read
the named site at the live head and let that decide — never a bare stop, and never a bare proceed.

Acceptance criteria — #5025, one at a time

Ten of the thirteen were verified in depth at 401d0538 and re-confirmed structurally unchanged by
the two-line delta above; the ones the repair could touch were re-verified in full at this head.

  • [PASS] Every contract verb implemented, no verb it does not specify — capture, take,
    read, claim, exactly four, matching the contract's verb inventory.
  • [PASS] Group shape — src/handoff/ holds a pure core per verb plus a thin <verb>-verb.ts,
    each with a *.unit.test.ts beside it, assembled by handoff/command.ts, plus one
    handoff.cli.test.ts.
  • [PASS] Registered and discoverable by registration alone — two inserted lines in
    src/registry.ts, no parallel list.
  • [PASS] Verbs are pure functions of their dependencies — every run* returns a VerbOutcome;
    the only process.exit and the only stream writes are in command.ts's emit adapter; every
    refusal is asserted in-process by code and by the bytes on each channel.
  • [PASS] Exit codes from verb.ts, proven outcomes at 3+, refusals empty on stdout — this
    is the criterion the prior FAIL sat on, and it now passes.
    refuse hardcodes empty stdout, and
    every seat in take-verb.ts now goes through a named constant (zero refuse(<integer>) sites).
  • [PASS] Each verb's header states whether empty is a fact or a failed read — unchanged by the
    repair; the four reads that can fail (default branch, ground derivation, comment paging, ACL) all
    seat 11 with "nothing was written".
  • [PASS] All shared I/O through src/io/, no second path to a resource — repoDefaultBranch
    and pullsForBranch are appended to the existing modules and reuse their execCapture /
    pagedJson seams.
  • [PASS] ADR 0238, no tether to v1 — scanned every added line for pipeline-cli,
    kampus-pipeline and packages/pipeline-cli: zero hits, re-run at this head.
  • [PASS] No wrapper verb, no second answer to a gated question — handoff emits no verdict
    marker and is absent from the ship namespaces.
  • [PASS] Extends the shared surface rather than minting a copy — HANDOFF_SEATS reuses
    GRILL_SEATS; the wire format takes a row in the existing registry; the two io/ reads are
    appended.
  • [PASS] Under-determined clauses surfaced, not invented over — four clauses named in the body,
    each surfaced on Implement the handoff contract's verbs in packages/fabrika-cli/ #5025; neither SKILL.md nor contract.md is touched by this diff.
  • [PASS] pnpm typecheck / pnpm lint green and the new tests pass — re-run at this head:
    typecheck clean, biome clean, 243 files / 3530 tests passing.
  • [PASS] Repo-relative paths only, no operator name or email. One line matched a broad
    machine-local-path scan: a deliberate negative-test fixture at
    packages/fabrika-cli/src/handoff/take-verb.unit.test.ts:109, whose entire purpose is to prove
    that the 5 LEAKED_PATH refusal fires on a machine-local path. Cleared on two grounds — the user
    segment is synthetic, carrying no operator's real name, and the identical idiom already ships
    on main in packages/fabrika-cli/src/ledger/child-verb.unit.test.ts. A leak-detection test that
    contains no sample leak cannot test anything. Not a finding.
  • [PASS] deviation-disclosure — the ## Deviations section is present, appended rather than
    rewritten, and its one disclosed Tier-M hit reproduces exactly. Nothing undisclosed that this gate
    could see.

Sub-gates that did not fire, each an explicit not-applicable skip rather than a silent green:
glossary-freshness (no new feature folder, public package or public export), comment-discipline
(the repair added no comment lines — the two changed lines are argument swaps), flag-gating (#5025
carries no **Containment:** flag marker), and the session-caching two-axis gate (no new
session-caching path).


Verdict

PASS at 35225fce9cf5fb2a6ca38b4ae6a3350791926a81. The one blocking defect is discharged, the
repair is exactly two lines in one file and carries nothing else, all five previously-graded
judgments survived the rebase intact, deletions across the PR are zero, and the deviation disclosure
is honest and appended rather than rewritten. The stale-verdict override was the right call and was
disclosed properly.

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

Verdict-written: 2026-08-10T18:05:28Z

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.

Implement the handoff contract's verbs in packages/fabrika-cli/

1 participant