fix(crews): Crew notices report only what the platform measured

[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. ⬅️ this PR
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 ✅ merged
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 ✅ merged
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)
7. feat(crews): custom seats default to Full access and the roster flags seats that will stop #347: custom seats default to Full access

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.

Problem

Two Crew notices could state things the platform never checked (#225, #226). A seat's finish notice told the Captain a handoff was "missing" when the file only failed to read, and recorded that so no retry corrected it. A seat's sign-in or permission failure was appended to whatever ask the Captain had open with me, so answering my own question closed the alert with it.

What I changed

• apps/server/src/j5/a2a/CrewSeatFinishNotifier.ts → handoffFact: only a not_found read yields missing; any other ArtifactWorkspaceError fails the notice.
• apps/server/src/j5/a2a/crewFailureAlert.ts: one alert exchange per Crew, ids under CREW_ALERT_EXCHANGE_PREFIX plus the URI-encoded Crew id; the reuse lookup matches only that Crew's prefix.
• apps/server/src/j5/a2a/migrations/020_CrewAlertExchanges.ts: recreates j5_a2a_exchange_open_pair_idx, excluding alert ids.
• apps/server/src/j5/a2a/SendService.ts: the "join an open exchange" lookup for a new ask skips alert exchanges.

Why this shape

The ledger allowed one open exchange per sender and receiver (migration 008), and the alert is sent as the Captain so replies route back to it. That index is why the old code joined the Captain's ask, and why the Captain couldn't ask me anything while an alert was open. I rejected sending the alert as the failed seat (my reply would go to a dead seat, not the Captain) and skipping the alert while an ask is open (I'd miss it in exactly that case). For #226 I chose retry over an explicit "handoff unavailable" fact: the platform shouldn't record a fact it didn't measure.

Invariants

• Replay: an alert still has one receipt per failed run; a replay appends nothing and never reopens an answered alert.
• The open-pair rule still holds for every exchange except platform alerts.
• A failed notice leaves no durable command, so the next finish or the boot sweep retries it.

Surfaces

Surface	Decision
Entry points	Unaffected: server-produced notices only.
Clients	Unaffected: the Inbox already lists any number of exchanges; no client change.
Providers	Unaffected: provider-agnostic.
Contracts	Unchanged.
Reverse states	An alert closes when I answer it; a later failure opens a fresh one.
Connection modes	Unaffected: no new route.
Upstream files / FORK.md	None touched; FORK.md unchanged.
Docs	crews.md still accurate; the #226 wording lands in #229.

Out of scope

• A Crew thread's provider approvals and questions reach the Inbox (server) #263: routing provider approvals and questions to the Inbox is a separate producer.
• Sync the Crews pages and tool contract with the shipped surface #229: the agent-tools.md seat-finish paragraph.

Upgrade and data

Migration 020 drops and recreates one partial index; no rows change. Open alerts in the old id format (exchange:j5-crew-human-alert:<runId>) match the prefix, so they're exempt too, and new failures open a fresh per-Crew alert beside them.

Verification

• vp test run apps/server/src/j5/a2a: 338 passed, 1 skipped.
• New crewFailureAlert.test.ts (the module's first direct test) fails without migration 020 on the unique index.
• New CrewSeatFinishNotifier.test.ts case: not-found, transient read error, recovery through the boot sweep, and unchanged-handoff dedup.
• Lint, format, and typecheck clean on the changed files.

Review focus

• The partial-index predicate in migration 020 and the substr check in SendService: is anything else relying on one open exchange per pair?
• handoffFact now fails notifyIfFinished after the failure alert has already fired. That alert is idempotent per run, but check the ordering.

Closes #225
Closes #226

Claude Opus 5.5 via Claude 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: 121686a7-dd8e-4337-94ff-cfa6d39e28a0

---

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

[Jacksondr5]

[Review panel: Opus 5.5 + Astra] Bug (pre-existing, but in scope for this PR): a Crew sign-in or permission alert is saved but never delivered until something else wakes the delivery worker.

makeCrewFailureAlert commits the alert and then only calls writer.publishCommitted, which publishes to the ledger PubSub (LedgerService.ts ~760). The DeliveryWorker daemon doesn't subscribe to that; when nothing is scheduled it blocks on Queue.take(wakeups), with no periodic poll (DeliveryWorker.ts ~507–520). Every other producer calls worker.notify after committing: mcp/handlers.ts, HumanInboxHttp, LifecycleService, SilenceDetector, MachineSenderHttp. This one doesn't. The same missing wake existed before this PR, but this PR's whole point is that the person sees this alert.

Live reproduction (Astra, in a real client on this stack):

• 14:57:12: a seat's second run fails with provider_error, code 401.
• 14:58:06: the alert's delivery row is still pending, attempts=0, next_attempt_at=null. It has no inbox projection, and the Inbox shows only the Captain's own ask. The alert exchange is open in the ledger.
• The person answers the Captain's ask, which calls worker.notify, and the alert is delivered immediately (attempts=1).

So the alert shows up only after some unrelated send, or after a restart.

The tests miss it because crewFailureAlert.test.ts calls worker.drain by hand, which hides the missing wake.

Suggested fix: depend on A2ADeliveryWorker and yield* worker.notify after publishCommitted. Add a regression test that runs the worker as its daemon and waits on the delivery receipt or signal, without calling drain manually.

[Jacksondr5]

Interaction evidence from Astra, reviewed with Opus 5.5, on the merged Crews stack tip aba91720e5. Used an isolated copy of the real DB, a real Codex Captain, and a controlled prepared-run.fail authentication error on a Crew seat. This injected the normal durable failure path; it did not revoke real provider credentials.

The alert has its own Inbox item. Answering it leaves the Captain's separate ask open:

The independent exchange behavior passed once delivered. End-to-end unsolicited delivery failed: the alert stayed pending with attempts=0 until answering an earlier Captain ask woke the delivery worker. See the panel's missing-notify finding. The screenshots show the delivered alert beside a fresh, separate Captain ask.

[bryantderosier]

@Jacksondr5 Both panel findings are fixed on this branch:

• The alert never woke the delivery worker (fbcd5b7). makeCrewFailureAlert now calls worker.notify after publishCommitted, like every other producer. The launch reporter and the finish notifier get the one worker instance from runtimeLayer.
  • On the regression test: a daemon-mode test on its own can't tell the difference, because the worker's wake queue can already hold a spare wake from startup, so it passes with or without the fix. So I added a test that counts wakes instead: one when the alert commits, none on a replay. It fails without the fix. I kept a daemon-mode test with no manual drain as the end-to-end check.
• A permanent handoff read failure blocked the finish notice (3b1a5f6). Details are in the thread.

I also merged the latest j5/main. CI is running.

[Jacksondr5]

[Review panel: Opus 5.5 + Astra] Verified: the alert that never woke the delivery worker (round one) is fixed in fbcd5b7. makeCrewFailureAlert calls worker.notify after publishCommitted, and runtimeLayer reuses the same deliveryWorkerProvided layer value as the other producers, so there's one worker. Live (Astra), an injected auth failure's alert was delivered 99 ms after commit on attempt 1, with no other send, answer, or manual drain, and it was in the Inbox within about 400 ms.

Test note (no change needed beyond a comment): with the notify line removed locally, only the wake-count test fails; the daemon-mode test still passes, as you said. Keep it as an end-to-end smoke check, but please correct its comment "or this waits until the test times out", which isn't true.

[Jacksondr5]

Posted by an AI agent on Jackson's behalf.

Approved. Both round-one findings are verified by the review panel (Opus 5.5 + Astra), in code and live:

• handoffs over the size limit read as written, and a directory or an out-of-root link reads as unavailable (the live cards match);
• the failure alert wakes the delivery worker by itself (99 ms, first attempt).

The daemon-test comment nit is optional.

Merge order: #292, #313, #300, then #315 (after #349 lands). #306 rebases after the stack.

[Jacksondr5]

[Review panel: Opus 5.5 + Astra]

Round-two web verification: all three handoff cases pass. Before and after use the same copied database, artifact fixtures, light theme, and 1440×1000 viewport.

6 MiB handoff — before: reported missing.

6 MiB handoff — after: reported written, without an inline body.

Directory and escaping symlink — before: both reported missing.

Directory and escaping symlink — after: unavailable, with a reason for each.

Alert wake — pass: a controlled authentication failure reached the Inbox without another send or a manual worker drain. The delivery committed 99 ms after creation, on its first attempt.

Test scope: real Codex Captain and seat threads, with controlled terminal events and artifact fixtures in isolated copied state. The handoff comparisons used a test-only cursor marker to prevent the separately reported historical replay bug #349 from contaminating the cards. The alert-wake test did not use that workaround.