fix(tools): enforce a disabled run_in_background at execution time (review findings)

enableRunInBackground: false removed the parameter from the advertised
schema only — the arg validator deliberately allows undeclared keys, so
a caller (or a model that has seen the parameter elsewhere) could still
force run_in_background: true and start background work past the
deployment's opt-out, in both tool-bash and tool-subagent. Both
producers now refuse the forced key loud in execute(); tests pin the
refusal (and that nothing spawns) alongside the untouched foreground
path; the schema-omission-is-advertising rule is recorded in the
runtime RFC and both READMEs.
This commit is contained in:
Yichen Jiang
2026-07-09 23:37:51 +08:00
parent 3ad561862f
commit 35acd34fd9
7 changed files with 41 additions and 3 deletions

View File

@@ -14,7 +14,7 @@ The tool description and the `prompt` parameter description are DERIVED from the
|---|---|
| `provider` (required) | The `ctx.subagents` provider name to start runs on (`spawn`, `fork`, `acp`, …). |
| `toolName` | The model-facing tool name to register (default `subagent`). Set a distinct value per load when exposing multiple providers, e.g. `subagent` + `subagent_acp`. |
| `enableRunInBackground` | Expose `run_in_background` in this instance's schema (default `true`). Disabled, the parameter is absent entirely — delegation through this instance stays strictly synchronous. |
| `enableRunInBackground` | Expose `run_in_background` in this instance's schema (default `true`). Disabled, the parameter is absent entirely AND a caller that forces the key anyway is refused at execution time (the arg validator allows undeclared keys) — delegation through this instance stays strictly synchronous. |
| `agentOptions` | Default per-child `{ model? }` applied to every spawned child. (No per-child persona: the deployment persona is a context-wide section every agent shares.) |
## Foreground lifecycle (synchronous collect)

View File

@@ -253,6 +253,13 @@ export function apply(ctx: Context, config: Config): void {
}
if (args.run_in_background === true) {
// The schema omission is advertising, not enforcement — the arg
// validator deliberately allows undeclared keys, so a caller (or a
// model that has seen the parameter elsewhere) can still send it.
// A disabled instance must refuse at execution time, loud.
if (!backgroundEnabled) {
throw new Error('run_in_background is disabled for this tool instance (enableRunInBackground: false)')
}
// The generic runtime owns everything task-shaped; without it a task
// id would be uncollectable — fail loud with the fix, not a dangle.
const tasks = ctx.get('tasks')

View File

@@ -81,6 +81,21 @@ describe('dsh-tool-subagent', () => {
expect(schema!.description).not.toContain('task_output')
})
it('refuses a forced run_in_background at execution time when the instance disables it (review finding)', async () => {
// Schema omission is advertising, not enforcement: the arg validator
// allows undeclared keys, so the opt-out must also hold in execute().
const ctx = await setup({ provider: 'mock', enableRunInBackground: false })
const parent = { id: AgentId('agent-sess-off'), inject: () => {}, session: { header: { version: 0, id: 'sess-off', createdAt: 0 } } } as unknown as Agent
const forced = await callSubagent(ctx, { description: 'd', prompt: 'p', run_in_background: true }, { agent: parent })
expect(forced.isError).toBe(true)
expect(text(forced)).toContain('run_in_background is disabled for this tool instance')
// The provider was never asked to start a child.
expect(ctx.subagents.getProvider('mock')).toBeDefined()
const foreground = await callSubagent(ctx, { description: 'd', prompt: 'p' }, { agent: parent })
expect(foreground.isError).toBe(false)
})
it.each([
{ stopReason: 'aborted' as const, fragment: 'cancelled' },
{ stopReason: 'error' as const, fragment: 'failed' },