fix(crews): a proposal launches once and reports every seat

FORK.md

@@ -187,7 +187,7 @@ Bryant requested a shared agent picker for Codex and Claude, invoking a saved ag
 
 `packages/client-runtime/src/state/models.ts` accepts a status-only runtime in `threadRuntimeIsActive`; J5 crew state reuses that exported predicate without importing the upstream-private status helper. Preserve this narrow input contract when carrying crew reads forward.
 
-The proposal flow's server modules are J5-owned and need no upstream registration: `AgentCrewInstanceService`, `AgentCrewProposalService`, `CrewLaunchService`, `CrewProposalService`, `CrewLaunchReporter`, `crewGateNotice`, `crewFailureAlert`, `CrewSeatFinishNotifier`, and `CrewProposalsHttp` under `apps/server/src/j5/a2a/`, the `list_personas`, `propose_crew`, `request_crew_member`, and `archive_crew` verbs in the J5 toolkit, and J5 migrations 14 (`j5_agent_crew_instance`, `j5_agent_crew_member`, `j5_agent_crew_proposal`) and 16 (proposal claims and launch-report receipts), plus 15 (`agent_id` nullable on `j5_agent_crew_member`: a custom seat names no saved agent and defaults to the Captain's runtime, with explicit harness/model/options/access selections stored in the proposal and bound to human approval). `archive_crew` composes the J5 `ArchiveAgentService` (which gained a side-effect-free `readFacts` and a `confirmationSatisfied` input for unit archives), and `archive_agent` refuses Crew members. Names are one line each and seat names are lowercase-hyphen words (`crewLimits.ts`), enforced by the MCP schemas and again in `CrewProposalService.validateSeats` for the human's edited card, because the platform writes them into the gate and seat notices ahead of the fields the Captain's card parser trusts. A roster launch records its Crew (every planned seat under deterministic ids) before any seat thread exists, so a spawn that fails partway leaves nothing a Crew record does not know: the gate is handed back with the human's approved seats kept, a retry converges on the same seats (a renamed seat's stale reservation is dropped, and a seat whose brief already dispatched is compared on the Captain's brief and its own instructions, not on the roster block, so dropping a failed seat does not refuse the rest), and a decline retires the record and archives whatever seat threads the failed launch created (2026-09-17, after Jackson's review of PR #148). Crew handoff artifacts (2026-09-10, consolidated 2026-09-14) add no path of their own: a seat whose definition declares an `outputArtifact` writes the same shared handoff artifact every saved agent writes (`handoffs/<agent>/<Artifact>-<task>.md`, PR #142), the handoff gate in `apps/server/src/j5/agents/agentHandoffObserver.ts` checks for it and reminds the seat once, and J5-owned `CrewSeatFinishNotifier.ts` tells the Captain in its thread when the seat finishes and whether the file exists; it settles no thread (2026-09-17, Jackson's review: upstream's settle rules apply to seats). That notice is keyed by the seat's participant (one Captain's several Crews reuse seat names), carries the handoff's size and digest so a rewritten body too long to inline still reads as changed, escapes the opening seat tag as well as the closing body tag in quoted handoffs, and a failed notice is left for the next pass rather than swallowed (2026-09-17, after Jackson's review of PR #150). The earlier `deliver_artifact` verb, `j5_agent_artifact` table, and crew-only read verbs were removed before shipping. Two leaf allowlists carry `write_artifact` for read-only personas so a seat can write its handoff artifact into application storage: `J5_CLAUDE_MCP_ALLOWED_TOOLS` (spread into Claude's read-only pre-approval list) and `J5_CODEX_PREAPPROVED_TOOLS` (per-tool `approval_mode` entries on the `t3-code` server for Codex under approval policy `never`). The crew gate rides the J5 client-runtime surface (`packages/client-runtime/src/j5/http.ts`, `state.ts`; contracts in `packages/contracts/src/j5.ts`), so proposals are a per-environment read source like the inbox. Crew launch remains out of scope for `delegate_task`.
+The proposal flow's server modules are J5-owned and need no upstream registration: `AgentCrewInstanceService`, `AgentCrewProposalService`, `CrewLaunchService`, `CrewProposalService`, `CrewLaunchReporter`, `crewGateNotice`, `crewFailureAlert`, `CrewSeatFinishNotifier`, and `CrewProposalsHttp` under `apps/server/src/j5/a2a/`, the `list_personas`, `propose_crew`, `request_crew_member`, and `archive_crew` verbs in the J5 toolkit, and J5 migrations 14 (`j5_agent_crew_instance`, `j5_agent_crew_member`, `j5_agent_crew_proposal`) 16 (launch-report receipts; its proposal claim states are retired by 21), and 21 (a proposal resolves once: rows a server left `approving` or `declining` go back to `open`), plus 15 (`agent_id` nullable on `j5_agent_crew_member`: a custom seat names no saved agent and defaults to the Captain's runtime, with explicit harness/model/options/access selections stored in the proposal and bound to human approval). `archive_crew` composes the J5 `ArchiveAgentService` (which gained a side-effect-free `readFacts` and a `confirmationSatisfied` input for unit archives), and `archive_agent` refuses Crew members. Names are one line each and seat names are lowercase-hyphen words (`crewLimits.ts`), enforced by the MCP schemas and again in `CrewProposalService.validateSeats` for the human's edited card, because the platform writes them into the gate and seat notices ahead of the fields the Captain's card parser trusts. An approval records the Crew (every planned seat under deterministic ids) and resolves the proposal once, before any seat spawns; every seat is then attempted, a seat whose thread was never created is dropped from the roster, and the launch report names each seat's outcome. A failure before the Crew is recorded leaves the proposal open, and a decline only retires a Crew record that never launched (#311, 2026-09-25; the earlier retry-and-converge model from PR #148 is gone). Crew handoff artifacts (2026-09-10, consolidated 2026-09-14) add no path of their own: a seat whose definition declares an `outputArtifact` writes the same shared handoff artifact every saved agent writes (`handoffs/<agent>/<Artifact>-<task>.md`, PR #142), the handoff gate in `apps/server/src/j5/agents/agentHandoffObserver.ts` checks for it and reminds the seat once, and J5-owned `CrewSeatFinishNotifier.ts` tells the Captain in its thread when the seat finishes and whether the file exists; it settles no thread (2026-09-17, Jackson's review: upstream's settle rules apply to seats). That notice is keyed by the seat's participant (one Captain's several Crews reuse seat names), carries the handoff's size and digest so a rewritten body too long to inline still reads as changed, escapes the opening seat tag as well as the closing body tag in quoted handoffs, and a failed notice is left for the next pass rather than swallowed (2026-09-17, after Jackson's review of PR #150). The earlier `deliver_artifact` verb, `j5_agent_artifact` table, and crew-only read verbs were removed before shipping. Two leaf allowlists carry `write_artifact` for read-only personas so a seat can write its handoff artifact into application storage: `J5_CLAUDE_MCP_ALLOWED_TOOLS` (spread into Claude's read-only pre-approval list) and `J5_CODEX_PREAPPROVED_TOOLS` (per-tool `approval_mode` entries on the `t3-code` server for Codex under approval policy `never`). The crew gate rides the J5 client-runtime surface (`packages/client-runtime/src/j5/http.ts`, `state.ts`; contracts in `packages/contracts/src/j5.ts`), so proposals are a per-environment read source like the inbox. Crew launch remains out of scope for `delegate_task`.
 
 Following Jackson's 2026-09-14 review, the J5-only `invoke_agent` tool is gone. J5 registers its own `delegate_task` (`apps/server/src/j5/agents/agentDelegation.ts`): upstream's input plus an optional `persona` id, with a J5-written description that leads with `persona` and presents the plain child as the fallback for cross-provider or T3-tracked work (upstream's text tells the model to delegate for any subagent request, which would contradict the native-subagent steering). With `agent` it resolves the saved definition, route, and permissions in the J5 invocation module and passes the pinned assignment to `OrchestratorMcpService.delegateTask`; without it, it is upstream's plain child delegation. `task_status` and `task_cancel` are consumed from upstream unchanged so a parent can follow the child. Raw thread creation (`create_threads`, `t3_thread_start`, `t3_thread_send`, `t3_thread_interrupt`) stays excluded.
 

apps/server/src/j5/a2a/AgentCrewProposalService.test.ts

@@ -10,8 +10,6 @@ import {
   layer as proposalLayer,
   type CreateCrewProposalInput,
 } from "./AgentCrewProposalService.ts";
-import { crewSeatThreadId } from "./crewSeatIds.ts";
-import { participantIdForThread } from "./HomeRegistrar.ts";
 import { A2ALedger, layer as ledgerLayer } from "./LedgerService.ts";
 import { runJ5A2AMigrations } from "./Migrations.ts";
 import { ParticipantId, SquadronId } from "./contracts.ts";
@@ -40,17 +38,6 @@ const rosterMember = (crewId: string, name: string) => ({
 });
 
 /** The row an addition reserves for one of its seats, under the ids the launcher derives. */
-const reservedMember = (proposalId: string, name: string) => {
-  const threadId = crewSeatThreadId(proposalId, name);
-  return {
-    seatName: name,
-    agentId: "scout",
-    participantId: participantIdForThread(threadId),
-    threadId,
-    reason: null,
-  };
-};
-
 const addition = (
   id: string,
   crewInstanceId: string,
@@ -99,17 +86,11 @@ it.effect(
         maxSeats: 12,
       });
       assert.equal(declined.status, "created");
-      const claimed = yield* proposals.claim({
-        id: "proposal:d",
-        decision: "decline",
-        approvedSeats: null,
-      });
-      assert.isNotNull(claimed);
       assert.isNotNull(
-        yield* proposals.complete({
+        yield* proposals.resolve({
           id: "proposal:d",
           decision: "decline",
-          crewInstanceId: crewId,
+          approvedSeats: null,
           resolvedAt: createdAt,
         }),
       );
@@ -138,39 +119,58 @@ it.effect(
     }).pipe(Effect.provide(testLayer)),
 );
 
-it.effect("an open addition whose failed launch left its reserved row is counted once", () =>
+it.effect("a proposal resolves once: the second resolution finds it closed", () =>
   Effect.gen(function* () {
     yield* runJ5A2AMigrations();
     yield* (yield* A2ALedger).createSquadron({
       squadron: { id: squadronId, name: "Proposal Squadron", createdAt },
     });
     const crews = yield* AgentCrewInstanceService;
     const proposals = yield* AgentCrewProposalService;
-    const crewId = "crew:reserved";
+    const crewId = "crew:resolve-once";
     yield* crews.record({
       id: crewId,
       squadronId,
       captainParticipantId: captain,
       captainThreadId,
-      displayName: "Reserved",
+      displayName: "Resolve once",
       brief: "Join in.",
       createdAt,
-      members: Array.from({ length: 10 }, (_, index) => rosterMember(crewId, `s${index}`)),
+      members: [rosterMember(crewId, "s0")],
+    });
+    const filed = yield* proposals.admit(addition("proposal:once", crewId, ["p"]), {
+      maxSeats: 12,
     });
-    // P is open again after its launch failed, with its one reserved row still on the Crew.
-    const p = yield* proposals.admit(addition("proposal:p", crewId, ["p"]), { maxSeats: 12 });
-    assert.equal(p.status, "created");
-    assert.equal(
-      (yield* crews.addMembers(crewId, [reservedMember("proposal:p", "p")])).status,
-      "added",
+    assert.equal(filed.status, "created");
+    const approvedSeats = [
+      { ...addition("proposal:once", crewId, ["renamed"]).requestedSeats[0]! },
+    ];
+    const approved = yield* proposals.resolve({
+      id: "proposal:once",
+      decision: "approve",
+      approvedSeats,
+      resolvedAt: createdAt,
+    });
+    assert.equal(approved?.status, "approved");
+    assert.deepStrictEqual(
+      approved?.approvedSeats?.map(({ seat }) => seat),
+      ["renamed"],
     );
-
-    // Eleven held, not twelve: Q takes the last seat and R is refused.
-    const q = yield* proposals.admit(addition("proposal:q", crewId, ["q"]), { maxSeats: 12 });
-    assert.equal(q.status, "created");
+    // Neither a second approval nor a decline can follow the first resolution.
+    for (const decision of ["approve", "decline"] as const)
+      assert.isNull(
+        yield* proposals.resolve({
+          id: "proposal:once",
+          decision,
+          approvedSeats: null,
+          resolvedAt: createdAt,
+        }),
+      );
+    assert.equal((yield* proposals.read("proposal:once"))?.status, "approved");
+    // An approved request is no longer counted as pending; its seat is held once it is a row.
     assert.deepStrictEqual(
-      yield* proposals.admit(addition("proposal:r", crewId, ["r"]), { maxSeats: 12 }),
-      { status: "cap-exceeded", held: 12, adding: 1 },
+      yield* proposals.admit(addition("proposal:next", crewId, ["q"]), { maxSeats: 1 }),
+      { status: "cap-exceeded", held: 1, adding: 1 },
     );
   }).pipe(Effect.provide(testLayer)),
 );