Coder/deepseek-harness
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(tool-subagent): enforce depth only at runtime

Browse Source
This commit is contained in:
Tianyi Cui
2026-07-20 17:58:57 +08:00
parent c40e63d04b
commit 46d60d6d90
26 changed files with 192 additions and 302 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
packages/subagent/subagent-inprocess/tests/subagent-inprocess.spec.ts
View File

@@ -69,9 +69,8 @@ describe('startInProcessRun', () => {
})
it('counts a RESUMED child by its persisted header depth, not the absent runtime depth', async () => {
// The review-reproduced failure chain: a depth-1 child comes back from
// persistence with a fresh AgentOptions (no subagentDepth). Its header must
// stay authoritative, or maxDepth: 1 would let it delegate as top-level.
// Resume rebuilds runtime options, so the durable header must keep this
// depth-1 child from delegating as though it were top-level.
const { ctx } = await setup([textResponse('unused')])
const resumed = (await ctx.agents.create({
sessionId: SessionId('resumed-child'),

2
packages/subagent/tool-subagent/README.md
View File

@@ -22,7 +22,7 @@ With `run_in_background: true`, the tool registers the parent-owned task before
| `agentOptions` | Default child options, currently including `model`. |
| `persona` | Per-child persona; requires provider `persona` capability. |
| `toolFilter` | Per-child global-tool restriction; requires `toolFilter` capability. |
| `maxDepth` | Absolute delegation-depth cap, default `3` (`0` forbids delegation); a numeric cap requires the `depthLimit` capability and fails the mount without it. `'provider-managed'` sends no cap — for an out-of-process provider whose budget belongs to the child harness. A child AT the cap also loses this tool from its schema when the provider supports `toolFilter` (prompt-face hiding; the service still rejects on the execution face). |
| `maxDepth` | Absolute delegation-depth cap, default `3` (`0` forbids delegation); a numeric cap requires the `depthLimit` capability and fails the mount without it. `'provider-managed'` sends no cap for an out-of-process provider whose budget belongs to the child harness. The tool stays visible at the cap; each attempted start checks the calling agent's current depth and returns an errored tool result when rejected. |
## Concurrency

30
packages/subagent/tool-subagent/src/index.ts
View File

@@ -12,7 +12,7 @@ import z from 'schemastery'
import { defineTool } from '@deepseek-ai/dsh-tools'
import type { Agent, AgentOptions } from '@deepseek-ai/dsh-agent'
import type { ContentBlock } from '@deepseek-ai/dsh-llm'
import { assertSubagentMaxDepth, delegationDepthOf } from '@deepseek-ai/dsh-subagent'
import { assertSubagentMaxDepth } from '@deepseek-ai/dsh-subagent'
import type { SubagentProvider, SubagentResult, SubagentRun, SubagentStartRequest } from '@deepseek-ai/dsh-subagent'
import type { TaskOutcome } from '@deepseek-ai/dsh-tasks'
@@ -57,9 +57,8 @@ export interface Config {
* Maximum child depth: a non-negative safe integer (default `3`; `0` forbids
* delegation entirely), or `'provider-managed'` to send no cap. A numeric cap
* requires the provider's `depthLimit` capability (mount fails loud
* otherwise), and a child AT the cap additionally loses this tool from its
* schema when the provider supports `toolFilter` — the prompt face of the
* budget; the service keeps rejecting on the execution face.
* otherwise). The provider checks the calling agent's current depth at every
* start; the tool remains model-visible so runtime policy owns rejection.
* `'provider-managed'` is for an out-of-process provider (ACP) whose
* recursion budget belongs to the child harness's own deployment.
*/
@@ -199,28 +198,15 @@ function providerWording(inheritsConversation: boolean): { description: string;
}
}
function startRequest(
config: Config,
prompt: string,
parent: Agent,
signal: AbortSignal,
hideAtCapToolName: string | undefined,
): SubagentStartRequest {
function startRequest(config: Config, prompt: string, parent: Agent, signal: AbortSignal): SubagentStartRequest {
const maxDepth = typeof config.maxDepth === 'number' ? config.maxDepth : undefined
// A child AT the cap cannot delegate further: deny it this tool so its
// schema hides what the service would reject anyway (prompt face; the
// depth check at start remains the execution face).
const childAtCap = maxDepth !== undefined && delegationDepthOf(parent) + 1 >= maxDepth
const toolFilter = childAtCap && hideAtCapToolName !== undefined
? { ...config.toolFilter, deny: [...config.toolFilter?.deny ?? [], hideAtCapToolName] }
: config.toolFilter
return {
prompt: [{ type: 'text', text: prompt }],
parent,
signal,
...config.agentOptions !== undefined ? { agentOptions: config.agentOptions } : {},
...config.persona !== undefined ? { persona: config.persona } : {},
...toolFilter !== undefined ? { toolFilter } : {},
...config.toolFilter !== undefined ? { toolFilter: config.toolFilter } : {},
...maxDepth !== undefined ? { maxDepth } : {},
}
}
@@ -257,9 +243,6 @@ export function apply(ctx: Context, config: Config): void {
+ 'set maxDepth: \'provider-managed\' to leave the recursion budget to the provider',
)
}
// Schema hiding rides the child toolFilter, so it needs that capability;
// without it the depth check at start remains the only fence.
const hideAtCapToolName = provider.capabilities.toolFilter ? config.toolName ?? 'subagent' : undefined
const wording = providerWording(provider.inheritsParentContext)
const backgroundEnabled = config.enableRunInBackground !== false
disposeTool = ctx.tools.register(defineTool({
@@ -314,7 +297,7 @@ export function apply(ctx: Context, config: Config): void {
const controller = new AbortController()
const start = ctx.subagents.start(
config.provider,
startRequest(config, args.prompt, parent, controller.signal, hideAtCapToolName),
startRequest(config, args.prompt, parent, controller.signal),
)
return {
cancel: (reason?: string) => {
@@ -333,7 +316,6 @@ export function apply(ctx: Context, config: Config): void {
args.prompt,
parent,
exec.signal ?? new AbortController().signal,
hideAtCapToolName,
)
const run: SubagentRun = await ctx.subagents.start(config.provider, request)

45
packages/subagent/tool-subagent/tests/tool-subagent.spec.ts
View File

@@ -23,13 +23,9 @@ import { SessionId } from '@deepseek-ai/dsh-session'
* shipping code path.
*/
/** A minimal parent Agent: the tool reads `agent.id` plus the delegation depth off its header/options. */
function fakeAgent(id = 'parent-1', delegationDepth?: number): Agent {
return {
id: SessionId(id),
options: {},
session: { header: { ...delegationDepth === undefined ? {} : { delegationDepth } } },
} as unknown as Agent
/** A minimal parent Agent passed through to the provider request. */
function fakeAgent(id = 'parent-1'): Agent {
return { id: SessionId(id) } as unknown as Agent
}
async function setup(toolConfig: tool.Config, mockConfig: Partial<mock.Config> = {}) {
@@ -886,7 +882,7 @@ describe('background preflight failure (no orphaned child, by construction)', ()
})
})
describe('depth budget defaults and schema hiding', () => {
describe('depth budget configuration', () => {
/** Mount the tool over a request-capturing provider with full capabilities. */
async function captureSetup(config: Omit<tool.Config, 'provider'> = {}) {
const requests: SubagentStartRequest[] = []
@@ -916,37 +912,14 @@ describe('depth budget defaults and schema hiding', () => {
const { ctx, requests } = await captureSetup()
await callSubagent(ctx, { description: 'd', prompt: 'p' })
expect(requests[0]?.maxDepth).toBe(3)
expect(requests[0]?.toolFilter?.deny ?? []).not.toContain('subagent')
expect(requests[0]?.toolFilter).toBeUndefined()
})
it('denies its own toolName to a child at the depth cap', async () => {
// The child of a depth-0 parent under maxDepth 1 sits AT the cap: any
// delegation it attempted would be rejected, so the tool must not appear in
// its schema at all (prompt-face hiding; the service still rejects).
const { ctx, requests } = await captureSetup({ maxDepth: 1 })
it('forwards an explicit tool filter unchanged instead of encoding the depth policy into it', async () => {
const { ctx, requests } = await captureSetup({ toolFilter: { deny: ['dangerous'] }, maxDepth: 0 })
await callSubagent(ctx, { description: 'd', prompt: 'p' })
expect(requests[0]?.toolFilter?.deny).toContain('subagent')
})
it('merges the cap denial into a configured tool filter', async () => {
const { ctx, requests } = await captureSetup({ toolFilter: { deny: ['dangerous'] }, maxDepth: 1 })
await callSubagent(ctx, { description: 'd', prompt: 'p' })
expect(requests[0]?.toolFilter?.deny).toEqual(expect.arrayContaining(['dangerous', 'subagent']))
})
it('keeps the tool visible for a child below the cap', async () => {
const { ctx, requests } = await captureSetup({ maxDepth: 2 })
await callSubagent(ctx, { description: 'd', prompt: 'p' })
expect(requests[0]?.maxDepth).toBe(2)
expect(requests[0]?.toolFilter?.deny ?? []).not.toContain('subagent')
})
it('counts the parent by its persisted header depth when hiding', async () => {
// A resumed depth-1 parent under maxDepth 2: its child is AT the cap and
// must lose the tool even though the parent's runtime options carry no depth.
const { ctx, requests } = await captureSetup({ maxDepth: 2 })
await callSubagent(ctx, { description: 'd', prompt: 'p' }, { agent: fakeAgent('resumed-parent', 1) })
expect(requests[0]?.toolFilter?.deny).toContain('subagent')
expect(requests[0]?.maxDepth).toBe(0)
expect(requests[0]?.toolFilter).toEqual({ deny: ['dangerous'] })
})
it('rejects a numeric maxDepth on a provider without the depthLimit capability at mount', async () => {

57
packages/support/acp-snapshot/src/suite.ts
View File

@@ -100,18 +100,6 @@ export interface Scenario {
* {@link headerClass}.
*/
configPath?: string
/**
* Global tool names allowed to be ABSENT from a non-primary (child) session's
* request/header relative to the class pin — the delegation tool a child at
* its depth cap loses to tool-subagent's schema hiding. Each child header is
* compared against the pin minus exactly the declared names it actually
* omitted, so any other divergence (or an undeclared omission) still fails.
* A child that omitted a declared tool also skips the text-level initial
* system prompt pin: the prompt embeds the toolset (Code Mode SDK sections),
* so a reduced child cannot equal the full-composition expected output — the
* structural header assertion remains its pin. Meaningless on the primary log.
*/
childToolOmissions?: string[]
}
/** One suite's inputs: the agent to boot, where its fixtures live, and its scenario table. */
@@ -294,36 +282,6 @@ export function restorePinnedToolSchemas(header: unknown, schemas: readonly unkn
return { ...header, tools: schemas }
}
/**
* The pinned header with exactly the DECLARED omissions a child actually made
* removed from its tool list. A child at its depth cap legitimately lacks the
* delegation tool that spawned it (tool-subagent schema hiding); removing only
* declared-AND-actually-absent names keeps every other divergence — including
* an undeclared omission — a loud mismatch.
* @param pinned The class-pinned full header (tool schemas restored).
* @param actual The child session's normalized header under comparison.
* @param allowed The scenario's declared {@link Scenario.childToolOmissions}.
* @returns The expected header for this child log.
*/
export function applyChildToolOmissions(pinned: unknown, actual: unknown, allowed: readonly string[]): unknown {
if (pinned === null || typeof pinned !== 'object' || Array.isArray(pinned)) {
throw new Error('acp-snapshot: pinned request header must be an object')
}
const toolNames = (header: unknown): Set<string> => {
const tools = (header as { tools?: unknown }).tools
return new Set(Array.isArray(tools)
? tools.map(tool => (tool as { name?: unknown }).name).filter((name): name is string => typeof name === 'string')
: [])
}
const actualNames = toolNames(actual)
const pinnedTools = (pinned as { tools?: unknown[] }).tools ?? []
const tools = pinnedTools.filter((tool) => {
const name = (tool as { name?: unknown }).name
return !(typeof name === 'string' && allowed.includes(name) && !actualNames.has(name))
})
return { ...pinned, tools }
}
/**
* Render a normalized prompt as a repository-friendly Markdown snapshot.
* Prompt text is unchanged except that a missing terminal newline is added so
@@ -667,20 +625,9 @@ export function defineAcpSnapshotSuite(options: SnapshotSuiteOptions): void {
.toBe(headers.length)
for (const [k, header] of headers.entries()) {
const expected = expectedChanges > 0 ? pinnedHeaders[k] : pinnedHeaders[0]
// A child (non-primary) log may omit declared delegation tools —
// schema hiding at the depth cap; see Scenario.childToolOmissions.
const childOmissions = logIndex === 0 ? [] : scenario.childToolOmissions ?? []
const target = childOmissions.length === 0
? expected
: applyChildToolOmissions(expected, header, childOmissions)
expect(header, `session ${log.id}: request/header #${k + 1} diverged from the pinned (${pinningScenario.name}) header`)
.toEqual(target)
// A child that omitted a declared tool cannot equal the text-level
// prompt pin (the prompt embeds the toolset); its header assertion
// above remains the structural pin.
const omittedDeclaredTool = target !== expected
&& (target as { tools?: unknown[] }).tools?.length !== (expected as { tools?: unknown[] }).tools?.length
if (expectedChanges === 0 && !omittedDeclaredTool) {
.toEqual(expected)
if (expectedChanges === 0) {
expect(formatSystemPromptSnapshot(prompts[k] as string), `session ${log.id}: initial system prompt #${k + 1} diverged from ${pinningScenario.name}/${SYSTEM_PROMPT_SNAPSHOT}`)
.toEqual(initialPromptSnapshot)
}

116
packages/support/acp-snapshot/tests/fixtures/suite/child-omission/behavior.json vendored
View File

@@ -1,116 +0,0 @@
{
"prompt": "respond",
"echoWorkspace": true,
"logs": [
{
"file": "b/parent.jsonl",
"lines": [
{
"type": "session",
"id": "{{SID}}",
"createdAt": 200,
"cwd": "{{CWD}}"
},
{
"type": "request/header",
"seq": 0,
"time": 5,
"data": {
"header": {
"config": {
"model": "fake"
},
"system": "SYS PROMPT",
"tools": [
{
"name": "t1",
"description": "D1",
"parameters": {
"type": "object"
}
}
]
},
"reason": "initial"
}
},
{
"type": "assistant/chunk",
"seq": 1,
"time": 5,
"data": {
"turn": 1,
"step": 1,
"chunk": {
"type": "text-delta",
"index": 0,
"text": "hi"
}
}
}
]
},
{
"file": "b/child1.jsonl",
"lines": [
{
"type": "session",
"id": "eeeeeeee-1111-4222-8333-444444444444",
"createdAt": 300,
"cwd": "{{CWD}}",
"parentSession": "{{SID}}"
},
{
"type": "request/header",
"seq": 0,
"time": 6,
"data": {
"header": {
"config": {
"model": "fake"
},
"system": "SYS PROMPT",
"tools": []
},
"reason": "initial"
}
}
]
},
{
"file": "b/child2.jsonl",
"lines": [
{
"type": "session",
"id": "ffffffff-2222-4333-8444-555555555555",
"createdAt": 400,
"cwd": "{{CWD}}",
"parentSession": "{{SID}}"
},
{
"type": "request/header",
"seq": 0,
"time": 6,
"data": {
"header": {
"config": {
"model": "fake"
},
"system": "SYS PROMPT",
"tools": [
{
"name": "t1",
"description": "D1",
"parameters": {
"type": "object"
}
}
]
},
"reason": "initial"
}
}
]
}
]
}

1
packages/support/acp-snapshot/tests/fixtures/suite/child-omission/input.json vendored
View File

@@ -1 +0,0 @@
{ "steps": [{ "op": "initialize" }, { "op": "newSession" }, { "op": "prompt", "text": "plain" }] }

2
packages/support/acp-snapshot/tests/fixtures/suite/child-omission/session.1.jsonl vendored
View File

@@ -1,2 +0,0 @@
{"type":"session","id":"eeeeeeee-1111-4222-8333-444444444444","createdAt":12,"cwd":"/rec/plain-cwd","parentSession":"56565656-7878-4989-8a9a-9b9b9b9b9b9b"}
{"type":"request/header","seq":0,"time":12,"data":{"header":{"config":{"model":"fake"},"system":"{{system}}","tools":"{{tools}}"},"reason":"initial"}}

2
packages/support/acp-snapshot/tests/fixtures/suite/child-omission/session.2.jsonl vendored
View File

@@ -1,2 +0,0 @@
{"type":"session","id":"ffffffff-2222-4333-8444-555555555555","createdAt":13,"cwd":"/rec/plain-cwd","parentSession":"56565656-7878-4989-8a9a-9b9b9b9b9b9b"}
{"type":"request/header","seq":0,"time":12,"data":{"header":{"config":{"model":"fake"},"system":"{{system}}","tools":"{{tools}}"},"reason":"initial"}}

3
packages/support/acp-snapshot/tests/fixtures/suite/child-omission/session.jsonl vendored
View File

@@ -1,3 +0,0 @@
{"type":"session","id":"56565656-7878-4989-8a9a-9b9b9b9b9b9b","createdAt":11,"cwd":"/rec/plain-cwd"}
{"type":"request/header","seq":0,"time":11,"data":{"header":{"config":{"model":"fake"},"system":"{{system}}","tools":"{{tools}}"},"reason":"initial"}}
{"type":"assistant/chunk","seq":1,"time":11,"data":{"turn":1,"step":1,"chunk":{"type":"text-delta","index":0,"text":"hi"}}}

5
packages/support/acp-snapshot/tests/fixtures/suite/child-omission/stdout.expected.jsonl vendored
View File

@@ -1,5 +0,0 @@
{"jsonrpc":"2.0","id":1,"result":{"protocolVersion":1,"agentCapabilities":{"loadSession":false}}}
{"jsonrpc":"2.0","id":2,"result":{"sessionId":"{{sessionId}}"}}
{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_message_chunk","content":{"type":"text","text":"thinking about it"}}}}
{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_message_chunk","content":{"type":"text","text":"workspace:seed.txt"}}}}
{"jsonrpc":"2.0","id":3,"result":{"stopReason":"end_turn"}}

1
packages/support/acp-snapshot/tests/fixtures/suite/child-omission/workspace/seed.txt vendored
View File

@@ -1 +0,0 @@
seeded

36
packages/support/acp-snapshot/tests/suite.spec.ts
View File

@@ -16,7 +16,6 @@ import {
parseToolSchemasSnapshot,
refreshFixtureReplacements,
sessionFixtureNames,
applyChildToolOmissions,
restorePinnedToolSchemas,
stabilizeRefreshLog,
unknownToolCallIds,
@@ -48,10 +47,6 @@ const RECORD_SRC = fileURLToPath(new URL('./fixtures/record-suite', import.meta.
const REPLAY_SCENARIOS: Scenario[] = [
{ name: 'pin-turn', hasModelTurn: true, recorded: true, pinsHeader: true, expectedHeaderChanges: 1, headerClass: 'main' },
{ name: 'plain-turn', hasModelTurn: true, recorded: true, headerClass: 'main', configPath: AGENT.configPath },
// Two scripted children under a declared omission: one omits t1 (header pin
// minus the declared tool, prompt pin skipped), one keeps the full set (pin
// and prompt compared verbatim) — the childToolOmissions branches.
{ name: 'child-omission', hasModelTurn: true, recorded: false, headerClass: 'main', childToolOmissions: ['t1'] },
{ name: 'no-model', hasModelTurn: false, recorded: false, headerClass: 'main' },
{ name: 'blocked-log', hasModelTurn: false, comparesLog: true, recorded: false, headerClass: 'main' },
{ name: 'authored-error', hasModelTurn: true, recorded: false, overridden: true, headerClass: 'main' },
@@ -371,37 +366,6 @@ describe('tool-schema snapshots', () => {
})
})
describe('applyChildToolOmissions', () => {
const pinned = { system: 's', tools: [{ name: 'bash' }, { name: 'subagent' }, { name: 'subagent_fork' }] }
it('removes exactly the declared tools the child actually omitted', () => {
const actual = { system: 's', tools: [{ name: 'bash' }, { name: 'subagent_fork' }] }
expect(applyChildToolOmissions(pinned, actual, ['subagent', 'subagent_fork']))
.toEqual({ system: 's', tools: [{ name: 'bash' }, { name: 'subagent_fork' }] })
})
it('keeps a declared tool the child still carries and an undeclared omission', () => {
// The child omitted `bash` (undeclared) — the expectation keeps it, so the
// equality assertion downstream still fails loudly on the real divergence.
const actual = { system: 's', tools: [{ name: 'subagent' }, { name: 'subagent_fork' }] }
expect(applyChildToolOmissions(pinned, actual, ['subagent']))
.toEqual(pinned)
})
it('tolerates a headerless tool list and unnamed tool entries', () => {
expect(applyChildToolOmissions({ system: 's' }, { tools: 'not-an-array' }, ['subagent']))
.toEqual({ system: 's', tools: [] })
const unnamed = { system: 's', tools: [{ name: 42 }] }
expect(applyChildToolOmissions(unnamed, { tools: [] }, ['subagent'])).toEqual(unnamed)
})
it('rejects a non-object pinned header', () => {
expect(() => applyChildToolOmissions(null, {}, [])).toThrow(/must be an object/)
expect(() => applyChildToolOmissions([], {}, [])).toThrow(/must be an object/)
expect(() => applyChildToolOmissions('x', {}, [])).toThrow(/must be an object/)
})
})
describe('unknownToolCallIds', () => {
it('returns structured UNKNOWN_TOOL call ids and ignores other results', () => {
const log = [