diff --git a/docs/rfc/proposed/feature/2026-07-07-claude-code-and-codex-subagent-backends.md b/docs/rfc/proposed/feature/2026-07-07-claude-code-and-codex-subagent-backends.md index 911d28cf1f..c05ae8d67f 100644 --- a/docs/rfc/proposed/feature/2026-07-07-claude-code-and-codex-subagent-backends.md +++ b/docs/rfc/proposed/feature/2026-07-07-claude-code-and-codex-subagent-backends.md @@ -12,7 +12,7 @@ Two sibling provider packages, structural variants of the ACP backend, plus one - `@deepseek-ai/dsh-subagent-claude-code` — drives a Claude Code child through `@anthropic-ai/claude-agent-sdk`'s `query()` (the SDK runs in the parent process and spawns its bundled `claude` CLI as the subprocess). Provider name `claude-code`: the child is the Claude Code *product*, not an Anthropic model adapter — "claude" stays reserved for a future `dsh-llm` adapter. - `@deepseek-ai/dsh-subagent-codex` — spawns `codex app-server` and drives one thread/turn over its JSON-RPC-over-stdio protocol with a hand-rolled newline-JSON client (~200–300 lines) in the package. -- `@deepseek-ai/dsh-subagent-process` — a pure library (the `subagent-inprocess` precedent) extracting what `dsh-subagent-acp` already carries and both new backends need: the credential env scrub (`SENSITIVE_ENV_PATTERN`/`buildChildEnv`), the EOF → SIGTERM → SIGKILL dispose ladder, and new isolated-config-dir helpers (`mkdtemp` create, best-effort remove). The ACP backend migrates onto it; `bash-local`'s sibling copy is left alone to bound the change. +- `@deepseek-ai/dsh-subagent-process` — a pure library (the `subagent-inprocess` precedent) extracting what `dsh-subagent-acp` already carries and both new backends need: the credential env scrub (`buildChildEnv`), the EOF → SIGTERM → SIGKILL dispose ladder, and new isolated-config-dir helpers (`mkdtemp` create, best-effort remove). The ACP backend migrates onto it; `bash-local`'s sibling copy is left alone to bound the change. Both providers copy the ACP backend's seam posture verbatim: fresh child per `start`, exactly one prompt round-trip, capabilities all `false`, `inheritsParentContext: false`, `request.parent`/`request.agentOptions` ignored, `id = SessionId(randomUUID())`, `result` never rejects — child-level failure flattens to a stop reason and the original error goes to `ctx.logger` via an `onError` spec callback. Model exposure is zero new code: `dsh-tool-subagent` is loaded once per provider with a distinct `toolName` (`subagent_claude_code`, `subagent_codex`). No new session events are needed — the only model-visible artifact is the tool result, so reconstructability holds exactly as it did for ACP. To be explicit about the boundary: the session log reconstructs the model-visible transcript, not workspace mutation history — a child granted write access mutates files as an ambient side effect outside the log, exactly as the bash tools and the ACP backend already do; replay reproduces requests, not the disk. diff --git a/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts b/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts index 115d5c3e97..cc4d8307f8 100644 --- a/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts +++ b/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts @@ -6,7 +6,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { fileURLToPath } from 'node:url' import SubagentService from '@deepseek-ai/dsh-subagent' -import { buildChildEnv, SENSITIVE_ENV_PATTERN } from '@deepseek-ai/dsh-subagent-subprocess' +import { buildChildEnv } from '@deepseek-ai/dsh-subagent-subprocess' import type { Agent } from '@deepseek-ai/dsh-agent' import * as acp from '../src/index.ts' import { acpStopReason, acpContentText, DEFAULT_DISPOSE_EOF_GRACE_MS, DEFAULT_DISPOSE_GRACE_MS, startAcpRun, toAcpPrompt, type AcpRunSpec } from '../src/run.ts' @@ -112,7 +112,6 @@ describe('buildChildEnv', () => { // The explicitly-supplied key survives (an opt-in for the child's creds). expect(env.DEEPSEEK_API_KEY).toBe('explicit') // A normal ambient var is forwarded. - expect(SENSITIVE_ENV_PATTERN.test('PATH')).toBe(false) expect(env.PATH).toBe(process.env.PATH) } finally { delete process.env.DSH_ACP_TEST_SECRET_TOKEN diff --git a/packages/subagent/subagent-inprocess/README.md b/packages/subagent/subagent-inprocess/README.md index 48fd0a201d..83c2ba3500 100644 --- a/packages/subagent/subagent-inprocess/README.md +++ b/packages/subagent/subagent-inprocess/README.md @@ -26,7 +26,7 @@ After fulfillment, the caller owns the run. Provider-plugin unload does not revo `InProcessRunOptions` is `{ seed?: SessionEvent[] }`. Spawn omits it. Fork supplies a balanced completed-turn prefix and records its length so the result reader never mistakes a seeded parent message for child output. -`depthOf(agent)` reads `AgentOptions.subagentDepth`, treating absence as top-level depth zero and rejecting malformed stored values. `SubagentDepthError` reports an attempted child depth above `maxDepth`; an unrepresentable depth above the safe-integer domain is a `RangeError`. +Depth enforcement is internal to `startInProcessRun`: it reads `AgentOptions.subagentDepth`, treats absence as top-level depth zero, rejects malformed stored values, and reports an attempted child depth above `maxDepth`. An unrepresentable depth above the safe-integer domain is a `RangeError`. ## Structured output diff --git a/packages/subagent/subagent-inprocess/src/index.ts b/packages/subagent/subagent-inprocess/src/index.ts index 965d9c8e78..9676c74b5b 100644 --- a/packages/subagent/subagent-inprocess/src/index.ts +++ b/packages/subagent/subagent-inprocess/src/index.ts @@ -36,7 +36,7 @@ declare module '@deepseek-ai/dsh-agent' { * @param agent - the agent whose options carry the depth. * @returns its non-negative safe-integer depth. */ -export function depthOf(agent: Agent): number { +function depthOf(agent: Agent): number { const depth = agent.options.subagentDepth if (depth === undefined) return 0 if (!Number.isSafeInteger(depth) || depth < 0 || Object.is(depth, -0)) { @@ -46,7 +46,7 @@ export function depthOf(agent: Agent): number { } /** Thrown when starting a child would exceed the requested depth cap. */ -export class SubagentDepthError extends Error { +class SubagentDepthError extends Error { constructor(public readonly attemptedDepth: number, public readonly maxDepth: number) { super(`subagent depth ${attemptedDepth} exceeds maxDepth ${maxDepth}`) this.name = 'SubagentDepthError' diff --git a/packages/subagent/subagent-inprocess/tests/subagent-inprocess.spec.ts b/packages/subagent/subagent-inprocess/tests/subagent-inprocess.spec.ts index aa0f946c89..c594e27fa8 100644 --- a/packages/subagent/subagent-inprocess/tests/subagent-inprocess.spec.ts +++ b/packages/subagent/subagent-inprocess/tests/subagent-inprocess.spec.ts @@ -10,7 +10,7 @@ import AgentLoop from '@deepseek-ai/dsh-agent-loop' import * as Invariants from '@deepseek-ai/dsh-invariants' import SubagentService from '@deepseek-ai/dsh-subagent' import { MockAdapter, textResponse } from '../../../core/agent-loop/tests/mock-adapter.ts' -import { depthOf, SubagentDepthError, startInProcessRun } from '../src/index.ts' +import { startInProcessRun } from '../src/index.ts' type Script = ConstructorParameters[0] @@ -37,19 +37,6 @@ function text(blocks: readonly { type: string; text?: string }[]): string { return blocks.filter(block => block.type === 'text').map(block => block.text).join('') } -describe('depthOf', () => { - it('reads zero for a top-level agent and an explicit child depth', async () => { - const { parent } = await setup([]) - expect(depthOf(parent)).toBe(0) - expect(depthOf({ options: { subagentDepth: 3 } } as unknown as Agent)).toBe(3) - }) - - it.each([Number.NaN, 1.5, -1, -0, Number.MAX_SAFE_INTEGER + 1])('rejects malformed depth %s', (value) => { - expect(() => depthOf({ options: { subagentDepth: value } } as unknown as Agent)) - .toThrow('non-negative safe integer') - }) -}) - describe('startInProcessRun', () => { it('returns only after publication, drives a fresh child, and disposes it', async () => { const { ctx, parent } = await setup([textResponse('driver answer')]) @@ -58,7 +45,7 @@ describe('startInProcessRun', () => { const result = await run.result expect(result.stopReason).toBe('completed') expect(text(result.output)).toBe('driver answer') - expect(depthOf(ctx.agents.get(run.id)!)).toBe(1) + expect(ctx.agents.get(run.id)!.options.subagentDepth).toBe(1) await run.dispose() await run.dispose() expect(ctx.agents.get(run.id)).toBeUndefined() @@ -83,7 +70,12 @@ describe('startInProcessRun', () => { await expect(startInProcessRun({ ...request(parent), maxDepth: -1 }, {})) .rejects.toThrow('non-negative safe integer') await expect(startInProcessRun({ ...request(parent), maxDepth: 0 }, {})) - .rejects.toBeInstanceOf(SubagentDepthError) + .rejects.toMatchObject({ name: 'SubagentDepthError' }) + for (const value of [Number.NaN, 1.5, -1, -0, Number.MAX_SAFE_INTEGER + 1]) { + const malformed = { options: { subagentDepth: value } } as unknown as Agent + await expect(startInProcessRun(request(malformed), {})) + .rejects.toThrow('agent subagentDepth must be a non-negative safe integer') + } const maxParent = { options: { subagentDepth: Number.MAX_SAFE_INTEGER } } as unknown as Agent await expect(startInProcessRun(request(maxParent), {})).rejects.toBeInstanceOf(RangeError) }) diff --git a/packages/subagent/subagent-spawn/tests/subagent-spawn.spec.ts b/packages/subagent/subagent-spawn/tests/subagent-spawn.spec.ts index 06aa40ca9f..9b9e2a8fd2 100644 --- a/packages/subagent/subagent-spawn/tests/subagent-spawn.spec.ts +++ b/packages/subagent/subagent-spawn/tests/subagent-spawn.spec.ts @@ -13,7 +13,7 @@ import * as Invariants from '@deepseek-ai/dsh-invariants' import SubagentService, { type SubagentStartRequest } from '@deepseek-ai/dsh-subagent' import { MockAdapter, maxTokensResponse, textResponse, toolCallResponse } from '../../../core/agent-loop/tests/mock-adapter.ts' import * as spawn from '../src/index.ts' -import { depthOf, STRUCTURED_OUTPUT_TOOL, SubagentDepthError } from '@deepseek-ai/dsh-subagent-inprocess' +import { STRUCTURED_OUTPUT_TOOL } from '@deepseek-ai/dsh-subagent-inprocess' type Script = ConstructorParameters[0] @@ -118,11 +118,11 @@ describe('dsh-subagent-spawn', () => { it('stamps child depth = parent depth + 1 (via the merged AgentOptions field)', async () => { const { ctx, parent } = await setup([textResponse('x')]) - expect(depthOf(parent)).toBe(0) + expect(parent.options.subagentDepth).toBeUndefined() const run = await start(ctx, 'spawn', { prompt: [{ type: 'text', text: 'p' }], parent }) await run.result const child = ctx.agents.get(run.id)! - expect(depthOf(child)).toBe(1) + expect(child.options.subagentDepth).toBe(1) await run.dispose() }) @@ -130,7 +130,7 @@ describe('dsh-subagent-spawn', () => { const { ctx, parent } = await setup([]) // parent is depth 0, child would be depth 1 — cap at 0 forbids any child. await expect(start(ctx, 'spawn', { prompt: [{ type: 'text', text: 'p' }], parent, maxDepth: 0 })) - .rejects.toThrow(SubagentDepthError) + .rejects.toThrow('subagent depth 1 exceeds maxDepth 0') }) it('maps a child that hit its token ceiling to stopReason "max-tokens"', async () => { diff --git a/packages/subagent/subagent-subprocess/README.md b/packages/subagent/subagent-subprocess/README.md index ccc68bf31b..c8b8a06437 100644 --- a/packages/subagent/subagent-subprocess/README.md +++ b/packages/subagent/subagent-subprocess/README.md @@ -6,7 +6,7 @@ Every tunable is a **parameter**: the dispose ladder takes its grace periods per ## What it exports -### `SENSITIVE_ENV_PATTERN` / `buildChildEnv(extra)` +### `buildChildEnv(extra)` The credential env scrub (same pattern as the [bash executor](../../bash/bash-local/README.md)): the child env is the ambient env minus credential-shaped vars (`/KEY|SECRET|TOKEN/i`), with `extra` layered on top AFTER the scrub. `PATH`, `HOME`, `TMPDIR`, locale, and proxy vars survive, so the child CLI runs normally; the parent's own secrets never leak implicitly, while an explicitly supplied credential (the child's OWN key in a backend's `env` config) still reaches the child. @@ -14,10 +14,6 @@ The credential env scrub (same pattern as the [bash executor](../../bash/bash-lo Spawn-failure capture: a promise that resolves (never rejects) with the child's first `error` event. A spawn failure such as `ENOENT` is an event, not a thrown exception — without a listener Node crashes the parent process — so call this in the same tick as `spawn()` and race it in the run's result path; a bad command then settles as an ordinary child-level failure. For a child that spawns cleanly the promise never settles. -### `waitForExit(child)` / `exitsWithin(child, ms)` - -Exit waits over a `ChildProcess`: resolve once the child exits by any code or signal (immediately if it is already gone), or race that against a timer (`true` = exited in time). The race cleans up after itself on both outcomes — the pending timer is `unref()`ed and cleared on exit, the exit listener removed on timeout — so repeated calls (the dispose ladder's tiers, a poll loop) never accumulate listeners on the child. - ### `disposeChildProcess(child, graces)` The three-tier dispose ladder. Resolves only once the child has ACTUALLY exited — quiescence reached, not merely requested (see [defensive patterns](../../../docs/defensive-patterns.md)): @@ -28,6 +24,8 @@ The three-tier dispose ladder. Resolves only once the child has ACTUALLY exited The two graces (`DisposeLadderGraces`) come from the consuming plugin's `disposeEofGraceMs`/`disposeGraceMs` Config fields; the EOF window is deliberately a separate — usually wider — grace than the signal tier, since a cooperative child's EOF teardown may itself await a signal-trapping grandchild plus a final flush. +The exit waits are internal to this ladder. They clean up their timer and listener on either outcome, so escalation never accumulates listeners on the child. + ### `createIsolatedConfigDir(prefix, pinnedPath?)` A per-run isolated config directory for an external CLI child (the target of `CLAUDE_CONFIG_DIR` / `CODEX_HOME`-style redirection), so child behavior is a function of deployment config alone — never of whatever `~/.claude` / `~/.codex`-style state exists on the host. Returns an `IsolatedConfigDir` handle: `path` goes into the child env, `remove()` runs on dispose. diff --git a/packages/subagent/subagent-subprocess/src/index.ts b/packages/subagent/subagent-subprocess/src/index.ts index 35d7383456..2ee2745985 100644 --- a/packages/subagent/subagent-subprocess/src/index.ts +++ b/packages/subagent/subagent-subprocess/src/index.ts @@ -32,7 +32,7 @@ import { join } from 'node:path' * the scrub, so an intended `DEEPSEEK_API_KEY` survives while an incidental * `AWS_SECRET_ACCESS_KEY` does not. */ -export const SENSITIVE_ENV_PATTERN = /KEY|SECRET|TOKEN/i +const SENSITIVE_ENV_PATTERN = /KEY|SECRET|TOKEN/i /** * The ambient env minus credential-shaped vars, plus the caller's explicit @@ -72,7 +72,7 @@ export function spawnFailure(child: ChildProcess): Promise { * already gone. * @param child - the child process to await. */ -export function waitForExit(child: ChildProcess): Promise { +function waitForExit(child: ChildProcess): Promise { if (child.exitCode !== null || child.signalCode !== null) return Promise.resolve() return new Promise(resolve => child.once('exit', () => { resolve() })) } @@ -87,7 +87,7 @@ export function waitForExit(child: ChildProcess): Promise { * @returns `true` if the child exits within `ms` (immediately if it is * already gone), `false` on timeout. */ -export function exitsWithin(child: ChildProcess, ms: number): Promise { +function exitsWithin(child: ChildProcess, ms: number): Promise { if (child.exitCode !== null || child.signalCode !== null) return Promise.resolve(true) return new Promise((resolve) => { const onExit = (): void => { diff --git a/packages/subagent/subagent-subprocess/tests/subagent-subprocess.spec.ts b/packages/subagent/subagent-subprocess/tests/subagent-subprocess.spec.ts index 2766ed0a41..8e19a8a4a1 100644 --- a/packages/subagent/subagent-subprocess/tests/subagent-subprocess.spec.ts +++ b/packages/subagent/subagent-subprocess/tests/subagent-subprocess.spec.ts @@ -9,10 +9,7 @@ import { buildChildEnv, createIsolatedConfigDir, disposeChildProcess, - exitsWithin, - SENSITIVE_ENV_PATTERN, spawnFailure, - waitForExit, } from '../src/index.ts' // `rm` is wrapped (real-passthrough by default) so ONE test can inject a @@ -47,6 +44,8 @@ interface FakeChildScript { diesOn?: LethalTrigger /** Delay (ms) between the lethal trigger and the exit event. */ delayMs?: number + /** Complete the scripted exit inside the triggering call. */ + synchronousExit?: boolean /** `false` models a child spawned without a stdin pipe. */ stdin?: boolean } @@ -80,11 +79,13 @@ class FakeChild extends EventEmitter { // SIGKILL is uncatchable — it always fells the child; any other trigger // only when the scenario scripts it as the lethal one. if (trigger !== 'SIGKILL' && this.script.diesOn !== trigger) return - setTimeout(() => { + const exit = (): void => { if (trigger === 'eof') this.exitCode = 0 else this.signalCode = trigger this.emit('exit', this.exitCode, this.signalCode) - }, this.script.delayMs ?? 0) + } + if (this.script.synchronousExit === true) exit() + else setTimeout(exit, this.script.delayMs ?? 0) } } @@ -93,7 +94,7 @@ function asChild(fake: FakeChild): ChildProcess { return fake as unknown as ChildProcess } -describe('buildChildEnv / SENSITIVE_ENV_PATTERN', () => { +describe('buildChildEnv', () => { it('drops credential-shaped ambient vars (KEY/SECRET/TOKEN, case-insensitive)', () => { process.env.DSH_PROC_TEST_API_KEY = 'leak' process.env.dsh_proc_test_secret = 'leak' @@ -111,7 +112,6 @@ describe('buildChildEnv / SENSITIVE_ENV_PATTERN', () => { }) it('forwards normal ambient vars', () => { - expect(SENSITIVE_ENV_PATTERN.test('PATH')).toBe(false) expect(buildChildEnv({}).PATH).toBe(process.env.PATH) }) @@ -149,7 +149,7 @@ describe('spawnFailure', () => { const fake = new FakeChild({ diesOn: 'SIGTERM' }) const failure = spawnFailure(asChild(fake)) fake.kill('SIGTERM') - await waitForExit(asChild(fake)) + await new Promise(resolve => fake.once('exit', () => { resolve() })) // A clean lifecycle emits `exit`, never `error` — the capture stays // pending forever, so a race against it is decided by the other arms. const settled = await Promise.race([ @@ -160,51 +160,6 @@ describe('spawnFailure', () => { }) }) -describe('waitForExit / exitsWithin', () => { - it('resolves immediately for a child that already exited by code', async () => { - const fake = new FakeChild() - fake.exitCode = 0 - await expect(waitForExit(asChild(fake))).resolves.toBeUndefined() - }) - - it('resolves immediately for a child that already died by signal', async () => { - const fake = new FakeChild() - fake.signalCode = 'SIGTERM' - await expect(waitForExit(asChild(fake))).resolves.toBeUndefined() - }) - - it('resolves on the exit event of a live child', async () => { - const fake = new FakeChild({ diesOn: 'SIGTERM', delayMs: 5 }) - const exited = waitForExit(asChild(fake)) - fake.kill('SIGTERM') - await expect(exited).resolves.toBeUndefined() - expect(fake.signalCode).toBe('SIGTERM') - }) - - it('exitsWithin resolves true immediately for an already-exited child (no listener attached)', async () => { - const fake = new FakeChild() - fake.exitCode = 0 - await expect(exitsWithin(asChild(fake), 1000)).resolves.toBe(true) - expect(fake.listenerCount('exit')).toBe(0) - }) - - it('exitsWithin resolves true when the child exits inside the window', async () => { - const fake = new FakeChild({ diesOn: 'SIGTERM', delayMs: 5 }) - fake.kill('SIGTERM') - await expect(exitsWithin(asChild(fake), 1000)).resolves.toBe(true) - // The once-listener fired and the grace timer was cleared — nothing lingers. - expect(fake.listenerCount('exit')).toBe(0) - }) - - it('exitsWithin resolves false on timeout for a child that never exits', async () => { - const fake = new FakeChild() // nothing short of SIGKILL fells it; no signal sent - await expect(exitsWithin(asChild(fake), 20)).resolves.toBe(false) - // The timeout arm removed its exit listener: repeated waits (a poll loop, - // the ladder's tiers) never accumulate listeners on the same child. - expect(fake.listenerCount('exit')).toBe(0) - }) -}) - describe('disposeChildProcess', () => { it('returns immediately for an already-exited child (no EOF, no signals)', async () => { const fake = new FakeChild() @@ -230,12 +185,28 @@ describe('disposeChildProcess', () => { expect(fake.exitCode).toBe(0) }) + it('recognizes a child that exits synchronously on stdin EOF', async () => { + const fake = new FakeChild({ diesOn: 'eof', synchronousExit: true }) + await disposeChildProcess(asChild(fake), { disposeEofGraceMs: 1000, disposeGraceMs: 1000 }) + expect(fake.exitCode).toBe(0) + expect(fake.listenerCount('exit')).toBe(0) + }) + it('tier 2: a child that ignores EOF but honors SIGTERM dies on the middle rung', async () => { const fake = new FakeChild({ diesOn: 'SIGTERM', delayMs: 5 }) await disposeChildProcess(asChild(fake), { disposeEofGraceMs: 20, disposeGraceMs: 1000 }) expect(fake.stdinEnded).toBe(true) expect(fake.kills).toEqual(['SIGTERM']) expect(fake.signalCode).toBe('SIGTERM') + expect(fake.listenerCount('exit')).toBe(0) + }) + + it('recognizes a child that exits synchronously on SIGTERM', async () => { + const fake = new FakeChild({ diesOn: 'SIGTERM', synchronousExit: true }) + await disposeChildProcess(asChild(fake), { disposeEofGraceMs: 20, disposeGraceMs: 1000 }) + expect(fake.kills).toEqual(['SIGTERM']) + expect(fake.signalCode).toBe('SIGTERM') + expect(fake.listenerCount('exit')).toBe(0) }) it('tier 3: a SIGTERM-trapping child is SIGKILLed, and dispose resolves only after the exit', async () => { @@ -247,6 +218,13 @@ describe('disposeChildProcess', () => { expect(fake.signalCode).toBe('SIGKILL') }) + it('recognizes a child already gone when the final exit wait begins', async () => { + const fake = new FakeChild({ synchronousExit: true }) + await disposeChildProcess(asChild(fake), { disposeEofGraceMs: 20, disposeGraceMs: 20 }) + expect(fake.kills).toEqual(['SIGTERM', 'SIGKILL']) + expect(fake.signalCode).toBe('SIGKILL') + }) + it('walks the ladder for a child spawned without a stdin pipe', async () => { const fake = new FakeChild({ stdin: false, diesOn: 'SIGTERM', delayMs: 5 }) await disposeChildProcess(asChild(fake), { disposeEofGraceMs: 20, disposeGraceMs: 1000 })