fix(crews): no seat launches into a retired Crew or under a gone Captain

[bryantderosier]

Important

⚠️ Merge order: merge these in this exact order

This PR is part of the Crews stack. Merge top to bottom, one at a time, and let each land on j5/main before the next. #292 isn't in the stack but has to merge before #313.

1. fix(crews): Crew notices report only what the platform measured #292: Crew notices report only what the platform measured (adds migration 020). Not stacked, but it must merge before fix(crews): a proposal launches once and reports every seat #313.
2. fix(crews): no seat launches into a retired Crew or under a gone Captain #271: No seat launches into a retired Crew or under a gone Captain ⬅️ this PR
3. fix(crews): unit stop and archive finish over a seat that was never created #279: Unit stop and archive finish over a seat that was never created
4. fix(crews): a proposal launches once and reports every seat #313: A proposal launches once and reports every seat (adds migration 021)
5. fix(crews): Crew groups keep seats without thread facts as unknown #300: Crew groups keep seats without thread facts as unknown
6. fix(crews): a Crew follows its Captain through every lifecycle step #315: A Crew follows its Captain through every lifecycle step (adds migration 022)

Why #292 goes first: J5 migrations run in id order, and the migrator skips any id at or below the newest one a database has already applied. If #313's migration 021 ships before #292's 020, 020 never runs on that database.

Two open windows let seats come into existence where they shouldn't. First, a roster or addition left open while its Captain was archived or deleted could still be approved. Second, unit archive read the roster first and marked the Crew retired last, so an addition approved in between could spawn a live seat under a retired Crew.

What I changed:

• A gone Captain is refused at approval. Preview and approval refuse a proposal whose Captain thread is archived or deleted, and the message names the Captain. An archived Captain can be unarchived to approve. This is a check when the person approves, not a guarantee held through the launch: a Captain archived after that read isn't caught. Decline still works; for a deleted Captain it skips the notice, because the orchestrator refuses commands on a deleted thread and the gate would otherwise never close.
• One per-Crew lock for every unit step. Launch (record through briefs), addition (reservation through briefs), and unit archive (roster read through the retired stamp) run under AgentCrewInstanceService.serialize, which is built on orchestration-v2's KeyedSerialExecutor, so an idle Crew's entry is released. A seat either exists before the archive reads the roster and retires with the rest, or it waits and meets the existing retired check in addMembers before any seat is reserved. The Captain cascade goes through the same archive.
• No migration. The lock is in-process. An archive cut short by a restart never reached the stamp, so the Crew it leaves is still live.

Left out on purpose:

• The cascade boot reconcile and the proposal boot-sweep decline: cascade removal (product(j5): decide what archive, unarchive, and settle mean for agents, Crews, and their children #254) and launch-once remove that code.
• A launch retry into a retired Crew: retries only exist through the gate reopening, which launch-once removes.
• A seat reserved but never spawned when the server restarts: that's Unit stop and archive complete when a reserved seat was never created #224 / fix(crews): unit stop and archive finish over a seat that was never created #279.

Tests (Deferred and a queue, no sleeps):

• CrewProposalService.test.ts: approve after the Captain was archived, and after it was deleted or is missing, for both a roster and an addition; decline still works in each case.
• CrewLaunchService.test.ts: the real launcher and the real unit archive over one Crew store. An archive that arrives while an addition is spawning waits, then retires the new seat with the rest; this one fails with the lock swapped for a pass-through. An addition that arrives while archive holds the roster ends up refused with no seat of its own. That test proves the outcome, not the lock, because without the lock the addition can still happen to land after the stamp.

Closes #223

Done by Claude Opus 5.5 in Claude Code (T3 Code).

🤖 Generated with Claude Code

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Repository: Jacksondr5/j5code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: ced0b1ab-de3d-4ec8-9771-d99a65b49744

---

_{Comment @coderabbitai help to get the list of available commands.}

[Jacksondr5]

Claimed for review by Jackson with Claude Opus 5.5 and GPT-6-Astra (the crews review thread). Other agents: please skip this one.

[Jacksondr5]

Bryant, a sequencing question before we review these line by line, covering both this PR and #279. Neither review pass found a correctness blocker: no reachable deadlock in the per-Crew lock, no false "never created" for a thread that committed, and CI is green. What we can't square is where these two sit relative to the two follow-ups you scoped last week, neither of which is open yet:

1. Launch-once. On fix(crews): launch retries converge on the final approved roster #235 you said you'd fold the AC7 rewrite, the removal of the claim states, reopen, and the boot sweep into one PR, including dropping the row of a seat whose thread.create failed and naming it in the launch report. On fix(crews): the twelve-seat cap counts pending additions atomically #239 you noted it should land after that one.
2. Cascade removal. On product(j5): decide what archive, unarchive, and settle mean for agents, Crews, and their children #254 you chose option A and said a separate PR removes the Captain→Crew cascade, its boot sweep, the lone-seat guard and the Captain refusal, with a Crew that outlives its Captain grouping at the Squadron root. You said the same on #244.

#271 and #279 mostly harden the code those two delete, so landing them first means the next two PRs unwind about a thousand lines. Sorted by whether each change survives:

Survives both, worth landing:

• fix(crews): no seat launches into a retired Crew or under a gone Captain #271: the decline under a deleted Captain skipping the notice so the proposal can close. That's a real bug fix. Also refusing approval under a deleted Captain, since the orchestrator refuses commands on a deleted thread and seats could never report.
• fix(crews): unit stop and archive finish over a seat that was never created #279: unit stop passing over a row with no thread and still interrupting the running seats after it. A batch stop still needs that under option A.
• The per-Crew lock, but only while fix(crews): unit stop and archive finish over a seat that was never created #279's two-absence proof depends on it. Without it, a read can land between a seat's thread commit and its home registration. If it stays, swapping the hand-rolled map for orchestration-v2/KeyedSerialExecutor.ts gets idle-key eviction for free. Today every Crew id ever touched, including mistyped ones, keeps an entry for the life of the server.

Hardens code the follow-ups remove:

• fix(crews): no seat launches into a retired Crew or under a gone Captain #271, refusing an archived Captain. Under option A a Crew that outlives its Captain is legal, and the check is point-in-time anyway: it reads the Captain before the claim and the lock, so a Captain archived in that gap still gets a Crew beneath it (the code comment at CrewProposalService.ts:282 claims more than that).
• fix(crews): no seat launches into a retired Crew or under a gone Captain #271, the launch retry into a retired Crew. Retries only exist through reopen.
• fix(crews): unit stop and archive finish over a seat that was never created #279, unit archive and Captain-cascade skipping. Both are headed out.
• fix(crews): unit stop and archive finish over a seat that was never created #279's main case, a row left by a failed create, which launch-once drops at launch. What's left is a restart mid-launch. Separately, a crash after a seat's thread commits but before its home registers still blocks unit archive (correctly, with a mismatch error), so "every partial Crew can retire" overclaims.

Our suggestion: land the surviving pieces on their own (trimmed #271, and #279 narrowed to stop), then open launch-once and cascade removal next, so nothing else lands against code that's about to be deleted. If there's a reason these needed to go first, for example something in dogfood that's broken today and can't wait, tell us and we'll review them as they stand.

Reviewed by Claude Opus 5.5 in Claude Code, with an independent pass by GPT-6-Astra in Codex.

[bryantderosier]

Thanks, this is a fair read, and I agree with the sequencing: launch-once and cascade removal are next, and nothing else lands against code they delete. I've narrowed both PRs in place instead of splitting them.

Changed (5c1a614 on #271, 6976480 on #279):

• The lock now runs on KeyedSerialExecutor, so an idle Crew's entry is released. I should have reused it in the first place.
• The launch-retry refusal and its test are gone; retries only exist through reopen.
• The Captain-check comment now says what it is: a read when the person approves, not a guarantee. The PR body says the same.
• neverCreatedSeats is gone. The HTTP routes just leave those seats out of members; the MCP results keep never_created per seat.
• The cascade test on fix(crews): unit stop and archive finish over a seat that was never created #279 is gone.
• fix(crews): unit stop and archive finish over a seat that was never created #279's body now names the case it doesn't cover: a crash after a seat's thread commits but before its home registers still stops unit archive with a mismatch error.
• One correction to my own claim: the "addition arrives while archive holds the roster" test proves the outcome, not the lock. Without the lock it can still pass by landing after the stamp. The mid-spawn test is the one that fails without the lock, and the body now says so.

Where I'm keeping things:

1. Refusing an archived Captain stays. It isn't there for the cascade. A new Crew's seat briefs name the Captain as the one they report to, and delivery refuses archived participants, so seats approved under an archived Captain start out unable to reach it. Under option A a Crew can outlive its Captain, but I don't want the gate creating a new one under a Captain nobody can message. Unarchiving to approve is the way out, and it's a point-in-time check, which the comment now says plainly.
2. Unit archive keeps its never-created handling on fix(crews): unit stop and archive finish over a seat that was never created #279. The cascade is going, but per my call on product(j5): decide what archive, unarchive, and settle mean for agents, Crews, and their children #254, archive_crew and the Fleet's Archive crew stay as batch conveniences. Launch-once drops a failed create's row, but a restart mid-launch still leaves rows with no thread, and the batch archive shouldn't fail on them any more than the batch stop should. Only the cascade case came out.
3. The Crew-stop test stays in runtimeLayer.test.ts. I looked at moving it. It runs on the live orchestrator (SharedApplicationDataPlaneTestLayer), which that file builds privately from upstream testkits. A J5 copy would duplicate about 30 lines of upstream wiring that drift on every sync, which is worse for the fork than two mock lines inside a block Record the Crew-stop replay test under FORK.md case 3 #232 already tracks. I'll record it under Record the Crew-stop replay test under FORK.md case 3 #232.

If any of those three still look wrong to you, say so and I'll take them out; none of them block the rest.

[Jacksondr5]

Approving at 5c1a614. Verified: the lock now runs on KeyedSerialExecutor so idle Crew entries are released, the launch-retry refusal is gone (the remaining retired checks predate this PR), and the Captain-check comment describes a point-in-time read. Your case for keeping the archived-Captain refusal holds: seat briefs name the Captain as the one they report to and delivery refuses archived participants, so a new Crew under an archived Captain would start unable to reach it. Thanks for correcting the race-test claim too. Merges before #279; launch-once and cascade removal next, as agreed.

Reviewed by Claude Opus 5.5 in Claude Code, with an independent pass by GPT-6-Astra in Codex.