Skip to content

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

Merged
justinhelmer merged 1 commit into
mainfrom
feat/u15-retire-ship-round-loop
Sep 14, 2026
Merged

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
justinhelmer merged 1 commit into
mainfrom
feat/u15-retire-ship-round-loop

Conversation

@justinhelmer

@justinhelmer justinhelmer commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Switchboard's agent:ship used 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:

  • runShipPipeline and its endings are gone from src/core/shipPipeline.ts, which keeps the ship config block, the caps, shipPresetFor and the round header; src/core/ship/childRound.ts is deleted; codingChild.ts and reviewChild.ts keep 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 to handOffToCoordinator.
  • ship.coordinator is removed and refused at load by name, with a migration note under 1.208.0.
  • The resume at review (agent:ship <ship PR URL>) 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 first review round.
  • The runner's unit-end now renders the last coding child's typed handoff into the board comment — the loop's coding round was the only path that posted it.
  • Rebased over #1005 (the program plan's Phase G) and #1013 (the review post as a recorded fact): both land intact; fix(ship): the runner trusts the review child's own record of its post — a review posted a second ago no longer reads as unposted, and GitHub is asked patiently only when the record is silent #1013's two-line change to reviewChild.test.ts targeted tests this PR removes, and ReviewPostOutcome stays where it always lived, in reviewRound.ts.
  • docs/reference/specs/agent-ship.md is rewritten as the runner's spec; the ship-restart plan is superseded by 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_URL or the coordinator bearer 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 no entry.resume guard remain.

const caps = { ...resolveShipCaps(deps.config.config.ship), maxMinutes: profile.minutes };
const shim = () => ({
baseUrl: process.env.PUBLIC_BASE_URL,
tokens: processSecrets.get("SWITCHBOARD_INGRESS_TOKENS"),
});
try {
// The hand-off (agent-ship.md item 16): the request — a plan, a task, or a
// resume at review — becomes a plan runner instance; the bot writes the
// records, asks its shim for the Workflow, and this run ends with where the
// plan runs. A deployment without the runner's prerequisites — run history
// on the state Worker, `PUBLIC_BASE_URL`, the `coordinator` bearer — is
// refused by name here, never run some other way.
outcome = await root.span("dispatch.ship_hand_off", () =>
handOffToCoordinator(
{
readFile: (repo, path, ref) => githubCapabilityFor(deps, msg.userId).api.readFile(repo, path, ref),
instances: deps.coordinatorInstances ?? new NullCoordinatorInstanceStore(),
create: deps.createCoordinatorInstance ?? ((id) => createInstanceViaShim(shim(), id)),
status: deps.fetchCoordinatorInstanceStatus ?? ((id) => fetchInstanceStatusViaShim(shim(), id)),
},
{
entry,
requestText: directives.text,
msg,
runId: run.id,
label,
caps,
...(card.handle !== undefined ? { card: card.handle } : {}),
now: clock(),
},
),
);

2. What shipPipeline.ts still is

The 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.

// What every ship surface shares (docs/reference/specs/agent-ship.md items 8
// and 12): the `ship` config block a deployment declares, the caps resolved
// from it, the ship preset as this deployment declares it, and the card's round
// header. The pipeline itself — coding → review → fix to LGTM — is the plan
// runner's (ship/coordinator.ts, driven from the bot's shim Worker; the hand-off
// in dispatch/ship.ts and coordinator/handOff.ts): every `agent:ship` request
// becomes a runner instance whose rounds are child `dispatch()` runs, so no
// round loop lives in this process. The caps' shape and the interrupted note
// live with the runner's machine (Worker-importable) and are re-exported here
// for the bot-side callers — the reclaim at boot, the coordinator routes.

3. The ship block loses its switch

ShipConfig is two knobs; the interrupted note and the caps' shape come from ship/coordinator.ts, which the bot-side callers (the reclaim at boot, the coordinator routes) reach through here as before.

export interface ShipConfig {
/** Review rounds per pipeline (>= 1). Default 3. */
maxRounds?: number;
/** The ship preset's declared wall-clock budget in minutes (>= 1). Default:
* the registry's `AGENTS.ship.maxMinutes` (120). */
maxMinutes?: number;
}
import { shipInterruptedNote, type ShipCaps } from "./ship/coordinator.js";
export { shipInterruptedNote, type ShipCaps };

4. ship.coordinator is refused at load, by name

The ship block now refuses unknown keys like the spawn block does. coordinator gets 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.

const SHIP_KEYS: Record<keyof ShipConfig, true> = { maxRounds: true, maxMinutes: true };
/** `ship` caps (docs/reference/specs/agent-ship.md item 8): both bounds enforced at load
* so a typo cannot silently become "no cap" (mirrors validateRunHistory). Any
* other key is refused by name — `coordinator` above all, the switch that once
* chose between the plan runner and an in-process loop: the runner is the one
* ship implementation now, so a config still carrying the key is told to drop
* it rather than left believing it chose anything (docs/reference/migrations.md). */
function validateShip(ship: ShipConfig): void {
if (typeof ship !== "object" || ship === null) throw new Error("config.yaml: ship must be a mapping");
for (const key of unknownKeys(ship, SHIP_KEYS)) {
if (key === "coordinator")
throw new Error(
"config.yaml: ship.coordinator is no longer a key — every agent:ship request runs on the plan runner; remove it (docs/reference/migrations.md)",
);
throw new Error(`config.yaml: ship.${key} is not a known key`);
}
for (const key of ["maxRounds", "maxMinutes"] as const) {
const v = ship[key];
if (v !== undefined && (!Number.isInteger(v) || v < 1))
throw new Error(`config.yaml: ship.${key} must be an integer >= 1`);
}
}

5. The coding child's prompt blocks are all that is left of it

codingChild.ts shrinks to buildShipFixTurn and withContractInFirstUserTurn — what coordinator/briefs.ts and the dispatcher use for a spawned child; reviewChild.ts to buildShipReviewTurn. shipBranchContract went with the loop: the runner's child is dispatched on branch <ref> and its contract's first instruction names the branch.

// The coding child's prompt blocks (docs/reference/specs/agent-ship.md items 7
// and 13): what a coding round of the ship pipeline is told beyond a plain
// coding run. The child itself is an ordinary `dispatch()` run the plan
// runner's spawn route starts as the requesting user (coordinator/briefs.ts
// composes the turn from these); the fix turn carries the review's findings
// and the loop contract, and the unit's contract enters the first user turn.
import type { ChatMessage } from "../../providers/types.js";

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's openUnitPipeline already took. The validator accepts the number with optional head and url and refuses anything else.

/** Resume at review (agent-ship item 10): the open pull request of ship's own
* the requester named, so the unit's pipeline opens at its first review round
* — no pre-check, no branch, no round 0. A task string's row only; written by
* the hand-off, read by the driver into the machine's input. */
resume?: { pr: number; headSha?: string; url?: string };

const isPr = (v: unknown): boolean => isObject(v) && isFinite(v.number) && isText(v.url, 2048);
const isResume = (v: unknown): boolean =>
isObject(v) &&
isFinite(v.pr) &&
(v.headSha === undefined || isText(v.headSha)) &&
(v.url === undefined || isText(v.url, 2048));

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).

if (request === undefined) {
const id = `ship-${input.runId}`;
const resume = entry.resume;
const prUrl =
resume?.url ?? (resume !== undefined ? `https://github.com/${entry.repo}/pull/${resume.pr}` : undefined);
return {
ok: true,
planned: {
kind: "task",
instance: { id, ...identity, branch: entry.branch },
units: [
{
instanceId: id,
unit: TASK_UNIT,
slug: TASK_UNIT,
branch: entry.branch,
dependsOn: [],
rounds: [],
...(resume !== undefined ? { resume } : {}),
},
],
where:
resume !== undefined
? `the review loop of ${prUrl} resumes at its next review round on \`${entry.branch}\` in this thread under your grants — no new coding round first; this card follows it and the report lands here.`
: `the task runs on \`${entry.branch}\` in this thread under your grants; this card follows it and the report lands here.`,
},
};

8. The driver reads it into the machine

One lookup: the unit's row's resume, if any, goes into openUnitPipeline, which already opens a resumed pipeline at nextReview — no pre-check, no branch, no round 0. Nothing in the machine changed.

// A resume at review (agent-ship item 10) rides the unit's row: the pull
// request of ship's own the requester named opens the pipeline at its first
// review round, with no pre-check, no branch and no round 0.
const resume = plan.units.find((u) => u.unit === unit)?.resume;
let state: UnitPipelineState = openUnitPipeline(
{
unit: { id: unit, branch: node.branch },
repo: plan.repo,
base: plan.base,
caps: plan.caps,
childMinutes: plan.childMinutes,
// The branch decides who merges (record 0031's merge grant): a plan
// branch the runner opened is the runner's to squash once the review
// approved at its head and the checks are green; any other branch — a
// task string's ship branch — waits for a person.
merge: parsePlanBranch(node.branch) !== undefined ? "runner" : "person",
...(resume !== undefined ? { resume } : {}),
},
start.at,

9. The unit's end names the last coding child

The unit-end body carries codingRunId (the machine's lastCodingRunId) so the bot can find the child whose record holds the unit's handoff.

// The last coding child's run is named so the bot can put its handoff
// — the deviations it recorded — on the unit's board issue beside the
// ending (agent-ship item 14).
const body = {
...tag,
ending: { kind: note.ending.kind, report: renderUnitReport(state) },
...(state.pr !== undefined ? { pr: state.pr } : {}),
...(state.lastCodingRunId !== undefined ? { codingRunId: state.lastCodingRunId } : {}),
};

10. The board comment carries the rendered handoff

When the row names an issue, the ending's comment appends renderHandoffComment of 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.

// The unit's ending reaches the board (agent-ship item 14's destination):
// when the row names an issue, the report lands there too — a merge GitHub
// refused, a cap, a stop — and under it the last coding child's typed handoff
// as the parent renders it, so a deviation the child recorded reaches the
// board without a person copying it over. Best effort, like the thread's.
if (row.issue !== undefined) {
const handoff = await codingHandoffOf(deps, instance, body.codingRunId);
const rendered =
handoff !== undefined
? renderHandoffComment(handoff, { unitId: row.unit, ...(updated.pr !== undefined ? { pr: updated.pr } : {}) })
: undefined;
const comment =
`**Plan runner — ${row.unit} ended \`${ending.kind}\`**${updated.pr ? ` · ${updated.pr.url}` : ""}\n\n${ending.report}` +
(rendered !== undefined ? `\n\n${rendered}` : "");
await deps.github
.commentIssue(instance.repo, row.issue, comment)
.catch((err) =>
(deps.log ?? console.warn)(
`[coordinator] ${instance.id} ${row.unit}: the board comment could not be posted: ${describe(err)}`,

11. Reading the child's handoff, and no other instance's

The child's record is read the way read-record reads 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.

async function codingHandoffOf(
deps: AdminCoordinatorDeps,
instance: CoordinatorInstance,
runId: unknown,
): Promise<Handoff | undefined> {
if (typeof runId !== "string" || !RUN_ID_PATTERN.test(runId)) return undefined;
const res = await deps.runs.getRun(runId).catch(() => undefined);
if (res === undefined || !res.ok || res.value.parentInstanceId !== instance.id) return undefined;
return isHandoffShape(res.value.handoff) ? res.value.handoff : undefined;
}

12. The source scan — the plan's test scenario

A property no behavioural test can hold ("this code does not exist"): shipPipeline.ts exports 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 on main (3/3 failed) before any source changed.

describe("the ship modules after the round loop's retirement — a source scan", () => {
it("shipPipeline.ts keeps the config block, the caps, the preset and the round header, and nothing of the loop", () => {
const src = read("shipPipeline.ts");
expect(exportsOf(src).sort()).toEqual(
[
"ShipConfig",
"shipInterruptedNote",
"ShipCaps",
"SHIP_DEFAULT_MAX_ROUNDS",
"SHIP_DEFAULT_MAX_MINUTES",
"resolveShipCaps",
"shipPresetFor",
"shipRoundHeader",
].sort(),
);
for (const gone of [
"runShipPipeline",
"runRounds",
"ShipPipelineInput",
"ShipOutcome",
"ShipGithub",
"coordinator?",
])
expect(src, `${gone} is gone`).not.toContain(gone);
expect(src).not.toMatch(/from "\.\/ship\/(codingChild|reviewChild|childRound)\.js"/);
});

13. The branch's refusal when the runner is missing

With the default shim client, a process without PUBLIC_BASE_URL, then one without a coordinator entry in the token map, gets the reply naming exactly that; the card closes ⚠️ and nothing runs.

// agent-ship.md item 16: a deployment without the runner's prerequisites gets
// a refusal naming what is missing — the default shim client answers by
// reason when the bot cannot address its own shim or present the bearer —
// never a silent fallback to some other ship implementation.
it("a deployment without the runner's prerequisites is refused naming the missing one: no `PUBLIC_BASE_URL`, then no `coordinator` entry in the token map; the records are written, nothing runs", async () => {
const s = setup("slack:UADMIN");
delete s.deps.createCoordinatorInstance;
delete s.deps.fetchCoordinatorInstanceStatus;
await runShipBranch(s.deps, s.msg, s.io, s.ctx);
expect(s.replies).toEqual([
"⚠️ The plan runner could not be started: PUBLIC_BASE_URL is not set — the bot cannot address its own shim. Nothing ran; re-issue the request to try again.",
]);
expect(JSON.stringify(s.closes[0])).toContain("⚠️");
vi.stubEnv("PUBLIC_BASE_URL", "https://bot.example");
const t = setup("slack:UADMIN");
delete t.deps.createCoordinatorInstance;
delete t.deps.fetchCoordinatorInstanceStatus;
await runShipBranch(t.deps, t.msg, t.io, t.ctx);
expect(t.replies[0]).toBe(
"⚠️ The plan runner could not be started: SWITCHBOARD_INGRESS_TOKENS has no single `coordinator` entry — the bot cannot present the coordinator bearer. Nothing ran; re-issue the request to try again.",
);
});

14. The dispatcher's ship suite becomes the hand-off's

The old agent:ship (pipeline) describe drove the in-process loop through dispatch() 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 and afterEach asserts no workspace was attached and no model turn ran.

/** The dispatcher's deps for a ship request: a provider that must never be
* asked (the runner's children are runs of their own, none of them here),
* the preflight's GitHub facts, and the runner's seams — its records and its
* shim — as doubles that remember what was written and created. */
function shipDeps(yaml = SHIP_YAML) {
const provider = capturingProvider("never asked");
const deps = makeDeps(yaml, provider);
deps.resolveRepoContext = () => ({ repo: "acme/api" });
deps.statusUpdateMinMs = 0;
deps.fetchRepoShipInfo = vi.fn(async () => ({ allowAutoMerge: false, defaultBranch: "main" }));
deps.fetchPrFacts = vi.fn(async () => openBotPr());
deps.fetchSelfIdentity = vi.fn(async () => SHIP_BOT);
const instances = new InMemoryCoordinatorInstanceStore();
const created: string[] = [];
deps.coordinatorInstances = instances;
deps.createCoordinatorInstance = vi.fn(async (id: string) => {
created.push(id);
return { kind: "created" as const, id };
});
deps.fetchCoordinatorInstanceStatus = vi.fn(async () => ({ kind: "absent" as const }));
return { deps, provider, instances, created };
}
/** The one task instance a request became, with its row — what the runner is handed. */
async function handed(instances: InMemoryCoordinatorInstanceStore, runId: string) {

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.

it("a deployment without the runner's prerequisites is refused naming the missing one — here `PUBLIC_BASE_URL` — the card closes ⚠️, and nothing runs some other way", async () => {
const { deps, provider } = shipDeps();
delete deps.createCoordinatorInstance;
delete deps.fetchCoordinatorInstanceStatus;
const { io, replies, statuses } = fakeIO();
await dispatch(deps, msg(TASK_MSG, "slack:UADMIN"), io);
expect(replies).toEqual([
"⚠️ The plan runner could not be started: PUBLIC_BASE_URL is not set — the bot cannot address its own shim. Nothing ran; re-issue the request to try again.",
]);
expect(statuses[statuses.length - 1]!.title).toContain("⚠️");
expect(provider.requests).toHaveLength(0);
expect(fetchGuard).not.toHaveBeenCalled(); // the shim is never addressed without its URL
});
it("the ship request is claimed on the run ledger without a seed (item 35) and finishes through it: the live row goes, the record lands in the ledger, the plain store is never the fallback", async () => {
const registry = new RunRegistry({ genId: () => "rship-l", genToken: () => "tship-l" });

16. The driver's resume, on the wire

A task row carrying resume yields the step names plan, 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 ends merge_ready; the ending names no coding run.

describe("the plan runner's driver — a resume at review (agent-ship item 10)", () => {
it("a task row carrying a resume opens the unit at its first review round: 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 ends merge-ready for a person, and the ending names no coding run", async () => {
const s = steps({ "task/1/review/wait/1": "event" });
const b = bot({
plan: [

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.

// The last coding child's typed handoff (agent-ship item 14) rides the
// comment under the report when the ending names the child's run: read
// from its record, a run of this instance and no other — a run outside the
// instance, one the history lacks, or a record without a handoff leaves the
// comment as the report alone.
const handoff = {
deviations: [{ from: "one table", to: "two tables", why: "the row outgrew the record" }],
followUps: [],
unproven: [{ criterion: "the live receipt", why: "needs staging" }],
};
await h.store.put(
record("run-c0", {
parentInstanceId: PLAN_INSTANCE.id,
idempotencyKey: `${PLAN_INSTANCE.id}:U10/1/fix`,

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).

16. **Every `agent:ship` request hands to the plan runner** (`src/core/coordinator/handOff.ts`, `src/core/dispatch/ship.ts`; [record 0031](../../decisions/0031-the-coordinator-runs-a-plan-not-a-pull-request.md); the runner itself is [http-ingress.md](http-ingress.md) item 9). There is no switch: the `ship` config block holds `maxRounds` and `maxMinutes` and refuses any other key at load by name (`validateShip`) — `ship.coordinator`, the key that once chose between the runner and an in-process loop, with a line saying the runner is the one implementation and pointing at the [migration notes](../migrations.md). The branch runs the preflight (item 2's compound gate, item 3's branch name, item 10's entry checks), registers the run record and card, and hands the request to the runner: the request names a plan (`plan <path>.md [units U<n>, …]`, `parseShipPlanRequest` over the directive-stripped text) or a task; a plan is read at the pull request's base ref through the App (`GithubApi.readFile`), parsed with item 15's `parsePlanGraph` and narrowed by item 15's cursor to the selected units in the plan's order, and a task is a plan of one unit, `task`, on the ship branch of item 3 in the requesting thread — a resume at review (item 10) is that one unit with the pull request on its row (`CoordinatorUnit.resume`), on the pull request's own head branch and base. The bot writes the instance record — the requester, channel and thread, the card's handle, the caps as the profile gate clipped them (item 8: the rounds cap from the block, the wall clock the profile's minutes), the run id and the card's label ([run-history.md](run-history.md) item 49) — under the plan's id (`planInstanceId`, `plan-<plan-id>`; a task's under `ship-<run id>`), then one unit row per selected unit with its branch and the dependencies the plan states ([run-history.md](run-history.md) item 50), then asks its own shim for the Workflow instance: `POST <PUBLIC_BASE_URL>/admin/coordinator/instances` with the `coordinator` bearer of the token map the bot holds (`createInstanceViaShim`; the answer read by `readCreateInstanceAnswer`, the reverse of the shim's `createInstanceResponse`). The run then ends `completed` with where the plan runs as its answer and reply — the instance, the plan and its units in dependency order (a task: its branch in this thread; a resume: that the review loop resumes at its next review round with no coding round first), that each unit runs in a thread of its own in the channel under the requester's grants, that the card follows the plan and its summary lands in the requesting thread — and the card closes ✅; the runner's steps call back into the bot from there, and its `finish` writes the plan's story under the same run id ([http-ingress.md](http-ingress.md) item 9). Every refusal is a reply and an `aborted` hand-off (the card closes ⚠️; the run record still says `completed`, because the request was answered), never a throw, and nothing is created on one: no base branch known; a plan file the repository lacks at the base, naming the path, the ref and GitHub's words; a plan file whose name is not a plan id; a plan with no unit headings; a unit the plan lacks or a dependency cycle, in the cursor's words; a process without a durable instance store (`NullCoordinatorInstanceStore` — run history not on the state Worker); the shim's `duplicate_instance` for an id the state Worker had no record of; the shim's `create_failed`, a shim that could not be reached, `PUBLIC_BASE_URL` unset or a token map without a single `coordinator` entry, each by reason — **a deployment without the runner's prerequisites is told which one is missing and runs ship no other way**. The records are written before the create. **A plan runs by attempts**: no record under `plan-<plan-id>` is a first attempt; a record there is an earlier attempt's, and the shim's status of the latest attempt (`GET /admin/coordinator/instances/<id>`, the highest suffix with a record) decides before anything is written — still running (`queued`, `running`, `paused`, `waiting`, `waitingForPause`) refuses the request naming the instance and its status, and its rows, a live runner's threads, rounds, pull requests and endings, are never touched; no such instance (the leftover of a create that failed after the records were written) is replaced with this request's record and rows under the same id and attempt (`replace`, [run-history.md](run-history.md) item 49), the units earlier attempts merged skipped there too, and the reply says so; ended (`complete`, `errored`, `terminated`) is resumed as the next attempt under `plan-<plan-id>-<n>` with `attempt` on the record, rerunning only the units the earlier attempts did not merge (a `merged` ending on any attempt's row), each dependency kept on the row as the plan states it so a merged one counts as done, the reply naming what is left and what was merged; every named unit merged already is a refusal; a status the bot does not read as ended, a shim that could not say, and a `duplicate_instance` for an id the state Worker had no record of (a store wiped or restored) are refusals a person decides, nothing written. Thread follow-ups steer the runner's children like any run ([thread-admission.md](thread-admission.md) item 2): a reply in a unit's thread reaches the child in flight there; the requesting thread's own run is over once the hand-off answered.

10. **Entry checks make restarts safe.** Ship never opens a duplicate PR (open-or-edit by head branch; a re-issued plan finds its unit's pull request at the pre-check, item 15). A ship invocation **resumes at review** — skipping round 0 — only when ALL hold: a user turn names the PR (the thread→PR inference reads user turns only), it is open, authored by this bot's own GitHub identity (the App's bot user — `resolveGithubIdentity`, resolved from GitHub once per process and matched by login AND immutable id; the code names no bot), same-repo head, and the invocation carries no new task text. An identity that cannot be resolved refuses fail-closed: an open PR's authorship cannot be judged. A PR reference binds the entry checks only when the invocation carries no other task text (a resume request) or the PR is ship's own; a PR number quoted as evidence inside new task text does not bind — it stays in the task as context and round 0 starts from the repo's default branch (or a user-named `on <ref>`), never from the quoted PR's head branch. This holds even when the cited PR's facts cannot be fetched: a PR named in the CURRENT message (`repoContext.prFromMessage`) beside new task text falls through to round 0 rather than refusing on the transient failure, and the round-0 base drops any PR-derived ref (`repoContext.refFromPr`, flagged at the resolver) so the failed fetch cannot leak the stranger's head branch. Kept fail-closed on a fetch failure: an INHERITED thread PR (not in-message), and a BARE in-message reference with no task text (a resume attempt must verify the PR first). A new task over ship's own still-open thread PR is refused naming the PR; a bare reference to a human-authored PR is refused as not ship's to drive. A resume is handed to the runner like every request (item 16): the one task unit's row carries `resume` — the pull request, its head and url — and the machine opens that unit at its first review round with no pre-check, no branch and no round 0 (`openUnitPipeline`'s `resume`), on the pull request's own head branch and base; the review child pins the head and the approve ends `merge_ready` for a person, as item 9 has it. No pipeline state lives in the bot process — the runner's is the Workflow's, durable across bot deaths (item 15) — so a bot death under a pipeline interrupts a **child**: the next generation closes that run `interrupted` and tells its thread ([run-history item 36](run-history.md)), the runner reads the record and ends the unit with the same note (`shipInterruptedNote`): the PR it had opened, if any, and the exact re-issue that continues the loop — `agent:ship` with only the PR URL (this item's resume-at-review), or with the task when no PR existed (round 0 again on the same deterministic branch; a plan is re-issued as its next attempt, item 16). Nothing restarts a pipeline unattended (the ship-restart plan's D1, which the runner keeps).

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.

## 1.208.0
- `ship.coordinator` is gone from `config.yaml`, and the load refuses it by name (`ship.coordinator is no longer a key — every agent:ship request runs on the plan runner; remove it`). The in-process ship round loop it switched off no longer exists: every `agent:ship` request — a task, `plan <path>.md [units …]`, or a ship pull request's URL to resume at review — runs on the plan runner, whose rounds are child runs of their own ([agent-ship.md](specs/agent-ship.md) item 16). Delete the key from the `ship` block. A deployment that ran ship without the runner must now carry the runner's prerequisites — a `coordinator` entry in `SWITCHBOARD_INGRESS_TOKENS` with `grants.http:coordinator: { actions: [coordinator:step] }` (add `plan:merge` for the runner to merge plan branches), `PUBLIC_BASE_URL`, and run history on the state Worker ([Turn features on and off](../how-to/turn-features-on-and-off.md), the `coordinator` row) — or `agent:ship` is refused naming the missing one; nothing else about the request's shape changes.

20. The ship-restart plan is superseded

Only the two status lines change; the body is untouched, as decisions:check requires.

---
title: Ship pipeline after a bot death - Plan
type: feat
date: 2026-09-08
status: superseded
superseded_by: 2026-09-10-001-feat-orchestration-program-plan.md
extends: 2026-09-08-001-feat-durable-runs-plan.md
artifact_contract: ce-unified-plan/v1

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.

### U15. Retire the in-process round loop and settle the inherited follow-ups
- **Status**: built. The loop is deleted and every `agent:ship` request hands to the runner; two decisions the unit left open were taken at build time: the `ship.coordinator` key is removed and refused at load by name (a deployment without the runner's prerequisites is refused naming what is missing, never served some other way), and the resume at review — `agent:ship` with a ship pull request's URL — runs on the runner too, as a one-unit instance whose row carries the pull request so the machine opens at its review round (the machine already had the input; the driver and the row gained the field). The runner's `unit-end` now renders the last coding child's handoff into the board comment, since the loop's coding round was the only path that posted it. The ledger below has no row without a disposition; the ship-restart plan is `superseded` by this plan.
- **Goal**: One ship implementation, and every follow-up this plan inherited has a disposition.

22. The example config says what ship needs

The ship block's comment names the runner and its prerequisites and drops the coordinator line.

# agent:ship pipeline caps (docs/reference/specs/agent-ship.md). `agent:ship in owner/repo:
# <task>` hands the coding → review → fix loop to LGTM to the plan runner — the
# ShipCoordinator Workflow in the bot Worker, whose rounds are child runs — which
# needs the `coordinator` ingress entry, its grants, run history on the state
# Worker and PUBLIC_BASE_URL (docs/how-to/turn-features-on-and-off.md); without
# them the request is refused naming what is missing. Per unit: at most
# `maxRounds` review rounds and `maxMinutes` minutes of wall clock — whichever
# hits first ends the unit, and each child round's own budget is clipped to the
# remaining time. `maxMinutes` is the ship preset's declared budget: a channel or
# user `boundary.maxMinutes` and a per-message `budget:` directive clip it like
# any preset's, and the card says so. Defaults shown; both must be integers >= 1;
# any other key under `ship` fails the load by name.
# ship:
# maxRounds: 3 # review rounds per unit
# maxMinutes: 120 # the ship preset's wall-clock budget in minutes (the registry's default)

23. Remaining changes

  • src/core/ship/childRound.ts — deleted (the shared child-round slices had no reader left).
  • src/core/ship/reviewChild.ts — shrinks to buildShipReviewTurn; 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 — the shipPipeline.ts/ship/ row, the dispatch/ row's ship.ts entry and the coordinator/ row.
  • docs/how-to/turn-features-on-and-off.md — the ship.coordinator row removed; the coordinator row's On/Off columns say agent:ship always 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.round stays in the span table for the records the loop wrote.

Decisions

  • ship.coordinator is 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.
  • The resume at review runs on the runner. The machine already had resume on openUnitPipeline and 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.
  • The handoff comment moves to 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.
  • No ! in the title. The release pin is off since the 1.200.0 cut, so check:pr-title accepts a ! only with a ## 2.0.0 migration 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).
  • Item 11 tells the truth about the children. The loop's "never a per-round cold clone" is not how the runner works: a child is an ordinary run — a repository not onboarded is refused at its authorize stage (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.
  • Kept as data compatibility: ship.round in the stream-span table and its display name, so the records the loop wrote still classify; tracing.md says no emitter opens it.
  • Not done here: parallel ready units (noted as a gap in the spec); U14's deploy stage stays deferred with its own trigger.

Validation

  • npm run verify on 0b1a693c (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 tests 369 files / 6737 passed, 2 skipped, check:dist ok, every workspace green, check:site ok. Log: u15-verify-rebased.log in the session scratchpad; the pre-rebase head aee9d958 passed 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.ts ship 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.ts 12 → 4; reviewChild.test.ts 7 → 4; ship.test.ts 9 → 9 (rewritten); the guard's check: lines are these renames and the lower expect() 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.
  • TDD red step: src/core/ship/surface.test.ts ran 3/3 failed on the tree before any source change, then green.
  • Added tests: the source scan (3); the resume — validator (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 through dispatch(); ship.coordinator refused 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:check accepts status: superseded + superseded_by on the ship-restart plan with the program plan's body amended while proposed.
  • Human-gated, still owed after the release that carries this deploys: a production agent:ship <task> runs on the runner with no other path (the card says 🧭 Handed to the plan runner ship-<run id>, runs get shows a parent record with no model.turn of its own); a resume agent: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

@coreplane-switchboard coreplane-switchboard Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 the ship.coordinator switch are gone; dispatch/ship.ts now 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 to handOffToCoordinator.
  • validateShip refuses ship.coordinator by name with a migration pointer and rejects any other unknown key (SHIP_KEYS held equal to ShipConfig) — 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 resume field: isCoordinatorUnit validates it, the driver feeds it into openUnitPipeline, which opens directly at nextReview with lastReviewHead seeded — no pr-check, no branch, no round 0. Preflight prefers the PR's own base, as the spec row says.
  • unit-end now renders the last coding child's handoff into the board comment via codingHandoffOf, correctly gated: run-id pattern check, parentInstanceId must match the instance (the field exists on RunRecord/RunView), isHandoffShape check, never a refusal — the ending posts either way.
  • The new surface.test.ts source 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.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto-approved: coreplane-switchboard[bot] reviewed this PR and posted an LGTM verdict (see its review). This repository opted in through its REVIEW_BOT_LOGIN and REVIEW_BOT_ID variables.

…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>
@justinhelmer
justinhelmer force-pushed the feat/u15-retire-ship-round-loop branch from aee9d95 to 0b1a693 Compare September 14, 2026 03:10
@justinhelmer

Copy link
Copy Markdown
Contributor Author

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 src/core/ship/reviewChild.test.ts (this PR's rewrite kept; #1013's two toMatchObject tweaks targeted the runShipReviewChild tests this PR removes). ReviewPostOutcome stays defined in reviewRound.ts, imported by runLoop.ts. npm run verify on the new head: exit 0; specs:check 2706 proofs; test-guard ok. The Tour's anchors are regenerated at the new head.

@coreplane-switchboard coreplane-switchboard Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 messagegit 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.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto-approved: coreplane-switchboard[bot] reviewed this PR and posted an LGTM verdict (see its review). This repository opted in through its REVIEW_BOT_LOGIN and REVIEW_BOT_ID variables.

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.

1 participant