fix(tasks): claim the teardown report before the producer cancel runs

A throwing producer cancel jumped to the force-fail branch before
`reported` was set, so `settle()` announced an unreported completion and
the default wakeup delivery started a model turn on an owner the host
was already destroying — the exact failure mode marking the record
reported exists to prevent.

Teardown claims the report before calling the producer, because that
decision does not depend on whether the producer's cancel succeeds.

Also reject a `maxConsecutiveWakes` that cannot bound anything: the
field exists to cap a runaway chain, and `Infinity` removed the cap
while a fraction never named a turn.

Correct the module JSDoc and the background-task runtime note, both of
which still promised that notices never wake an idle agent.
This commit is contained in:
Yichen Jiang
2026-08-11 20:29:08 +08:00
parent 4675914d74
commit 75b26988dc
12 changed files with 81 additions and 19 deletions

View File

@@ -466,14 +466,17 @@ export class LocalTaskService extends TaskService {
private cancelForTeardown(tasks: TrackedTask[], reason: string): void {
for (const task of tasks) {
if (isTerminal(task.status)) continue
// Teardown cancellation is a kill without a caller, so it claims the
// terminal report the same way `kill()` does. Nothing will read a notice
// for a task whose owner or service is being destroyed, and a waking
// reporter would spend a model request per teardown layer. This is
// decided before the producer runs: the force-failure below settles the
// record too, so a throwing cancel must not be the one path that
// announces an unreported completion into a disposing owner.
task.reported = true
try {
task.cancel(reason)
task.status = 'stopping'
// Teardown cancellation is a kill without a caller, so it claims the
// terminal report the same way `kill()` does. Nothing will read a
// notice for a task whose owner or service is being destroyed, and a
// waking reporter would spend a model request per teardown layer.
task.reported = true
// Teardown reaches settlement only after the producer releases, which a
// slow stop can defer; announcing the transition here is what keeps an
// observer from showing `running` for that whole window.

View File

@@ -1,8 +1,9 @@
/**
* Model-facing `task_output`, `task_list`, and `task_kill` tools over
* `ctx.tasks`. Loading the plugin attaches the controller required by
* producers. It also injects unreported completions as durable context for the
* owner's next request; notices do not wake idle agents.
* producers. It also delivers unreported completions to the owning agent:
* injected into a busy owner's next step, or opening a turn on an idle one
* under the default `wakeup` delivery, bounded per owner.
* @module @deepseek-ai/dsh-tool-tasks
*/
@@ -214,6 +215,11 @@ export function apply(ctx: Context, config: Config): void {
if (waitDefault > waitCap) {
throw new Error(`tool-tasks: waitTimeoutMs (${waitDefault}) exceeds maxWaitTimeoutMs (${waitCap})`)
}
// A budget is a count of turns. `Infinity` would leave the runaway chain this
// field exists to bound unbounded, and a fraction never names a turn at all.
if (!Number.isSafeInteger(wakeBudget)) {
throw new Error(`tool-tasks: maxConsecutiveWakes (${wakeBudget}) must be a whole number of turns`)
}
ctx.on('agent/inbox/claimed', ({ agent, message }) => {
// Claiming is the point the human's input actually enters a step; a notice
// this plugin itself queued must not refill the budget it just spent.

View File

@@ -128,6 +128,28 @@ describe('tool-tasks setup', () => {
.rejects.toThrow('waitTimeoutMs (100) exceeds maxWaitTimeoutMs (50)')
})
it('rejects a wake budget that cannot bound anything', async () => {
// Reports the load outcome as text: a resolved fiber is not safely printable.
const loadWith = async (maxConsecutiveWakes: number): Promise<string> => {
const ctx = new Context()
await ctx.plugin(SystemPrompt)
await ctx.plugin(ToolRegistry)
await ctx.plugin(LocalTaskService)
try {
await ctx.plugin(ToolTasks, { maxConsecutiveWakes })
return 'loaded'
} catch (error: unknown) {
return String(error)
}
}
// The field exists to bound runaway waking; a fractional budget counts
// nothing and an infinite one removes the bound it was configured for.
expect(await loadWith(Number.POSITIVE_INFINITY)).toContain('maxConsecutiveWakes')
expect(await loadWith(2.5)).toContain('maxConsecutiveWakes')
expect(await loadWith(1)).toBe('loaded')
})
it('renders status lines with and without producer detail', () => {
const base = { id: 'bash-1', kind: 'bash', label: 'x', startedAt: 0, reported: false } as unknown as TaskSnapshot
expect(statusLine({ ...base, status: 'running' })).toBe('[status: running]')
@@ -607,6 +629,33 @@ describe('completion notice delivery', () => {
expect(inject).not.toHaveBeenCalled()
})
it('neither wakes nor injects when the teardown cancel itself threw', async () => {
const { ctx } = await setup()
const warn = vi.spyOn(ctx.logger, 'warn').mockImplementation(() => {})
const inject = vi.fn()
const followup = vi.fn()
const owner = fakeAgent(ctx, 'sess-1', { inject, followup, status: 'idle' })
ctx.tasks.start({
kind: 'bash',
label: 'broken producer',
owner,
run: () => ({
cancel() { throw new Error('cancel boom') },
done: new Promise<TaskOutcome>(() => {}),
}),
})
// The registry force-fails the record instead of deadlocking. That path
// settles the task too, so it must claim the report as the ordinary
// teardown cancel does — otherwise a throwing producer is all it takes to
// spend a model request on an owner being destroyed.
await disposeAgentScope(owner)
await tick()
expect(warn).toHaveBeenCalledWith(expect.stringContaining('work may be orphaned'))
expect(followup).not.toHaveBeenCalled()
expect(inject).not.toHaveBeenCalled()
})
it('keeps the budget spent when the owner only claims plugin notices', async () => {
const { ctx } = await setup({ maxConsecutiveWakes: 1 })
const followup = vi.fn()