feat(ship): one ship implementation — every agent:ship request runs on the plan runner, the in-process round loop is deleted and ship.coordinator is gone - #1011
Conversation
There was a problem hiding this comment.
LGTM: Clean retirement of the in-process ship round loop: every agent:ship request now hands to the plan runner, the ship.coordinator key is refused at load by name, resume-at-review rides the unit row, and specs/tests/migration notes move with the code — no blocking issues found.
Verdict: approve — a large but disciplined deletion (43 files, +1031/−3784) that retires the in-process ship round loop and makes the plan runner the one ship implementation. I read the whole diff and the full current contents of the key files; no blocking issues.
What the change does, verified against the code:
runShipPipeline, the child-round stages (runShipCodingChild,runShipReviewChild,childRound.ts) and theship.coordinatorswitch are gone;dispatch/ship.tsnow runs the preflight, registers the one run record/card (tombstone-first, ledger claim, truthful card close, failed-on-thrown-reply — all mirroring the main path), and hands every admitted request tohandOffToCoordinator.validateShiprefusesship.coordinatorby name with a migration pointer and rejects any other unknown key (SHIP_KEYSheld equal toShipConfig) — matching the migrations.md 1.208.0 entry and config.example.yaml.- Resume-at-review (formerly in-process only) now rides the task row's
resumefield:isCoordinatorUnitvalidates it, the driver feeds it intoopenUnitPipeline, which opens directly atnextReviewwithlastReviewHeadseeded — no pr-check, no branch, no round 0. Preflight prefers the PR's own base, as the spec row says. unit-endnow renders the last coding child's handoff into the board comment viacodingHandoffOf, correctly gated: run-id pattern check,parentInstanceIdmust match the instance (the field exists onRunRecord/RunView),isHandoffShapecheck, never a refusal — the ending posts either way.- The new
surface.test.tssource scan pins the retirement itself ("this code does not exist"), a sensible proof for a deletion.
Spec contradiction check: specs:coverage --changed passes; agent-ship.md is rewritten in the same diff to match (items 1–16 recast around the runner, criteria re-bound to the new tests), and agent-coding.md, agent-review.md, code-map, how-to and migrations all move consistently. No contradiction.
Test guard: many removals in dispatcher.test.ts, codingChild.test.ts, reviewChild.test.ts — every removed: and check: line ends — allowed by docs/reference/specs/agent-ship.md (spec changed in the same diff), and the guard ends test-guard ok — no verification removed without its spec. The check: heuristics (expect-count drops, describe renames) are the licensed migration of loop-scenario proofs to the coordinator machine's tests (coordinator.test.ts, driver.test.ts, handOff.test.ts, adminCoordinator.test.ts) — refactor, verification intact.
No findings.
…n the plan runner, the in-process round loop is deleted and ship.coordinator is gone The runner merged a plan unit in production, so the in-process ship round loop retires: `runShipPipeline` and its endings leave `shipPipeline.ts`, which keeps the `ship` config block, the caps, the preset and the round header; `ship/childRound.ts` is deleted and `codingChild.ts` / `reviewChild.ts` keep only the prompt blocks the runner's spawn route composes a child's turn from; the ship branch hands every admitted request to `handOffToCoordinator`. Two decisions the unit left open. The `ship.coordinator` key is removed and refused at load by name, pointing at the migration note — a deployment without the runner's prerequisites is refused naming what is missing, never served some other way. The resume at review runs on the runner too: the task row carries `resume`, the driver reads it into the machine's existing `openUnitPipeline` input, so the unit opens at its review round. The loop's coding round was the only path that posted a child's handoff to the board, so the runner's `unit-end` now names the last coding child and the bot renders that record's handoff under the ending's comment. The agent-ship spec is rewritten as the runner's; the ship-restart plan is superseded by the program plan; the program plan's U15 carries its status. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
aee9d95 to
0b1a693
Compare
|
Re-review requested at 0b1a693 — rebased over #1013 (the review post as a recorded fact) and #1005 (the program plan's Phase G); no changes of this PR's own since the LGTM at aee9d95. The four conflicts were three spec rows in agent-review.md, http-ingress.md and tracing.md (both sides' edits kept) and |
There was a problem hiding this comment.
LGTM: Rebase-only delta since the round-1 approval at aee9d95: same single commit, conflict resolutions confined to three spec files and the deleted reviewChild.test.ts; totals still match GitHub (43 files, +1031/−3784), test guard still passes.
Verdict: approve — re-review of the rebased head 0b1a693; the delta since the round-1 approval at aee9d95 is exactly the rebase, no code changes of this PR's own.
Verified:
• One commit, same message — git log origin/main..HEAD shows the single squashed commit; git range-diff between the two heads confines the differences to four files, all conflict-resolution context, matching your description.
• Spec conflicts resolved correctly, both edits kept: agent-review.md keeps #1013's new items 17/18 rows (the post as a recorded fact, the record carrying reviewPost) while this PR's rewrite of the reviewChild-bound composer row lands, re-bound to reviewRound.test.ts only — correct, since reviewChild.test.ts is deleted here. http-ingress.md item 9 merges #1013's reviewPosted-from-record paragraph with this PR's "a task string's ship branch still never merges" phrasing. tracing.md item 18 keeps #1013's run.review_post_step span while item 17 takes this PR's retirement of ship.round. agent-coding.md context only.
• reviewChild.test.ts: the file stays deleted (this PR's rewrite kept); the only delta is that the removed lines now show #1013's two toMatchObject tweaks — tests this PR removes anyway, licensed by the agent-ship.md rewrite.
• Totals: git diff origin/main...HEAD --shortstat = 43 files, +1031/−3784 — exactly GitHub's size.
• Guards: specs:coverage --changed --test-guard still ends test-guard ok; every removed/check line on the deleted file remains — allowed by docs/reference/specs/agent-ship.md.
Prior round had no findings to re-verify. No new findings — the round-1 approval carries over to this head.
Switchboard's
agent:shipused to have two implementations — a round loop inside the bot process and, behind a config switch, the plan runner. This PR deletes the loop and the switch: every ship request runs on the plan runner, whose rounds are child runs that survive a bot restart, and a deployment without the runner's prerequisites is told exactly what is missing.What & why
Unit U15 of the orchestration program plan, board item #837. Its dependency is met: the runner carried a plan to a merge it made in production on 2026-09-14 (receipt on #835), which is the trigger record 0031 and the plan set for retiring the in-process path. What lands:
runShipPipelineand its endings are gone fromsrc/core/shipPipeline.ts, which keeps theshipconfig block, the caps,shipPresetForand the round header;src/core/ship/childRound.tsis deleted;codingChild.tsandreviewChild.tskeep only the prompt blocks the runner's spawn route composes a child's turn from; the ship branch (src/core/dispatch/ship.ts) hands every admitted request tohandOffToCoordinator.ship.coordinatoris removed and refused at load by name, with a migration note under 1.208.0.agent:ship <ship PR URL>) runs on the runner too: the task row carriesresume, the driver reads it into the machine's existingopenUnitPipelineinput, so the unit opens at its first review round.unit-endnow renders the last coding child's typed handoff into the board comment — the loop's coding round was the only path that posted it.reviewChild.test.tstargeted tests this PR removes, andReviewPostOutcomestays where it always lived, inreviewRound.ts.docs/reference/specs/agent-ship.mdis rewritten as the runner's spec; the ship-restart plan issupersededby the program plan; the program plan's U15 carries its status and its Appendix B ledger has no row without a disposition.Tour
1. The ship branch hands every admitted request to the runner
After the preflight and the run record, the branch builds the runner's caps from the effective profile and hands off — a plan, a task or a resume alike. A deployment without run history on the state Worker,
PUBLIC_BASE_URLor thecoordinatorbearer is refused by name from inside this call; there is no other path to fall back to. Look for: no branch on a config switch and noentry.resumeguard remain.switchboard/src/core/dispatch/ship.ts
Lines 314 to 345 in 0b1a693
2. What
shipPipeline.tsstill isThe module's new header states its scope: the config block, the caps, the preset, the round header, and two re-exports from the runner's machine. Everything about the round loop is gone with the 500 lines below it.
switchboard/src/core/shipPipeline.ts
Lines 1 to 11 in 0b1a693
3. The
shipblock loses its switchShipConfigis two knobs; the interrupted note and the caps' shape come fromship/coordinator.ts, which the bot-side callers (the reclaim at boot, the coordinator routes) reach through here as before.switchboard/src/core/shipPipeline.ts
Lines 23 to 33 in 0b1a693
4.
ship.coordinatoris refused at load, by nameThe
shipblock now refuses unknown keys like thespawnblock does.coordinatorgets its own line pointing at the migration note, so a config that still carries it is told the runner is the one implementation rather than left believing it chose anything. Look for: the message names the key and the fix, and any other stray key is refused too.switchboard/src/config/validate.ts
Lines 305 to 327 in 0b1a693
5. The coding child's prompt blocks are all that is left of it
codingChild.tsshrinks tobuildShipFixTurnandwithContractInFirstUserTurn— whatcoordinator/briefs.tsand the dispatcher use for a spawned child;reviewChild.tstobuildShipReviewTurn.shipBranchContractwent with the loop: the runner's child is dispatchedon branch <ref>and its contract's first instruction names the branch.switchboard/src/core/ship/codingChild.ts
Lines 1 to 8 in 0b1a693
6. The resume rides the task row
The unit row gains
resume— the pull request of ship's own the requester named, its head and url — the same shape the machine'sopenUnitPipelinealready took. The validator accepts the number with optional head and url and refuses anything else.switchboard/src/core/coordinator/contract.ts
Lines 148 to 152 in 0b1a693
switchboard/src/core/coordinator/contract.ts
Lines 170 to 176 in 0b1a693
7. The hand-off writes it, and says the review resumes
A task request whose entry carries a resume puts it on the one task row; the reply tells the thread the review loop resumes at its next review round with no coding round first. Look for: the row is otherwise the task row as before — same id, same branch (the pull request's own head branch from the preflight).
switchboard/src/core/coordinator/handOff.ts
Lines 126 to 152 in 0b1a693
8. The driver reads it into the machine
One lookup: the unit's row's
resume, if any, goes intoopenUnitPipeline, which already opens a resumed pipeline atnextReview— no pre-check, no branch, no round 0. Nothing in the machine changed.switchboard/src/core/coordinator/driver.ts
Lines 414 to 432 in 0b1a693
9. The unit's end names the last coding child
The
unit-endbody carriescodingRunId(the machine'slastCodingRunId) so the bot can find the child whose record holds the unit's handoff.switchboard/src/core/coordinator/driver.ts
Lines 445 to 453 in 0b1a693
10. The board comment carries the rendered handoff
When the row names an issue, the ending's comment appends
renderHandoffCommentof that child's handoff under the report. Before this PR the only caller of that renderer was the deleted in-process coding round, so the runner never put a child's deviations on the board. Look for: best effort, like the report — a failed comment never fails the step.switchboard/src/channels/adminCoordinator.ts
Lines 1057 to 1075 in 0b1a693
11. Reading the child's handoff, and no other instance's
The child's record is read the way
read-recordreads it: a run outside the instance, a run the history lacks, a record without a handoff or an id that is not a run id all yield nothing, never a refusal.switchboard/src/channels/adminCoordinator.ts
Lines 1093 to 1103 in 0b1a693
12. The source scan — the plan's test scenario
A property no behavioural test can hold ("this code does not exist"):
shipPipeline.tsexports exactly the surviving eight names and none of the loop's, the child modules export only the prompt blocks and import no runner, and the ship branch imports the hand-off and reads no switch. It was red onmain(3/3 failed) before any source changed.switchboard/src/core/ship/surface.test.ts
Lines 36 to 61 in 0b1a693
13. The branch's refusal when the runner is missing
With the default shim client, a process without⚠️ and nothing runs.
PUBLIC_BASE_URL, then one without acoordinatorentry in the token map, gets the reply naming exactly that; the card closesswitchboard/src/core/dispatch/ship.test.ts
Lines 266 to 289 in 0b1a693
14. The dispatcher's ship suite becomes the hand-off's
The old
agent:ship (pipeline)describe drove the in-process loop throughdispatch()with scripted children (50 tests). The new one keeps every preflight and entry-check scenario and asserts what the runner is handed instead; its provider is never asked andafterEachasserts no workspace was attached and no model turn ran.switchboard/src/core/dispatcher.test.ts
Lines 7327 to 7351 in 0b1a693
15. The same refusal through
dispatch()The prerequisites refusal end to end: the reply, the⚠️ close, no provider call, and the fetch guard proving the shim was never addressed without its URL.
switchboard/src/core/dispatcher.test.ts
Lines 7691 to 7706 in 0b1a693
16. The driver's resume, on the wire
A task row carrying
resumeyields the step namesplan,task/start,task/1/review, … — no pr-check, no branch, no coding child; the review child is briefed with the pull request and the head; the approve on a ship branch endsmerge_ready; the ending names no coding run.switchboard/src/core/coordinator/driver.test.ts
Lines 570 to 574 in 0b1a693
17. The board comment with and without a handoff
The unit-end test now seeds three records — a child of this instance with a handoff, another instance's, and one without — and asserts the exact comment for the first and the report alone for the rest.
switchboard/src/channels/adminCoordinator.test.ts
Lines 1849 to 1862 in 0b1a693
18. The spec: every request hands to the runner
Item 16 is now the one ship implementation — the prerequisites and their named refusals, the attempts, and that the switch is gone; item 10 states the resume's path through the runner and where a bot death now lands (a child, never the pipeline).
switchboard/docs/reference/specs/agent-ship.md
Line 27 in 0b1a693
switchboard/docs/reference/specs/agent-ship.md
Line 21 in 0b1a693
19. The migration note
Under 1.208.0: what no longer works, what replaces it, and the smallest edit — delete the key; carry the runner's prerequisites.
switchboard/docs/reference/migrations.md
Lines 7 to 9 in 0b1a693
20. The ship-restart plan is superseded
Only the two status lines change; the body is untouched, as
decisions:checkrequires.switchboard/docs/plans/2026-09-08-003-feat-ship-restart-plan.md
Lines 1 to 8 in 0b1a693
21. The program plan's U15 carries its status
The amendment convention earlier units used (U13, U14): a Status bullet naming the two decisions taken at build time and that the ledger is complete. Rebased over #1005's Phase G text; both edits stand.
switchboard/docs/plans/2026-09-10-001-feat-orchestration-program-plan.md
Lines 519 to 522 in 0b1a693
22. The example config says what ship needs
The
shipblock's comment names the runner and its prerequisites and drops thecoordinatorline.switchboard/config/config.example.yaml
Lines 280 to 294 in 0b1a693
23. Remaining changes
src/core/ship/childRound.ts— deleted (the shared child-round slices had no reader left).src/core/ship/reviewChild.ts— shrinks tobuildShipReviewTurn; header rewritten.src/core/ship/codingChild.test.ts,src/core/ship/reviewChild.test.ts— the stage tests (12 and 7) become tests of the prompt blocks (4 and 4).src/core/dispatch/ship.test.ts— rewritten for the hand-off: 9 tests before, 9 after (the task, the resume, the caps, the shim refusal, no store, the prerequisites, a throw, a throwing reply, the preflight refusal).src/core/coordinator/handOff.test.ts,src/core/coordinator/contract.test.ts— one test each for the resume row and its validator; header comments no longer name a switch.src/core/coordinator/briefs.ts,src/core/coordinator/briefs.test.ts,src/core/ship/coordinator.ts,src/core/ship/coordinator.test.ts,src/core/ship/preflight.ts,src/core/dispatcher.ts,src/index.ts— comments that described the loop or the switch now describe the runner; the dispatcher's ship fork no longer passes the memory block (the branch composed no child prompt).src/config.test.ts— the switch's parse test becomes the refusal test; an import the removed test used goes.docs/reference/specs/agent-ship.md— the intro, Code header, a new Tests header, items 1–16 and the validation table rewritten for the runner; roadmap gaps for the loop dropped, a parallel-units gap added.docs/reference/code-map.md— theshipPipeline.ts/ship/row, thedispatch/row'sship.tsentry and thecoordinator/row.docs/how-to/turn-features-on-and-off.md— theship.coordinatorrow removed; thecoordinatorrow's On/Off columns sayagent:shipalways runs on it and is refused without it.docs/reference/specs/README.md,http-ingress.md,tracing.md,thread-admission.md,run-history.md,mcp-tools.md,pr-description.md,agent-coding.md,agent-review.md— rows and sentences that named the loop, its spans, its steering or its tests are rewritten or rebound;ship.roundstays in the span table for the records the loop wrote.Decisions
ship.coordinatoris removed, not kept as a no-op. The plan's intent is one implementation; a documented no-op invites a config that "chose" something. The load refuses the key by name with the fix and the migration link; a deployment without the runner's prerequisites is refused at hand-off naming the missing one (PUBLIC_BASE_URL is not set…,SWITCHBOARD_INGRESS_TOKENS has no single \coordinator` entry…,needs run history on the state Worker`) — never served some other way.resumeonopenUnitPipelineand a test for it; the cost was a field on the unit row, a lookup in the driver and the hand-off writing it — smaller than a refusal plus a rewrite of the interrupted note, which promises exactly this re-issue. Both the preflight's entry checks and the runner's review round are unchanged.unit-end.renderHandoffComment's only caller was the deleted coding round, and record 0031's R28 ("a child's deviation reaches the board without a person") had no live path under the runner. The driver names the last coding child; the bot reads its record and appends the rendered handoff under the ending's report in the one comment it already leaves.!in the title. The release pin is off since the 1.200.0 cut, socheck:pr-titleaccepts a!only with a## 2.0.0migration section — i.e. it would cut 2.0.0, which the program plan reserves for the release moment Justin picks and CONTRIBUTING says waits for the public launch. The change ships as a minor with its section under 1.208.0 (the same shape every pre-launch breaking cleanup has used).repo_not_onboarded, the unit ends with the gate's name), and a resident that cannot attach falls back exactly as a plain coding or review run does.ship.roundin the stream-span table and its display name, so the records the loop wrote still classify; tracing.md says no emitter opens it.Validation
npm run verifyon0b1a693c(the rebased head) — exit 0: consistency (specs:check ok — 43 spec(s), 2706 proof reference(s),decisions:check ok — 45 record(s)…superseded_by resolves,docs:check ok — 10 file(s),public-hygiene ok,agents:check ok), typecheck, lint, format, root tests369 files / 6737 passed, 2 skipped,check:dist ok, every workspace green,check:site ok. Log:u15-verify-rebased.login the session scratchpad; the pre-rebase headaee9d958passed the same gate (u15-verify-final.log) and drew the clean LGTM below.npm run specs:coverage -- --changed origin/main...HEAD --test-guard:every changed source path has a covering spec;test-guard ok — 12 test file(s) changed, no verification removed without its spec. Removed-test accounting:dispatcher.test.tsship describe 50 → 21 (29 loop scenarios removed, each covered by the rewritten agent-ship.md rows and, for the run-history/thread-admission/tracing/mcp-tools/pr-description rows that bound them, by those specs' changes in this range);codingChild.test.ts12 → 4;reviewChild.test.ts7 → 4;ship.test.ts9 → 9 (rewritten); the guard'scheck:lines are these renames and the lowerexpect()counts, none a weakening — the behaviours moved to the runner's suites (coordinator.test.ts,driver.test.ts,adminCoordinator.test.ts) or left with the code.src/core/ship/surface.test.tsran 3/3 failed on the tree before any source change, then green.contract.test.ts), hand-off row and reply (handOff.test.ts), driver steps (driver.test.ts); the board comment with the handoff (adminCoordinator.test.ts); the prerequisites refusal in the branch and throughdispatch();ship.coordinatorrefused at load (config.test.ts).npm run check:pr-title -- "feat(ship): …"ok; the!variant is refused:a breaking change (\!`) needs its migration note: add a `## 2.0.0` section`.decisions:checkacceptsstatus: superseded+superseded_byon the ship-restart plan with the program plan's body amended whileproposed.agent:ship <task>runs on the runner with no other path (the card says🧭 Handed to the plan runner ship-<run id>,runs getshows a parent record with nomodel.turnof its own); a resumeagent:ship <ship PR URL>opens at the review round; a unit's board issue gains the runner's ending comment with**Handoff — U<n>**under the report. These are the spec's live rows and go on #837.🤖 Generated with Claude Code