milestone bridge: distinguish a settled-then-cancelled run, fix the leader-facing worker model

packages/extension/dist/index.js

@@ -13419,7 +13419,7 @@ function registerRunTool(pi, ctx) {
   pi.registerTool({
     name: CREW_RUN_TOOL_NAME,
     label: "Crew Run",
-    description: "Use to execute, monitor, or manage task execution by external workers. Use op: 'submit' to start execution (requires taskId from crew_task, workerId from crew_worker, and prompt -- the instruction text the worker executes), op: 'get' to check progress/status of a run, op: 'result' to read a run's output text and token usage (requires runId; readable once the run is terminal OR has settled a turn without exiting -- a TUI vendor's process outlives its turn, so 'waitingUser' with a journaled turn-end already qualifies, per ADR-0027 -- chain work by passing resultText into the next submit's prompt), op: 'list' to list runs for a task, op: 'retry' to re-execute a terminal run (creates a new runId and starts a fresh worker process; pass prompt again), op: 'cancel' to stop a running run immediately, op: 'timeoutAck' to decide a WorkerTimeout fact (requires runId and decision: 'extend' | 'nudge' | 'abort'), or op: 'finish' to end a run as the leader's own settle decision (requires runId; optional outcome: 'succeeded' | 'failed', default 'succeeded' -- states the outcome rather than inferring it, and works on any non-terminal run, not only one that has settled a turn). After submitting, monitor with op: 'get'. If the run fails, retry with op: 'retry' (new runId). If stuck, cancel with op: 'cancel'.",
+    description: "Use to execute, monitor, or manage task execution by external workers. Use op: 'submit' to start execution (requires taskId from crew_task, workerId from crew_worker, and prompt -- the instruction text the worker executes), op: 'get' to check progress/status of a run, op: 'result' to read a run's output text and token usage (requires runId; readable once the run is terminal OR has settled a turn without exiting -- a TUI worker persists between turns and a protocol worker exits when its turn ends, but in both cases a settled turn is when the answer is readable, so 'waitingUser' with a journaled turn-end already qualifies, per ADR-0027 -- chain work by passing resultText into the next submit's prompt), op: 'list' to list runs for a task, op: 'retry' to re-execute a terminal run (creates a new runId and starts a fresh worker process; pass prompt again), op: 'cancel' to stop a running run immediately, op: 'timeoutAck' to decide a WorkerTimeout fact (requires runId and decision: 'extend' | 'nudge' | 'abort'), or op: 'finish' to end a run as the leader's own settle decision (requires runId; optional outcome: 'succeeded' | 'failed', default 'succeeded' -- states the outcome rather than inferring it, and works on any non-terminal run, not only one that has settled a turn). After submitting, monitor with op: 'get'. A run's NORMAL end is a rendered verdict, not a retry or a cancel -- but which of 'result'/'finish' applies depends on whether the run is still parked or already terminal, and the two must not be joined: if op: 'get' (or a milestone) shows the run settled but still non-terminal ('waitingUser'), read the answer with op: 'result' and close it out with op: 'finish' -- a protocol worker runs one turn and exits, so that park is transient by construction: by the time you act the run commonly already reads terminal instead of parked, which is expected, not a race you lost. If op: 'get' already shows the run terminal (including 'cancelled' after a settled turn -- the same run, no verdict rendered, but nothing left to render one on), op: 'result' is the whole of it: read it, and do not call op: 'finish' -- a terminal run refuses it ('invalid_params', already finished). Only retry with op: 'retry' if op: 'get' actually reports the run failed. Only cancel with op: 'cancel' if op: 'get' shows it genuinely stuck (no activity), never merely because it looks quiet while settled and awaiting your own read.",
     parameters: params,
     approval: (args) => typeof args === "object" && args !== null && ("op" in args) && (args.op === "submit" || args.op === "retry" || args.op === "cancel" || args.op === "timeoutAck" || args.op === "finish") ? "exec" : "read",
     async execute(_toolCallId, input, _signal, _onUpdate, extCtx) {
@@ -13742,8 +13742,11 @@ class MilestoneTracker {
         return false;
     }
   }
+  hasSettledTurn(runId) {
+    return this.#sawSettled.has(runId);
+  }
 }
-function formatDigest(e, lookup) {
+function formatDigest(e, lookup, tracker) {
   const event = e.event;
   const runId = lookupKey(e);
   const row = runId !== undefined ? lookup[runId] : undefined;
@@ -13759,6 +13762,9 @@ function formatDigest(e, lookup) {
         return `${capitalize(who)} succeeded. ${READ_THE_REPORT}`;
       }
       if (state === "cancelled") {
+        if (runId !== undefined && tracker.hasSettledTurn(runId)) {
+          return `${capitalize(who)} finished its turn and ended without a verdict; the result is complete. ${READ_THE_REPORT}`;
+        }
         return `${capitalize(who)} was cancelled. ${READ_ANY_PARTIAL_OUTPUT}`;
       }
       if (state === "lost") {
@@ -13812,7 +13818,7 @@ function attachMilestoneBridge(pi, monitor) {
     }
     try {
       const rows = monitor.getState().rows;
-      const digest = formatDigest(e, rows);
+      const digest = formatDigest(e, rows, tracker);
       if (digest === undefined) {
         return;
       }

packages/extension/skills/crew-orchestration/SKILL.md

@@ -21,14 +21,14 @@ To run any task via Claude, Codex, Copilot, or OMP-RPC:
 
 ## Run lifecycle: a run settles, it doesn't just exit
 
-A TUI vendor process never exits between turns — it sits there waiting for the next instruction. So a run's answer becomes readable, and the leader's turn to act begins, the moment the vendor finishes a turn, not only when the whole run reaches a terminal state (`succeeded`/`failed`/`cancelled`/`lost`):
-
-1. A finished turn parks the run at `waitingUser` and journals the turn-end. The `/crew` monitor now emits a milestone for this the same way it does for terminal states, worker questions, and timeouts — you do not have to poll for it.
-2. `crew_run { op: "result", runId }` is readable as soon as that happens (a settled `waitingUser`, not only a terminal run) — no need to cancel the run first to read its answer.
-3. From there you have three real choices, none of which requires a terminal state first:
-   - `crew_send { runId, text, kind }` to follow up — a follow-up on an existing run is never refused, so there is no need to cancel-and-resubmit just to keep steering the same conversation.
-   - `crew_run { op: "finish", runId, outcome: "succeeded" | "failed" }` — the leader's own decision that this conversation is done. States the outcome explicitly (default `"succeeded"`); never inferred from the vendor's own turn markers, because a finished turn only means "the vendor stopped talking," not "the task went well." Works on any non-terminal run, not only one that has settled a turn.
-   - `crew_stop { runId, outcome: "done" | "abort" }` to end it by killing the worker instead (see below) — reach for `finish` when the conversation is genuinely complete, `stop` when you need the process gone regardless.
+A TUI worker persists between turns; a protocol worker exits when its turn ends. In both cases a settled turn is when the answer is readable — so a run's answer becomes readable, and the leader's turn to act begins, the moment the vendor finishes a turn, not only when the whole run reaches a terminal state (`succeeded`/`failed`/`cancelled`/`lost`):
+
+1. A finished turn parks the run at `waitingUser` and journals the turn-end — a TUI worker, which stays alive between turns, sits there. A protocol worker runs one turn and exits, so this park is transient by construction: the run you actually observe commonly already reads terminal (`cancelled`, ADR-0027's `unrendered_verdict()` — real work done, no `run/finish` call ever rendered a verdict on it). Either way, the `/crew` monitor emits a milestone for the settle the same way it does for terminal states, worker questions, and timeouts — you do not have to poll for it.
+2. `crew_run { op: "result", runId }` is readable as soon as the turn settles, whether the run is still parked at `waitingUser` (TUI, or a protocol run you caught in time) or has already gone terminal (`cancelled`, the common protocol case) — no need to cancel the run first to read its answer either way.
+3. From there:
+   - `crew_send { runId, text, kind }` to follow up **while the worker's own process is still alive** — a follow-up on a live run is never refused, so there is no need to cancel-and-resubmit just to keep steering that conversation. Once a protocol worker's process has exited (the run already reads terminal), there is no live adapter left to steer: the message still journals, but nothing is driving it — continuing the work means a fresh `crew_run { op: "submit" }`, not a follow-up.
+   - `crew_run { op: "finish", runId, outcome: "succeeded" | "failed" }` — the leader's own decision that this conversation is done. States the outcome explicitly (default `"succeeded"`); never inferred from the vendor's own turn markers, because a finished turn only means "the vendor stopped talking," not "the task went well." Works on any non-terminal run, not only one that has settled a turn — which is exactly why it refuses once the run is already terminal (the protocol case above, `invalid_params`): there is no verdict left to render, and `op: "result"` is the whole of what you get.
+   - `crew_stop { runId, outcome: "done" | "abort" }` to end it by killing the worker instead (see below) — reach for `finish` when the conversation is genuinely complete and still open to a verdict, `stop` when you need a live process gone regardless.
 
 `crew_finish { runIds }` is a different, narrower tool: it **cancels** the named plan runs in a loop (the same effect as `crew_stop { outcome: "abort" }`, batched) — it does not settle anything and never states an outcome. Use `crew_run { op: "finish" }` when you want to record how one run's conversation actually went; use `crew_finish` only to bulk-cancel a plan's remaining live runs once you're done with the plan as a whole. Neither ever releases a workspace lease behind your back.
 

packages/extension/src/milestones.test.ts

@@ -179,6 +179,7 @@ test("paneDowngraded digest names the requested/actual backends and the reason",
       event: { type: "paneDowngraded", payload: { runId: "run-1", requestedBackend: "tmux", requestedPlacement: "splitDown", actualBackend: "hidden", attempted: ["tmux"], reason: "tmux exploded" } },
     }),
     ROWS,
+    tracker(),
   );
   expect(digest).toBeDefined();
   expect(digest).toContain("tmux");
@@ -205,7 +206,7 @@ test("failed digest names the reason but never claims a failure count on its own
   const rows: RunLookup = {
     "run-1": { ...ROWS["run-1"], latestActivity: "process exited 1" } as MonitorRow,
   };
-  const digest = formatDigest(run("run-1", "failed"), rows);
+  const digest = formatDigest(run("run-1", "failed"), rows, tracker());
   expect(digest).toBeDefined();
   expect(digest).toContain("FAILED");
   expect(digest).toContain("process exited 1");
@@ -233,6 +234,7 @@ test("escalation digest for a genuine repeat failure carries the runtime's own q
       },
     }),
     ROWS,
+    tracker(),
   );
   expect(digest).toBeDefined();
   expect(digest).toContain("repeated_failure");
@@ -246,7 +248,7 @@ test("succeeded digest tells the leader how to read the report", () => {
   // settled-turn digest below has always carried this instruction and had a
   // test pinning it; the terminal digests carried neither, which is how the
   // omission survived.
-  const digest = formatDigest(run("run-1", "succeeded"), ROWS);
+  const digest = formatDigest(run("run-1", "succeeded"), ROWS, tracker());
   expect(digest).toBeDefined();
   expect(digest).toContain("succeeded");
   expect(digest).toContain('crew_run { op: "result"');
@@ -256,16 +258,45 @@ test("non-succeeded terminal digests offer partial output without promising it",
   // `run/result` accepts every terminal state and returns whatever visible
   // text the journal accumulated, which for a run that died is usually
   // partial and is sometimes null. The wording must offer it without
-  // claiming it exists.
+  // claiming it exists. `cancelled` here is the run's ONLY settle episode
+  // never happening at all -- see the dedicated `cancelled` tests below for
+  // the branch where it did.
   for (const state of ["failed", "cancelled", "lost"]) {
     const rows: RunLookup = { "run-1": { ...ROWS["run-1"], latestActivity: "process exited 1" } as MonitorRow };
-    const digest = formatDigest(run("run-1", state), rows);
+    const digest = formatDigest(run("run-1", state), rows, tracker());
     expect(digest, `${state} must produce a digest`).toBeDefined();
     expect(digest, `${state} must point at run/result`).toContain('crew_run { op: "result"');
     expect(digest, `${state} must not promise output exists`).toContain("there may be none");
   }
 });
 
+test("cancelled after a settled turn says the result is complete, not 'may be none'", () => {
+  // ADR-0027's `unrendered_verdict()`: this run did real work and settled a
+  // turn, but nothing ever called run/finish to render a verdict -- the
+  // SAME `cancelled` state a never-started run gets, but a very different
+  // fact for the leader. The state itself does not change (the
+  // maintainer's own ruling); only the digest text does.
+  const t = tracker();
+  expect(t.isMilestone(flags("run-1", true))).toBe(true); // the turn settles first
+  const digest = formatDigest(run("run-1", "cancelled"), ROWS, t);
+  expect(digest).toBeDefined();
+  expect(digest).toContain("finished its turn and ended without a verdict");
+  expect(digest).toContain("the result is complete");
+  expect(digest).toContain('crew_run { op: "result"');
+  expect(digest).not.toContain("there may be none");
+  expect(digest).not.toContain("was cancelled");
+});
+
+test("cancelled with no settled turn keeps the existing partial-output wording", () => {
+  const t = tracker();
+  // No settle episode recorded for this run at all.
+  const digest = formatDigest(run("run-1", "cancelled"), ROWS, t);
+  expect(digest).toBeDefined();
+  expect(digest).toContain("was cancelled");
+  expect(digest).toContain("there may be none");
+  expect(digest).not.toContain("finished its turn and ended without a verdict");
+});
+
 test("a settled turn (runFlagsEvent turnSettled:true) is a milestone, once per settle episode", () => {
   const t = tracker();
   // First settle: milestone.
@@ -287,7 +318,7 @@ test("a runFlagsEvent with turnSettled false is never a milestone on its own", (
 });
 
 test("settled-turn digest points the leader at crew_run result and finish", () => {
-  const digest = formatDigest(flags("run-1", true), ROWS);
+  const digest = formatDigest(flags("run-1", true), ROWS, tracker());
   expect(digest).toBeDefined();
   expect(digest).toContain("settled a turn");
   expect(digest).toContain('crew_run { op: "result"');
@@ -301,6 +332,7 @@ test("question digest contains the question text and triage instruction", () =>
       event: { type: "workerQuestion", payload: { runId: "run-1", taskId: "task-1", workerId: "w1", question: "should I delete the index?" } },
     }),
     ROWS,
+    tracker(),
   );
   expect(digest).toBeDefined();
   expect(digest).toContain("should I delete the index?");
@@ -314,6 +346,7 @@ test("escalation digest with no question renders exactly today's sentence", () =
       event: { type: "escalationRaised", payload: { runId: "run-1", taskId: "task-1", workerId: "w1", reason: "write_violation", question: null } },
     }),
     ROWS,
+    tracker(),
   );
   expect(digest).toBe("Escalation raised on run run-1 (claude adapter) for task task-1: write_violation.");
 });
@@ -334,6 +367,7 @@ test("escalation digest with a question appends it after the reason", () => {
       },
     }),
     ROWS,
+    tracker(),
   );
   expect(digest).toBeDefined();
   expect(digest).toContain("vendorFirstRunGate");