fix: address codex review round 2

- Wire the control service and send_message tool into every shipped
  composition with a resumable provider and background enabled
  (headless-agent, tui-agent, and the SDK helper's subagent feature
  base resources); jsonrpc-agent disables background and is unchanged.
- Resolve the send_message availability check in the CALLER's tool
  scope so a restriction that removes the follow-up tool from one
  agent also blocks that agent's continuable start.
- Control-service disposal now cancels live activations and awaits
  producer settlement instead of stranding them: TaskService keeps
  producer Tasks across a reload, so the disposing service aborts each
  activation-owned controller, resolves its terminal gate (the
  effect-scoped onTaskDone listener is already gone), and awaits done.
  A new test kills a mid-start activation through HMR disposal.
This commit is contained in:
Dudu-0223
2026-07-23 18:14:37 +08:00
committed by imccyu
parent 71570d7bec
commit 4eda48d002
12 changed files with 136 additions and 9 deletions

View File

@@ -209,7 +209,13 @@ config:
id: 'subagent',
summary: 'Delegate work to child agents',
mode: 'multiple',
baseResources: [{ kind: 'npm-cordis-config-entry', id: 'subagent', package: '@deepseek-ai/dsh-subagent' }],
// The control pair rides every resumable in-process option: background
// delegation on spawn/fork is continuable and advertises send_message.
baseResources: [
{ kind: 'npm-cordis-config-entry', id: 'subagent', package: '@deepseek-ai/dsh-subagent' },
{ kind: 'npm-cordis-config-entry', id: 'subagent-control', package: '@deepseek-ai/dsh-subagent-control' },
{ kind: 'npm-cordis-config-entry', id: 'tool-subagent-control', package: '@deepseek-ai/dsh-tool-subagent-control' },
],
options: [
{
id: 'spawn',

View File

@@ -86,6 +86,10 @@ interface ActiveActivation {
taskId: TaskId | undefined
/** Filled when the provider publishes; `undefined` while starting or resuming. */
run: SubagentRun | undefined
/** The activation-owned cancellation authority, created before any await. */
readonly controller: AbortController
/** The producer's settlement (run disposed, outcome produced); assigned when the Task registers. */
done: Promise<TaskOutcome> | undefined
/** Resolved by the completion listener when the Task's terminal snapshot is recorded. */
readonly terminal: PromiseWithResolvers<void>
}
@@ -164,7 +168,21 @@ export class SubagentControlService extends Service {
if (activation.taskId === snapshot.id) activation.terminal.resolve()
}
})
ctx.effect(() => () => { this.activations.clear() }, 'subagentControl.activations()')
// TaskService deliberately keeps producer Tasks alive across a
// control-surface or producer reload, so this service's disposal must not
// strand the activations it can no longer route to: cancel each one and
// await producer settlement (run disposal) before releasing the map. The
// effect-scoped onTaskDone listener above is already gone by then, so
// terminal publication is resolved here instead of waiting forever.
ctx.effect(() => async () => {
const active = [...this.activations.values()]
this.activations.clear()
for (const activation of active) {
activation.controller.abort('subagent control service disposed')
activation.terminal.resolve()
}
await Promise.allSettled(active.map(activation => activation.done ?? Promise.resolve()))
}, 'subagentControl.activations()')
}
/**
@@ -368,6 +386,8 @@ export class SubagentControlService extends Service {
const activation: ActiveActivation = {
taskId: undefined,
run: undefined,
controller: new AbortController(),
done: undefined,
terminal: Promise.withResolvers<void>(),
}
this.activations.set(childId, activation)
@@ -378,21 +398,21 @@ export class SubagentControlService extends Service {
label,
owner,
run: (): TaskHooks => {
const controller = new AbortController()
const done = (async (): Promise<TaskOutcome> => {
try {
const run = await begin(controller.signal)
const run = await begin(activation.controller.signal)
activation.run = run
return await settleRun(run)
} catch (error: unknown) {
// A pre-publication abort rejects only after the provider's
// creation transaction rolled back to quiescence, so recording
// `killed` here honors the settlement-after-rollback contract.
return controller.signal.aborted
return activation.controller.signal.aborted
? { status: 'killed' }
: { status: 'failed', detail: String(error) }
}
})()
activation.done = done
void Promise.allSettled([done, activation.terminal.promise]).then(() => {
/* v8 ignore else -- service teardown clears the map while a producer is still settling. */
if (this.activations.get(childId) === activation) this.activations.delete(childId)
@@ -401,7 +421,7 @@ export class SubagentControlService extends Service {
cancel: (reason?: string) => {
// Cancellation targets the whole activation: every message that
// joined this turn shares the `killed` outcome.
controller.abort(reason ?? 'subagent activation killed')
activation.controller.abort(reason ?? 'subagent activation killed')
},
done,
// No readOutput: the child session owns intermediate detail.

View File

@@ -482,6 +482,51 @@ describe('SubagentControlService.sendMessage', () => {
})
})
describe('service disposal with live activations', () => {
it('cancels and settles a starting activation on service disposal instead of stranding it', async () => {
const ctx = new Context()
await mountAgentLoopTestDependencies(ctx)
const root = mkdtempSync(join(tmpdir(), 'dsh-subagent-control-hmr-'))
roots.push(root)
await ctx.plugin(JsonlSessionPersistence, { root })
await ctx.plugin(AgentLoop, { agents: [] })
await ctx.plugin(SubagentService)
await ctx.plugin(TaskService)
await ctx.plugin(ToolTasks, {})
// A provider that stays pending until its signal aborts, so the activation
// is observably mid-start when the control service is disposed.
let sawAbort = false
ctx.subagents.registerProvider({
name: 'pending',
capabilities: { outputSchema: false, depthLimit: false, toolFilter: false, persona: false },
inheritsParentContext: false,
start: request => new Promise((_resolve, reject) => {
request.signal.addEventListener('abort', () => {
sawAbort = true
reject(new Error('startup aborted'))
}, { once: true })
}),
resume: () => Promise.reject(new Error('unreachable')),
})
const controlFiber = await ctx.plugin(SubagentControlService)
ctx.llm.registerAdapter(['mock'], new MockAdapter([]))
const parent = ctx.agentLoop.create(SessionId('parent'), { provider: 'mock', model: 'mock' })
const control = ctx.get('subagentControl')!
const started = control.startContinuable({
provider: 'pending',
label: 'will be interrupted',
request: { prompt: message('go'), parent },
})
// TaskService keeps the producer Task; the disposing control service must
// cancel its activation and await settlement rather than strand it.
await controlFiber.dispose()
expect(sawAbort).toBe(true)
const snapshot = await waitTerminal(ctx, started.taskId, parent)
expect(snapshot.status).toBe('killed')
})
})
describe('outcome mapping helpers', () => {
it('runOutcome maps the stop-reason vocabulary onto task outcomes', () => {
const output = [{ type: 'text' as const, text: 'partial' }]

View File

@@ -288,8 +288,10 @@ export function apply(ctx: Context, config: Config): void {
// The schema above tells the model to follow up with
// `send_message`; starting a durable child the model cannot
// continue would make that advertisement false. Sibling load order
// is undetermined at mount, so the check lives at the operation.
if (ctx.tools.get('send_message') === undefined) {
// is undetermined at mount, so the check lives at the operation,
// and it resolves in the CALLER's scope so a restriction that
// removes send_message from this agent also blocks the start.
if (ctx.tools.get('send_message', parent) === undefined) {
throw new Error('continuable background subagents unavailable: load @deepseek-ai/dsh-tool-subagent-control (the advertised send_message tool is not registered)')
}
// The control service owns the durable child id, descriptor

View File

@@ -903,6 +903,22 @@ describe('dsh-tool-subagent continuable background mode', () => {
// Nothing was started: no Task exists for the parent.
expect(ctx.tasks.list(parent)).toEqual([])
})
it('resolves send_message availability in the CALLER scope, not the global registry', async () => {
// A scoped restriction that keeps this delegation tool but removes
// send_message means this agent cannot execute the promised follow-up;
// the availability check must see the caller's surface.
const { ctx, parent } = await continuableSetup()
parent.ctx.tools.restrict({ deny: ['send_message'] })
const result = await callSubagent(
ctx,
{ description: 'd', prompt: 'p', run_in_background: true },
{ agent: parent },
)
expect(result.isError).toBe(true)
expect(text(result)).toContain('load @deepseek-ai/dsh-tool-subagent-control')
expect(ctx.tasks.list(parent)).toEqual([])
})
})
describe('background preflight failure (no orphaned child, by construction)', () => {