refactor(tasks): declare-then-execute — ctx.tasks.start() replaces register()

start({ kind, label, owner, run }) preflights everything that can fail
(the attachSurface fence, validation, the owner-cleanup attach) BEFORE
invoking the producer's run() starter, then commits atomically —
'work started but never got a collectable id' is now structurally
impossible instead of a producer try/catch rollback obligation (the
P1 review fix, rebuilt on #185's declare/execute split). Producers
lose their catch-wraps; the leak tests now pin the stronger property
that a failed preflight never spawns anything. TaskRegistration splits
into TaskStart (identity + run) and TaskHooks (cancel/done/readOutput);
docs, type-equiv manifest, catalogs, and both RFCs move with it.
This commit is contained in:
Yichen Jiang
2026-07-09 21:53:48 +08:00
parent f858c647a6
commit bd59fddacd
22 changed files with 281 additions and 240 deletions

View File

@@ -361,25 +361,22 @@ export function apply(ctx: Context, config: Config): void {
// (cancellation belongs to task_kill / owner cleanup), so the check
// happens here, once, instead of passing the signal to start().
if (exec.signal?.aborted) throw new Error('command aborted')
const proc = ctx.bash.start(ctx.bash.resolve(request))
let id: string
try {
id = tasks.register({
kind: 'bash',
label: args.command,
...exec.agent ? { owner: exec.agent } : {},
cancel: () => void proc.kill(),
done: proc.done.then(() => processOutcome(proc)),
readOutput: () => renderProcessRead(proc.readOutput()),
})
} catch (error: unknown) {
// A failed registration must not leak the just-started process: the
// model never received an id, so nothing could ever task_kill it.
// Kill, await quiescence, then fail the call with the real cause.
proc.kill()
await proc.done
throw error
}
// tasks.start preflights (surface fence, owner cleanup) BEFORE run()
// spawns anything, and cannot fail after — the process can never
// start without a collectable id.
const id = tasks.start({
kind: 'bash',
label: args.command,
...exec.agent ? { owner: exec.agent } : {},
run: () => {
const proc = ctx.bash.start(ctx.bash.resolve(request))
return {
cancel: () => void proc.kill(),
done: proc.done.then(() => processOutcome(proc)),
readOutput: () => renderProcessRead(proc.readOutput()),
}
},
})
return [{ type: 'text', text: `started background task ${id}` }]
}
const result = await ctx.bash.run(ctx.bash.resolve({

View File

@@ -354,36 +354,30 @@ describe('background execution through the task runtime', () => {
expect((ctx.bash as CountingStartExecutor).starts).toBe(0)
})
it('a failed registration kills the just-started process (no orphan without an id)', async () => {
it('never spawns the process when tasks.start preflight throws (no orphan, by construction)', async () => {
class LeakProbeExecutor extends BashExecutor {
kills = 0
starts = 0
resolve(request: BashExecRequest): BashExecSpec {
return { command: request.command, workdir: request.workdir ?? '/x', timeoutMs: request.timeoutMs ?? 0 }
}
run(): Promise<BashRunResult> { return Promise.reject(new Error('unused')) }
start(spec: BashExecSpec): BashProcess {
let close!: () => void
const done = new Promise<void>((res) => { close = res })
const proc: BashProcess = {
this.starts += 1
return {
command: spec.command,
status: 'running',
exitCode: null,
status: 'completed',
exitCode: 0,
signal: null,
done,
done: Promise.resolve(),
readOutput: () => ({ delta: '', lossy: false }),
kill: () => {
this.kills += 1
proc.status = 'killed'
close()
return true
},
kill: () => false,
}
return proc
}
}
// TaskService WITHOUT any control surface: register() throws AFTER the
// process already started — the producer must kill and await it.
// TaskService WITHOUT any control surface: tasks.start preflights that
// fence BEFORE invoking the producer's run(), so the executor is never
// asked to spawn — there is no orphan to roll back.
const ctx = new Context()
await ctx.plugin(SystemPrompt)
await ctx.plugin(ToolRegistry)
@@ -395,8 +389,8 @@ describe('background execution through the task runtime', () => {
const result = await call(ctx, 'bash', { command: 'sleep 60', description: 'test command', run_in_background: true })
expect(result.isError).toBe(true)
expect(text(result)).toContain('no control surface is attached')
// The call resolved only after the kill landed (the catch awaits done).
expect((ctx.bash as LeakProbeExecutor).kills).toBe(1)
// Declare-then-execute: the failed preflight means no process ever ran.
expect((ctx.bash as LeakProbeExecutor).starts).toBe(0)
})
it('enableRunInBackground: false removes the parameter and flips the description', async () => {