diff --git a/.pylon/features.yaml b/.pylon/features.yaml index 284da3ca0d..7c09a6582e 100644 --- a/.pylon/features.yaml +++ b/.pylon/features.yaml @@ -296,3 +296,23 @@ decisions: revisit_when: - Prime upstream makes the session retry loop failure-kind aware and honors Retry-After with a delay ceiling. - Prime upstream makes exactly one retry layer own policy so provider SDK retries cannot multiply session retries. + + turn-scoped-subagent-cancellation: + area: runtime-reliability + state: candidate + owner: pylon-prime-integration + decision: redesign + pylon_refs: + - https://github.com/pylon-code/prime-agent/issues/22 + - https://github.com/pylon-code/prime-agent/issues/25 + - https://github.com/pylon-code/prime-agent/pull/35 + upstream_refs: + - https://github.com/PrimeIntellect-ai/prime-agent/pull/346 + - https://github.com/PrimeIntellect-ai/prime-agent/pull/464 + - https://github.com/PrimeIntellect-ai/prime-agent/pull/1253 + - https://github.com/PrimeIntellect-ai/prime-agent/tree/c382f09856d4a8c8d2b765179657047d58691f25 + fork_change: candidate + upstream_support: Prime through c382f09856d4 keeps two cancel semantics. `AgentSession.requestAbort()` deliberately leaves active RLM child runs alive (PR #346 pinned that with a characterization test) while only `abort()` and `abortForUpdateRestart()` cascade, and every user-facing cancel path routes through `requestAbort()`. PR #464 made per-child cancellation explicit but never bound it to a turn abort, and open PR #1253 scopes child cancellation to kernel host teardown only. No upstream contract cancels the children a cancelled turn owns. + revisit_when: + - Prime makes a user-facing turn abort cancel the child runs it owns, or exposes an equivalent turn-scoped cancellation contract. + - Prime introduces a real detached or background spawn mode, which would need a per-child survival decision instead of one retained-versus-active rule. diff --git a/.pylon/upstream-review.md b/.pylon/upstream-review.md index f533e0a56f..335ac39ed7 100644 --- a/.pylon/upstream-review.md +++ b/.pylon/upstream-review.md @@ -137,3 +137,14 @@ This ledger records Prime upstream evidence and the decision taken for each over - `child-scoped-provider-identity`: **retain**. Inline RLM children now run their payload, response, and context hooks through their own extension runner, converging with the daemon path that already gets a per-child runner from `createAgentSession`. Side questions and the compaction, branch-summary, refine, and auto-refine-review passes run through a scoped view of the owning session whose `getSessionId()` returns `/`; every other accessor still reports the owning session. The extension contract stays additive: existing `before_provider_request` handlers keep working and simply observe one identity per agent instead of the parent's for all of them. - Out of scope by design: `SessionManager.sessionId` still changes on fork and branch. A genuinely divergent history deserves a new provider key, so persisting identity across forks is a separate decision. - Validation: `npm run check` clean. `test/suite/regressions/23-child-provider-identity.test.ts` passes 3/3 and fails on the pre-fix inline-child wiring. Adjacent suites pass: side questions, fast-mode children, compaction (suite, extensions, summary reasoning), refinement, subagent runtime host, subagent model selection, subagent terminal messages, agent-session runtime, recursion, context tree, concurrent sessions, daemon agent connection, and the `packages/ai` faux provider — 620 passes with 8 skips across 18 files. + +## 2026-08-31 — turn-scoped subagent cancellation + +- Upstream evidence: `PrimeIntellect-ai/prime-agent@c382f09856d4a8c8d2b765179657047d58691f25` (current upstream `main`); latest audited release remains `v0.8.1`. Reviewed the upstream `AgentSession` abort surface (`requestAbort`, `abort`, `abortForUpdateRestart`, `_cancelActiveRlmChildRuns`, `_cancelRlmChildRun`, `_abandonRlmRunForQuiescence`), the RLM child run lifecycle, and upstream PRs #346, #464, and #1253 plus issue searches for cancel/abort cascade work. +- `turn-scoped-subagent-cancellation`: **redesign**. Upstream keeps two cancel semantics. `requestAbort()` suspends the scheduler and aborts retry, compaction, branch summary, bash, refine, and the provider stream, but deliberately leaves active RLM child runs running; only `abort()` and `abortForUpdateRestart()` call `_cancelActiveRlmChildRuns`. PR #346 pinned the split with the characterization test "does not cancel active rlm children when only the parent turn is interrupted". Because every user-facing cancel path (interactive ctrl-c through `AgentConnection.abort`, ACP/in-process `abort` and `abortAndClearQueue`, daemon `abort` and `abort_and_clear_queue`) routes through `requestAbort()`, a cancelled parent turn left its children streaming until they finished or idled out. Pylon replaces the two semantics with one: `requestAbort()` now performs the same cascade `abort()` always did, and `abort()` inherits it instead of repeating it. +- Not superseded upstream. PR #464 (closed unmerged) made per-child cancellation explicit and idempotent, which is the `cancelRlmChildRun` machinery the fork already has, but never bound it to a turn abort. Open PR #1253 cancels admitted child runs when the owning kernel host is disposed, killed, or stopped, which is teardown-scoped rather than turn-scoped and does not change `requestAbort()`. +- Retained children survive by design. `_activeRlmChildRuns` holds every spawn run that has not finished, including one admitted by an earlier turn under the documented fire-and-forget delegation pattern; `RLMSpawnHandle` confirms admission only, so no Python cell ever awaits a child answer and there is no host-visible marker separating awaited from detached spawns. `_rlmChildSessions` holds children that already finished their run and stay addressable for `agent_message`, `rlm.list_subagents`, and inspectors. The cascade covers the first set and recurses into grandchildren through the child's own `abort()`. It does not touch the second set, because `abort()` never did, because a retained child's activity belongs to a later explicit request rather than the cancelled turn, and because the user already has `cancelRlmChildRun` / `rlm.delete_subagent` for a specific one. Prime exposes no detached or background spawn mode: `rlm.run` accepts only `name`, `model`, and `thinking`, so "retained versus active" is the only real distinction available. +- Ordering matters for correctness and is preserved: the cascade runs after the scheduler suspension, so `_cancelRlmChildRun` routes each cancelled run through `_abandonRlmRunForQuiescence`. Cancelled children therefore leave no unsettled quiescence work, cannot report success, and cannot inject a late terminal notice into the next turn. Session-local only; no daemon command, event, or schema change. +- Validation: new faux-provider regression `packages/coding-agent/test/suite/regressions/25-request-abort-rlm-cascade.test.ts` (2 tests) proves the child's provider stream is cut, that no further child request reaches the provider, that the parent settles and its next turn runs, and pins retained-child survival; it fails on the pre-change implementation. The upstream characterization test was inverted to "cancels active rlm children when the parent turn is interrupted", and the ACP-close terminal-notice retention test now settles the child before the scheduler cut, since a live child no longer survives it. `test/agent-session-recursion.test.ts` passes 112/112 and a 17-file affected batch across abort, RLM, subagent, queue, prompt, compaction, ACP, and correlated-lifecycle suites passes 396/396. `npm run check` is clean. +- Fork change: [pylon-code/prime-agent#35](https://github.com/pylon-code/prime-agent/pull/35). +- Revisit when Prime makes a user-facing turn abort cancel the children it owns, or introduces a real detached/background spawn mode that needs a per-child survival decision. diff --git a/packages/coding-agent/.changes/25-request-abort-rlm-cascade.md b/packages/coding-agent/.changes/25-request-abort-rlm-cascade.md new file mode 100644 index 0000000000..594758a449 --- /dev/null +++ b/packages/coding-agent/.changes/25-request-abort-rlm-cascade.md @@ -0,0 +1 @@ +- Fixed cancelling a turn so in-flight subagent runs stop with it instead of continuing to consume provider capacity; retained subagents keep running and are still cancelled individually ([#25](https://github.com/pylon-code/prime-agent/issues/25)). diff --git a/packages/coding-agent/docs/rlm-runtime.md b/packages/coding-agent/docs/rlm-runtime.md index 63625ccbc9..4e5217009a 100644 --- a/packages/coding-agent/docs/rlm-runtime.md +++ b/packages/coding-agent/docs/rlm-runtime.md @@ -168,6 +168,8 @@ audit = await rlm("slow independent audit", name="audit-reviewer") End the turn instead of waiting for completion. Children send requested answers with `await agent_message.send(message, receiver_role="parent")`, and replies arrive as ordinary agent messages over later turns. A child may instead write results to files for the parent to read. The host runs each admitted child as an independent `AgentSession`; daemon-backed children can be retained as independently addressable session workers. +A run admitted this way still belongs to the parent session's lifecycle: cancelling the parent turn cancels every child run that has not finished, so a cancel does not leave a fleet streaming. Children that already finished stay retained and addressable, and keep running any later work of their own. + ## Parent-Scoped Sub-Agent Registry The TypeScript parent maintains the authoritative direct-child registry. `await rlm.list_subagents()` returns stable child IDs, active-session IDs when daemon-backed, session IDs, names, directories, and running/completed status. @@ -248,6 +250,7 @@ Provider credentials are resolved by the TypeScript host. The bounded model cata | Requested model unavailable | Spawn fails instead of substituting another model. | | Host connection closed | Pending `host_request` calls fail with `RuntimeError` so awaiting cells unblock. | | Child cancellation | Host aborts the child and removes failed/cancelled registry entries. | +| Parent turn cancelled | Every in-flight child run is cancelled, including one admitted by an earlier turn. Retained child sessions keep running; stop one with `rlm.delete_subagent()` or the interactive per-child stop. | | Parent teardown | Active descendants are cancelled and their runtimes are closed. | ## Focused Validation diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index ba50b67fcb..c3d07b21d9 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -7462,6 +7462,15 @@ export class AgentSession { return this._resourceLoader; } + /** + * Cancel the active turn and everything it owns, including every in-flight RLM + * child run. Every user-facing cancel (interactive ctrl-c, ACP/daemon `abort`, + * `abort_and_clear_queue`) routes here, so an unfinished spawn run must die with + * the cancel rather than keep holding provider capacity, even when an earlier + * turn admitted it. Retained child sessions have already finished their run and + * stay addressable, so they keep running; cancel one by id with + * {@link cancelRlmChildRun}. + */ requestAbort(): void { this._failDeferredPromptLifecycles(); for (const run of [...this._unsettledRlmChildRuns]) { @@ -7490,13 +7499,15 @@ export class AgentSession { this._autoRefineReviewAbort?.abort(); this._refineAbortController?.abort(); this.agent.abort(); + // After the scheduler suspension above, so cancelled runs are abandoned for + // quiescence and cannot inject a late terminal notice into the next turn. + this._cancelActiveRlmChildRuns("Parent session aborted"); } async abort(): Promise { const compactionOperation = this._compactionOperation; const branchSummaryOperation = this._branchSummaryOperation; this.requestAbort(); - this._cancelActiveRlmChildRuns("Parent session aborted"); this._goalAbortInProgress = this._goalState.status === "active"; try { await Promise.allSettled([ diff --git a/packages/coding-agent/test/agent-session-recursion.test.ts b/packages/coding-agent/test/agent-session-recursion.test.ts index f68daa6d00..e43ce9900e 100644 --- a/packages/coding-agent/test/agent-session-recursion.test.ts +++ b/packages/coding-agent/test/agent-session-recursion.test.ts @@ -1722,18 +1722,21 @@ describe("AgentSession rlm recursion", () => { await root.runRlmChild("finish during ACP close", { name: "paused-terminal-worker" }); await childStarted.promise; const inputPause = root.acquireSessionInputPause(); - root.requestAbort(); - await expect(root.prompt("external prompt", { resumeIfIdle: false })).rejects.toThrow( - "session input admission is paused", - ); - - childCompletion.resolve(); const internals = root as unknown as InspectableRlmSession; const deferredNotices = () => root .getPendingNextTurnMessageSnapshots() .filter((message) => message.customType === "rlm_child_terminal_notice"); + // The child settles under the input pause. requestAbort cancels live child runs + // and suppresses their notices, so retention across the scheduler cut only + // applies to a notice that already exists. + childCompletion.resolve(); await vi.waitFor(() => expect(deferredNotices()).toHaveLength(1)); + root.requestAbort(); + await expect(root.prompt("external prompt", { resumeIfIdle: false })).rejects.toThrow( + "session input admission is paused", + ); + expect(deferredNotices()).toHaveLength(1); expect(synthesizedAgentMessageSend).not.toHaveBeenCalled(); const restartSnapshot = root.getPendingNextTurnMessageSnapshots(); await vi.waitFor(() => expect(internals._unsettledRlmChildRuns.size).toBe(0)); @@ -3001,7 +3004,7 @@ describe("AgentSession rlm recursion", () => { expect(promptAndWait).not.toHaveBeenCalled(); }); - it("does not cancel active rlm children when only the parent turn is interrupted", async () => { + it("cancels active rlm children when the parent turn is interrupted", async () => { let releaseChild: () => void = () => {}; const release = new Promise((resolve) => { releaseChild = resolve; @@ -3025,14 +3028,22 @@ describe("AgentSession rlm recursion", () => { await waitFor(() => childStarted); const runs = (root as unknown as InspectableRlmSession)._activeRlmChildRuns; expect(runs.size).toBe(1); - const run = [...runs.values()][0]; + const childId = [...runs.keys()][0]; + if (!childId) { + throw new Error("Missing child run id"); + } + const run = runs.get(childId); root.requestAbort(); - expect(run.status).toBe("running"); - expect(run.error).toBeUndefined(); + expect(run?.status).toBe("cancelled"); + expect(run?.error).toBe("Parent session aborted"); + // The cut is authoritative: a cancelled child neither blocks the next strong + // barrier nor injects a terminal notice into a later turn. + expect(run?.abandonedForQuiescence).toBe(true); releaseChild(); - await waitFor(() => run.status === "done"); + await waitFor(() => !runs.has(childId)); + expect(root.hasRunningRlmChildren()).toBe(false); }); it("cancels a single rlm child run by id and reports unknown ids", async () => { diff --git a/packages/coding-agent/test/suite/regressions/25-request-abort-rlm-cascade.test.ts b/packages/coding-agent/test/suite/regressions/25-request-abort-rlm-cascade.test.ts new file mode 100644 index 0000000000..fd31274fc3 --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/25-request-abort-rlm-cascade.test.ts @@ -0,0 +1,156 @@ +import { type FauxResponseStep, fauxAssistantMessage } from "@earendil-works/pi-ai"; +import { afterEach, describe, expect, it } from "vitest"; +import type { CustomMessage } from "../../../src/core/messages.js"; +import { createHarness, getAssistantTexts, type Harness } from "../harness.js"; + +interface Gate { + promise: Promise; + open: () => void; +} + +function gate(): Gate { + let open = () => {}; + const promise = new Promise((resolve) => { + open = resolve; + }); + return { promise, open }; +} + +/** Faux step that holds the provider stream open until the gate opens or the request aborts. */ +function heldResponse(options: { record: () => void; release: Promise; text: string }): FauxResponseStep { + return async (_context, streamOptions) => { + options.record(); + const signal = streamOptions?.signal; + await new Promise((resolve) => { + if (signal?.aborted) { + resolve(); + return; + } + signal?.addEventListener("abort", () => resolve(), { once: true }); + void options.release.then(() => resolve()); + }); + return fauxAssistantMessage(options.text); + }; +} + +function terminalNotices(messages: readonly unknown[]): CustomMessage[] { + return messages.filter( + (message): message is CustomMessage => + typeof message === "object" && + message !== null && + (message as { role?: unknown }).role === "custom" && + ((message as { customType?: unknown }).customType === "rlm_child_terminal_notice" || + (message as { customType?: unknown }).customType === "rlm_child_failure"), + ); +} + +describe("issue #25 requestAbort cascades into active RLM child runs", () => { + const harnesses: Harness[] = []; + const gates: Gate[] = []; + + afterEach(() => { + while (gates.length > 0) { + gates.pop()?.open(); + } + while (harnesses.length > 0) { + harnesses.pop()?.cleanup(); + } + }); + + async function createParent(child: Harness): Promise { + const parent = await createHarness({ + rlmDepth: 0, + rlmMaxDepth: 1, + subagentRuntimeHost: { + createRlmSubagentRuntime: async () => ({ session: child.session }), + deleteRlmSubagentRuntime: async () => {}, + }, + }); + harnesses.push(parent); + return parent; + } + + it("terminates an in-flight child provider stream and settles the parent", async () => { + const child = await createHarness(); + harnesses.push(child); + const release = gate(); + gates.push(release); + let childRequests = 0; + child.setResponses([ + heldResponse({ + record: () => { + childRequests++; + }, + release: release.promise, + text: "child finished after the abort", + }), + fauxAssistantMessage("child must not start a second turn"), + ]); + const parent = await createParent(child); + parent.setResponses([fauxAssistantMessage("parent recovered")]); + + const spawned = await parent.session.runRlmChild("long shard", { name: "cascade-worker" }); + await expect.poll(() => childRequests).toBe(1); + expect(parent.session.getRlmChildRunStatus(spawned.rlm_child_id)).toBe("running"); + + parent.session.requestAbort(); + + expect(parent.session.getRlmChildRunStatus(spawned.rlm_child_id)).toBe("cancelled"); + // The run unwinds on its own once the child's provider stream is cut. + await expect.poll(() => parent.session.getRlmChildRunStatus(spawned.rlm_child_id)).toBeUndefined(); + expect(parent.session.hasRunningRlmChildren()).toBe(false); + expect(child.session.isStreaming).toBe(false); + // No further child request reached the provider. + expect(child.faux.state.callCount).toBe(1); + expect(child.getPendingResponseCount()).toBe(1); + // A cancelled child does not report an outcome to the parent. + expect(terminalNotices(parent.session.messages)).toEqual([]); + expect(parent.session.getPendingNextTurnMessageSnapshots()).toEqual([]); + + // The cut leaves no quiescence waiter behind, and the next turn runs. + await expect(parent.session.waitForRlmQuiescence()).resolves.toBeUndefined(); + parent.session.resumeQueuedWork(); + await parent.session.prompt("what happened?"); + await expect(parent.session.waitForRlmQuiescence()).resolves.toBeUndefined(); + expect(getAssistantTexts(parent)).toEqual(["parent recovered"]); + }); + + it("leaves a retained background subagent running", async () => { + const child = await createHarness(); + harnesses.push(child); + const release = gate(); + gates.push(release); + let childRequests = 0; + child.setResponses([ + fauxAssistantMessage("first shard done"), + heldResponse({ + record: () => { + childRequests++; + }, + release: release.promise, + text: "background work finished", + }), + ]); + const parent = await createParent(child); + parent.setResponses([fauxAssistantMessage("parent consumed the child result")]); + + const spawned = await parent.session.runRlmChild("first shard", { name: "retained-worker" }); + await expect.poll(() => terminalNotices(parent.session.messages)).toHaveLength(1); + const retained = parent.session.getRlmChildSession(spawned.rlm_child_id); + expect(retained).toBe(child.session); + + // Retained children stay addressable, so their own work is background work the + // parent turn does not own. + const background = child.session.prompt("keep working in the background"); + await expect.poll(() => childRequests).toBe(1); + + parent.session.requestAbort(); + + expect(child.session.isStreaming).toBe(true); + release.open(); + await background; + expect(child.session.getLastAssistantText()).toBe("background work finished"); + expect(child.faux.state.callCount).toBe(2); + expect(parent.session.getRlmChildSession(spawned.rlm_child_id)).toBe(child.session); + }); +});