From b06ae91fa8acb38cd62c1b5e23992d95dc032201 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Wed, 15 Jul 2026 21:45:30 +0800 Subject: [PATCH] fix(tasks): address task API review feedback The public kill result used the awkward phrase already-terminal. Rename it to already-finished and keep the model-facing response aligned; not-alive would be inaccurate because a force-failed registry record can still correspond to orphaned producer work. Task kinds were open strings even though producer namespaces are an extension point. Add the merge-extensible TaskKindMap and derived TaskKind, cover consumer declarations in task and bundle tests, and retain the runtime non-empty check for untyped callers. With exactOptionalPropertyTypes, owner?: Agent | undefined allowed an explicit undefined value that no caller needs. Tighten the property to owner?: Agent so unowned work is expressed by omitting it. Record the requested task-service/backend split as a follow-up, using a systemd-backed runtime as a concrete candidate without guessing its durability and ownership contract in this PR. Regenerate the type and Cordis catalogs so public docs match the declarations. --- docs/cordis-catalog/services.md | 4 ++-- docs/core-data-structures/tasks.md | 23 ++++++++++++------ ...06-20-generic-long-running-tool-runtime.md | 8 +++---- .../cordis/tool-cordis/src/api-catalog.ts | 14 ++++++++--- .../agent-spine-demo/tests/agent-core.spec.ts | 6 +++++ packages/tasks/tasks/README.md | 3 ++- packages/tasks/tasks/src/index.ts | 14 +++++++---- packages/tasks/tasks/src/types.ts | 24 ++++++++++++++----- packages/tasks/tasks/tests/tasks.spec.ts | 19 ++++++++++----- packages/tasks/tool-tasks/src/index.ts | 2 +- .../tasks/tool-tasks/tests/tool-tasks.spec.ts | 2 +- scripts/type-equiv.manifest.json | 1 + 12 files changed, 84 insertions(+), 36 deletions(-) diff --git a/docs/cordis-catalog/services.md b/docs/cordis-catalog/services.md index 19c09d944b..18e00b7904 100644 --- a/docs/cordis-catalog/services.md +++ b/docs/cordis-catalog/services.md @@ -251,7 +251,7 @@ start(spec: TaskStart): TaskId list(caller?: Agent): TaskSnapshot[] get(id: TaskId, caller?: Agent): TaskSnapshot read(id: TaskId, caller?: Agent): TaskRead -kill(id: TaskId, caller?: Agent, reason?: string): 'requested' | 'already-terminal' +kill(id: TaskId, caller?: Agent, reason?: string): 'requested' | 'already-finished' async wait(id: TaskId, timeoutMs: number, caller?: Agent, signal?: AbortSignal): Promise onTaskDone(listener: TaskDoneListener): () => void attachSurface(name: string): () => void @@ -259,7 +259,7 @@ attachSurface(name: string): () => void Types: [Agent](../core-data-structures/core.md) -Source: [`packages/tasks/tasks/src/index.ts:72`](../../packages/tasks/tasks/src/index.ts) +Source: [`packages/tasks/tasks/src/index.ts:76`](../../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 0f84ac33b4..0482912af7 100644 --- a/docs/core-data-structures/tasks.md +++ b/docs/core-data-structures/tasks.md @@ -4,7 +4,16 @@ Types shared by long-running producers, `ctx.tasks`, and task control surfaces. ## Ids and status -`TaskId` is a [branded id](core.md#branded-ids) generated as `-N`. Access control relies on owner authorization, not id secrecy. `TaskStatus` is `'running' | 'stopping' | 'completed' | 'killed' | 'failed'`; producer-specific facts belong in `TaskSnapshot.detail`. +`TaskId` is a [branded id](core.md#branded-ids) generated as `-N`. Access control relies on owner authorization, not id secrecy. `TaskKind` derives from a merge-extensible map; the registry treats kinds as opaque id namespaces. + +```ts type-equiv +interface TaskKindMap { + bash: 'bash' + subagent: 'subagent' +} +``` + +`TaskStatus` is `'running' | 'stopping' | 'completed' | 'killed' | 'failed'`; producer-specific facts belong in `TaskSnapshot.detail`. ## Producer contract @@ -12,17 +21,17 @@ Types shared by long-running producers, `ctx.tasks`, and task control surfaces. ```ts type-equiv interface TaskStart { - /** Producer kind — also the id prefix (`bash`, `subagent`, …). Non-empty. */ - kind: string + /** Producer kind — also the id prefix (`bash`, `subagent`, …). */ + kind: TaskKind /** One-line model-facing label (the command; the delegation description). */ label: string /** * Owning live agent. Access is fenced by its session id, and agent disposal * cancels and awaits the task. The instance must be the one currently - * registered under its agent id. `undefined` creates an unowned task, open to - * any caller until service disposal. + * registered under its agent id. Omitting the owner creates an unowned task, + * open to any caller until service disposal. */ - owner?: Agent | undefined + owner?: Agent /** * Start the work after preflight and synchronously return its hooks. Called * once; a throw leaves nothing registered, and the producer must clean up any @@ -77,7 +86,7 @@ interface TaskSnapshot { /** The registry-issued id (`-N`). */ id: TaskId /** The producer kind the task was registered with. */ - kind: string + kind: TaskKind /** The producer-supplied one-line label. */ label: string /** 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 64c5dff5a4..c99eb9f0fe 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 @@ -17,7 +17,7 @@ The `tasks/` package group owns background-task semantics: Long-running tools are producers. `dsh-tool-bash` adapts a `BashProcess` into incremental output and process cancellation; `dsh-tool-subagent` adapts a child run into final output and child disposal. The execution seams remain independent of sessions and the task registry. -`TaskService` is a concrete service. There is one in-process implementation, so an interface/backend package split would be speculative. A durable or remote implementation can introduce that seam when its lifecycle requirements are known. +`TaskService` is a concrete, process-local service. TODO(task-service-backend): separate its public contract from the implementation when a second backend defines the required lifecycle; a systemd-backed runtime is one plausible driver, but this PR does not speculate about its durability, reconnect, ownership, or observation semantics. ## Runtime contract @@ -29,7 +29,7 @@ The producer hooks define three responsibilities: - `done` never rejects and settles only after the producer has released the task's resources. - Optional `readOutput()` returns the next consuming output delta. Omitting it declares a final-output task whose terminal result comes from `TaskOutcome.output`. -Statuses are `running`, `stopping`, `completed`, `killed`, and `failed`. Producer-specific information such as an exit code or stop reason belongs in `detail`; the registry does not interpret it. Task ids are branded and generated as `-N`, with a counter per kind. +Statuses are `running`, `stopping`, `completed`, `killed`, and `failed`. Producer-specific information such as an exit code or stop reason belongs in `detail`; the registry does not interpret it. Task kinds form a merge-extensible string union, and task ids are branded and generated as `-N`, with a counter per kind. The runtime attaches one continuation to `done`, records the first terminal outcome, resolves waiters, and invokes completion listeners with per-listener error containment. First-wins settlement matters during teardown: if `cancel` throws, the runtime force-fails the record and warns that work may be orphaned rather than waiting forever for a promise that may never settle. A later producer outcome cannot overwrite that diagnosis or notify twice. A `cancel` that returns without eventually settling `done` still blocks teardown because the runtime cannot distinguish it from a slow, valid stop. @@ -95,9 +95,9 @@ For background subagents, `dsh-tool-subagent` creates a task-owned `AbortControl Separate bash and subagent output/stop tools duplicate ids, isolation, cleanup, notification, and guidance while increasing the model's schema and protocol burden. One runtime keeps execution-specific behavior in producers without cloning the task lifecycle. -### An abstract task-runtime backend +### An immediate abstract task-runtime backend -No second backend exists. Durable work also changes owner and restart semantics, so its design should extract an interface from concrete requirements rather than preserve this implementation speculatively. +The current `TaskStart.run()` contract passes in-process callbacks and exact `Agent` objects. A durable backend changes identity, restart, ownership, and observation semantics, so extracting an interface before a second implementation exists would freeze the wrong boundary. ### Consumer-owned authorization or cleanup events diff --git a/packages/cordis/tool-cordis/src/api-catalog.ts b/packages/cordis/tool-cordis/src/api-catalog.ts index 9401425dd3..51b4a5155f 100644 --- a/packages/cordis/tool-cordis/src/api-catalog.ts +++ b/packages/cordis/tool-cordis/src/api-catalog.ts @@ -216,7 +216,7 @@ export const SERVICE_API: readonly ServiceApiEntry[] = [ 'list(caller?: Agent): TaskSnapshot[]', 'get(id: TaskId, caller?: Agent): TaskSnapshot', 'read(id: TaskId, caller?: Agent): TaskRead', - 'kill(id: TaskId, caller?: Agent, reason?: string): \'requested\' | \'already-terminal\'', + 'kill(id: TaskId, caller?: Agent, reason?: string): \'requested\' | \'already-finished\'', 'async wait(id: TaskId, timeoutMs: number, caller?: Agent, signal?: AbortSignal): Promise', 'onTaskDone(listener: TaskDoneListener): () => void', 'attachSurface(name: string): () => void', @@ -933,6 +933,14 @@ export const TYPE_API: readonly TypeApiEntry[] = [ name: 'TaskId', declaration: 'export type TaskId = Branded<\'TaskId\'>;', }, + { + name: 'TaskKind', + declaration: 'export type TaskKind = TaskKindMap[keyof TaskKindMap];', + }, + { + name: 'TaskKindMap', + declaration: 'export interface TaskKindMap {\n bash: \'bash\';\n subagent: \'subagent\';\n}', + }, { name: 'TaskOutcome', declaration: 'export interface TaskOutcome {\n status: \'completed\' | \'killed\' | \'failed\';\n detail?: string;\n output?: string;\n}', @@ -943,11 +951,11 @@ export const TYPE_API: readonly TypeApiEntry[] = [ }, { name: 'TaskSnapshot', - declaration: 'export interface TaskSnapshot {\n id: TaskId;\n kind: string;\n label: string;\n ownerSession?: SessionId;\n status: TaskStatus;\n detail?: string;\n startedAt: number;\n finishedAt?: number;\n reported: boolean;\n}', + declaration: 'export interface TaskSnapshot {\n id: TaskId;\n kind: TaskKind;\n label: string;\n ownerSession?: SessionId;\n status: TaskStatus;\n detail?: string;\n startedAt: number;\n finishedAt?: number;\n reported: boolean;\n}', }, { name: 'TaskStart', - declaration: 'export interface TaskStart {\n kind: string;\n label: string;\n owner?: Agent | undefined;\n run(): TaskHooks;\n}', + declaration: 'export interface TaskStart {\n kind: TaskKind;\n label: string;\n owner?: Agent;\n run(): TaskHooks;\n}', }, { name: 'TaskStatus', diff --git a/packages/examples/agent-spine-demo/tests/agent-core.spec.ts b/packages/examples/agent-spine-demo/tests/agent-core.spec.ts index a09479bdb9..74dbfe5484 100644 --- a/packages/examples/agent-spine-demo/tests/agent-core.spec.ts +++ b/packages/examples/agent-spine-demo/tests/agent-core.spec.ts @@ -9,6 +9,12 @@ import * as agentCore from '../src/index.ts' import { AgentId, agentEvents, type Agent } from '@deepseek-ai/dsh-agent' import { CallId, type Message } from '@deepseek-ai/dsh-llm' +declare module '@deepseek-ai/dsh-tasks' { + interface TaskKindMap { + probe: 'probe' + } +} + async function composePrefix(ctx: Context, cwd: string): Promise { const agent = { session: { header: { cwd } } } as unknown as Agent const empty: Message[] = [] diff --git a/packages/tasks/tasks/README.md b/packages/tasks/tasks/README.md index e9ddf43256..b061038714 100644 --- a/packages/tasks/tasks/README.md +++ b/packages/tasks/tasks/README.md @@ -1,6 +1,6 @@ # @deepseek-ai/dsh-tasks -The process-local background task registry (`ctx.tasks`). It gives long-running producers shared ids, owner isolation, reads, cancellation, waiting, notices, and cleanup. The service is concrete; a durable backend can introduce an interface when its different lifecycle is specified. +The process-local background task registry (`ctx.tasks`). It gives long-running producers shared ids, owner isolation, reads, cancellation, waiting, notices, and cleanup. Producer plugins extend `TaskKindMap` with their opaque id namespace. ## Service API @@ -29,6 +29,7 @@ Indirectly, through producer plugins and [`dsh-tool-tasks`](../tool-tasks/README ## Known Limitations and Deferred Work - **Tasks are process-local** — durable or cross-restart execution needs a separate lifecycle. +- **The service and implementation are not split** — a second backend must define the lifecycle that shapes that boundary. - **Stream output has one consuming cursor** — independent observers need a cursor or snapshot API. - **Foreground work cannot be promoted** — producers choose foreground or background before starting. - **A silently ineffective cancel can stall teardown** — only an explicit throw can be force-failed safely. diff --git a/packages/tasks/tasks/src/index.ts b/packages/tasks/tasks/src/index.ts index 68300a7079..e9c896c3a1 100644 --- a/packages/tasks/tasks/src/index.ts +++ b/packages/tasks/tasks/src/index.ts @@ -13,12 +13,14 @@ import { Context, Service } from 'cordis' import type { Agent } from '@deepseek-ai/dsh-agent' import { deadline, timeoutOf } from '@deepseek-ai/dsh-timeout' import { TaskId } from './types.ts' -import type { TaskDoneListener, TaskOutcome, TaskRead, TaskSnapshot, TaskStart, TaskStatus } from './types.ts' +import type { TaskDoneListener, TaskKind, TaskOutcome, TaskRead, TaskSnapshot, TaskStart, TaskStatus } from './types.ts' export { TaskId } from './types.ts' export type { TaskDoneListener, TaskHooks, + TaskKind, + TaskKindMap, TaskOutcome, TaskRead, TaskSnapshot, @@ -38,7 +40,7 @@ export const TASK_WAIT_TIMEOUT = 'TASK_WAIT_TIMEOUT' /** The registry's mutable per-task record (never handed out — see {@link TaskService.snapshot}). */ interface TrackedTask { id: TaskId - kind: string + kind: TaskKind label: string /** Exact lifecycle owner; session-id authorization is derived from it. */ owner: Agent | undefined @@ -69,6 +71,8 @@ function isTerminal(status: TaskStatus): boolean { * The `tasks` service: the runtime-global background task registry. See the * module doc for the ownership, isolation, and lifecycle contracts. */ +// TODO(task-service-backend): Separate the service contract from this +// process-local implementation when a second backend defines its lifecycle. export class TaskService extends Service { private store = new Map() private counters = new Map() @@ -191,14 +195,14 @@ export class TaskService extends Service { * @param id - task to cancel. * @param caller - killing agent checked against the owner. * @param reason - logged reason forwarded to the producer. - * @returns `requested` for live work, otherwise `already-terminal`. + * @returns `requested` for live work, otherwise `already-finished`. */ - kill(id: TaskId, caller?: Agent, reason?: string): 'requested' | 'already-terminal' { + kill(id: TaskId, caller?: Agent, reason?: string): 'requested' | 'already-finished' { const task = this.expect(id) this.assertAccess(task, caller) if (isTerminal(task.status)) { task.reported = true - return 'already-terminal' + return 'already-finished' } // Cancel first so a throw leaves both lifecycle and notice state unchanged. task.cancel(reason) diff --git a/packages/tasks/tasks/src/types.ts b/packages/tasks/tasks/src/types.ts index 34d0189913..96316260ec 100644 --- a/packages/tasks/tasks/src/types.ts +++ b/packages/tasks/tasks/src/types.ts @@ -29,6 +29,18 @@ export function TaskId(id: string): TaskId { */ export type TaskStatus = 'running' | 'stopping' | 'completed' | 'killed' | 'failed' +/** + * Producer-defined task kinds. Plugins extend this map by declaration merging; + * the registry treats every value as an opaque id namespace. + */ +export interface TaskKindMap { + bash: 'bash' + subagent: 'subagent' +} + +/** The merge-extensible union of registered producer kind names. */ +export type TaskKind = TaskKindMap[keyof TaskKindMap] + /** Terminal result supplied by a producer through {@link TaskHooks.done}. */ export interface TaskOutcome { /** How the task ended: finished (`completed`), cancelled (`killed`), or broke (`failed`). */ @@ -45,17 +57,17 @@ export interface TaskOutcome { * execution resources while the runtime owns identity and lifecycle state. */ export interface TaskStart { - /** Producer kind — also the id prefix (`bash`, `subagent`, …). Non-empty. */ - kind: string + /** Producer kind — also the id prefix (`bash`, `subagent`, …). */ + kind: TaskKind /** One-line model-facing label (the command; the delegation description). */ label: string /** * Owning live agent. Access is fenced by its session id, and agent disposal * cancels and awaits the task. The instance must be the one currently - * registered under its agent id. `undefined` creates an unowned task, open to - * any caller until service disposal. + * registered under its agent id. Omitting the owner creates an unowned task, + * open to any caller until service disposal. */ - owner?: Agent | undefined + owner?: Agent /** * Start the work after preflight and synchronously return its hooks. Called * once; a throw leaves nothing registered, and the producer must clean up any @@ -94,7 +106,7 @@ export interface TaskSnapshot { /** The registry-issued id (`-N`). */ id: TaskId /** The producer kind the task was registered with. */ - kind: string + kind: TaskKind /** The producer-supplied one-line label. */ label: string /** diff --git a/packages/tasks/tasks/tests/tasks.spec.ts b/packages/tasks/tasks/tests/tasks.spec.ts index b9b6cfdf01..f21eb9c433 100644 --- a/packages/tasks/tasks/tests/tasks.spec.ts +++ b/packages/tasks/tasks/tests/tasks.spec.ts @@ -4,7 +4,13 @@ import { Session, SessionId } from '@deepseek-ai/dsh-session' import AgentRegistry, { AgentId } from '@deepseek-ai/dsh-agent' import type { Agent } from '@deepseek-ai/dsh-agent' import TaskService, { TaskId } from '@deepseek-ai/dsh-tasks' -import type { TaskHooks, TaskOutcome, TaskSnapshot, TaskStart } from '@deepseek-ai/dsh-tasks' +import type { TaskHooks, TaskKind, TaskOutcome, TaskSnapshot, TaskStart } from '@deepseek-ai/dsh-tasks' + +declare module '@deepseek-ai/dsh-tasks' { + interface TaskKindMap { + workflow: 'workflow' + } +} const agentScopeDisposers = new WeakMap Promise>() @@ -81,7 +87,7 @@ describe('TaskService.start', () => { it('rejects an empty kind and an empty label', async () => { const ctx = await harness() - expect(() => ctx.tasks.start(producer({ kind: '' }).spec)).toThrow('invalid task kind') + expect(() => ctx.tasks.start(producer({ kind: '' as TaskKind }).spec)).toThrow('invalid task kind') expect(() => ctx.tasks.start(producer({ label: '' }).spec)).toThrow('invalid task label') }) @@ -90,6 +96,7 @@ describe('TaskService.start', () => { expect(ctx.tasks.start(producer().spec)).toBe('bash-1') expect(ctx.tasks.start(producer().spec)).toBe('bash-2') expect(ctx.tasks.start(producer({ kind: 'subagent' }).spec)).toBe('subagent-1') + expect(ctx.tasks.start(producer({ kind: 'workflow' }).spec)).toBe('workflow-1') }) }) @@ -222,13 +229,13 @@ describe('TaskService.kill', () => { expect(seen[0]).toMatchObject({ id, status: 'killed', reported: true }) }) - it('reports an already-terminal task instead of failing', async () => { + it('reports an already-finished task instead of failing', async () => { const ctx = await harness() const p = producer() const id = ctx.tasks.start(p.spec) p.settle({ status: 'completed' }) await tick() - expect(ctx.tasks.kill(id)).toBe('already-terminal') + expect(ctx.tasks.kill(id)).toBe('already-finished') }) it('propagates a throwing producer cancel and leaves the task untouched', async () => { @@ -254,7 +261,7 @@ describe('TaskService.kill', () => { expect(seen[0]).toMatchObject({ id, reported: false }) // notice would still fire broken = false - expect(ctx.tasks.kill(id)).toBe('already-terminal') + expect(ctx.tasks.kill(id)).toBe('already-finished') }) }) @@ -299,7 +306,7 @@ describe('TaskService.wait', () => { expect(ctx.tasks.get(id).status).toBe('running') }) - it('returns immediately for an already-terminal task', async () => { + it('returns immediately for an already-finished task', async () => { const ctx = await harness() const p = producer() const id = ctx.tasks.start(p.spec) diff --git a/packages/tasks/tool-tasks/src/index.ts b/packages/tasks/tool-tasks/src/index.ts index f0de782eea..2d556cada9 100644 --- a/packages/tasks/tool-tasks/src/index.ts +++ b/packages/tasks/tool-tasks/src/index.ts @@ -136,7 +136,7 @@ export function apply(ctx: Context, config: Config): void { execute(args, exec) { const id = validateTaskId(args.task_id) const result = ctx.tasks.kill(id, exec.agent, args.reason) - if (result === 'already-terminal') { + if (result === 'already-finished') { // A snapshot describes terminal state without consuming pending output. const snapshot = ctx.tasks.get(id, exec.agent) return Promise.resolve([{ type: 'text', text: `task ${id} had already finished ${statusLine(snapshot)}` }]) diff --git a/packages/tasks/tool-tasks/tests/tool-tasks.spec.ts b/packages/tasks/tool-tasks/tests/tool-tasks.spec.ts index d78bd1668a..1b5e221a3c 100644 --- a/packages/tasks/tool-tasks/tests/tool-tasks.spec.ts +++ b/packages/tasks/tool-tasks/tests/tool-tasks.spec.ts @@ -192,7 +192,7 @@ describe('task_kill', () => { expect(p.cancels).toEqual(['superseded']) }) - it('reports an already-terminal task without consuming its pending delta', async () => { + it('reports an already-finished task without consuming its pending delta', async () => { const { ctx } = await setup() let delta = 'unread tail' const p = producer({ readOutput: () => { const d = delta; delta = ''; return d } }) diff --git a/scripts/type-equiv.manifest.json b/scripts/type-equiv.manifest.json index b270dd50bf..c3e4533ba5 100644 --- a/scripts/type-equiv.manifest.json +++ b/scripts/type-equiv.manifest.json @@ -91,6 +91,7 @@ { "doc": "docs/core-data-structures/bash.md", "symbol": "BashProcess", "source": "packages/bash/bash/src/types.ts" }, { "doc": "docs/core-data-structures/bash.md", "symbol": "BashProcessRead", "source": "packages/bash/bash/src/types.ts" }, + { "doc": "docs/core-data-structures/tasks.md", "symbol": "TaskKindMap", "source": "packages/tasks/tasks/src/types.ts" }, { "doc": "docs/core-data-structures/tasks.md", "symbol": "TaskStart", "source": "packages/tasks/tasks/src/types.ts" }, { "doc": "docs/core-data-structures/tasks.md", "symbol": "TaskHooks", "source": "packages/tasks/tasks/src/types.ts" }, { "doc": "docs/core-data-structures/tasks.md", "symbol": "TaskOutcome", "source": "packages/tasks/tasks/src/types.ts" },