diff --git a/packages/extension/dist/index.js b/packages/extension/dist/index.js index 3198465e..649c2c00 100644 --- a/packages/extension/dist/index.js +++ b/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; } diff --git a/packages/extension/skills/crew-orchestration/SKILL.md b/packages/extension/skills/crew-orchestration/SKILL.md index e6367184..637080d9 100644 --- a/packages/extension/skills/crew-orchestration/SKILL.md +++ b/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. diff --git a/packages/extension/src/milestones.test.ts b/packages/extension/src/milestones.test.ts index 6602d09a..0711e7dc 100644 --- a/packages/extension/src/milestones.test.ts +++ b/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"); diff --git a/packages/extension/src/milestones.ts b/packages/extension/src/milestones.ts index f9ecf759..4e57f3bc 100644 --- a/packages/extension/src/milestones.ts +++ b/packages/extension/src/milestones.ts @@ -245,15 +245,37 @@ export class MilestoneTracker { return false; } } + + /** + * Whether `runId`'s current turn settled (ADR-0027's `waitingUser` + + * `turnSettled`) before whatever terminal event is being formatted now -- + * the same bookkeeping {@link isMilestone}'s `runFlagsEvent` arm + * maintains, exposed read-only so {@link formatDigest} can tell a + * cancellation that arrived after a real, complete turn apart from one + * that never got that far. Reflects the run's CURRENT settle episode + * only: `run/finish` (or a follow-up resuming the run) clears it, same + * as `isMilestone`'s own one-shot bookkeeping. See the `cancelled` case + * in {@link formatDigest} for the two populations this distinguishes and + * the one bound on relying on it (a connection opened between a settle + * and its terminal event, with the settle behind the replay cursor). + */ + hasSettledTurn(runId: string): boolean { + return this.#sawSettled.has(runId); + } } /** * Builds the compact prose digest for a milestone envelope. `lookup` names - * the run's adapter / task from the monitor's rows. Returns undefined when - * the envelope is not a milestone (callers should only call this after - * `isMilestone`). + * the run's adapter / task from the monitor's rows. `tracker` is the same + * {@link MilestoneTracker} instance `isMilestone` was already called + * against for this envelope -- its {@link MilestoneTracker.hasSettledTurn} + * is what lets the `cancelled` case below tell a run that finished a real + * turn (ADR-0027's `unrendered_verdict()`: real work done, nothing rendered + * a verdict on it) apart from one cancelled before ever getting that far. + * Returns undefined when the envelope is not a milestone (callers should + * only call this after `isMilestone`). */ -export function formatDigest(e: EventEnvelope, lookup: RunLookup): string | undefined { +export function formatDigest(e: EventEnvelope, lookup: RunLookup, tracker: MilestoneTracker): string | undefined { const event: RuntimeEvent = e.event; const runId = lookupKey(e); const row = runId !== undefined ? lookup[runId] : undefined; @@ -270,6 +292,32 @@ export function formatDigest(e: EventEnvelope, lookup: RunLookup): string | unde return `${capitalize(who)} succeeded. ${READ_THE_REPORT}`; } if (state === "cancelled") { + // `cancelled` is ADR-0027's `RunState::unrendered_verdict()`, and + // this branch deliberately covers TWO populations that land in it + // for different reasons, with the SAME next action for both: (1) a + // leader that never called run/finish to render a verdict on a + // real, completed turn, and (2) the daemon's own recovery sweep + // ending a `waitingUser`/`waitingPeer`/`paused` run at restart -- + // if that run's turn had genuinely settled before the restart, the + // sentence below is still true and the leader's next move is + // identical (read the result). The state itself never changes for + // either population (the maintainer's own 2026-09-09 ruling, + // shared with the leader-disconnect case -- ADR-0027 reserves + // `succeeded` for an explicit `run/finish`); only this digest text + // does. `tracker.hasSettledTurn` (the same bookkeeping + // `isMilestone`'s own `runFlagsEvent` arm keeps) is what tells the + // two apart from a run that never settled at all. + // + // Bound worth stating plainly: `hasSettledTurn` only knows what + // THIS bridge instance observed. A connection established between + // a run's settle and its terminal event, with the settle already + // behind the replay cursor, leaves it `false` for a run that did + // complete -- not a regression (that run got the same + // unsettled-looking wording before this branch existed), but the + // fallback below, not a claim that the turn never settled. + 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") { @@ -392,7 +440,7 @@ export function attachMilestoneBridge(pi: ExtensionAPI, monitor: MonitorControll } try { const rows = monitor.getState().rows; - const digest = formatDigest(e, rows); + const digest = formatDigest(e, rows, tracker); if (digest === undefined) { return; } diff --git a/packages/extension/src/tools/runs.ts b/packages/extension/src/tools/runs.ts index 55140597..8a9366b3 100644 --- a/packages/extension/src/tools/runs.ts +++ b/packages/extension/src/tools/runs.ts @@ -36,7 +36,7 @@ export function registerRunTool(pi: ExtensionAPI, ctx: OrchestrationToolContext) 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'.", + "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) {