feat(fabrika): implement the handoff contract's four verbs (#5025) - #5350
Conversation
No preview deploy
|
`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
|
review-code: FAIL @ 401d053 Gated against issue #5025 and the landed Required namespaces — Control-plane classification, both axes. Path axis: One blocking defectTwo bare
|
| 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. Nodriftverb, 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.tsbeside it, assembled byhandoff/command.ts, plus one
handoff.cli.test.tsfor 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 aVerbOutcome;
the onlyprocess.exitand the only stream writes are incommand.ts'semitadapter. 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
(refusehardcodes empty stdout), with the two bare4numerals 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.pullbeingnullis a fact".read: zero packs is a fact at exit
0.takeandclaimstate 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 seat11. - All shared I/O through
src/io/, no second path to a resource - PASS. The two additions
(repoDefaultBranch,pullsForBranch) are appended to the existingio/issues.tsand
io/pulls.tsrather than opened as a new path, and both reuse the module's own
execCapture/pagedJson/parseJsonseams. - ADR 0238 - no tether to v1 - PASS. Grepped every added line for
pipeline-cli,
kampus-pipelineandpackages/pipeline-cli: zero hits. No import, no subprocess, no wrapper. - No wrapper verb, no second answer to a gated question - PASS.
handoffis absent from
SHIP_NAMESPACES, emits no verdict marker, and neitherverdict-marker.ts'sNAMESPACEregex nor
itsNAMESPACE_PREFIXESgate 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_SEATSreusesGRILL_SEATS; the wire format takes a row in the existing
registry; the twoio/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. NeitherSKILL.mdnor
contract.mdis touched by this diff. pnpm typecheck/pnpm lintgreen, new tests pass - PASS, re-run at this head:pnpm typecheck(inpackages/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.
401d053 to
4903796
Compare
…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>
|
review-code: PASS @ 35225fc — merge-ready Re-gate of #5350 against issue #5025 at the repaired head. Read the PR head (§HEAD): every file Required namespaces — Control-plane, both axes. Path axis: The blocking defect is dischargedThe prior FAIL (
The repair delta is exactly what was reported, and nothing elseThe two-commit stat from I read the full patch, not only the stat. The whole delta is two hunk lines — a bare The five judgments confirmed to survive the rebase + repair — all unchangedRe-verified at
Insertion-only discipline — zero deletions, still holdingThe sibling lane (#5351) is appending to several of the same files, so this matters more after a
|
Fixes #5025
fabrika handoff <capture|take|read|claim>, built against the landedclaude-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.tsentry,each with a
*.unit.test.tsbeside it, assembled byhandoff/command.ts. Same shape asmap(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). Onehandoff.cli.test.tsfor the two facts no in-process test canestablish: 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 withreport/codes.tsarere-exported under aliases, never restated as numerals;
10is held asDELIBERATE_GAP(no verb accepts a label flag, writes a label, or composes a title);
12–15are this group's own.wire/handoff-pack.ts— the pack document as a registered wire format, with itssrc/wire/registry.tsrow (fixtures + brands) and its### handoff-packnarrative section.io/issues.ts/io/pulls.ts— two appended reads: the repository's default branch, and thepull requests on a branch. No second path to either resource is opened.
The exit table
0verb.ts1/2/1273 EMPTY_STDINreport/codes.ts4 BAD_SECTIONS5 LEAKED_PATH--redact)6 BARE_AT_PATH@path reference7 NO_TARGET8 WRITE_UNKNOWN9 READBACK_MISMATCH10DELIBERATE_GAP, deleted fromHANDOFF_SEATSon theUI_SEATSprecedent11 PRECONDITION_UNKNOWN12 WORK_UNREACHABLE13 NO_PACK14 PACK_MALFORMED15 PACK_CLAIMEDEvery proven verdict sits at
3+ with empty stdout (refusehardcodes that), and every refusal isasserted in a unit test by exit code and by the bytes on each channel.
1/127mean the callnever decided;
11means a read failed before any write;8means a write was attempted and itsoutcome is UNKNOWN. The three are three seats with three remedies.
Shared registration files — insertion-only
A concurrent lane (#5024, the
prototypinggroup) is on the same files, so every edit here is apure insertion with zero deleted lines.
git diff --numstaton the base, per file:packages/fabrika-cli/src/registry.tspackages/fabrika-cli/src/exit-code-alignment.tspackages/fabrika-cli/src/exit-code-alignment.unit.test.tspackages/fabrika-cli/src/wire/registry.tsclaude-plugins/fabrika/docs/wire-formats.mdpackages/fabrika-cli/README.mdpackages/fabrika-cli/src/io/issues.tspackages/fabrika-cli/src/io/pulls.tsThe wire-formats generated region was regenerated with
fabrika wire index --write, neverhand-merged. The README gained a
## The handoff groupsection and nothing else was reflowed.HANDOFF_SEATSis authored inexit-code-alignment.tsas an alias ofGRILL_SEATS, which the twogroups reach independently: both name
7NO_TARGETand both hold10empty.Verification
pnpm typecheckin-package (packages/fabrika-cli) — clean.pnpm vitest runin-package — 235 files, 3393 tests, all passing (80 of them this group's).pnpm lint:worktree— clean.Deviations
dev-tier-m.shreported 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 insideprocess.exit(inpackages/fabrika-cli/src/handoff/command.ts's emit adapter. Those bytes are byte-identical to thesame adapter in every sibling group (
map/command.ts,grill/command.ts,report/command.ts…) —it is the module-private
emitthe contract says each group declares for itself. No test isskipped, no lint rule is suppressed, and no
@ts-expect-error/biome-ignoreappears anywhere inthis diff.
(repair round 1) Rebased onto
mainafter the siblingspikelane (feat(fabrika): implement the prototyping contract's spike verbs (#5024) #5349) landed; nothingelse changed. The four shared files collided textually only in
packages/fabrika-cli/README.md;registry.ts,exit-code-alignment.tsandexit-code-alignment.unit.test.tsauto-merged. TheREADME conflict was resolved additively —
main's## The spike groupsection kept exactlywhere it sits, this branch's
## The handoff groupsection placed after it, no prose reflowed andno line dropped. The insertion-only table above still holds on the rebased diff
(
git diff --numstat origin/main HEADreports0deletions in every file). The wire-formatsgenerated region was regenerated with
fabrika wire index --writeand produced zero diff.Re-verified after the rebase: in-package
pnpm typecheckclean,pnpm lint:worktreeclean, andthe in-package suite now 243 files / 3530 tests passing (the growth is the
spikegroup arrivingon the base, not new tests here).
dev-tier-m.shre-run on the rebased diff reports the sameone suppression/skip line — the
process.exit(false positive already disclosed above — and zeroremoved-assertion lines, so no new deviation fired this round.
(repair round 2) Acted on a
review-codeFAIL bound to the pre-rebase head. The verdict waswritten
@ 401d0538; the round-1 rebase had already moved the head to4903796, so theSHA-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.tsand both bare4literals were re-readfirst-party at
4903796before the edit. Disclosed here rather than quietly relying on thedispatch 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 theseat table —
handoff/codes.tsre-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.tshad two call sites passing a bare4where the importedBAD_SECTIONSbelonged. Those two are fixed (2 insertions / 2 deletions, one file, nobehaviour 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-alignmentchecks seat mapsagainst 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:
git.base.branch's default has no source as written. The contract derives it from "therepository's default branch from the
getIssuerepo payload", butrepos/{owner}/{repo}/issues/{n}returns no
repositoryobject — the field does not exist. Taken: a dedicatedrepos/<repo>read,whose failure is
11, never a guessedmain.6 BARE_AT_PATHis unreachable as ordered. The section check runs before the leak scan, andthe shipped
isBareAtReferencetests only a body's first token — so a stdin that is a bare@pathfails4first, and a composed document always opens with the marker. Taken: thepredicate is applied per asserted section, which is the one reading that keeps the seat
reachable and its remedy distinct.
packedBranchisgone. The contract states rows 3–10 reportlive: null/movedand says nothing about the board rows. Taken: they are still compared — thepull 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.
14and never adisregardedrow, and that an unauthorized pack is one. Taken: parse first, sothe
14refusal wins — the fail-closed direction.One known absence, met as recorded:
src/eval/corpus.ts'sSTAGEShas no stage ahandoffevalentry could decode (#5241, owned by #4649's harness), so the skill-conventions §8 gate-3 leg is
blocked, not skipped. Nothing here widens
SHIP_NAMESPACESor the verdict-marker namespace gates.🤖 Generated with Claude Code