fix(tasks): contain async completion listeners

TaskService contained synchronous listener throws but discarded returned promises. An async TaskDoneListener could therefore reject as an unhandled process rejection even though the API promises per-listener containment.

Allow listeners to return PromiseLike<void> and attach a rejection handler to each result without awaiting it. This logs asynchronous failures independently while preserving the observation-only contract: later listeners, task waiters, and teardown are not delayed. Add a regression test and synchronize the public docs and generated API declaration.
This commit is contained in:
Tianyi Cui
2026-07-15 11:40:47 +08:00
parent 1a69a3debe
commit d462348556
7 changed files with 36 additions and 9 deletions

View File

@@ -141,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, 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).
`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 and effect-scoped). Listener calls contain synchronous throws and returned promise rejections independently; returned promises are not awaited and therefore do not delay task settlement. 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).

View File

@@ -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<TaskSnapshot>
onTaskDone(listener: (snapshot: TaskSnapshot, owner: Agent | undefined) => void): () => void // exact lifecycle owner; effect-scoped, contained
onTaskDone(listener: (snapshot: TaskSnapshot, owner: Agent | undefined) => void | PromiseLike<void>): () => void // exact owner; effect-scoped, contained
attachSurface(name: string): () => void // the misconfiguration fence, below
}
```
@@ -98,6 +98,8 @@ One system-prompt section (order 106, next to `tool:bash`) teaches the cross-cal
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 <id> (<kind>: <label>) finished [status: …]. Read its output with task_output.` through the exact owner captured at start, never a replacement found through a reused agent/session id, with the disposed-race contained. Notices are deduplicated the way Claude Code and Kimi Code both learned to: a task the model explicitly killed, or whose terminal state a read/wait already returned (including a wait pending at the moment of settlement), is marked `reported` and its notice suppressed — never a redundant "finished" for work the model just collected or ended. The model-visible ⟺ logged invariant holds with no new session event type.
Completion listeners are observation-only: each synchronous throw and returned promise rejection is logged independently, later listeners still run, and listener promises are not awaited before waiters or task teardown continue.
## Producer opt-in and schema exposure
Whether a producer tool offers `run_in_background` is that producer's own defaulted config: `enableRunInBackground?: boolean` on `dsh-tool-bash` and on each `dsh-tool-subagent` instance (both default `true` — bash keeps its always-exposed behavior, and a deployment disables either per instance from cordis.yml, no code edit). A bundle forwards the configs of the child plugins it owns: `dsh-agent-core` exposes `toolBash` for its built-in producer and `toolTasks` for the generic control surface, while independently composed producers such as subagent instances receive config directly. This forwarding is config reachability, not producer registration: future background-capable tools do not become `agent-core` fields unless that bundle also chooses to own them. A disabled producer omits the parameter from its schema entirely — and, because the arg validator deliberately allows undeclared keys, its `execute` ALSO refuses a forced `run_in_background: true` loud (the omission is advertising; the execution-time check is the enforcement). `ctx.tasks` plays no part in schema shaping — it never rewrites or decorates a producer's tool schema (Kimi Code regex-rewrites its bash description when background is disabled; config-owns-the-schema makes that trick unnecessary) — it only provides runtime registration. The two halves compose fail-loud: the producer's config decides what the model sees, and a background call that still reaches `start()` without a control surface throws the load-this-package error. `start()` preflights every failable check (the fence, validation, exact live owner instance, and owner-cleanup attach) BEFORE invoking the producer's `run()` and commits atomically after — background work started without a collectable id is structurally impossible, not a producer rollback obligation.

View File

@@ -939,7 +939,7 @@ export const TYPE_API: readonly TypeApiEntry[] = [
},
{
name: 'TaskDoneListener',
declaration: 'export type TaskDoneListener = (snapshot: TaskSnapshot, owner: Agent | undefined) => void;',
declaration: 'export type TaskDoneListener = (snapshot: TaskSnapshot, owner: Agent | undefined) => void | PromiseLike<void>;',
},
{
name: 'TaskHooks',

View File

@@ -9,7 +9,7 @@ The background task registry (`ctx.tasks`): a runtime-global, CONCRETE service (
- `read(id, caller?): TaskRead` — stream kinds consume the per-task cursor (v1's single intended reader is the owning model — a non-consuming multi-reader surface would be a cursor/snapshot API extension, not a `read` change); final kinds read the terminal output idempotently.
- `kill(id, caller?, reason?)``'requested'` (live task: producer `cancel` runs first — a throw fails the kill loud and leaves the task untouched — then `stopping`) or `'already-terminal'`. Every successful kill marks the task `reported` (the killer saw the end → completion notice suppressed).
- `wait(id, timeoutMs, caller?, signal?)` — resolves with the terminal snapshot (marked `reported`), or the live snapshot at timeout; an aborted signal rejects the WAIT only — unless the task already settled, in which case the wait still resolves and delivers the terminal snapshot (settlement suppressed the completion notice on this waiter's behalf, so rejecting would leave the finish both unreported and un-noticed). Timing is a [`dsh-timeout`](../../util/timeout/README.md) `deadline()` scoped to the `TASK_WAIT_TIMEOUT` code, so a nested foreign deadline never misreads as a wait timeout; timeout and abort detach their settlement resolver immediately, keeping retention bounded while the task remains live.
- `onTaskDone(listener)` — exactly once per terminal task record, with the snapshot plus its exact lifecycle owner (or `undefined`); effect-scoped, per-listener containment, silent after service disposal.
- `onTaskDone(listener)` — exactly once per terminal task record, with the snapshot plus its exact lifecycle owner (or `undefined`); effect-scoped, contains synchronous throws and returned promise rejections without awaiting listener work, silent after service disposal.
- `attachSurface(name)` — declares a control surface exists (the model tools, or a deployment's custom surface); effect-scoped.
Every read/kill/wait/get compares the task's owner session (`owner.session.header.id`) with the caller's and rejects a foreign one — ids are predictable (`bash-1`), so the fence, not id secrecy, is the isolation boundary.

View File

@@ -350,8 +350,9 @@ export class TaskService extends Service {
* Register a completion listener, called exactly once per terminal task
* record with its snapshot and exact lifecycle owner (or `undefined` for an
* unowned task). Effect-scoped (disposed with the calling fiber); per-listener
* containment (one throwing listener is logged, never starves the rest);
* never fires after this service is disposed.
* containment (one throwing or rejecting listener is logged, never starves
* the rest); returned promises are observed but not awaited; never fires
* after this service is disposed.
* @param listener - called with each terminal snapshot and its exact owner.
* @returns the disposer that unregisters the listener.
*/
@@ -440,7 +441,10 @@ export class TaskService extends Service {
const snapshot = this.snapshot(task)
for (const listener of this.listeners) {
try {
listener(snapshot, task.owner)
const returned = listener(snapshot, task.owner)
void Promise.resolve(returned).catch((error: unknown) => {
this.selfCtx.logger.warn(`tasks: onTaskDone listener rejected for ${task.id}: ${String(error)}`)
})
} catch (error: unknown) {
this.selfCtx.logger.warn(`tasks: onTaskDone listener threw for ${task.id}: ${String(error)}`)
}

View File

@@ -181,6 +181,10 @@ export interface TaskRead {
/**
* Completion callback registered via {@link TaskService.onTaskDone}.
* `owner` is the exact lifecycle instance supplied at start, not a registry
* lookup by reusable agent or session id; it is absent for unowned tasks.
* lookup by reusable agent or session id; it is absent for unowned tasks. A
* returned promise is observed for rejection but does not delay settlement.
*/
export type TaskDoneListener = (snapshot: TaskSnapshot, owner: Agent | undefined) => void
export type TaskDoneListener = (
snapshot: TaskSnapshot,
owner: Agent | undefined,
) => void | PromiseLike<void>

View File

@@ -155,6 +155,23 @@ describe('TaskService reads and settlement', () => {
expect(warn).toHaveBeenCalledWith(expect.stringContaining('listener boom'))
})
it('contains a rejecting onTaskDone listener without starving later listeners', async () => {
const ctx = await harness()
const warn = vi.spyOn(ctx.logger, 'warn').mockImplementation(() => {})
const seen: TaskId[] = []
ctx.tasks.onTaskDone(async () => { throw new Error('async listener boom') })
ctx.tasks.onTaskDone(snapshot => void seen.push(snapshot.id))
const p = producer()
const id = ctx.tasks.start(p.spec)
p.settle({ status: 'completed' })
await tick()
expect(seen).toEqual([id])
expect(warn).toHaveBeenCalledWith(expect.stringContaining('onTaskDone listener rejected'))
expect(warn).toHaveBeenCalledWith(expect.stringContaining('async listener boom'))
})
it('contains a rejecting done as a failed outcome (producer contract violation)', async () => {
const ctx = await harness()
const warn = vi.spyOn(ctx.logger, 'warn').mockImplementation(() => {})