From fced51d4ebb35bd105cb10e0d9196d7cc5325b04 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Mon, 27 Jul 2026 01:04:37 +0800 Subject: [PATCH] fix(subagent): route explicit DSH_* config entries onto the managed env channel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The acp run passed config.env wholesale into the spawn spec's ordinary channel, which rejects the reserved DSH_* namespace — a deployment fact like DSH_PERMISSION_MODE (the acp-agent example's own knob, used by the with-key e2e) crashed the spawn. The run now splits DSH_* entries onto dshEnv, where the scrubbed base expects current facts to arrive. New layering test drives the split through the real seam via a MOCK_ECHO_ENV knob on the mock server; README env prose updated (en+zh, re-recorded). --- .../subagent/subagent-acp/README.i18n.yaml | 4 ++-- packages/subagent/subagent-acp/README.md | 2 +- packages/subagent/subagent-acp/README.zh.md | 2 +- packages/subagent/subagent-acp/src/run.ts | 20 ++++++++++++++++--- .../subagent-acp/tests/mock-acp-server.ts | 10 +++++++++- .../subagent-acp/tests/subagent-acp.spec.ts | 15 ++++++++++++++ 6 files changed, 45 insertions(+), 8 deletions(-) diff --git a/packages/subagent/subagent-acp/README.i18n.yaml b/packages/subagent/subagent-acp/README.i18n.yaml index 10d4b9a7cb..61f41f5665 100644 --- a/packages/subagent/subagent-acp/README.i18n.yaml +++ b/packages/subagent/subagent-acp/README.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write -README.md: 317517f64f24d8a3ed01ebae08dcfd13668b9029 -README.zh.md: e10f435b13e7cdabb92ebfa5b4a5d0763f2af18f +README.md: 0c21e179bd5ca405057b682786c41c8447a78317 +README.zh.md: b349443cfcda80caa9b06cfa1cf80f6e780a98be diff --git a/packages/subagent/subagent-acp/README.md b/packages/subagent/subagent-acp/README.md index 317517f64f..0c21e179bd 100644 --- a/packages/subagent/subagent-acp/README.md +++ b/packages/subagent/subagent-acp/README.md @@ -57,7 +57,7 @@ ACP advertises no start-time capabilities because this process cannot enforce th ## Process boundary -The child spawns through the [`dsh-subprocess`](../../subprocess/subprocess/README.md) seam: credential-shaped ambient variables are removed by the shared scrub, then explicit `config.env` values merge after it (an intended `DEEPSEEK_API_KEY` survives), stderr is inherited to the parent's own stream, and disposal runs the seam's cooperative stdin-EOF→SIGTERM→SIGKILL ladder with this plugin's configured graces. The ACP wire is the real serialization boundary; same-process subagent values are not defensively cloned. +The child spawns through the [`dsh-subprocess`](../../subprocess/subprocess/README.md) seam: credential-shaped ambient variables are removed by the shared scrub, then explicit `config.env` values merge after it (an intended `DEEPSEEK_API_KEY` survives; `DSH_*` entries such as `DSH_PERMISSION_MODE` ride the seam's managed channel, since the scrubbed base reserves that namespace for current deployment facts), stderr is inherited to the parent's own stream, and disposal runs the seam's cooperative stdin-EOF→SIGTERM→SIGKILL ladder with this plugin's configured graces. The ACP wire is the real serialization boundary; same-process subagent values are not defensively cloned. The package has no default export. Cordis loader unwrapping would otherwise hide the named `inject` metadata; see [postmortem 0001](../../../docs/postmortem/0001-acp-default-export-drops-inject.md). diff --git a/packages/subagent/subagent-acp/README.zh.md b/packages/subagent/subagent-acp/README.zh.md index e10f435b13..b349443cfc 100644 --- a/packages/subagent/subagent-acp/README.zh.md +++ b/packages/subagent/subagent-acp/README.zh.md @@ -57,7 +57,7 @@ ACP 不声明任何启动时能力，因为当前进程无法强制执行远程 ## 进程边界 -子进程经由 [`dsh-subprocess`](../../subprocess/subprocess/README.md) seam spawn：共享的凭据清除先移除名称形似凭据的环境变量，显式 `config.env` 值在清除之后合并（有意转发的 `DEEPSEEK_API_KEY` 会保留下来），stderr 以 inherit 方式直通父进程自身的流，dispose 则以本插件配置的宽限期运行该 seam 的协作式 stdin EOF→SIGTERM→SIGKILL 阶梯。ACP 协议是真正的序列化边界；同进程 subagent 值不会为防御目的而克隆。 +子进程经由 [`dsh-subprocess`](../../subprocess/subprocess/README.md) seam spawn：共享的凭据清除先移除名称形似凭据的环境变量，显式 `config.env` 值在清除之后合并（有意转发的 `DEEPSEEK_API_KEY` 会保留下来；`DSH_PERMISSION_MODE` 这类 `DSH_*` 条目走该 seam 的受管通道，因为清除后的基底把这一命名空间保留给当前部署事实），stderr 以 inherit 方式直通父进程自身的流，dispose 则以本插件配置的宽限期运行该 seam 的协作式 stdin EOF→SIGTERM→SIGKILL 阶梯。ACP 协议是真正的序列化边界；同进程 subagent 值不会为防御目的而克隆。 本包没有默认导出。否则 Cordis loader 的解包会隐藏具名 `inject` 元数据；见[事故复盘 0001](../../../docs/postmortem/0001-acp-default-export-drops-inject.md)。 diff --git a/packages/subagent/subagent-acp/src/run.ts b/packages/subagent/subagent-acp/src/run.ts index 85f1b9fdde..d2fc52d876 100644 --- a/packages/subagent/subagent-acp/src/run.ts +++ b/packages/subagent/subagent-acp/src/run.ts @@ -25,7 +25,8 @@ import { import type { ContentBlock } from '@deepseek-ai/dsh-llm' import { SessionId } from '@deepseek-ai/dsh-session' import type { SubagentResult, SubagentRun, SubagentStartRequest, SubagentStopReason } from '@deepseek-ai/dsh-subagent' -import type { SubprocessHandle, SubprocessSpawnSpec } from '@deepseek-ai/dsh-subprocess' +import { DSH_ENV_PREFIX } from '@deepseek-ai/dsh-subprocess' +import type { DshEnvironmentKey, SubprocessHandle, SubprocessSpawnSpec } from '@deepseek-ai/dsh-subprocess' /** Fixed response to child permission requests: reject by default, or select the first allow option. */ export type PermissionPolicy = 'allow' | 'reject' @@ -49,6 +50,9 @@ export interface AcpRunSpec { * `DEEPSEEK_API_KEY`). Merged on top of the subprocess seam's scrubbed * parent env. A value here is forwarded even if its name matches the * credential-scrub pattern (an explicit opt-in for the child's own creds). + * Explicit `DSH_*` entries are deployment-owned facts for the child harness + * (e.g. `DSH_PERMISSION_MODE`) and ride the seam's managed channel, which + * the scrubbed base reserves for current values. */ env: Record /** @@ -166,13 +170,23 @@ export async function startAcpRun(request: SubagentStartRequest, spec: AcpRunSpe // Keep diagnostics on parent stderr ('inherit'); only ACP output contributes // to the result. The seam's scrub drops ambient credentials while spec.env - // (the child's own key) merges after it. + // (the child's own key) merges after it. Explicit DSH_* entries are the + // deployment's facts for the child and take the managed channel — the + // ordinary channel rejects that reserved namespace. + const env: Record = {} + const dshEnv: Record = {} + const isDshKey = (key: string): key is DshEnvironmentKey => key.startsWith(DSH_ENV_PREFIX) + for (const [key, value] of Object.entries(spec.env)) { + if (isDshKey(key)) dshEnv[key] = value + else env[key] = value + } const child = spec.spawn({ argv: [spec.command, ...spec.args], cwd: spec.cwd, stdio: { stdin: 'pipe', stdout: 'pipe', stderr: 'inherit' }, graceMs: spec.disposeGraceMs, - env: spec.env, + env, + dshEnv, }) /* v8 ignore start -- 'pipe' dispositions expose both streams by the seam contract; defensive. */ if (child.stdin === undefined || child.stdout === undefined) { diff --git a/packages/subagent/subagent-acp/tests/mock-acp-server.ts b/packages/subagent/subagent-acp/tests/mock-acp-server.ts index 6b3f8157e8..de5900906a 100644 --- a/packages/subagent/subagent-acp/tests/mock-acp-server.ts +++ b/packages/subagent/subagent-acp/tests/mock-acp-server.ts @@ -4,6 +4,9 @@ * fully scripted by environment variables — no model, no network: * * - `MOCK_TEXT` — the assistant text it streams as one `agent_message_chunk`. + * - `MOCK_ECHO_ENV` — if set to a variable NAME, stream that variable's value + * (or ``) instead of MOCK_TEXT — asserts what + * environment actually reached the child process. * - `MOCK_STOP` — the ACP `StopReason` it returns from `prompt` * (`end_turn` default, or `max_tokens`/`refusal`/…). * - `MOCK_HANG` — if `1`, `prompt` never resolves on its own (it waits for @@ -67,7 +70,12 @@ import { type StopReason, } from '@agentclientprotocol/sdk' -const TEXT = process.env.MOCK_TEXT ?? 'mock child answer' +// When MOCK_ECHO_ENV names a variable, stream that variable's value in place +// of MOCK_TEXT — lets a test assert exactly what env reached this process. +const echoEnvName = process.env.MOCK_ECHO_ENV +const TEXT = echoEnvName !== undefined + ? process.env[echoEnvName] ?? `<${echoEnvName} unset>` + : process.env.MOCK_TEXT ?? 'mock child answer' const ECHO_CWD = process.env.MOCK_ECHO_CWD === '1' const STOP = (process.env.MOCK_STOP ?? 'end_turn') as StopReason const HANG = process.env.MOCK_HANG === '1' diff --git a/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts b/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts index 919e736cb8..7c10463000 100644 --- a/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts +++ b/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts @@ -119,6 +119,21 @@ describe('child env layering (through the subprocess seam)', () => { delete process.env.ACP_TEST_AMBIENT_SECRET_TOKEN } }) + + it('routes explicit DSH_* config entries onto the managed channel', async () => { + // A deployment sets child-harness facts like DSH_PERMISSION_MODE in + // config.env; the run must split them onto the seam's managed channel + // (the ordinary channel rejects the reserved namespace) and the child + // must still see the value. + const ctx = await setup({ MOCK_ECHO_ENV: 'DSH_ACP_TEST_FACT', DSH_ACP_TEST_FACT: 'managed' }) + const parent = { id: 'parent', session: { header: { cwd: process.cwd() } } } as unknown as Agent + const run = await ctx.subagents.start('acp', { prompt: [{ type: 'text' as const, text: 'p' }], parent, signal: new AbortController().signal }) + const result = await run.result + await run.dispose() + const text = result.output.filter(b => b.type === 'text').map(b => (b as { text: string }).text).join('') + expect(text).toBe('managed') + await ctx.fiber.dispose() + }) }) describe('cwd resolution', () => {