From f85b831bd2cd0d37ca814473abc17ccfb2a6c287 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Tue, 14 Jul 2026 02:41:23 +0800 Subject: [PATCH 1/8] refactor: hide subagent implementation helpers --- ...claude-code-and-codex-subagent-backends.md | 2 +- .../subagent-acp/tests/subagent-acp.spec.ts | 3 +- .../subagent/subagent-inprocess/README.md | 2 +- .../subagent/subagent-inprocess/src/index.ts | 4 +- .../tests/subagent-inprocess.spec.ts | 24 ++---- .../tests/subagent-spawn.spec.ts | 8 +- .../subagent/subagent-subprocess/README.md | 8 +- .../subagent/subagent-subprocess/src/index.ts | 6 +- .../tests/subagent-subprocess.spec.ts | 84 +++++++------------ 9 files changed, 54 insertions(+), 87 deletions(-) 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 }) From 65521b589f3ffa1a912bb3d8db05a6e2ce204a85 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Tue, 14 Jul 2026 02:45:34 +0800 Subject: [PATCH 2/8] refactor: hide remaining subagent helpers --- ...t-variables-and-tool-guidance-ownership.md | 2 +- ...7-05-subagent-provider-lifecycle-events.md | 2 +- .../2026-06-22-fork-snapshot-scenarios.md | 2 +- packages/subagent/subagent-fork/README.md | 2 +- packages/subagent/subagent-fork/src/index.ts | 2 +- .../subagent-fork/tests/subagent-fork.spec.ts | 42 ++++++++----------- packages/subagent/tool-subagent/src/index.ts | 4 +- 7 files changed, 25 insertions(+), 31 deletions(-) diff --git a/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md b/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md index 0961830c1a..aae4564513 100644 --- a/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md +++ b/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md @@ -38,7 +38,7 @@ Per-tool semantics and when-to-use live in tool DESCRIPTIONS, which already ship ### The subagent conversation-history descriptor -`SubagentProvider` gains `readonly inheritsParentContext: boolean` — a DESCRIPTIVE conversation-history fact beside `capabilities`, not in it (capabilities are start-time validation; nothing validates against this flag). Spawn and ACP declare `false`, fork declares `true`. The name refers only to conversation seeding, not Cordis scope, services, tools, or authority. `dsh-tool-subagent` derives both the tool description and the `prompt` parameter description from the flag (`providerWording`): the fork instance now tells the model the child is seeded with the conversation's completed turns (not the in-flight turn) and that its prompt should state only what is new. Deriving the description from a provider that arrives on its own fiber is what forced the provider-lifecycle events and the tool's reactive registration — that mechanism, its Loader-concurrency rationale, and its rejected alternatives are recorded in [the provider-lifecycle-events RFC](2026-07-05-subagent-provider-lifecycle-events.md). +`SubagentProvider` gains `readonly inheritsParentContext: boolean` — a DESCRIPTIVE conversation-history fact beside `capabilities`, not in it (capabilities are start-time validation; nothing validates against this flag). Spawn and ACP declare `false`, fork declares `true`. The name refers only to conversation seeding, not Cordis scope, services, tools, or authority. `dsh-tool-subagent` derives both the tool description and the `prompt` parameter description from the flag: the fork instance now tells the model the child is seeded with the conversation's completed turns (not the in-flight turn) and that its prompt should state only what is new. Deriving the description from a provider that arrives on its own fiber is what forced the provider-lifecycle events and the tool's reactive registration — that mechanism, its Loader-concurrency rationale, and its rejected alternatives are recorded in [the provider-lifecycle-events RFC](2026-07-05-subagent-provider-lifecycle-events.md). ## Alternatives considered diff --git a/docs/rfc/implemented/architecture/2026-07-05-subagent-provider-lifecycle-events.md b/docs/rfc/implemented/architecture/2026-07-05-subagent-provider-lifecycle-events.md index 46e7ea4e71..4a96ef7419 100644 --- a/docs/rfc/implemented/architecture/2026-07-05-subagent-provider-lifecycle-events.md +++ b/docs/rfc/implemented/architecture/2026-07-05-subagent-provider-lifecycle-events.md @@ -4,7 +4,7 @@ Status: implemented ## Problem -[The prompt-variables RFC](2026-07-05-prompt-variables-and-tool-guidance-ownership.md) makes `dsh-tool-subagent` DERIVE its model-facing wording from its provider: `SubagentProvider.inheritsParentContext` (spawn/ACP `false`, fork `true`) drives both the tool description and the `prompt` parameter description (`providerWording`), so the fork tool stops lying about context inheritance. That fix created a cross-fiber data dependency: a tool's description is fixed at TOOL REGISTRATION (deliberately — the description is where tool-choice guidance lives), but the provider arrives on its own plugin fiber, on no particular schedule. +[The prompt-variables RFC](2026-07-05-prompt-variables-and-tool-guidance-ownership.md) makes `dsh-tool-subagent` DERIVE its model-facing wording from its provider: `SubagentProvider.inheritsParentContext` (spawn/ACP `false`, fork `true`) drives both the tool description and the `prompt` parameter description, so the fork tool stops lying about context inheritance. That fix created a cross-fiber data dependency: a tool's description is fixed at TOOL REGISTRATION (deliberately — the description is where tool-choice guidance lives), but the provider arrives on its own plugin fiber, on no particular schedule. The first implementation resolved the provider at the tool plugin's `apply` time and threw when it was absent — an implicit load-order requirement ("list the backend before the tool in cordis.yml"). Review reproduced the failure that requirement hides: the cordis Loader starts sibling entries CONCURRENTLY (`Promise.all` over the group) and `Entry.init()` does not await activation, so a backend whose activation is delayed leaves the tool's fiber permanently failed even when "listed first". The ordering the requirement leaned on is not a contract the Loader offers — "async state is not synchronous state" ([defensive patterns](../../../defensive-patterns.md)). diff --git a/docs/rfc/implemented/testing/2026-06-22-fork-snapshot-scenarios.md b/docs/rfc/implemented/testing/2026-06-22-fork-snapshot-scenarios.md index b2c39047b9..34bebd1194 100644 --- a/docs/rfc/implemented/testing/2026-06-22-fork-snapshot-scenarios.md +++ b/docs/rfc/implemented/testing/2026-06-22-fork-snapshot-scenarios.md @@ -17,7 +17,7 @@ Record two scenarios against the real API, both replayed keyless in the default ### Why a completed turn-1 is required -The fork backend seeds the child with the parent's **balanced completed-turn prefix** ([`completedTurnPrefix`](../../../../packages/subagent/subagent-fork)). A parent that forks on its very first turn has no completed turn to inherit, so the seed is empty (≡ a fresh spawn, `seedLength` 0) — which would NOT exercise the slice. Both scenarios therefore use a two-prompt input: the first prompt completes a turn (establishing a codeword the child is later asked to recall), the second delegates the fork. The recalled codeword in the child's transcript is incidental to the model's behavior; the load-bearing artifact is the child fixture's recorded `seedLength`, which the replay slice consumes. +The fork backend seeds the child with the parent's **balanced completed-turn prefix**. A parent that forks on its very first turn has no completed turn to inherit, so the seed is empty (≡ a fresh spawn, `seedLength` 0) — which would NOT exercise the slice. Both scenarios therefore use a two-prompt input: the first prompt completes a turn (establishing a codeword the child is later asked to recall), the second delegates the fork. The recalled codeword in the child's transcript is incidental to the model's behavior; the load-bearing artifact is the child fixture's recorded `seedLength`, which the replay slice consumes. ## Consequences diff --git a/packages/subagent/subagent-fork/README.md b/packages/subagent/subagent-fork/README.md index bf90ecdf52..0909dc0bf1 100644 --- a/packages/subagent/subagent-fork/README.md +++ b/packages/subagent/subagent-fork/README.md @@ -6,7 +6,7 @@ The fork provider creates an in-process child seeded with the parent's completed The parent's current tool-calling turn is still open when a subagent starts: its log contains the assistant tool call but not the matching tool result or `turn/end`. Copying that raw log would give the child an invalid, unbalanced session. -Fork therefore uses `completedTurnPrefix(parent.session.events)`: the contiguous prefix ending at the last `turn/end`. The child sees all completed parent turns and none of the in-flight turn. If the parent has not completed a turn yet, the seed is empty and the child behaves like a fresh spawn. +Fork therefore computes the contiguous prefix ending at the last `turn/end`. The child sees all completed parent turns and none of the in-flight turn. If the parent has not completed a turn yet, the seed is empty and the child behaves like a fresh spawn. The seed transfers conversation history only. The child still receives a fresh flat registration scope; it does not inherit the parent's tool restrictions or authority. diff --git a/packages/subagent/subagent-fork/src/index.ts b/packages/subagent/subagent-fork/src/index.ts index ebf64e7b70..22be47cc56 100644 --- a/packages/subagent/subagent-fork/src/index.ts +++ b/packages/subagent/subagent-fork/src/index.ts @@ -54,7 +54,7 @@ export const Config: z = z.object({ * @param parent - the agent whose session log to slice. * @returns the seed events, contiguous from seq 0; empty when no turn has completed. */ -export function completedTurnPrefix(parent: Agent): SessionEvent[] { +function completedTurnPrefix(parent: Agent): SessionEvent[] { const events = parent.session.events const lastEnd = events.findLast(e => e.type === 'turn/end') if (lastEnd === undefined) return [] diff --git a/packages/subagent/subagent-fork/tests/subagent-fork.spec.ts b/packages/subagent/subagent-fork/tests/subagent-fork.spec.ts index 360098e369..bfb0bedac7 100644 --- a/packages/subagent/subagent-fork/tests/subagent-fork.spec.ts +++ b/packages/subagent/subagent-fork/tests/subagent-fork.spec.ts @@ -14,7 +14,6 @@ import { MockAdapter, textResponse, toolCallResponse } from '../../../core/agent import type { StreamChunk } from '@deepseek-ai/dsh-llm' import * as fork from '../src/index.ts' import { STRUCTURED_OUTPUT_TOOL } from '@deepseek-ai/dsh-subagent-inprocess' -import { completedTurnPrefix } from '../src/index.ts' type Script = ConstructorParameters[0] @@ -52,28 +51,6 @@ function text(blocks: { type: string; text?: string }[]): string { return blocks.filter(b => b.type === 'text').map(b => b.text).join('') } -describe('completedTurnPrefix', () => { - it('returns an empty prefix for a parent that has never completed a turn', async () => { - const { parent } = await setup([]) - expect(completedTurnPrefix(parent)).toEqual([]) - }) - - it('returns the balanced prefix up to and including the last turn/end', async () => { - const { parent } = await setup([textResponse('first'), textResponse('second')]) - parent.send([{ type: 'text', text: 'q1' }]) - await parent.whenIdle() - parent.send([{ type: 'text', text: 'q2' }]) - await parent.whenIdle() - - const prefix = completedTurnPrefix(parent) - // Ends exactly at the last turn/end; seq is contiguous from 0. - expect(prefix.at(-1)?.type).toBe('turn/end') - expect(prefix.map(e => e.seq)).toEqual(prefix.map((_, i) => i)) - // Both completed turns are present. - expect(prefix.filter(e => e.type === 'turn/end')).toHaveLength(2) - }) -}) - describe('dsh-subagent-fork', () => { it('emits subagent/start only after the seeded child is published', async () => { const { ctx, parent } = await setup([textResponse('child answer')]) @@ -96,7 +73,6 @@ describe('dsh-subagent-fork', () => { // The parent has never completed a turn → empty prefix → the provider omits // the seed → the child runs fresh. Exercises the `seed.length > 0` false arm. const { ctx, parent } = await setup([textResponse('fresh child')]) - expect(completedTurnPrefix(parent)).toEqual([]) const run = await start(ctx, 'fork', { prompt: [{ type: 'text', text: 'child q' }], parent }) const result = await run.result expect(result.stopReason).toBe('completed') @@ -104,6 +80,24 @@ describe('dsh-subagent-fork', () => { const child = ctx.agents.get(run.id)! // Only the child's own turn — no seeded parent turns. expect(child.session.events.filter(e => e.type === 'turn/end')).toHaveLength(1) + expect(child.session.header.seedLength).toBeUndefined() + await run.dispose() + }) + + it('seeds every completed parent turn through the last turn/end', async () => { + const { ctx, parent } = await setup([textResponse('first'), textResponse('second'), textResponse('child')]) + parent.send([{ type: 'text', text: 'q1' }]) + await parent.whenIdle() + parent.send([{ type: 'text', text: 'q2' }]) + await parent.whenIdle() + const parentPrefixLen = parent.session.events.length + + const run = await start(ctx, 'fork', { prompt: [{ type: 'text', text: 'child q' }], parent }) + await run.result + const child = ctx.agents.get(run.id)! + expect(child.session.header.seedLength).toBe(parentPrefixLen) + expect(child.session.events.slice(0, parentPrefixLen).at(-1)?.type).toBe('turn/end') + expect(child.session.events.slice(0, parentPrefixLen).filter(e => e.type === 'turn/end')).toHaveLength(2) await run.dispose() }) diff --git a/packages/subagent/tool-subagent/src/index.ts b/packages/subagent/tool-subagent/src/index.ts index d07e6726dd..426753dbb9 100644 --- a/packages/subagent/tool-subagent/src/index.ts +++ b/packages/subagent/tool-subagent/src/index.ts @@ -161,13 +161,13 @@ function stopReasonError(result: SubagentResult): string | undefined { * A fresh child needs a standalone prompt; a forked child already sees the * conversation's completed turns — telling the model to restate everything * (or, worse, that the child "does not see this conversation") would be false - * for a fork. Exported for tests. + * for a fork. * @param inheritsConversation - whether the child's conversation is seeded * with the parent's completed turns; this says nothing about tool, service, * scope, or authority inheritance. * @returns the tool `description` and the `prompt` parameter description. */ -export function providerWording(inheritsConversation: boolean): { description: string; promptDescription: string } { +function providerWording(inheritsConversation: boolean): { description: string; promptDescription: string } { if (inheritsConversation) { return { description: From 8ada835396a021a7dce8f72d6b025dcd3a264c15 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Tue, 14 Jul 2026 08:03:33 +0800 Subject: [PATCH 3/8] docs: remove links to private subagent helpers --- packages/subagent/subagent-subprocess/src/index.ts | 12 +++++------- packages/subagent/tool-subagent/src/index.ts | 10 +++++----- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/packages/subagent/subagent-subprocess/src/index.ts b/packages/subagent/subagent-subprocess/src/index.ts index 2ee2745985..bd792e7f23 100644 --- a/packages/subagent/subagent-subprocess/src/index.ts +++ b/packages/subagent/subagent-subprocess/src/index.ts @@ -2,11 +2,10 @@ * Shared machinery for OUT-OF-PROCESS subagent backends — providers that spawn * an external agent as a child process and must keep the parent deployment's * credentials out of it, tear it down to quiescence, and isolate it from the - * host user's on-disk CLI state. The pieces: the credential env scrub - * ({@link SENSITIVE_ENV_PATTERN} / {@link buildChildEnv}), the spawn-failure - * capture ({@link spawnFailure}), the child-exit waits ({@link waitForExit} / - * {@link exitsWithin}), the stdin-EOF → SIGTERM → SIGKILL dispose ladder - * ({@link disposeChildProcess}), and the per-run isolated config dir + * host user's on-disk CLI state. The pieces: credential-shaped env scrubbing + * ({@link buildChildEnv}), spawn-failure capture ({@link spawnFailure}), + * bounded child-exit waits inside the stdin-EOF → SIGTERM → SIGKILL dispose + * ladder ({@link disposeChildProcess}), and the per-run isolated config dir * ({@link createIsolatedConfigDir}). * * This package owns no provider and registers nothing; it is a pure library @@ -37,8 +36,7 @@ const SENSITIVE_ENV_PATTERN = /KEY|SECRET|TOKEN/i /** * The ambient env minus credential-shaped vars, plus the caller's explicit * env. `PATH`, `HOME`, `TMPDIR`, locale, and proxy vars survive the scrub, so - * a child CLI runs normally; only {@link SENSITIVE_ENV_PATTERN}-shaped names - * are dropped. + * a child CLI runs normally; only credential-shaped names are dropped. * @param extra - explicit vars layered on top AFTER the scrub, so a * credential-shaped name supplied deliberately still reaches the child. * @returns the environment to spawn the child with. diff --git a/packages/subagent/tool-subagent/src/index.ts b/packages/subagent/tool-subagent/src/index.ts index 426753dbb9..ff54bc109a 100644 --- a/packages/subagent/tool-subagent/src/index.ts +++ b/packages/subagent/tool-subagent/src/index.ts @@ -12,11 +12,11 @@ * sees only `{ description, prompt }`. * * The tool DESCRIPTION is derived from the bound provider's conversation-history - * descriptor ({@link providerWording}): a fresh-conversation provider (spawn, - * ACP) gets the standalone-prompt wording, while a seeded-conversation provider - * (fork) tells the model the child already sees the conversation's completed - * turns. This descriptor says nothing about Cordis scope, services, tools, or - * authority. The tool MIRRORS the + * descriptor ({@link SubagentProvider.inheritsParentContext}): a + * fresh-conversation provider (spawn, ACP) gets the standalone-prompt wording, + * while a seeded-conversation provider (fork) tells the model the child already + * sees the conversation's completed turns. This descriptor says nothing about + * Cordis scope, services, tools, or authority. The tool MIRRORS the * provider's lifecycle via `subagent/provider-added`/`-removed` — it registers * when the provider is (or becomes) available and unregisters when the * provider goes away — so no load-order requirement exists and an HMR reload From d33a819d15964fac1212f7b716f50106c8bb0ed1 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Tue, 14 Jul 2026 08:55:38 +0800 Subject: [PATCH 4/8] fix: share scope carrier across built JSON-RPC --- AGENTS.md | 2 +- docs/testing.md | 2 +- packages/ui/jsonrpc/package.json | 2 + .../jsonrpc/tests/built-scope-carrier.e2e.ts | 121 ++++++++++++++++++ pnpm-lock.yaml | 3 + scripts/run-gates.ts | 1 + 6 files changed, 129 insertions(+), 2 deletions(-) create mode 100644 packages/ui/jsonrpc/tests/built-scope-carrier.e2e.ts diff --git a/AGENTS.md b/AGENTS.md index 4dfbedb97b..1494d3fcf8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -76,7 +76,7 @@ printf '%s\n' "$out" | grep -q '\[tool call\] echo({"text":"ci smoke"})' printf '%s\n' "$out" | grep -q '\[tool result\] ECHO: CI SMOKE' test -n "$(find .sessions -path '.sessions/cwd-*/main-session-*.jsonl' -type f -print -quit)" rm -rf .sessions -pnpm exec vitest run --config vitest.e2e.config.ts packages/ui/stdio-agent/tests/built-bin.e2e.ts packages/ui/acp-agent/tests/built-bin.e2e.ts packages/workflow/workflow-workerthread/tests/built-worker.e2e.ts packages/code-runtime/code-runtime-worker/tests/built-lib.e2e.ts +pnpm exec vitest run --config vitest.e2e.config.ts packages/ui/stdio-agent/tests/built-bin.e2e.ts packages/ui/acp-agent/tests/built-bin.e2e.ts packages/ui/jsonrpc/tests/built-scope-carrier.e2e.ts packages/workflow/workflow-workerthread/tests/built-worker.e2e.ts packages/code-runtime/code-runtime-worker/tests/built-lib.e2e.ts ``` `test:coverage`, not `test`, is the gating run ([why](docs/testing.md)); a sign-off counts only for commands actually run. diff --git a/docs/testing.md b/docs/testing.md index 23cca651fe..a1a841296a 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -25,7 +25,7 @@ An e2e assertion re-runs the command or re-reads the file externally; a keyword - A plugin shipped via `cordis.yml` needs at least one test through the REAL Loader path: hand-built `ctx.plugin({...})` mounts bypass `unwrapExports` and cannot catch a broken export shape ([postmortem 0001](postmortem/0001-acp-default-export-drops-inject.md); export-shape rules in [packages/AGENTS.md](../packages/AGENTS.md)). - A guard only guards if the regression actually fails it. For a plugin without `inject` (bundle/composition plugins), a Loader smoke stays green under a broken export shape — add an explicit `expect('default' in mod).toBe(false)` plus an `unwrapExports` round-trip assertion, and prove it: introduce the regression, watch red, revert. -- "Real entry path" means the published artifact: the package `bin` points at built `lib/bin.js` under plain `node`, which tsx masks (settle races, module resolution, a swallowed load failure exiting 0). The same applies to any non-index runtime entry the built package resolves at run time (the worker-thread runtime's sibling `lib/worker.cjs`). Keep the built-artifact smokes green (`packages/ui/*/tests/built-bin.e2e.ts`, `packages/code-runtime/code-runtime-worker/tests/built-lib.e2e.ts`), and assert a genuinely-missing config exits non-zero. +- "Real entry path" means the published artifact: a package `bin` runs built `lib/bin.js` under plain `node`, exposing failures tsx masks (settle races, module resolution, swallowed load failures). The same applies to non-index runtime entries (the worker-thread sibling `lib/worker.cjs`) and singleton modules shared across bundles (`packages/ui/jsonrpc/tests/built-scope-carrier.e2e.ts`). Keep the built-artifact smokes green (`packages/ui/*/tests/built-bin.e2e.ts`, `packages/code-runtime/code-runtime-worker/tests/built-lib.e2e.ts`), and assert a genuinely-missing config exits non-zero. - An e2e that spawns an example from a temp cwd sets `TSX_TSCONFIG_PATH` to the repo-root tsconfig, or it silently falls back to stale built `lib/` ([examples/AGENTS.md](../examples/AGENTS.md)). ## When a snapshot test is required diff --git a/packages/ui/jsonrpc/package.json b/packages/ui/jsonrpc/package.json index be19ab7217..bf4fe39118 100644 --- a/packages/ui/jsonrpc/package.json +++ b/packages/ui/jsonrpc/package.json @@ -28,6 +28,7 @@ "@deepseek-ai/dsh-agent": "^0.0.1", "@deepseek-ai/dsh-llm": "^0.0.1", "@deepseek-ai/dsh-llm-deepseek": "^0.0.1", + "@deepseek-ai/dsh-scope": "^0.0.1", "@deepseek-ai/dsh-session": "^0.0.1", "@deepseek-ai/dsh-subagent": "^0.0.1", "cordis": "^4.0.0-rc.6" @@ -38,6 +39,7 @@ "@deepseek-ai/dsh-agent-core": "workspace:^", "@deepseek-ai/dsh-llm": "workspace:^", "@deepseek-ai/dsh-llm-deepseek": "workspace:^", + "@deepseek-ai/dsh-scope": "workspace:^", "@deepseek-ai/dsh-session": "workspace:^", "@deepseek-ai/dsh-session-persistence-jsonl": "workspace:^", "@deepseek-ai/dsh-subagent": "workspace:^", diff --git a/packages/ui/jsonrpc/tests/built-scope-carrier.e2e.ts b/packages/ui/jsonrpc/tests/built-scope-carrier.e2e.ts new file mode 100644 index 0000000000..159600e49c --- /dev/null +++ b/packages/ui/jsonrpc/tests/built-scope-carrier.e2e.ts @@ -0,0 +1,121 @@ +/** + * Built-artifact guard for the scope carrier shared by `dsh-subagent` and + * `dsh-jsonrpc`. The carrier registry is module-local, so both bundles must + * externalize `dsh-scope`; source-mode tests cannot expose an accidentally + * inlined second registry. This test runs the real `lib/index.js` bundles in a + * plain Node subprocess, disposes the child before settlement, and requires the + * SDK completion notification to retain the delegating parent. + */ + +import { execFile } from 'node:child_process' +import { existsSync } from 'node:fs' +import { fileURLToPath } from 'node:url' +import { promisify } from 'node:util' +import { describe, expect, it } from 'vitest' + +const repoRoot = fileURLToPath(new URL('../../../../', import.meta.url)) +const jsonrpcBundle = fileURLToPath(new URL('../lib/index.js', import.meta.url)) +const execFileAsync = promisify(execFile) + +const builtRuntimeProbe = String.raw` +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; +import { pathToFileURL } from "node:url"; + +const load = (path) => import(pathToFileURL(resolve(path)).href); +const [ + { Context }, + agentCore, + { default: SubagentService }, + { default: SessionPersistenceJsonl }, + { HarnessSdkServer }, + { SessionId }, +] = await Promise.all([ + load("vendor/cordis/lib/index.js"), + load("packages/core/agent-core/lib/index.js"), + load("packages/subagent/subagent/lib/index.js"), + load("packages/session-persistence/session-persistence-jsonl/lib/index.js"), + load("packages/ui/jsonrpc/lib/index.js"), + load("packages/core/session/lib/index.js"), +]); + +const storageRoot = await mkdtemp(join(tmpdir(), "jsonrpc-built-scope-")); +const ctx = new Context(); +try { + await ctx.plugin(agentCore); + await ctx.plugin(SubagentService); + await ctx.plugin(SessionPersistenceJsonl, { root: storageRoot }); + await new Promise((ready) => setTimeout(ready, 50)); + + const notifications = []; + const server = new HarnessSdkServer(ctx, { + request() { return Promise.reject(new Error("unexpected host request")); }, + notify(method, params) { notifications.push({ method, params }); }, + }); + const parent = await ctx.agents.create({ + sessionId: SessionId("built-parent"), + meta: { cwd: storageRoot }, + agentOptions: { model: "test" }, + }); + const child = await ctx.agents.create({ + sessionId: SessionId("built-child"), + meta: { cwd: storageRoot, parentSession: SessionId("built-parent") }, + agentOptions: { model: "test" }, + }); + const result = Promise.withResolvers(); + const unregister = ctx.subagents.registerProvider({ + name: "built-local", + capabilities: { outputSchema: false, depthLimit: false, toolFilter: false, persona: false }, + inheritsParentContext: false, + start() { + return Promise.resolve({ + id: child.agent.id, + result: result.promise, + dispose() { return Promise.resolve(); }, + }); + }, + }); + const run = await ctx.subagents.start("built-local", { + parent: parent.agent, + prompt: [], + signal: new AbortController().signal, + }); + await child.dispose(); + result.resolve({ output: [], stopReason: "completed" }); + await run.result; + await Promise.resolve(); + + console.log(JSON.stringify(notifications.filter(({ method }) => method === "subagent.finished"))); + await run.dispose(); + unregister(); + await parent.dispose(); + await server.shutdown(); +} finally { + await ctx.fiber.dispose(); + await rm(storageRoot, { recursive: true, force: true }); +} +` + +describe.skipIf(!existsSync(jsonrpcBundle))('dsh-jsonrpc BUILT scope carrier', () => { + it('preserves parent-scoped completion after child disposal', async () => { + const { stdout, stderr } = await execFileAsync(process.execPath, ['--input-type=module', '-e', builtRuntimeProbe], { + cwd: repoRoot, + timeout: 15_000, + }) + + expect(stderr).not.toContain('listener threw') + expect(JSON.parse(stdout) as unknown).toEqual([{ + method: 'subagent.finished', + params: { + provider: 'built-local', + agentId: 'built-child', + parentSessionId: 'built-parent', + childSessionId: 'built-child', + status: 'ok', + stopReason: 'completed', + lastAssistantMessage: [], + }, + }]) + }) +}) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6214564eae..dec8c3024f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1288,6 +1288,9 @@ importers: '@deepseek-ai/dsh-llm-deepseek': specifier: workspace:^ version: link:../../llm/llm-deepseek + '@deepseek-ai/dsh-scope': + specifier: workspace:^ + version: link:../../core/scope '@deepseek-ai/dsh-session': specifier: workspace:^ version: link:../../core/session diff --git a/scripts/run-gates.ts b/scripts/run-gates.ts index 131501abb0..e1c41149ca 100644 --- a/scripts/run-gates.ts +++ b/scripts/run-gates.ts @@ -332,6 +332,7 @@ function builtBinSmokeGate(): Gate { 'vitest.e2e.config.ts', 'packages/ui/stdio-agent/tests/built-bin.e2e.ts', 'packages/ui/acp-agent/tests/built-bin.e2e.ts', + 'packages/ui/jsonrpc/tests/built-scope-carrier.e2e.ts', // The worker-entry packages' built bundles: the only automated proof // that lib/index.js resolves its sibling lib/worker.cjs under plain node // (the e2e lane runs unbuilt, so these files self-skip there). From 39c6c1120abdcc0414325bbdd1624917b0cb8624 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Tue, 14 Jul 2026 08:56:21 +0800 Subject: [PATCH 5/8] Revert "fix: share scope carrier across built JSON-RPC" This reverts commit 835fd3ca4f46e0c6464098e3d0864dc56f31398d. --- AGENTS.md | 2 +- docs/testing.md | 2 +- packages/ui/jsonrpc/package.json | 2 - .../jsonrpc/tests/built-scope-carrier.e2e.ts | 121 ------------------ pnpm-lock.yaml | 3 - scripts/run-gates.ts | 1 - 6 files changed, 2 insertions(+), 129 deletions(-) delete mode 100644 packages/ui/jsonrpc/tests/built-scope-carrier.e2e.ts diff --git a/AGENTS.md b/AGENTS.md index 1494d3fcf8..4dfbedb97b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -76,7 +76,7 @@ printf '%s\n' "$out" | grep -q '\[tool call\] echo({"text":"ci smoke"})' printf '%s\n' "$out" | grep -q '\[tool result\] ECHO: CI SMOKE' test -n "$(find .sessions -path '.sessions/cwd-*/main-session-*.jsonl' -type f -print -quit)" rm -rf .sessions -pnpm exec vitest run --config vitest.e2e.config.ts packages/ui/stdio-agent/tests/built-bin.e2e.ts packages/ui/acp-agent/tests/built-bin.e2e.ts packages/ui/jsonrpc/tests/built-scope-carrier.e2e.ts packages/workflow/workflow-workerthread/tests/built-worker.e2e.ts packages/code-runtime/code-runtime-worker/tests/built-lib.e2e.ts +pnpm exec vitest run --config vitest.e2e.config.ts packages/ui/stdio-agent/tests/built-bin.e2e.ts packages/ui/acp-agent/tests/built-bin.e2e.ts packages/workflow/workflow-workerthread/tests/built-worker.e2e.ts packages/code-runtime/code-runtime-worker/tests/built-lib.e2e.ts ``` `test:coverage`, not `test`, is the gating run ([why](docs/testing.md)); a sign-off counts only for commands actually run. diff --git a/docs/testing.md b/docs/testing.md index a1a841296a..23cca651fe 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -25,7 +25,7 @@ An e2e assertion re-runs the command or re-reads the file externally; a keyword - A plugin shipped via `cordis.yml` needs at least one test through the REAL Loader path: hand-built `ctx.plugin({...})` mounts bypass `unwrapExports` and cannot catch a broken export shape ([postmortem 0001](postmortem/0001-acp-default-export-drops-inject.md); export-shape rules in [packages/AGENTS.md](../packages/AGENTS.md)). - A guard only guards if the regression actually fails it. For a plugin without `inject` (bundle/composition plugins), a Loader smoke stays green under a broken export shape — add an explicit `expect('default' in mod).toBe(false)` plus an `unwrapExports` round-trip assertion, and prove it: introduce the regression, watch red, revert. -- "Real entry path" means the published artifact: a package `bin` runs built `lib/bin.js` under plain `node`, exposing failures tsx masks (settle races, module resolution, swallowed load failures). The same applies to non-index runtime entries (the worker-thread sibling `lib/worker.cjs`) and singleton modules shared across bundles (`packages/ui/jsonrpc/tests/built-scope-carrier.e2e.ts`). Keep the built-artifact smokes green (`packages/ui/*/tests/built-bin.e2e.ts`, `packages/code-runtime/code-runtime-worker/tests/built-lib.e2e.ts`), and assert a genuinely-missing config exits non-zero. +- "Real entry path" means the published artifact: the package `bin` points at built `lib/bin.js` under plain `node`, which tsx masks (settle races, module resolution, a swallowed load failure exiting 0). The same applies to any non-index runtime entry the built package resolves at run time (the worker-thread runtime's sibling `lib/worker.cjs`). Keep the built-artifact smokes green (`packages/ui/*/tests/built-bin.e2e.ts`, `packages/code-runtime/code-runtime-worker/tests/built-lib.e2e.ts`), and assert a genuinely-missing config exits non-zero. - An e2e that spawns an example from a temp cwd sets `TSX_TSCONFIG_PATH` to the repo-root tsconfig, or it silently falls back to stale built `lib/` ([examples/AGENTS.md](../examples/AGENTS.md)). ## When a snapshot test is required diff --git a/packages/ui/jsonrpc/package.json b/packages/ui/jsonrpc/package.json index bf4fe39118..be19ab7217 100644 --- a/packages/ui/jsonrpc/package.json +++ b/packages/ui/jsonrpc/package.json @@ -28,7 +28,6 @@ "@deepseek-ai/dsh-agent": "^0.0.1", "@deepseek-ai/dsh-llm": "^0.0.1", "@deepseek-ai/dsh-llm-deepseek": "^0.0.1", - "@deepseek-ai/dsh-scope": "^0.0.1", "@deepseek-ai/dsh-session": "^0.0.1", "@deepseek-ai/dsh-subagent": "^0.0.1", "cordis": "^4.0.0-rc.6" @@ -39,7 +38,6 @@ "@deepseek-ai/dsh-agent-core": "workspace:^", "@deepseek-ai/dsh-llm": "workspace:^", "@deepseek-ai/dsh-llm-deepseek": "workspace:^", - "@deepseek-ai/dsh-scope": "workspace:^", "@deepseek-ai/dsh-session": "workspace:^", "@deepseek-ai/dsh-session-persistence-jsonl": "workspace:^", "@deepseek-ai/dsh-subagent": "workspace:^", diff --git a/packages/ui/jsonrpc/tests/built-scope-carrier.e2e.ts b/packages/ui/jsonrpc/tests/built-scope-carrier.e2e.ts deleted file mode 100644 index 159600e49c..0000000000 --- a/packages/ui/jsonrpc/tests/built-scope-carrier.e2e.ts +++ /dev/null @@ -1,121 +0,0 @@ -/** - * Built-artifact guard for the scope carrier shared by `dsh-subagent` and - * `dsh-jsonrpc`. The carrier registry is module-local, so both bundles must - * externalize `dsh-scope`; source-mode tests cannot expose an accidentally - * inlined second registry. This test runs the real `lib/index.js` bundles in a - * plain Node subprocess, disposes the child before settlement, and requires the - * SDK completion notification to retain the delegating parent. - */ - -import { execFile } from 'node:child_process' -import { existsSync } from 'node:fs' -import { fileURLToPath } from 'node:url' -import { promisify } from 'node:util' -import { describe, expect, it } from 'vitest' - -const repoRoot = fileURLToPath(new URL('../../../../', import.meta.url)) -const jsonrpcBundle = fileURLToPath(new URL('../lib/index.js', import.meta.url)) -const execFileAsync = promisify(execFile) - -const builtRuntimeProbe = String.raw` -import { mkdtemp, rm } from "node:fs/promises"; -import { tmpdir } from "node:os"; -import { join, resolve } from "node:path"; -import { pathToFileURL } from "node:url"; - -const load = (path) => import(pathToFileURL(resolve(path)).href); -const [ - { Context }, - agentCore, - { default: SubagentService }, - { default: SessionPersistenceJsonl }, - { HarnessSdkServer }, - { SessionId }, -] = await Promise.all([ - load("vendor/cordis/lib/index.js"), - load("packages/core/agent-core/lib/index.js"), - load("packages/subagent/subagent/lib/index.js"), - load("packages/session-persistence/session-persistence-jsonl/lib/index.js"), - load("packages/ui/jsonrpc/lib/index.js"), - load("packages/core/session/lib/index.js"), -]); - -const storageRoot = await mkdtemp(join(tmpdir(), "jsonrpc-built-scope-")); -const ctx = new Context(); -try { - await ctx.plugin(agentCore); - await ctx.plugin(SubagentService); - await ctx.plugin(SessionPersistenceJsonl, { root: storageRoot }); - await new Promise((ready) => setTimeout(ready, 50)); - - const notifications = []; - const server = new HarnessSdkServer(ctx, { - request() { return Promise.reject(new Error("unexpected host request")); }, - notify(method, params) { notifications.push({ method, params }); }, - }); - const parent = await ctx.agents.create({ - sessionId: SessionId("built-parent"), - meta: { cwd: storageRoot }, - agentOptions: { model: "test" }, - }); - const child = await ctx.agents.create({ - sessionId: SessionId("built-child"), - meta: { cwd: storageRoot, parentSession: SessionId("built-parent") }, - agentOptions: { model: "test" }, - }); - const result = Promise.withResolvers(); - const unregister = ctx.subagents.registerProvider({ - name: "built-local", - capabilities: { outputSchema: false, depthLimit: false, toolFilter: false, persona: false }, - inheritsParentContext: false, - start() { - return Promise.resolve({ - id: child.agent.id, - result: result.promise, - dispose() { return Promise.resolve(); }, - }); - }, - }); - const run = await ctx.subagents.start("built-local", { - parent: parent.agent, - prompt: [], - signal: new AbortController().signal, - }); - await child.dispose(); - result.resolve({ output: [], stopReason: "completed" }); - await run.result; - await Promise.resolve(); - - console.log(JSON.stringify(notifications.filter(({ method }) => method === "subagent.finished"))); - await run.dispose(); - unregister(); - await parent.dispose(); - await server.shutdown(); -} finally { - await ctx.fiber.dispose(); - await rm(storageRoot, { recursive: true, force: true }); -} -` - -describe.skipIf(!existsSync(jsonrpcBundle))('dsh-jsonrpc BUILT scope carrier', () => { - it('preserves parent-scoped completion after child disposal', async () => { - const { stdout, stderr } = await execFileAsync(process.execPath, ['--input-type=module', '-e', builtRuntimeProbe], { - cwd: repoRoot, - timeout: 15_000, - }) - - expect(stderr).not.toContain('listener threw') - expect(JSON.parse(stdout) as unknown).toEqual([{ - method: 'subagent.finished', - params: { - provider: 'built-local', - agentId: 'built-child', - parentSessionId: 'built-parent', - childSessionId: 'built-child', - status: 'ok', - stopReason: 'completed', - lastAssistantMessage: [], - }, - }]) - }) -}) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index dec8c3024f..6214564eae 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1288,9 +1288,6 @@ importers: '@deepseek-ai/dsh-llm-deepseek': specifier: workspace:^ version: link:../../llm/llm-deepseek - '@deepseek-ai/dsh-scope': - specifier: workspace:^ - version: link:../../core/scope '@deepseek-ai/dsh-session': specifier: workspace:^ version: link:../../core/session diff --git a/scripts/run-gates.ts b/scripts/run-gates.ts index e1c41149ca..131501abb0 100644 --- a/scripts/run-gates.ts +++ b/scripts/run-gates.ts @@ -332,7 +332,6 @@ function builtBinSmokeGate(): Gate { 'vitest.e2e.config.ts', 'packages/ui/stdio-agent/tests/built-bin.e2e.ts', 'packages/ui/acp-agent/tests/built-bin.e2e.ts', - 'packages/ui/jsonrpc/tests/built-scope-carrier.e2e.ts', // The worker-entry packages' built bundles: the only automated proof // that lib/index.js resolves its sibling lib/worker.cjs under plain node // (the e2e lane runs unbuilt, so these files self-skip there). From 64a3933270a0f4636b36b485197597c66945599b Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Tue, 14 Jul 2026 19:03:07 +0800 Subject: [PATCH 6/8] docs: refresh config catalog after parent merge --- docs/config-catalog.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/config-catalog.md b/docs/config-catalog.md index 14cc973995..6f74792213 100644 --- a/docs/config-catalog.md +++ b/docs/config-catalog.md @@ -899,7 +899,7 @@ export interface Config { Depends on: [`AgentOptions`](../packages/core/agent/src/index.ts) -Source: [`packages/subagent/tool-subagent/src/index.ts:24`](../packages/subagent/tool-subagent/src/index.ts) +Source: [`packages/subagent/tool-subagent/src/index.ts:47`](../packages/subagent/tool-subagent/src/index.ts) ## `@deepseek-ai/dsh-tool-web` From ed3654da00fd14597b79e8e97bf61f09c5981ced Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Tue, 14 Jul 2026 23:38:53 +0800 Subject: [PATCH 7/8] docs: reconcile hidden internals with prose standard --- ...t-variables-and-tool-guidance-ownership.md | 16 ++--- .../subagent/subagent-subprocess/src/index.ts | 61 +++++-------------- packages/subagent/tool-subagent/src/index.ts | 49 +++------------ 3 files changed, 31 insertions(+), 95 deletions(-) diff --git a/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md b/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md index aae4564513..854807c109 100644 --- a/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md +++ b/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md @@ -24,21 +24,21 @@ The assembled system prompt had four defects, all of one family: facts the harne ### Prompt variables -Plugins contribute named values via `ctx.systemPrompt.variable(name, provider)`; prompt text references them as `{{name}}`. Providers are functions of the `AssembleContext` and may return `undefined` — "no value for THIS assembly". `assemble()` resolves every registered variable into `PromptAssembly.variables` (waterfall listeners can see, add, or override); `renderPrompt` interpolates. Rendering is STRICT — fail loud beats shipping a malformed prompt: a reference to an unregistered name throws (listing what exists; lookup is `Object.hasOwn`, so a prototype property like `{{constructor}}` is unknown, not a function spliced into the prompt), a registered-but-valueless reference throws, a complete `{{…}}` group that is not a well-formed name (`[a-z][a-z0-9_]*`, e.g. `{{ model }}`) throws, and a `{{` that opens no complete group while a `}}` still follows (`{{{model}}}`, `{{a{b}}`) throws. A lone `{{` with no `}}` anywhere after it is ordinary prose and passes through verbatim; substituted values are never re-scanned. Registration rejects duplicate and unreferenceable names, mirroring the tool registry — and `section()` now rejects duplicate section names, making the documented dedup real. +Plugins register `{{name}}` values through `ctx.systemPrompt.variable(name, provider)`. Assembly resolves them into the waterfall-visible variable map. Rendering rejects unknown own-property references, registered providers that return `undefined`, malformed complete references, and unbalanced references that still contain a closing `}}`; a lone unmatched `{{` remains prose, and substituted values are not rescanned. Registration rejects invalid or duplicate variable names, and section names are unique. `dsh-agent-loop` registers the two built-ins, both pure projections of the context agent: `model` (= `options.model`) and `cwd` (= `session.header.cwd`). The example personas write `powered by the {{model}} model` — the model name is stated once, in the `model:` config key. `{{cwd}}` is demonstrated in the ACP example only: every ACP session carries the client's cwd, while config-pre-created stdio agents have none (a persona claiming `{{cwd}}` there fails the turn — by design). The variables stay on the loop plugin (unlike the sections below): they are runtime facts of the agents THIS loop drives, and a replacement loop supplies its own. ### Persona as the order-0 section -`dsh-system-prompt` itself registers the two harness-owned sections (they must survive a swapped loop plugin, so they do NOT live on `dsh-agent-loop`): the static `harness:identity` at order `-100` — every prompt opens by stating the agent is powered by the DeepSeek Harness SDK — and the global default `deployment:persona` at order 0, whose text is the plugin's own `persona` config. `AgentOptions.systemPrompt` and the loop's special-case join are gone: `fullSystemPrompt ≡ renderPrompt(assembly)`, one ordered pipeline for everything the model sees, and `agent/pre-step` (compaction's token-pressure input) measures exactly the real prompt. An agent-scoped section with the same `deployment:persona` name shadows the default for that agent; programmatic setup may register one directly, and the subagent persona feature installs one before publishing an in-process child when the selected provider supports it. Order bands are convention: harness identity `-100`, persona `0`, tool guidance `100–199`; other negative orders also render before the persona. +`dsh-system-prompt` owns `harness:identity` at order `-100` and the configured `deployment:persona` at order 0, so both survive a replacement loop. Prompt rendering has one path, `renderPrompt(assembly)`, and `agent/pre-step` therefore measures the exact prompt used for compaction. An agent-scoped `deployment:persona` shadows the global default and lets subagent providers install a persona before publication. The conventional order bands are identity `-100`, persona `0`, and tool guidance `100–199`. ### Tool guidance ownership -Per-tool semantics and when-to-use live in tool DESCRIPTIONS, which already ship in every request — the YAML prose was ~fully redundant with them. Sections carry only the cross-call habits a single call's description cannot: `dsh-tool-bash` contributes `tool:bash` (order 105) — check the `[exit code: N]` marker on every result; `dsh-tool-fs`'s read section gains the "not shell commands like cat" contrast. `todo_write` and the subagent tools need NO section — their descriptions already carry the whole contract. The leaf personas shrink to identity + behavior (verify your work; keep answers brief), and the welcome banner stops enumerating tools. +Per-tool semantics and selection guidance live in tool descriptions. Prompt sections carry only cross-call habits, such as checking bash exit markers or preferring filesystem tools over shell commands. `todo_write` and subagent tools need no section because their descriptions contain the full contract. Deployment personas contain only role and behavior. ### The subagent conversation-history descriptor -`SubagentProvider` gains `readonly inheritsParentContext: boolean` — a DESCRIPTIVE conversation-history fact beside `capabilities`, not in it (capabilities are start-time validation; nothing validates against this flag). Spawn and ACP declare `false`, fork declares `true`. The name refers only to conversation seeding, not Cordis scope, services, tools, or authority. `dsh-tool-subagent` derives both the tool description and the `prompt` parameter description from the flag: the fork instance now tells the model the child is seeded with the conversation's completed turns (not the in-flight turn) and that its prompt should state only what is new. Deriving the description from a provider that arrives on its own fiber is what forced the provider-lifecycle events and the tool's reactive registration — that mechanism, its Loader-concurrency rationale, and its rejected alternatives are recorded in [the provider-lifecycle-events RFC](2026-07-05-subagent-provider-lifecycle-events.md). +`SubagentProvider.inheritsParentContext` describes conversation seeding, not scope, services, tools, or authority. Spawn and ACP set it to `false`; fork sets it to `true`. `dsh-tool-subagent` derives its tool and prompt-parameter descriptions from the flag, including that fork inherits completed turns but not the in-flight turn. Provider lifecycle events keep that wording synchronized with reactive provider registration; their rationale lives in the [provider-lifecycle-events RFC](2026-07-05-subagent-provider-lifecycle-events.md). ## Alternatives considered @@ -56,10 +56,10 @@ Per-tool semantics and when-to-use live in tool DESCRIPTIONS, which already ship ## Shipped invariants -- `renderPrompt(await assemble(assembleContextFor(agent)))` for the coding-agent example renders the harness identity, then the persona (with the agent's model name interpolated), then the fs/bash/web guidance sections; the loop has no other prompt-composition path. -- The `subagent_fork` schema description says the child inherits the conversation; the `subagent` one says it does not. The tool follows its provider: absent before the backend activates, present after, gone when the backend unloads, re-worded from the fresh provider on reload. -- Unknown/valueless/malformed/unbalanced `{{…}}` references throw with the section name in the message; duplicate section, variable, and tool-name registrations all throw. -- Snapshot goldens are prompt-independent by construction: llm-replay keys replay on (turn, step) chunk streams and never re-verifies the outgoing request. +- The coding-agent prompt renders identity, persona with the interpolated model, then fs/bash/web guidance through one assembly path. +- Fork and fresh subagent descriptions reflect whether the provider inherits completed conversation turns; the tool appears, disappears, and is reworded with provider lifecycle changes. +- Unknown, valueless, malformed, or unbalanced variable references name the section and throw; duplicate section, variable, and tool registrations also throw. +- Snapshot replay is prompt-independent: it keys recorded chunk streams by turn and step without comparing the outgoing request. ## Consequences diff --git a/packages/subagent/subagent-subprocess/src/index.ts b/packages/subagent/subagent-subprocess/src/index.ts index bd792e7f23..3831d2bb6a 100644 --- a/packages/subagent/subagent-subprocess/src/index.ts +++ b/packages/subagent/subagent-subprocess/src/index.ts @@ -1,19 +1,8 @@ /** - * Shared machinery for OUT-OF-PROCESS subagent backends — providers that spawn - * an external agent as a child process and must keep the parent deployment's - * credentials out of it, tear it down to quiescence, and isolate it from the - * host user's on-disk CLI state. The pieces: credential-shaped env scrubbing - * ({@link buildChildEnv}), spawn-failure capture ({@link spawnFailure}), - * bounded child-exit waits inside the stdin-EOF → SIGTERM → SIGKILL dispose - * ladder ({@link disposeChildProcess}), and the per-run isolated config dir - * ({@link createIsolatedConfigDir}). - * - * This package owns no provider and registers nothing; it is a pure library - * the out-of-process backend packages depend on (the `subagent-inprocess` - * shape, for the process boundary). Every tunable — the ladder's grace - * periods, a pinned config dir — is a PARAMETER here: defaults belong in each - * consuming plugin's Config, per the no-hardcoded-tunables rule. - * + * Shared machinery for OUT-OF-PROCESS subagent backends — providers that spawn an external + * agent as a child process and must keep the parent deployment's credentials out of it, tear + * it down to quiescence, and isolate it from the host user's on-disk CLI state. This package + * registers no provider; consuming plugins own and validate every timing or path default. * @module @deepseek-ai/dsh-subagent-subprocess */ @@ -50,11 +39,8 @@ export function buildChildEnv(extra: Record): NodeJS.ProcessEnv } /** - * Capture the child's spawn-level failure as a promise the run's result path - * can race. A spawn failure (e.g. `ENOENT` for a bad command) is emitted as an - * `error` EVENT, not a thrown exception — and without a listener Node treats - * it as an unhandled error and crashes the parent process. Call this in the - * SAME TICK as `spawn()`, so no window exists for the event to fire unheard. + * Capture the child's spawn-level `error` event as a promise. Call in the same tick as + * `spawn()`; otherwise an early event can be unhandled and crash the parent. * @param child - the just-spawned child process. * @returns a promise that RESOLVES (never rejects) with the child's first * `error` event; for a child that spawns cleanly it never settles. @@ -123,15 +109,8 @@ export interface DisposeLadderGraces { } /** - * Tear a child process down to QUIESCENCE: resolves only once the child has - * actually exited (or was already gone), never merely after requesting it. - * Three-tier escalation — - * - * 1. stdin EOF (when stdin is piped), then wait `disposeEofGraceMs`: a - * cooperative child quiesces on its own, its teardown and flushes intact; - * 2. `SIGTERM`, then wait `disposeGraceMs`; - * 3. `SIGKILL`, then await the (now-certain) exit — a child that ignores EOF - * and traps `SIGTERM` must not wedge dispose forever. + * Tear a child process down to quiescence, resolving only after exit: close stdin and allow + * cooperative flush, then send `SIGTERM`, then `SIGKILL` and await the forced exit. * * @param child - the child process to tear down. * @param graces - the two grace periods, from the consuming plugin's Config. @@ -139,10 +118,7 @@ export interface DisposeLadderGraces { export async function disposeChildProcess(child: ChildProcess, graces: DisposeLadderGraces): Promise { // Already gone: nothing to reap. if (child.exitCode !== null || child.signalCode !== null) return - // 1. Graceful: end the request stream (stdin EOF) and let the child quiesce - // on its own. Sending SIGTERM in the same tick (or too soon) would - // default-terminate a cooperative child mid-flush, orphaning its nested - // work. A child spawned without a stdin pipe skips straight to the wait. + // 1. Close stdin and allow cooperative teardown and durable-state flush. child.stdin?.end() if (await exitsWithin(child, graces.disposeEofGraceMs)) return // 2. SIGTERM, escalating if the child still does not exit within the grace. @@ -171,16 +147,9 @@ export interface IsolatedConfigDir { } /** - * An isolated config dir for one child run, so the child's behavior is a - * function of deployment config alone — never of whatever `~/.claude` / - * `~/.codex`-style state happens to exist on the host machine. Two modes: - * - * - no `pinnedPath` (the default): creates a FRESH private (0700) `mkdtemp` - * dir under the OS temp root; {@link IsolatedConfigDir.remove} deletes it - * best-effort; - * - `pinnedPath` set (a deployment deliberately sharing state across runs): - * the pinned path is returned as-is — never created, never removed — the - * deployment owns that directory's lifecycle. + * An isolated config dir for one child run, independent of host CLI state. Without + * `pinnedPath`, creates a private temp directory and removes it best-effort; a pinned directory + * is returned unchanged and remains deployment-owned. * * @param prefix - the `mkdtemp` name prefix for a fresh dir (e.g. * `dsh-subagent-codex-`); ignored when `pinnedPath` is set. @@ -207,10 +176,8 @@ export async function createIsolatedConfigDir(prefix: string, pinnedPath?: strin try { await rm(path, { recursive: true, force: true }) } catch { - // Best-effort by contract: swallows rm failures (EACCES/EBUSY-style — - // e.g. the dead child left an unreadable entry behind). The dir lives - // under the OS temp root, which reclaims it; failing dispose over - // cleanup would be worse than a leftover temp dir. + // Best-effort by contract: swallows rm failures (EACCES/EBUSY-style — e.g. the dead + // child left an unreadable entry behind). } }, } diff --git a/packages/subagent/tool-subagent/src/index.ts b/packages/subagent/tool-subagent/src/index.ts index ff54bc109a..59821147af 100644 --- a/packages/subagent/tool-subagent/src/index.ts +++ b/packages/subagent/tool-subagent/src/index.ts @@ -1,34 +1,11 @@ /** - * The model-facing `subagent` tool: delegate a task to a child agent and return - * its final output. Pure schema + lifecycle shaping — every transport concern - * lives behind the `ctx.subagents` provider registry - * (`@deepseek-ai/dsh-subagent`), so an in-process, ACP, or future A2A backend - * swaps in without touching what the model sees. - * - * Provider selection is config, not model-facing: this plugin is bound to - * EXACTLY ONE provider name (`Config.provider`). To expose more than one - * transport, load the plugin more than once, each bound to a different provider - * — there is no provider/type parameter in the model-facing schema. The model - * sees only `{ description, prompt }`. - * - * The tool DESCRIPTION is derived from the bound provider's conversation-history - * descriptor ({@link SubagentProvider.inheritsParentContext}): a - * fresh-conversation provider (spawn, ACP) gets the standalone-prompt wording, - * while a seeded-conversation provider (fork) tells the model the child already - * sees the conversation's completed turns. This descriptor says nothing about - * Cordis scope, services, tools, or authority. The tool MIRRORS the - * provider's lifecycle via `subagent/provider-added`/`-removed` — it registers - * when the provider is (or becomes) available and unregisters when the - * provider goes away — so no load-order requirement exists and an HMR reload - * of the backend re-derives the wording from the fresh provider. - * - * Collection is SYNCHRONOUS this cut: `execute` starts a run and awaits - * `run.result` inside a `try/finally` that always disposes the run, so the - * owned child agent/session is torn down on every path (success, error, abort) - * and never leaks as a live idle child. A non-`completed` stop reason maps to an - * `isError` tool result (by throwing) rather than returning partial output as - * success. + * Model-facing delegation tool bound by configuration to one provider; transport selection is not + * exposed in its `{ description, prompt }` schema. Provider lifecycle controls registration and + * re-derives conversation-history wording after reload, so load order is irrelevant. * + * Execution synchronously awaits the child result and always disposes the run. Non-completed stop + * reasons become error results, while transport details remain behind `ctx.subagents`. Load this + * plugin more than once to expose multiple configured providers. * @module @deepseek-ai/dsh-tool-subagent */ @@ -105,16 +82,8 @@ export const Config: z = z.object({ model: z.string(), }).default(undefined as unknown as { model: string }), persona: z.string(), - // A schemastery object materializes {} (with [] for nested arrays) when the - // key is omitted — for toolFilter that would mean an EMPTY ALLOW-LIST, i.e. - // deny-everything, silently. Force the omitted key to stay absent (the same - // shape discipline as SystemPrompt's toolOrder); the cast is needed because - // .default() expects the object type. - // The NESTED arrays get the same treatment as the object itself: a partial - // filter ({deny: […]}) must not materialize allow: [] beside it — an empty - // allow-list means deny-EVERYTHING, so the materialized default would turn - // a deny-one config into deny-all. An EXPLICIT allow: [] (grant-only - // children) survives, since only the omitted key defaults to undefined. + // Schemastery otherwise materializes omitted objects and nested arrays as `{ allow: [] }`, which + // silently means deny all. Preserve omission while retaining an explicit empty allow-list. toolFilter: z.object({ allow: z.array(z.string()).default(undefined as unknown as string[]), deny: z.array(z.string()).default(undefined as unknown as string[]), @@ -290,7 +259,7 @@ export function apply(ctx: Context, config: Config): void { if (present !== undefined) { mount(present) } else { - // Not an error: the backend's fiber may simply activate after this one. + // Not an error: the backend's fiber may activate after this one. // The tool appears the moment the provider registers; a typo'd provider // name shows up as this note plus a tool that never materializes. ctx.logger.info(`subagent provider "${config.provider}" not registered yet; the "${config.toolName ?? 'subagent'}" tool will register when it appears`) From cb177090f9573b30947af31daa876d586bea844a Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Tue, 14 Jul 2026 23:53:23 +0800 Subject: [PATCH 8/8] docs: refresh subagent config catalog --- docs/config-catalog.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/config-catalog.md b/docs/config-catalog.md index 093e534d23..d9e42366ba 100644 --- a/docs/config-catalog.md +++ b/docs/config-catalog.md @@ -915,7 +915,7 @@ export interface Config { Depends on: [`AgentOptions`](../packages/core/agent/src/index.ts) -Source: [`packages/subagent/tool-subagent/src/index.ts:47`](../packages/subagent/tool-subagent/src/index.ts) +Source: [`packages/subagent/tool-subagent/src/index.ts:24`](../packages/subagent/tool-subagent/src/index.ts) ## `@deepseek-ai/dsh-tool-web`