Repository navigation
fix(crews): a Crew follows its Captain through every lifecycle step - #315
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Repository: Jacksondr5/j5code/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: Comment |
A Crew proposal now resolves exactly once, open to approved or declined, by compare-and-set under a per-proposal lock. The claimed states, reopen, and the boot sweep for lost claims are gone; migration 021 hands any leftover approving or declining row back as open. Once seats start spawning, a seat that fails does not stop the others. A seat whose thread was never created is dropped from the roster and the launch report names it on a seat_not_created line; a seat whose brief never went out reports not_started. The web launch card shows both. Closes #311 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The migration 021 test this branch adds still called NodeSqliteClient.layerMemory(), which the sync replaced with NodeSqliteClient.layer({ filename: ":memory:" }).
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The upstream sync's Badge lint (shadcn/no-restyle) refuses spacing and typography classes on <Badge>; the not-created row now uses size="sm" like the roster rows beside it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The sidebar expander dropped any Crew seat whose thread was not in client state, and the Fleet page grouped a Crew only from placed participants, so a Crew could under-count or vanish while its seats were unknown. The spawned-children read now gives a Captain's row every seat on its live rosters that no parent holds, and the Fleet read emits a thread-less row for a roster seat the ledger never recorded. The sidebar keeps a seat with no thread as an unknown row, and the Fleet tree hangs an unplaced seat under its Captain, so both summaries count every seat. Closes #227 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There is never a Crew without its Captain. The Captain cascade already retired a Captain's live Crews on archive or delete; it now also brings back the Crews that retired with it when the Captain is unarchived, seat threads included, and sends a Captain's settle or unsettle to every seat of its live Crews. Unsettle reaches only seats that were settled, so a seat that never settled keeps upstream's automatic settlement. Migration 022 records whether a Crew retired with its Captain, so a Crew retired on its own through archive_crew or Archive crew stays retired. The boot sweep also settles the seats of a settled Captain and restores Crews under a Captain that is live again. A seat is still never archived on its own. Closes #312 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The upstream sync withheld the archive Undo notice whenever an archive might retire a Captain's Crews (decision #7, 2026-09-24), because unarchiving the Captain did not bring them back. With this branch it does: the Crews that retired with their Captain return with its unarchive. So the undoable plumbing is gone, useThreadActions.archiveThread is upstream's again, and the Sidebar, header menu, and LegacySidebar doors call it as upstream does. Refs #312 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
5143d85 to
99969ca
Compare
|
[Review panel: Opus 5.5 + Astra] FORK.md: the file-table row for a file this PR edits is stale. This PR edits The rest checks out. |
|
[Review panel: Opus 5.5 + coordinator (Opus 5.5)] Docs: missing History line. This PR rewrites |
|
Heads-up for the register of divergences (added in #329, |
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…th their own ids The Captain cascade derived each seat's archive command ids from the Captain and the Crew alone, so archive, unarchive, then archive again reused the first archive's ids: the orchestrator replayed the old receipt, the seat stayed unarchived, and the Crew stayed live under an archived Captain. The retire request key now carries the occurrence, the triggering event id on the stream and the Captain's archive or delete time on the boot sweep. Each Crew step runs once and logs its failure with the cause; the two in-session retry loops are gone. The boot sweep keeps only its retire leg for Crews of an archived or gone Captain; its settle and restore legs are removed, so unarchive, settle, and unsettle have no restart recovery. A Captain's settle skips a seat with a pending runtime request, a live run, or background work, mirroring upstream's isAutoSettlementCandidate through threadShellFromProjection. Seat reads use getThreadProjectionIfPresent, so only a genuine not-found skips a seat and any other failure fails the step. Refs #312 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Crews AC18 now reads that J5 never settles a seat because its run finished, settling the Captain settles its seats, and upstream's settle rules apply to every thread; the Definition says the same. The docs no longer claim the boot sweep settles or restores Crews, say that Undo and unarchive don't restore interrupted runs or dropped Exchanges, and give the repair for an unarchive that stops partway. The retired Crew roster shows why each seat joined, since it records no approver, and the Fleet page history records the AC28 change. FORK.md's useThreadActionMenu row points at case 21, which now names the file and describes the cascade's run-once steps and retire-only sweep. Refs #312 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
FORK.md's proposal-flow paragraph still described the retry model this PR removes (the handed-back gate, converging retries, a decline that archives spawned seats) and listed migration 16's claims as current. It now says a proposal resolves once before any seat spawns and names migration 21. The agent-tools page gains its History line for the change. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@Jacksondr5 Everything from the panel and your 2026-09-26 decisions is in (1e5fc2a, b2abd86):
I also merged #313's docs fixes and the latest |
|
[Review panel: Opus 5.5 + Astra] Merge prerequisite: the cascade's stream replays all history on every boot (#349). This bug is already on On This PR widens the damage, because the cascade now also acts on replayed Please land the #349 fix before this merges, or in this PR: take the high-water mark from the event store's |
|
[Review panel: Opus 5.5 + Astra] Verified in b2abd86:
The one open item on this PR is the #349 merge prerequisite above. |
|
[Review panel: Opus 5.5 + Astra] Round-two web verification: archive → Undo → archive retires the Crew again. Undo and Settings unarchive restore the Crew and its seats. Before/after use the same seeded Crew, light theme, and 1440×1000 viewport. After archiving a Captain — before: no Undo control. After archiving a Captain — after: Undo is available. Settings unarchive — before: the Crew remains retired. Settings unarchive — after: the Crew and all three seats are restored in Fleet. Watch archive → Undo → archive. Busy-seat check also passes: settling the Captain skipped a running seat and a seat awaiting approval, settled the idle seat, and unsettling the Captain reached only the settled seat. Restart remains FAIL due to #349, already posted as a merge prerequisite. The removed settle sweep is fixed, but the legacy high-water query replays old lifecycle events. The paired baseline screenshots used a test-only cursor marker to isolate UI behavior from that replay; the main archive/Undo and busy-seat tests used the unmodified review tip. |
|
@Jacksondr5 The #349 fix is up as #352. The finish notifier and the launch reporter now start their streams, and SilenceDetector seeds its cursor, from |
Jacksondr5
left a comment
There was a problem hiding this comment.
Posted by an AI agent on Jackson's behalf.
Approved. The review panel (Opus 5.5 + Astra) verified every round-one finding, in code and live on a copy of real data, with evidence on this PR:
- archive → Undo → archive retires the Crew again;
- settling a Captain skips busy seats, and unsettling reaches only settled ones;
- the startup restore and settle steps and the retry loops are gone;
- AC18 uses Jackson's wording.
The only new commit since the review is the merge of j5/main after #292, #313 and #300 landed.
Merge after #353 (the #349 restart-replay fix). Without it, a restart replays history through this cascade too, including unarchive, settle and unsettle.
Conflicts in the Crews docs resolved to the shipped wording from #313 and #315, keeping this branch's corrections: - crews.md: the Lifecycle sentence now says a Crew comes back only with its Captain, and the 2026-09-24 History line drops "Crews never unarchive as a unit" (Jackson's suggestion). The roster snapshot records each member's reason and no approver. History is back in date order. - agent-tools.md: the launch report keeps not_started, seat_not_created, and resolve-once, and the seat finish notice adds the unavailable status and over-read-limit case. - fleet-page.md: AC28 takes #315's wording, with both History lines. - personas.md: #315's paragraph, with Retired crews described as one section across every Squadron. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…s product (#329) * docs(j5): agents know what J5 owns and stop before changing upstream's product AGENTS.md and the PR template become J5-owned. Agents get a map of J5's domain, three zones (J5's domain, code overlap, upstream's product), and a rule to bring product changes to the person with trade-offs. A register of approved divergences lives in docs/j5/product/upstream.md. The PR checklist and screenshot guide cover the failures reviews keep finding. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(j5): seed the register of divergences from FORK.md and the worklog Records every product divergence from upstream with its ruling, cost and FORK.md cases, and lists the ones no human has ruled on as awaiting a decision. Drops the retired truthful-steer clause from the scoped principles. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(j5): address review of the agent guidance and rewrite the register AGENTS.md keeps upstream's structure: Theo's note, Taste and Additional tips return as their own sections, J5's additions sit apart, and the screenshot wording is upstream's again. Archive and the PR pane leave the J5 overview. The restart principle becomes "Repair beats edge-case machinery". The register gets IDs and one subsection per divergence (upstream, J5, why, consequences, decided), with attributions checked against the records. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(j5): D1 points at #336, D2 explains J5's own tools, D3 marked provisional Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(j5): record that D4-D6 are still needed against upstream V2 (2026-09-28) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(j5): D4-D6 say J5 wants to drop them once upstream fixes the cause Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(j5): fold Squadron surfaces into D7, retire the steering principle D8 (draft wording) and the Squadron half of D12 (cards) become part of D7; the Crew anchor and expander are additive UI, not a divergence. The scheduled-task entry now says J5 wants to close the gap (#273, #38), and multi-model send points at #338. The steering scoped principle is removed: only one behavior still rested on it, and D4 carries its reasoning. Entries are renumbered D1-D27 while the register is unmerged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(j5): keep mobile gaps out of the register; D13 retires with #315 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(j5): D14 watches upstream A2A, D15 points at #339/#340, D16 at #341 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(j5): settle most awaiting divergences and renumber the register Folds organize authorization and merge-back into the Squadron entry and branding into on-disk separation; moves the Codex floor, the wizard stage and the plan export into the decided sections; explains the @ menu, the usage-limit Stop rule and pair in plain terms; Astra aliases are marked for removal (#342). The register now runs D1-D24. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(j5): pair is decided; D22 and D23 say concretely what changes Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(j5): Squadrons scope project actions; usage-limit Stop to follow upstream The Squadron definition's "never a boundary" now covers communication and reading; actions upstream scopes to a project are scoped to the Squadron, so D8 no longer conflicts with it. The @ menu is decided. The usage-limit Stop extension (#343) and the Astra aliases (#342) move to a new "To be removed" section. Only D22 still awaits a decision. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(j5): resolve the #329 review findings from Bryant, Basti and CodeRabbit AGENTS.md: scope the plans/research and docs/internals rules to what J5 actually commits, fix the J5CODE_HOME wording and the --share command, drop npx t3, and say that Cursor's priority isn't parity. Overview names both human-contact modes and says the PR pane is defined but unbuilt. Squadron merge-back includes threads with no home. The register keeps retired entries under Retired, corrects D2, D6 and D22 against the code, completes D18 and D21, and says why D5 isn't an edge case. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(j5): stacks merge together, so review them as the code that lands Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(j5): Playbook tool permissions move out of upstream's provider docs The section J5 added to docs/internals/providers.md recorded a J5 decision (Playbook tools are pre-approved on Codex and Claude only), not how upstream's providers work. It now lives in the Playbooks definition and D3, the file returns to upstream's text, FORK.md drops its row, and AGENTS.md says plainly that J5 doesn't edit upstream's docs. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(j5): D5 moves to To be removed; follow upstream on committed Stop Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(j5): Squadron checks add to project scope; stack rule keeps per-PR FORK records Squadron scope adds a check on top of upstream's project scope rather than replacing it (squadron.md and D8). D14 applies the busy-seat exception to settle only; unsettle reaches the settled seats. The stack-review rule covers final behavior, while each PR still records its own upstream edits. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(j5): D3 is settled: Bryant accepted the #233 ruling Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>




Important
This PR is part of the Crews stack. Merge top to bottom, one at a time, and let each land on
j5/mainbefore the next. #352 isn't in the stack but has to merge before this PR.fix(crews): Crew notices report only what the platform measured #292: Crew notices report only what the platform measured (adds migration 020)✅ mergedfix(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✅ mergedfix(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✅ mergedfix(crews): a proposal launches once and reports every seat #313: A proposal launches once and reports every seat (adds migration 021)✅ mergedfix(crews): Crew groups keep seats without thread facts as unknown #300: Crew groups keep seats without thread facts as unknown✅ mergedWhy #352 goes first: without it the finish notifier's stream starts at sequence 0 and replays every event on boot. This PR's cascade also acts on
thread.unarchived,thread.settled, andthread.unsettled, so a restart would settle, unsettle, and restore Crews from history (#349).Problem
My rule is that there's never a Crew without its Captain (#312). Before this, only archive and delete moved the Crew with its Captain:
What I changed
apps/server/src/j5/a2a/CrewCaptainArchiveCascade.ts→handleStoredEvent:thread.archived/thread.deletedstill retire the Captain's live Crews as units. They now passwithCaptain: true, so the Crew records that it retired with its Captain.thread.unarchived→restore: unarchives each archived seat thread, then clears the Crew's retired stamp (restoreWithCaptain), all undercrews.serialize. The lifecycle reactor already restores each seat's agent from thethread.unarchivedevent.thread.settled/thread.unsettled→moveSeats: sendsthread.settleto each unsettled seat, orthread.unsettle(reasonuser) to each settled seat.reconcile(the boot sweep): retires live Crews whose Captain is archived or gone, asj5/mainalready did. No restart recovery for restore, settle, or unsettle (Jackson, 2026-09-26): each Crew step runs once and a failure is logged.getThreadProjectionIfPresent.AgentCrewInstanceService.ts:markArchived(id, archivedAt, { withCaptain }),listRetiredWithCaptain(captainThreadId?), andrestoreWithCaptain(id), which is compare-and-set onretired_with_captain = 1.ArchiveCrewService.ts:ArchiveCrewInput.withCaptainis passed through to the store.migrations/022_CrewRetiredWithCaptain.ts:retired_with_captain INTEGER NOT NULL DEFAULT 0onj5_agent_crew_instance.undoableplumbing:apps/web/src/hooks/useThreadActions.ts→archiveThreadis upstream's again, the Sidebar, header menu (useThreadActionMenu.ts), andLegacySidebar.tsxdoors call it as upstream does, andarchiveFlow.tsdropsarchiveMayRetireCrews(with its test andarchiveUndo.test.ts).docs/j5/product/features/crews.md(the new "A Crew follows its Captain" rule, AC17, AC20, History),fleet-page.mdAC28,docs/user/personas.md, and FORK.md case 21.Why this shape
archive_crew. Crews retired before migration 022 read as retired on their own, so none of them come back unexpectedly.thread.unsettlesets the "active" override, which also blocks upstream's automatic settlement. Sending it to a seat that never settled would change that seat's behavior for no reason.restoreand skips seats already back.Invariants
retired_with_captain = 1).Surfaces
packages/contracts)useThreadActions.ts,useThreadActionMenu.ts,Sidebar.tsx,LegacySidebar.tsx), since the Undo carve-out is gone. FORK.md case 21 describes the wider cascade and the returned Undo.crews.md,fleet-page.md,docs/user/personas.md.Out of scope
Upgrade and data
retired_with_captainwith default 0. Crews already retired read as retired on their own and aren't restored by a later unarchive of their Captain.Verification
vp test run apps/server/src/j5 apps/web/src/j5: 1,013 tests pass (1 skipped).withCaptain: true.apps/serverandapps/webtypecheck with exit 0. Lint is clean on the changed files.Review focus
thread.archivedbeforethread.unarchivedfor the same Captain. Both ride the one stored-event stream in order, so the Crews retire and then come back.CrewCaptainArchiveCascade.restore: it unarchives seats outside the archive path's per-seat lifecycle service and relies on the lifecycle reactor'sthread.unarchivedhandling to restore each seat's agent. Challenge whether anything else a Crew retirement closes (Exchanges, placement) should reopen.moveSeatsonthread.settledfrom upstream's automatic settlement: it gives the seats an explicit settled override, which the settlement sweep itself wouldn't.Closes #312
Claude Opus 5.5 via Claude Code in J5 Code
🤖 Generated with Claude Code