Skip to content

fix(crews): unit stop and archive finish over a seat that was never created - #279

Merged
bryantderosier merged 4 commits into
j5/issue-223-trimmedfrom
j5/issue-224-never-created-seats
Sep 25, 2026
Merged

bryantderosier merged 4 commits into
j5/issue-223-trimmedfrom
j5/issue-224-never-created-seats

Conversation

@bryantderosier

@bryantderosier bryantderosier commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator

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
  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 ⬅️ this PR
  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.

A launch records every seat's row before any seat thread exists, so a launch or addition that failed partway leaves rows with no thread behind them. Unit stop and unit archive read each row as a live seat and failed on the first missing one: a stop gave up before it reached later seats that were still running, and the Crew couldn't be archived as a unit.

What I changed:

  • The proof of "never created". A seat qualifies only when two absences agree: no home is registered for its deterministic thread id, and the projection store answers not-found for it. Both reads run under the per-Crew lock from fix(crews): no seat launches into a retired Crew or under a gone Captain #271, so no launch or addition is creating the seat meanwhile. A store that can't answer, or a home with no thread behind it, still fails the operation loudly.
  • Unit stop takes the same lock, reports a missing seat as never_created, and still interrupts every running seat after it.
  • Unit archive (archive_crew and the Fleet's Archive crew) skips such seats, reports them as never_created, finishes on the seats that exist, and stamps the Crew retired. The row stays on the retired roster. Under the product(j5): decide what archive, unarchive, and settle mean for agents, Crews, and their children #254 ruling these doors stay as batch conveniences, and even with launch-once dropping failed-create rows, a restart mid-launch still leaves rows with no thread.
  • Wire shape. The HTTP routes leave never-created seats out of members, so every client decodes what it gets. The agent-facing MCP results say never_created per seat.

What this doesn't cover: a crash after a seat's thread commits but before its home registers still stops unit archive with a mismatch error. That's the loud failure I want there, since the seat exists, but it means not every partial Crew archives cleanly.

Tests:

  • ArchiveAgentService.test.ts: readFacts returns null only when both the home and the thread are absent; an unreadable store and a home with no thread both fail.
  • ArchiveCrewService.test.ts: the unit retires past a never-created seat and replays as already archived; a seat whose facts can't be read fails the unit and retires nothing.
  • CrewStopService.test.ts: a missing seat ahead of a running one still lets the stop interrupt it; an unreadable seat or a homed seat with no thread fails the stop.
  • CrewLaunchService.test.ts: with the real launcher, an addition reserves its seat, the spawn fails, and the unit archive still finishes.
  • CrewArchiveHttp.test.ts, CrewStopHttp.test.ts: never-created seats are left out of members.

One upstream-file touch: the existing J5 Crew-stop block in orchestration-v2/runtimeLayer.test.ts gains two mock lines (the lock and the home read). #232 already tracks recording that block in FORK.md.

Stacked on #271.

Closes #224

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

🤖 Generated with Claude Code

@bryantderosier bryantderosier self-assigned this Sep 24, 2026
@bryantderosier
bryantderosier added this pull request to stack #280 September 24, 2026 18:09
@coderabbitai

coderabbitai Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

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: c82f5ed4-5635-4d30-a184-431b541065bd


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

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 effective changed lines (test files excluded in mixed PRs). labels Sep 24, 2026
@Jacksondr5 Jacksondr5 added the jackson-direct Taken by Jackson + Astra outside the fleet methodology; lanes never staff these label Sep 24, 2026
@Jacksondr5

Copy link
Copy Markdown
Owner

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

Copy link
Copy Markdown
Owner

Covered together with #271 in one comment, since the question is the same for both: #271 (comment)

The short version for this PR: the unit-stop handling survives option A and is worth landing on its own. The unit-archive and Captain-cascade skipping hardens code your cascade-removal PR takes out, and the main case (a row left by a failed create) goes away when launch-once drops that row. Two smaller notes if it lands as is. Nothing reads the new neverCreatedSeats field yet. And the upstream orchestration-v2/runtimeLayer.test.ts touch waits on #232; moving that Crew-stop test into a J5-owned file would avoid the upstream edit entirely.

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

@bryantderosier
bryantderosier force-pushed the j5/issue-224-never-created-seats branch from f035381 to 6976480 Compare September 24, 2026 19:17
@bryantderosier

Copy link
Copy Markdown
Collaborator Author

Replied on #271 for both PRs, since the question was the same: #271. The short version for this one: I dropped neverCreatedSeats and the cascade test in 6976480, and fixed the body to name the thread-committed-but-home-missing case it doesn't cover. Unit archive's never-created handling stays, because archive_crew and the Fleet's Archive crew survive option A and a restart mid-launch still leaves rows with no thread. The runtimeLayer.test.ts touch stays, and I explain why on #271.

@Jacksondr5 Jacksondr5 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Approving at 6976480. neverCreatedSeats and the cascade test are gone, and the body now names the thread-committed-but-home-missing case it doesn't cover. You're right that unit archive keeps its never-created handling: archive_crew and the Fleet's Archive crew survive option A, and a restart mid-launch still leaves rows with no thread. The runtimeLayer.test.ts touch is the smaller cost than duplicating the upstream orchestrator wiring; recording it under #232 is the right place. Merges after #271.

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

bryantderosier and others added 2 commits September 24, 2026 16:32
…reated

A launch records every seat's row before any seat thread exists, so a
launch or addition that failed partway leaves rows with no thread. Unit
archive, the Captain cascade, and unit stop all read every row as a live
seat and failed on the first missing one: the Crew could not be retired,
and a stop gave up before interrupting later running seats.

A seat now counts as never created only when two absences agree: no home
is registered for its deterministic thread id, and the projection store
answers not-found for it. Both reads run under the Crew's unit lock, so no
launch or addition is creating the seat meanwhile. Unit archive and stop
pass such a seat by, report it as never_created, and finish on the seats
that exist; the row stays on the retired roster. A store that cannot
answer, or a home with no thread behind it, still fails loudly.

The HTTP responses the apps decode carry these seats in a new optional
neverCreatedSeats list rather than a new per-seat value, so older mobile
builds still decode them. The agent-facing MCP results say never_created
per seat.

Refs #224

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ade tests

Review follow-ups:
- Nothing in the apps reads a never-created list, so the optional
  neverCreatedSeats field is dropped. The HTTP routes leave those seats out
  of members, which every client already decodes; the MCP results still
  say never_created per seat.
- The Captain-cascade case is dropped because the cascade removal takes
  that code out. Unit archive keeps its never-created handling, since
  archive_crew and the Fleet's Archive crew survive as batch conveniences
  and a restart mid-launch still leaves rows with no thread.

Refs #224

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
bryantderosier and others added 2 commits September 25, 2026 14:55
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The sync replaced NodeSqliteClient.layerMemory() with NodeSqliteClient.layer({ filename: ":memory:" }); the Crew stop tests this branch adds still called the old one.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@bryantderosier
bryantderosier merged commit ce32280 into j5/main Sep 25, 2026
29 checks passed
@bryantderosier
bryantderosier deleted the j5/issue-224-never-created-seats branch September 25, 2026 20:23
bryantderosier added a commit that referenced this pull request Sep 25, 2026
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

# Conflicts:
#	apps/server/src/j5/a2a/runtimeLayer.ts
bryantderosier added a commit that referenced this pull request Sep 25, 2026
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jackson-direct Taken by Jackson + Astra outside the fleet methodology; lanes never staff these size:L 100-499 effective changed lines (test files excluded in mixed PRs). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unit stop and archive complete when a reserved seat was never created

2 participants