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.
This commit is contained in:
Tianyi Cui
2026-07-15 21:45:30 +08:00
parent f4bd2405cc
commit b06ae91fa8
12 changed files with 84 additions and 36 deletions

View File

@@ -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.

View File

@@ -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<TaskId, TrackedTask>()
private counters = new Map<string, number>()
@@ -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)

View File

@@ -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 (`<kind>-N`). */
id: TaskId
/** The producer kind the task was registered with. */
kind: string
kind: TaskKind
/** The producer-supplied one-line label. */
label: string
/**

View File

@@ -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<Agent, () => Promise<void>>()
@@ -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)

View File

@@ -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)}` }])

View File

@@ -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 } })