From 1a69a3debedbe6b54b53f069084cc68b6d3ee284 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Wed, 15 Jul 2026 11:40:37 +0800 Subject: [PATCH] fix(tasks): bind cleanup and notices to exact owners Task records previously retained only ownerSession. If an old agent scope unwound after another agent reused the same agent and session ids, cleanup selected both records and could cancel replacement work. The completion surface also re-resolved the session at settlement, which could inject an old task notice into the replacement agent. Retain the exact Agent instance for lifecycle work, select owner cleanup by object identity, and pass that exact owner to completion listeners. Keep read, list, kill, and wait authorization session-based as the runtime RFC intends. Add regressions for cleanup and notice routing under id reuse, then update the public docs and generated API catalogs. --- docs/cordis-catalog/services.md | 2 +- docs/core-data-structures/tasks.md | 13 +++--- ...06-20-generic-long-running-tool-runtime.md | 8 ++-- .../cordis/tool-cordis/src/api-catalog.ts | 2 +- packages/tasks/tasks/README.md | 4 +- packages/tasks/tasks/src/index.ts | 34 +++++++-------- packages/tasks/tasks/src/types.ts | 19 +++++---- packages/tasks/tasks/tests/tasks.spec.ts | 33 +++++++++++++++ packages/tasks/tool-tasks/README.md | 2 +- packages/tasks/tool-tasks/src/index.ts | 16 +++---- .../tasks/tool-tasks/tests/tool-tasks.spec.ts | 42 +++++++++++++++---- 11 files changed, 117 insertions(+), 58 deletions(-) diff --git a/docs/cordis-catalog/services.md b/docs/cordis-catalog/services.md index dd8663ae32..c8f587c765 100644 --- a/docs/cordis-catalog/services.md +++ b/docs/cordis-catalog/services.md @@ -259,7 +259,7 @@ attachSurface(name: string): () => void Types: [Agent](../core-data-structures/core.md) -Source: [`packages/tasks/tasks/src/index.ts:99`](../../packages/tasks/tasks/src/index.ts) +Source: [`packages/tasks/tasks/src/index.ts:98`](../../packages/tasks/tasks/src/index.ts) ## `ctx.tools` — `ToolRegistry` diff --git a/docs/core-data-structures/tasks.md b/docs/core-data-structures/tasks.md index 83351f0651..8ee766de78 100644 --- a/docs/core-data-structures/tasks.md +++ b/docs/core-data-structures/tasks.md @@ -99,11 +99,12 @@ interface TaskSnapshot { /** The producer-supplied one-line label. */ label: string /** - * The owner's session id (`session.header.id`), for surfaces that must - * reach the owning agent (the completion-notice injector); absent for - * unowned tasks. Session ids are runtime-shared identifiers, not secrets — - * the read/kill/wait/list FENCE is what isolation rests on. The shared - * {@link SessionId} brand is preserved across this package boundary. + * The owner's session id (`session.header.id`), for authorization and + * correlation; absent for unowned tasks. A listener that must reach the + * lifecycle owner receives the exact Agent separately through + * {@link TaskDoneListener}. Session ids are runtime-shared identifiers, not + * secrets — the read/kill/wait/list FENCE is what isolation rests on. The + * shared {@link SessionId} brand is preserved across this package boundary. */ ownerSession?: SessionId /** Current lifecycle state. */ @@ -140,4 +141,4 @@ interface TaskRead { ## The service -`TaskService` (`ctx.tasks` — [`packages/tasks/tasks/src/index.ts`](../../packages/tasks/tasks/src/index.ts)): `start` (preflight → producer `run()` → atomic commit, fenced by `attachSurface`), non-consuming `get`/`list` (caller-scoped — owned-by-caller plus unowned only), `read` (consuming for stream kinds), `kill` (producer `cancel` first; a throw leaves the task untouched), `wait` (bounded, abort cancels the wait only), and `onTaskDone` (a `TaskDoneListener` per terminal record, effect-scoped, contained). Start validates that an owned task names the exact live Agent instance currently registered under its id, so an old reference cannot bind work to a replacement agent's cleanup after id reuse. Every read/kill/wait/get separately compares the task's owner session with the caller's and rejects a foreign one. Owned tasks register one async cleanup through the exact owner's `agent.ctx`; scope disposal cancels them and normally awaits producer quiescence. A teardown cancel that throws force-fails only the registry record and reports that the underlying work may be orphaned, preventing disposal deadlock without claiming quiescence. The model-facing surface over all of this is [dsh-tool-tasks](../../packages/tasks/tool-tasks/README.md). +`TaskService` (`ctx.tasks` — [`packages/tasks/tasks/src/index.ts`](../../packages/tasks/tasks/src/index.ts)): `start` (preflight → producer `run()` → atomic commit, fenced by `attachSurface`), non-consuming `get`/`list` (caller-scoped — owned-by-caller plus unowned only), `read` (consuming for stream kinds), `kill` (producer `cancel` first; a throw leaves the task untouched), `wait` (bounded, abort cancels the wait only), and `onTaskDone` (a `TaskDoneListener` per terminal record, given the exact lifecycle owner, effect-scoped, contained). Start retains the exact live Agent instance validated under its id; owner-scope cleanup selects by that identity, so a reused agent/session id cannot make an old scope cancel replacement work. Read/kill/wait/get authorization remains session-based and rejects a foreign session. A teardown cancel that throws force-fails only the registry record and reports that the underlying work may be orphaned, preventing disposal deadlock without claiming quiescence. The model-facing surface over all of this is [dsh-tool-tasks](../../packages/tasks/tool-tasks/README.md). diff --git a/docs/rfc/implemented/architecture/2026-06-20-generic-long-running-tool-runtime.md b/docs/rfc/implemented/architecture/2026-06-20-generic-long-running-tool-runtime.md index 6c62f96a65..b179ceaa49 100644 --- a/docs/rfc/implemented/architecture/2026-06-20-generic-long-running-tool-runtime.md +++ b/docs/rfc/implemented/architecture/2026-06-20-generic-long-running-tool-runtime.md @@ -65,7 +65,7 @@ Registrations are NOT effect-scoped to the registering fiber: a task belongs to ## Authorization and the service surface -Cross-session isolation lives IN the runtime so every consumer gets the same rule for free: read/kill/wait/get take the caller (`Agent | undefined`), and a task whose owner session differs from the caller's session is rejected (`!== undefined` comparison — an unowned task is open, a no-agent caller cannot match an owned one). `list(caller)` returns only the caller-visible tasks (owned-by-caller or unowned) — a global listing would leak other sessions' labels. The snapshot carries that owner as the canonical branded `SessionId`, not a package-local token or bare string. Lifecycle ownership is checked independently at start: the supplied owner must be the exact live `Agent` instance currently registered under its id, so an old object cannot attach its session's work to a replacement agent's cleanup after id reuse. +Cross-session isolation lives IN the runtime so every consumer gets the same rule for free: read/kill/wait/get take the caller (`Agent | undefined`), and a task whose owner session differs from the caller's session is rejected (`!== undefined` comparison — an unowned task is open, a no-agent caller cannot match an owned one). `list(caller)` returns only the caller-visible tasks (owned-by-caller or unowned) — a global listing would leak other sessions' labels. The snapshot carries that authorization identity as the canonical branded `SessionId`, not a package-local token or bare string. Lifecycle ownership is independent: start retains the exact live `Agent` instance, owner cleanup selects by object identity, and completion listeners receive that exact owner, so id reuse cannot redirect cleanup or notices to a replacement. ```ts ignore-check class TaskService extends Service { // ctx.tasks @@ -75,7 +75,7 @@ class TaskService extends Service { // ctx.tasks read(id: TaskId, caller?: Agent): TaskRead // delta (stream kinds, consuming) or final output (final kinds, idempotent) + snapshot kill(id: TaskId, caller?: Agent, reason?: string): 'requested' | 'already-terminal' wait(id: TaskId, timeoutMs: number, caller?: Agent, signal?: AbortSignal): Promise - onTaskDone(listener: (snapshot: TaskSnapshot) => void): () => void // effect-scoped, contained, never fires after dispose + onTaskDone(listener: (snapshot: TaskSnapshot, owner: Agent | undefined) => void): () => void // exact lifecycle owner; effect-scoped, contained attachSurface(name: string): () => void // the misconfiguration fence, below } ``` @@ -96,7 +96,7 @@ class TaskService extends Service { // ctx.tasks One system-prompt section (order 106, next to `tool:bash`) teaches the cross-call habit the per-tool descriptions cannot: track every returned task id; you are notified in-session when a task finishes, so do not busy-poll or sleep on one — keep working on independent steps and do not duplicate a running task's work; do not produce a final answer while a relevant task still runs — call `task_output` (with `wait` when blocked) to collect it first; `task_kill` tasks that stopped mattering. The do-not-poll and do-not-duplicate sentences are near-verbatim convergent across Claude Code, Kimi Code, and OpenCode — they are the two failure modes every peer engineered against. -Completion notices stay durable context, not a wake-up (`agent.inject()` appends a logged `context/message` the next model request sees; it does not run the model): on `onTaskDone`, `dsh-tool-tasks` injects `background task (: