From cdd1ce2ad4ab5851de949f13ba085b306b8e70b2 Mon Sep 17 00:00:00 2001 From: pku-xht Date: Wed, 8 Jul 2026 15:31:14 +0800 Subject: [PATCH 01/13] feat(subagent): extract dsh-subagent-process shared out-of-process machinery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The credential env scrub (SENSITIVE_ENV_PATTERN/buildChildEnv), the spawn-failure capture (spawnFailure), the child-exit waits (waitForExit/exitsWithin), and the stdin-EOF -> SIGTERM -> SIGKILL dispose ladder move out of subagent-acp into a new pure library package (the subagent-inprocess shape), with the ladder taking its two grace periods as parameters — defaults stay in the plugin Config. New isolated-config-dir helpers (mkdtemp create, best-effort remove; a pinned dir is never removed) land alongside for the CLAUDE_CONFIG_DIR / CODEX_HOME redirection the RFC names. The ACP backend migrates onto the library with no semantic change: its suite passes with import-path edits only. bash-local keeps its sibling copy, per the RFC's blast-radius call. RFC: docs/rfc/proposed/feature/2026-07-07-claude-code-and-codex-subagent-backends.md --- docs/config-catalog.md | 1 + docs/module-graph.md | 5 +- knip.json | 5 + packages/subagent/README.md | 3 +- packages/subagent/subagent-acp/package.json | 2 + packages/subagent/subagent-acp/src/run.ts | 98 ++---- .../subagent-acp/tests/subagent-acp.spec.ts | 3 +- packages/subagent/subagent-acp/tsconfig.json | 3 + packages/subagent/subagent-process/README.md | 40 +++ .../subagent/subagent-process/package.json | 30 ++ .../subagent/subagent-process/src/index.ts | 207 ++++++++++++ .../tests/subagent-process.spec.ts | 307 ++++++++++++++++++ .../subagent/subagent-process/tsconfig.json | 11 + pnpm-lock.yaml | 9 + tsconfig.build.json | 1 + tsconfig.json | 1 + 16 files changed, 645 insertions(+), 81 deletions(-) create mode 100644 packages/subagent/subagent-process/README.md create mode 100644 packages/subagent/subagent-process/package.json create mode 100644 packages/subagent/subagent-process/src/index.ts create mode 100644 packages/subagent/subagent-process/tests/subagent-process.spec.ts create mode 100644 packages/subagent/subagent-process/tsconfig.json diff --git a/docs/config-catalog.md b/docs/config-catalog.md index e55066101f..c01d42f87d 100644 --- a/docs/config-catalog.md +++ b/docs/config-catalog.md @@ -816,3 +816,4 @@ Imported as libraries by other packages; a `cordis.yml` cannot load them. - `@deepseek-ai/dsh-brand` ([`packages/util/brand/src/index.ts`](../packages/util/brand/src/index.ts)) - `@deepseek-ai/dsh-hook-protocol` ([`packages/hooks/hook-protocol/src/index.ts`](../packages/hooks/hook-protocol/src/index.ts)) - `@deepseek-ai/dsh-subagent-inprocess` ([`packages/subagent/subagent-inprocess/src/index.ts`](../packages/subagent/subagent-inprocess/src/index.ts)) +- `@deepseek-ai/dsh-subagent-process` ([`packages/subagent/subagent-process/src/index.ts`](../packages/subagent/subagent-process/src/index.ts)) diff --git a/docs/module-graph.md b/docs/module-graph.md index 5e043d22d4..cadf8d0d14 100644 --- a/docs/module-graph.md +++ b/docs/module-graph.md @@ -43,6 +43,7 @@ flowchart TD pkg_subagent_acp["subagent-acp"] pkg_subagent_fork["subagent-fork"] pkg_subagent_inprocess["subagent-inprocess"] + pkg_subagent_process["subagent-process"] pkg_subagent_spawn["subagent-spawn"] pkg_tool_subagent["tool-subagent"] end @@ -171,6 +172,7 @@ flowchart TD pkg_subagent_acp --> pkg_agent pkg_subagent_acp --> pkg_llm pkg_subagent_acp --> pkg_subagent + pkg_subagent_acp --> pkg_subagent_process pkg_subagent_inprocess --> pkg_agent pkg_subagent_inprocess --> pkg_llm pkg_subagent_inprocess --> pkg_session @@ -211,6 +213,7 @@ flowchart TD | Package | Group | Depends on | | --- | --- | --- | | [`brand`](../packages/util/brand) | `util` | — | +| [`subagent-process`](../packages/subagent/subagent-process) | `subagent` | — | | [`acp-snapshot`](../packages/support/acp-snapshot) | `support` | — | | [`app-boot`](../packages/ui/app-boot) | `ui` | — | | [`code-runtime`](../packages/code-runtime/code-runtime) | `code-runtime` | — | @@ -248,7 +251,7 @@ flowchart TD | [`hooks-codex`](../packages/hooks/hooks-codex) | `hooks` | [`agent`](../packages/core/agent), [`hook-protocol`](../packages/hooks/hook-protocol), [`llm`](../packages/llm/llm), [`session`](../packages/core/session), [`tools`](../packages/core/tools) | | [`acp`](../packages/ui/acp) | `ui` | [`agent`](../packages/core/agent), [`llm`](../packages/llm/llm), [`session`](../packages/core/session), [`session-persistence`](../packages/session-persistence/session-persistence), [`tools`](../packages/core/tools) | | [`agent-core`](../packages/core/agent-core) | `core` | [`agent`](../packages/core/agent), [`agent-loop`](../packages/core/agent-loop), [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm), [`session`](../packages/core/session), [`system-prompt`](../packages/core/system-prompt), [`tool-bash`](../packages/bash/tool-bash), [`tools`](../packages/core/tools) | -| [`subagent-acp`](../packages/subagent/subagent-acp) | `subagent` | [`agent`](../packages/core/agent), [`llm`](../packages/llm/llm), [`subagent`](../packages/subagent/subagent) | +| [`subagent-acp`](../packages/subagent/subagent-acp) | `subagent` | [`agent`](../packages/core/agent), [`llm`](../packages/llm/llm), [`subagent`](../packages/subagent/subagent), [`subagent-process`](../packages/subagent/subagent-process) | | [`subagent-inprocess`](../packages/subagent/subagent-inprocess) | `subagent` | [`agent`](../packages/core/agent), [`llm`](../packages/llm/llm), [`session`](../packages/core/session), [`subagent`](../packages/subagent/subagent), [`system-prompt`](../packages/core/system-prompt), [`tools`](../packages/core/tools) | | [`tool-subagent`](../packages/subagent/tool-subagent) | `subagent` | [`agent`](../packages/core/agent), [`llm`](../packages/llm/llm), [`subagent`](../packages/subagent/subagent), [`tools`](../packages/core/tools) | | [`hooks-claude`](../packages/hooks/hooks-claude) | `hooks` | [`agent`](../packages/core/agent), [`hook-protocol`](../packages/hooks/hook-protocol), [`llm`](../packages/llm/llm), [`session`](../packages/core/session), [`subagent`](../packages/subagent/subagent), [`tools`](../packages/core/tools) | diff --git a/knip.json b/knip.json index 8c0f71f3af..1ab6d39a9e 100644 --- a/knip.json +++ b/knip.json @@ -66,6 +66,11 @@ "entry": ["tests/**/*.spec.ts", "tests/**/*.e2e.ts", "tests/mock-acp-server.ts"], "project": ["src/**/*.ts", "tests/**/*.ts"] }, + "packages/subagent/subagent-process": { + "entry": ["tests/**/*.spec.ts"], + "project": ["src/**/*.ts", "tests/**/*.ts"], + "ignoreDependencies": ["cordis"] + }, "packages/fs/tool-fs": { "entry": ["tests/**/*.spec.ts", "tests/**/*.e2e.ts"], "project": ["src/**/*.ts", "tests/**/*.ts"] diff --git a/packages/subagent/README.md b/packages/subagent/README.md index 6da8ee42f5..38c64d22a5 100644 --- a/packages/subagent/README.md +++ b/packages/subagent/README.md @@ -8,9 +8,10 @@ The subagent seam: an agent delegating work to a child agent. Like the [bash](.. | `subagent-inprocess/` | Shared in-process run driver (pure lib; registers nothing) | — | | `subagent-spawn/` | In-process backend: a fresh child agent | (registers on `ctx.subagents`) | | `subagent-fork/` | In-process backend: a child seeded with the parent's completed-turn prefix | (registers on `ctx.subagents`) | +| `subagent-process/` | Shared out-of-process machinery: env scrub, dispose ladder, isolated config dirs (pure lib; registers nothing) | — | | `subagent-acp/` | Out-of-process backend: a child agent in a spawned subprocess, driven over ACP | (registers on `ctx.subagents`) | | `tool-subagent/` | Model-facing `subagent` delegation tool over `ctx.subagents` | (registers on `ctx.tools`) | -The interface lives at `subagent/subagent/`. The in-process `subagent-spawn` / `subagent-fork` backends share the `subagent-inprocess` driver (a pure library — both depend on it, neither on the other), and the out-of-process `subagent-acp` backend ships alongside them here; the test-only `dsh-subagent-mock` (in [support](../support/README.md)) is separate. All **product** packages except the mock. +The interface lives at `subagent/subagent/`. The in-process `subagent-spawn` / `subagent-fork` backends share the `subagent-inprocess` driver (a pure library — both depend on it, neither on the other), the out-of-process `subagent-acp` backend builds on the `subagent-process` library (the credential env scrub, the dispose ladder, isolated config dirs) and ships alongside them here; the test-only `dsh-subagent-mock` (in [support](../support/README.md)) is separate. All **product** packages except the mock. The proposal and design rationale: [docs/rfc/implemented/feature/2026-06-21-subagent-capability-seam.md](../../docs/rfc/implemented/feature/2026-06-21-subagent-capability-seam.md). diff --git a/packages/subagent/subagent-acp/package.json b/packages/subagent/subagent-acp/package.json index 2c55051da9..45fc6b072c 100644 --- a/packages/subagent/subagent-acp/package.json +++ b/packages/subagent/subagent-acp/package.json @@ -25,6 +25,7 @@ "@deepseek-ai/dsh-agent": "^0.0.1", "@deepseek-ai/dsh-llm": "^0.0.1", "@deepseek-ai/dsh-subagent": "^0.0.1", + "@deepseek-ai/dsh-subagent-process": "^0.0.1", "cordis": "^4.0.0-rc.6" }, "dependencies": { @@ -35,6 +36,7 @@ "@deepseek-ai/dsh-agent": "workspace:^", "@deepseek-ai/dsh-llm": "workspace:^", "@deepseek-ai/dsh-subagent": "workspace:^", + "@deepseek-ai/dsh-subagent-process": "workspace:^", "@cordisjs/plugin-loader": "^1.0.0-rc.4", "cordis": "^4.0.0-rc.6" } diff --git a/packages/subagent/subagent-acp/src/run.ts b/packages/subagent/subagent-acp/src/run.ts index d7631f4d8d..a94c923f46 100644 --- a/packages/subagent/subagent-acp/src/run.ts +++ b/packages/subagent/subagent-acp/src/run.ts @@ -22,7 +22,7 @@ * @module @deepseek-ai/dsh-subagent-acp/run */ -import { spawn, type ChildProcess } from 'node:child_process' +import { spawn } from 'node:child_process' import { randomUUID } from 'node:crypto' import { Readable, Writable } from 'node:stream' import { @@ -40,6 +40,7 @@ import { import { AgentId } from '@deepseek-ai/dsh-agent' import type { ContentBlock } from '@deepseek-ai/dsh-llm' import type { SubagentResult, SubagentRun, SubagentStartRequest, SubagentStopReason } from '@deepseek-ai/dsh-subagent' +import { buildChildEnv, disposeChildProcess, spawnFailure } from '@deepseek-ai/dsh-subagent-process' /** * How the client answers a child's `session/request_permission`. The first cut @@ -110,31 +111,6 @@ export const DEFAULT_DISPOSE_EOF_GRACE_MS = 6_000 /** Default grace between SIGTERM and SIGKILL on dispose (the `disposeGraceMs` config; mirrors the bash executor). */ export const DEFAULT_DISPOSE_GRACE_MS = 3_000 -/** - * Credential-shaped ambient env vars are NOT forwarded to the child by default - * (the parent harness's own `DEEPSEEK_API_KEY`/secrets must not leak into a - * spawned process implicitly). Same pattern as the bash executor. The child - * agent needs its OWN credentials to reach a model — those are supplied - * explicitly via {@link AcpRunSpec.env}, which is layered on top AFTER 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 - -/** - * The ambient env minus credential-shaped vars, plus the spec's explicit env. - * @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. - */ -export function buildChildEnv(extra: Record): NodeJS.ProcessEnv { - const env: NodeJS.ProcessEnv = {} - for (const [key, value] of Object.entries(process.env)) { - if (!SENSITIVE_ENV_PATTERN.test(key)) env[key] = value - } - return { ...env, ...extra } -} - /** * Map an ACP {@link StopReason} to a harness {@link SubagentStopReason}. * @param reason - the terminal reason from the child's `session/prompt` response. @@ -196,24 +172,6 @@ function toError(value: unknown): Error { return value instanceof Error ? value : new Error(String(value)) } -/** Resolve once the child process exits (any code/signal); immediate if gone. */ -function waitForExit(child: ChildProcess): Promise { - // Already-exited fast path: dispose guards on exitCode before calling, so in - // tests the child is always still alive here. - /* v8 ignore next */ - if (child.exitCode !== null || child.signalCode !== null) return Promise.resolve() - return new Promise(resolve => child.once('exit', () => { resolve() })) -} - -/** Resolve `true` if the child exits within `ms`, `false` on timeout. */ -function exitsWithin(child: ChildProcess, ms: number): Promise { - return Promise.race([ - waitForExit(child).then(() => true), - // `.unref()` so a pending grace timer never keeps the parent's loop alive. - new Promise(resolve => setTimeout(() => { resolve(false) }, ms).unref()), - ]) -} - /** * Start an out-of-process ACP child for `request` and return a {@link SubagentRun}. * @@ -254,13 +212,11 @@ export function startAcpRun(request: SubagentStartRequest, spec: AcpRunSpec): Su env: buildChildEnv(spec.env), stdio: ['pipe', 'pipe', 'inherit'], }) - // A spawn-level failure (e.g. ENOENT for a bad command) is emitted as an - // `error` event, NOT a thrown exception — without a listener Node treats it as - // an unhandled error and crashes the parent. Capture it into a promise the - // result path races, so a bad command settles `error` like any child failure. - const spawnFailed = new Promise((resolve) => { - child.once('error', (err) => { resolve(err) }) - }) + // Same-tick capture (the library's contract): a spawn-level failure (e.g. + // ENOENT for a bad command) is an `error` EVENT that would crash the parent + // unheard; the result path races this promise, so a bad command settles + // `error` like any child failure. + const spawnFailed = spawnFailure(child) // Accumulate the child's streamed assistant text — the SubagentResult output. const output: string[] = [] @@ -393,33 +349,19 @@ export function startAcpRun(request: SubagentStartRequest, spec: AcpRunSpec): Su }, async dispose(): Promise { request.signal?.removeEventListener('abort', onAbort) - // Reach quiescence, not merely request it (dispose must AWAIT the child - // actually stopping). If the child is already gone, nothing to do. - if (child.exitCode !== null || child.signalCode !== null) return - const eofGraceMs = spec.disposeEofGraceMs - const graceMs = spec.disposeGraceMs - // 1. Graceful: end the ACP request stream (stdin EOF) and let the child - // quiesce ON ITS OWN. Our acp-agent has NO SIGTERM handler in a normal - // session — it tears down via the server bridge's connection-close path - // (conn.closed → per-agent dispose → final session/flush), driven by the - // stdin EOF, NOT by a signal. A prompt response can resolve from a - // turn/end BEFORE that post-turn flush lands, so the child still has - // durable work owed when dispose runs. Give the EOF-driven quiesce a real - // window — wider than a single signal-grace, since the child's own - // teardown may itself be awaiting a signal-trapping grandchild (a bash - // subprocess in its own SIGTERM→SIGKILL grace) plus a flush — and only - // escalate if it overruns. Sending SIGTERM in the same tick (or too soon) - // would default-terminate the child mid-flush, orphaning its nested work. - child.stdin.end() - if (await exitsWithin(child, eofGraceMs)) return - // 2. SIGTERM, then escalate to SIGKILL if it still does not exit within the - // grace period — a child that ignores EOF and traps SIGTERM must not - // wedge dispose forever (the seam requires bounded quiescence). - child.kill('SIGTERM') - if (await exitsWithin(child, graceMs)) return - // 3. Force-kill and await the (now-certain) exit. - child.kill('SIGKILL') - await waitForExit(child) + // Quiescent teardown via the shared ladder (stdin EOF → SIGTERM → + // SIGKILL, awaiting the actual exit). For THIS child the EOF tier is the + // one that matters: our acp-agent has NO SIGTERM handler in a normal + // session — it tears down via the server bridge's connection-close path + // (conn.closed → per-agent dispose → final session/flush), driven by the + // stdin EOF, NOT by a signal — and a prompt response can resolve from a + // turn/end BEFORE that post-turn flush lands, so the child still has + // durable work owed when dispose runs (hence the wide EOF grace; see + // DEFAULT_DISPOSE_EOF_GRACE_MS). + await disposeChildProcess(child, { + disposeEofGraceMs: spec.disposeEofGraceMs, + disposeGraceMs: spec.disposeGraceMs, + }) }, } } diff --git a/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts b/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts index 3eb12fac38..6d351c71d0 100644 --- a/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts +++ b/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts @@ -6,9 +6,10 @@ 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-process' import type { Agent } from '@deepseek-ai/dsh-agent' import * as acp from '../src/index.ts' -import { acpStopReason, acpContentText, buildChildEnv, DEFAULT_DISPOSE_EOF_GRACE_MS, DEFAULT_DISPOSE_GRACE_MS, SENSITIVE_ENV_PATTERN, startAcpRun, toAcpPrompt, type AcpRunSpec } from '../src/run.ts' +import { acpStopReason, acpContentText, DEFAULT_DISPOSE_EOF_GRACE_MS, DEFAULT_DISPOSE_GRACE_MS, startAcpRun, toAcpPrompt, type AcpRunSpec } from '../src/run.ts' /** * Keyless integration tests for the ACP subagent backend. Each spawns a REAL diff --git a/packages/subagent/subagent-acp/tsconfig.json b/packages/subagent/subagent-acp/tsconfig.json index 3c06fef150..ab24f60f93 100644 --- a/packages/subagent/subagent-acp/tsconfig.json +++ b/packages/subagent/subagent-acp/tsconfig.json @@ -25,6 +25,9 @@ }, { "path": "../subagent" + }, + { + "path": "../subagent-process" } ] } diff --git a/packages/subagent/subagent-process/README.md b/packages/subagent/subagent-process/README.md new file mode 100644 index 0000000000..597470f52a --- /dev/null +++ b/packages/subagent/subagent-process/README.md @@ -0,0 +1,40 @@ +# @deepseek-ai/dsh-subagent-process + +Shared machinery for **out-of-process subagent backends** — providers that spawn an external agent as a child process, such as the [ACP backend](../subagent-acp/README.md). A pure library (no provider, no registration, no Config): what every spawn-a-CLI-child backend needs to keep the parent deployment's credentials out of the child, tear the child down to quiescence, and isolate it from the host user's on-disk CLI state. Design rationale: [the Claude Code / Codex subagent backends RFC](../../../docs/rfc/proposed/feature/2026-07-07-claude-code-and-codex-subagent-backends.md). + +Every tunable is a **parameter**: the dispose ladder takes its grace periods per call, the config-dir helper takes an optional pinned path. Defaults live in each consuming plugin's Config (defaulted, validated fields changeable from `cordis.yml`), never in this library. + +## What it exports + +### `SENSITIVE_ENV_PATTERN` / `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. + +### `spawnFailure(child)` + +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 pending timer is `unref()`ed so a grace window never keeps the parent's event loop alive). + +### `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)): + +1. stdin EOF (when stdin is piped), then wait `graces.disposeEofGraceMs` — a cooperative child quiesces on its own, its flushes and nested-subprocess teardown intact; +2. `SIGTERM`, then wait `graces.disposeGraceMs`; +3. `SIGKILL`, then await the now-certain exit — a child that ignores EOF and traps `SIGTERM` cannot wedge dispose forever. + +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. + +### `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. + +- **Fresh (default)**: a private (0700) `mkdtemp` dir under the OS temp root; `remove()` deletes it best-effort (never rejects — a leftover temp dir beats a failed dispose) and is idempotent. +- **Pinned** (`pinnedPath` set): the path is returned as-is — never created, never removed. A deployment that pins a directory to share child state across runs owns that directory's lifecycle. + +## Testing + +`tests/subagent-process.spec.ts`: the env scrub and config-dir helpers run against the real process env and real filesystem (including an rm-failure path proving `remove()` never rejects); the exit waits and the dispose ladder run against a scriptable fake child, driving each escalation tier deterministically. The [ACP backend suite](../subagent-acp/README.md) exercises the same ladder against real subprocesses (EOF-cooperative, EOF-ignoring, and SIGTERM-trapping children) end to end. diff --git a/packages/subagent/subagent-process/package.json b/packages/subagent/subagent-process/package.json new file mode 100644 index 0000000000..218276be44 --- /dev/null +++ b/packages/subagent/subagent-process/package.json @@ -0,0 +1,30 @@ +{ + "name": "@deepseek-ai/dsh-subagent-process", + "description": "Shared out-of-process subagent machinery: credential env scrub, spawn-failure capture, child-exit waits, the EOF-to-SIGTERM-to-SIGKILL dispose ladder, and isolated config dirs (pure lib; registers nothing)", + "version": "0.0.1", + "private": true, + "type": "module", + "main": "lib/index.js", + "types": "lib/types/index.d.ts", + "exports": { + ".": { + "types": "./lib/types/index.d.ts", + "default": "./lib/index.js" + }, + "./src/*": "./src/*", + "./package.json": "./package.json" + }, + "files": [ + "lib/index.js", + "lib/types/**/*.d.ts", + "lib/types/**/*.d.ts.map", + "src" + ], + "license": "BSD-3-Clause", + "peerDependencies": { + "cordis": "^4.0.0-rc.6" + }, + "devDependencies": { + "cordis": "^4.0.0-rc.6" + } +} diff --git a/packages/subagent/subagent-process/src/index.ts b/packages/subagent/subagent-process/src/index.ts new file mode 100644 index 0000000000..b54681d3c7 --- /dev/null +++ b/packages/subagent/subagent-process/src/index.ts @@ -0,0 +1,207 @@ +/** + * 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 + * ({@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. + * + * @module @deepseek-ai/dsh-subagent-process + */ + +import type { ChildProcess } from 'node:child_process' +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +/** + * Credential-shaped ambient env vars are NOT forwarded to a child by default + * (the parent harness's own `DEEPSEEK_API_KEY`/secrets must not leak into a + * spawned process implicitly). Same pattern as the bash executor. The child + * agent needs its OWN credentials to reach a model — those are supplied + * explicitly via the `extra` layer of {@link buildChildEnv}, which lands AFTER + * 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 + +/** + * 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. + * @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. + */ +export function buildChildEnv(extra: Record): NodeJS.ProcessEnv { + const env: NodeJS.ProcessEnv = {} + for (const [key, value] of Object.entries(process.env)) { + if (!SENSITIVE_ENV_PATTERN.test(key)) env[key] = value + } + return { ...env, ...extra } +} + +/** + * 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. + * @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. + */ +export function spawnFailure(child: ChildProcess): Promise { + return new Promise((resolve) => { + child.once('error', (err) => { resolve(err) }) + }) +} + +/** + * Resolve once the child process exits (any code/signal); immediate if it is + * already gone. + * @param child - the child process to await. + */ +export function waitForExit(child: ChildProcess): Promise { + if (child.exitCode !== null || child.signalCode !== null) return Promise.resolve() + return new Promise(resolve => child.once('exit', () => { resolve() })) +} + +/** + * Race the child's exit against a timer. + * @param child - the child process to watch. + * @param ms - the wait window in milliseconds. + * @returns `true` if the child exits within `ms`, `false` on timeout. + */ +export function exitsWithin(child: ChildProcess, ms: number): Promise { + return Promise.race([ + waitForExit(child).then(() => true), + // `.unref()` so a pending grace timer never keeps the parent's loop alive. + new Promise(resolve => setTimeout(() => { resolve(false) }, ms).unref()), + ]) +} + +/** + * The two grace periods of the dispose ladder, supplied per call by the + * consuming backend — each plugin carries them as defaulted, validated + * `disposeEofGraceMs`/`disposeGraceMs` Config fields, so teardown timing is + * deployment-tunable and this library hardcodes nothing. + */ +export interface DisposeLadderGraces { + /** + * Tier-1 window (ms): after stdin EOF, how long the child gets to quiesce + * ON ITS OWN — flush durable state, tear down its own nested subprocesses — + * before the parent escalates to `SIGTERM`. A separate (usually WIDER) + * grace than {@link DisposeLadderGraces.disposeGraceMs}: a cooperative + * child's EOF-driven teardown may itself be waiting on a signal-trapping + * grandchild plus a final flush, needing more than one signal-grace of + * headroom. + */ + disposeEofGraceMs: number + /** Tier-2 window (ms): between `SIGTERM` and the `SIGKILL` escalation. */ + disposeGraceMs: number +} + +/** + * 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. + * + * @param child - the child process to tear down. + * @param graces - the two grace periods, from the consuming plugin's Config. + */ +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. + child.stdin?.end() + if (await exitsWithin(child, graces.disposeEofGraceMs)) return + // 2. SIGTERM, escalating if the child still does not exit within the grace. + child.kill('SIGTERM') + if (await exitsWithin(child, graces.disposeGraceMs)) return + // 3. Force-kill and await the (now-certain) exit. + child.kill('SIGKILL') + await waitForExit(child) +} + +/** + * A per-run config directory handle for an external CLI child — the target of + * `CLAUDE_CONFIG_DIR` / `CODEX_HOME`-style redirection. Hand {@link path} to + * the child's environment; call {@link remove} on dispose. + */ +export interface IsolatedConfigDir { + /** The directory to point the child at. */ + path: string + /** + * Best-effort cleanup: removes the directory (recursively) iff this handle + * CREATED it — a pinned directory is never removed. Idempotent; never + * rejects (a leftover dir under the OS temp root is preferable to a failed + * dispose). + */ + remove(): Promise +} + +/** + * 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. + * + * @param prefix - the `mkdtemp` name prefix for a fresh dir (e.g. + * `dsh-subagent-codex-`); ignored when `pinnedPath` is set. + * @param pinnedPath - a deployment-pinned directory to use instead of a + * fresh one. + * @returns the directory handle: `path` for the child env, `remove()` for + * dispose. + */ +export async function createIsolatedConfigDir(prefix: string, pinnedPath?: string): Promise { + if (pinnedPath !== undefined) { + return { + path: pinnedPath, + remove(): Promise { + // A pinned dir is deployment-owned state (config the user asked to + // persist across runs); removing it here would destroy it. No-op. + return Promise.resolve() + }, + } + } + const path = await mkdtemp(join(tmpdir(), prefix)) + return { + path, + async remove(): Promise { + 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. + } + }, + } +} diff --git a/packages/subagent/subagent-process/tests/subagent-process.spec.ts b/packages/subagent/subagent-process/tests/subagent-process.spec.ts new file mode 100644 index 0000000000..c4828d8498 --- /dev/null +++ b/packages/subagent/subagent-process/tests/subagent-process.spec.ts @@ -0,0 +1,307 @@ +import { describe, expect, it } from 'vitest' +import { EventEmitter } from 'node:events' +import { existsSync } from 'node:fs' +import { chmod, mkdir, mkdtemp, rm, stat, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import type { ChildProcess } from 'node:child_process' +import { + buildChildEnv, + createIsolatedConfigDir, + disposeChildProcess, + exitsWithin, + SENSITIVE_ENV_PATTERN, + spawnFailure, + waitForExit, +} from '../src/index.ts' + +/** + * Unit tests for the shared out-of-process machinery. The env scrub and the + * isolated-config-dir helpers run against the REAL process env and REAL + * filesystem; the exit waits and the dispose ladder run against a scriptable + * fake child so each escalation tier's timing is driven deterministically + * (the ACP backend's suite exercises the same ladder against real + * subprocesses end to end). + */ + +/** What fells a scripted {@link FakeChild}. */ +type LethalTrigger = 'eof' | NodeJS.Signals + +/** Per-scenario script for a {@link FakeChild}. */ +interface FakeChildScript { + /** + * The one trigger that makes the child exit (SIGKILL always does, + * uncatchable, like a real process). Omitted: only SIGKILL fells it. + */ + diesOn?: LethalTrigger + /** Delay (ms) between the lethal trigger and the exit event. */ + delayMs?: number + /** `false` models a child spawned without a stdin pipe. */ + stdin?: boolean +} + +/** + * A scriptable stand-in for a ChildProcess carrying exactly the surface the + * helpers read: `exitCode`/`signalCode`, `stdin.end()`, `kill()`, and the + * `exit` event. + */ +class FakeChild extends EventEmitter { + exitCode: number | null = null + signalCode: NodeJS.Signals | null = null + readonly kills: NodeJS.Signals[] = [] + stdinEnded = false + readonly stdin: { end: () => void } | null + + constructor(private readonly script: FakeChildScript = {}) { + super() + this.stdin = script.stdin === false + ? null + : { end: () => { this.stdinEnded = true; this.maybeDie('eof') } } + } + + kill(signal: NodeJS.Signals): boolean { + this.kills.push(signal) + this.maybeDie(signal) + return true + } + + private maybeDie(trigger: LethalTrigger): void { + // 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(() => { + if (trigger === 'eof') this.exitCode = 0 + else this.signalCode = trigger + this.emit('exit', this.exitCode, this.signalCode) + }, this.script.delayMs ?? 0) + } +} + +/** The helpers take a real ChildProcess; the fake carries the read surface. */ +function asChild(fake: FakeChild): ChildProcess { + return fake as unknown as ChildProcess +} + +describe('buildChildEnv / SENSITIVE_ENV_PATTERN', () => { + 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' + process.env.DSH_PROC_TEST_TOKEN = 'leak' + try { + const env = buildChildEnv({}) + expect(env.DSH_PROC_TEST_API_KEY).toBeUndefined() + expect(env.dsh_proc_test_secret).toBeUndefined() + expect(env.DSH_PROC_TEST_TOKEN).toBeUndefined() + } finally { + delete process.env.DSH_PROC_TEST_API_KEY + delete process.env.dsh_proc_test_secret + delete process.env.DSH_PROC_TEST_TOKEN + } + }) + + it('forwards normal ambient vars', () => { + expect(SENSITIVE_ENV_PATTERN.test('PATH')).toBe(false) + expect(buildChildEnv({}).PATH).toBe(process.env.PATH) + }) + + it('layers extras AFTER the scrub, so a deliberate credential-shaped name survives', () => { + process.env.DSH_PROC_TEST_EXTRA_TOKEN = 'ambient-leak' + try { + const env = buildChildEnv({ DSH_PROC_TEST_EXTRA_TOKEN: 'explicit' }) + // The ambient value was scrubbed; ONLY the explicit opt-in reaches the child. + expect(env.DSH_PROC_TEST_EXTRA_TOKEN).toBe('explicit') + } finally { + delete process.env.DSH_PROC_TEST_EXTRA_TOKEN + } + }) + + it('an extra overrides the ambient value of a non-credential var', () => { + process.env.DSH_PROC_TEST_PLAIN = 'ambient' + try { + expect(buildChildEnv({ DSH_PROC_TEST_PLAIN: 'override' }).DSH_PROC_TEST_PLAIN).toBe('override') + } finally { + delete process.env.DSH_PROC_TEST_PLAIN + } + }) +}) + +describe('spawnFailure', () => { + it('resolves (never rejects) with the first error event', async () => { + const fake = new FakeChild() + const failure = spawnFailure(asChild(fake)) + const err = new Error('spawn ENOENT') + fake.emit('error', err) + await expect(failure).resolves.toBe(err) + }) + + it('never settles for a child that spawns cleanly and exits', async () => { + const fake = new FakeChild({ diesOn: 'SIGTERM' }) + const failure = spawnFailure(asChild(fake)) + fake.kill('SIGTERM') + await waitForExit(asChild(fake)) + // 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([ + failure.then(() => 'settled'), + new Promise(resolve => setTimeout(() => { resolve('pending') }, 30)), + ]) + expect(settled).toBe('pending') + }) +}) + +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 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) + }) + + 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) + }) +}) + +describe('disposeChildProcess', () => { + it('returns immediately for an already-exited child (no EOF, no signals)', async () => { + const fake = new FakeChild() + fake.exitCode = 0 + await disposeChildProcess(asChild(fake), { disposeEofGraceMs: 1000, disposeGraceMs: 1000 }) + expect(fake.stdinEnded).toBe(false) + expect(fake.kills).toEqual([]) + }) + + it('returns immediately for a child already dead by signal', async () => { + const fake = new FakeChild() + fake.signalCode = 'SIGKILL' + await disposeChildProcess(asChild(fake), { disposeEofGraceMs: 1000, disposeGraceMs: 1000 }) + expect(fake.stdinEnded).toBe(false) + expect(fake.kills).toEqual([]) + }) + + it('tier 1: a cooperative child quiesces on stdin EOF — no signal is ever sent', async () => { + const fake = new FakeChild({ diesOn: 'eof', delayMs: 5 }) + await disposeChildProcess(asChild(fake), { disposeEofGraceMs: 1000, disposeGraceMs: 1000 }) + expect(fake.stdinEnded).toBe(true) + expect(fake.kills).toEqual([]) + expect(fake.exitCode).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') + }) + + it('tier 3: a SIGTERM-trapping child is SIGKILLed, and dispose resolves only after the exit', async () => { + const fake = new FakeChild({ delayMs: 5 }) // only SIGKILL fells it + await disposeChildProcess(asChild(fake), { disposeEofGraceMs: 20, disposeGraceMs: 20 }) + expect(fake.kills).toEqual(['SIGTERM', 'SIGKILL']) + // Quiescence, not a request: at resolution the child has ACTUALLY exited + // (the exit event landed, despite the scripted post-SIGKILL delay). + 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 }) + expect(fake.kills).toEqual(['SIGTERM']) + }) +}) + +describe('createIsolatedConfigDir', () => { + it('creates a fresh private mkdtemp dir under the OS temp root', async () => { + const dir = await createIsolatedConfigDir('dsh-subagent-process-test-') + try { + expect(dir.path.startsWith(join(tmpdir(), 'dsh-subagent-process-test-'))).toBe(true) + const st = await stat(dir.path) + expect(st.isDirectory()).toBe(true) + // Private (0700) per the defensive-patterns temp-dir rule. + expect(st.mode & 0o777).toBe(0o700) + } finally { + await dir.remove() + } + }) + + it('creates a distinct dir per call (per-run isolation)', async () => { + const a = await createIsolatedConfigDir('dsh-subagent-process-test-') + const b = await createIsolatedConfigDir('dsh-subagent-process-test-') + try { + expect(a.path).not.toBe(b.path) + } finally { + await a.remove() + await b.remove() + } + }) + + it('remove() deletes a fresh dir recursively and is idempotent', async () => { + const dir = await createIsolatedConfigDir('dsh-subagent-process-test-') + await writeFile(join(dir.path, 'settings.json'), '{}') + await dir.remove() + expect(existsSync(dir.path)).toBe(false) + // Second remove: nothing left to delete, still resolves. + await expect(dir.remove()).resolves.toBeUndefined() + }) + + it('returns a pinned dir verbatim and NEVER removes it', async () => { + const pinned = await mkdtemp(join(tmpdir(), 'dsh-subagent-process-pinned-')) + try { + const dir = await createIsolatedConfigDir('ignored-prefix-', pinned) + expect(dir.path).toBe(pinned) + await dir.remove() + // The deployment owns a pinned dir's lifecycle — remove() must not touch it. + expect(existsSync(pinned)).toBe(true) + } finally { + await rm(pinned, { recursive: true, force: true }) + } + }) + + it('does not create a missing pinned path (the deployment owns its lifecycle)', async () => { + const missing = join(tmpdir(), `dsh-subagent-process-missing-${process.pid}`) + const dir = await createIsolatedConfigDir('ignored-prefix-', missing) + expect(dir.path).toBe(missing) + expect(existsSync(missing)).toBe(false) + await dir.remove() + expect(existsSync(missing)).toBe(false) + }) + + it('remove() is best-effort: an rm failure resolves instead of rejecting', async () => { + const dir = await createIsolatedConfigDir('dsh-subagent-process-locked-') + const locked = join(dir.path, 'locked') + await mkdir(locked) + await writeFile(join(locked, 'entry'), 'x') + // An unreadable, unwritable non-empty subdir makes recursive rm fail + // (EACCES on readdir/unlink) for a non-root user. + await chmod(locked, 0o000) + try { + await expect(dir.remove()).resolves.toBeUndefined() + // rm really did fail — the locked subtree is still there. + expect(existsSync(locked)).toBe(true) + } finally { + await chmod(locked, 0o700) + await rm(dir.path, { recursive: true, force: true }) + } + }) +}) diff --git a/packages/subagent/subagent-process/tsconfig.json b/packages/subagent/subagent-process/tsconfig.json new file mode 100644 index 0000000000..749cb0208e --- /dev/null +++ b/packages/subagent/subagent-process/tsconfig.json @@ -0,0 +1,11 @@ +{ + "extends": "../../../tsconfig.base.json", + "compilerOptions": { + "rootDir": "src", + "outDir": "lib/types" + }, + "include": [ + "src" + ], + "references": [] +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 32ffa0d389..359b68b6d5 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -594,6 +594,9 @@ importers: '@deepseek-ai/dsh-subagent': specifier: workspace:^ version: link:../subagent + '@deepseek-ai/dsh-subagent-process': + specifier: workspace:^ + version: link:../subagent-process cordis: specifier: ^4.0.0-rc.6 version: 4.0.0-rc.6(@cordisjs/plugin-include@1.0.4)(@cordisjs/plugin-loader@1.0.0-rc.4) @@ -671,6 +674,12 @@ importers: specifier: ^4.0.0-rc.6 version: 4.0.0-rc.6(@cordisjs/plugin-include@1.0.4)(@cordisjs/plugin-loader@1.0.0-rc.4) + packages/subagent/subagent-process: + devDependencies: + cordis: + specifier: ^4.0.0-rc.6 + version: 4.0.0-rc.6(@cordisjs/plugin-include@1.0.4)(@cordisjs/plugin-loader@1.0.0-rc.4) + packages/subagent/subagent-spawn: dependencies: schemastery: diff --git a/tsconfig.build.json b/tsconfig.build.json index 3d99ad4e28..21fa726927 100644 --- a/tsconfig.build.json +++ b/tsconfig.build.json @@ -50,6 +50,7 @@ { "path": "./packages/support/subagent-mock" }, { "path": "./packages/subagent/tool-subagent" }, { "path": "./packages/subagent/subagent-inprocess" }, + { "path": "./packages/subagent/subagent-process" }, { "path": "./packages/subagent/subagent-spawn" }, { "path": "./packages/subagent/subagent-fork" }, { "path": "./packages/subagent/subagent-acp" }, diff --git a/tsconfig.json b/tsconfig.json index 2091283c93..c4127b2b5d 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -61,6 +61,7 @@ { "path": "./packages/support/subagent-mock" }, { "path": "./packages/subagent/tool-subagent" }, { "path": "./packages/subagent/subagent-inprocess" }, + { "path": "./packages/subagent/subagent-process" }, { "path": "./packages/subagent/subagent-spawn" }, { "path": "./packages/subagent/subagent-fork" }, { "path": "./packages/subagent/subagent-acp" }, From 7ccf31a59b5227d66a95b2d52932d68b59e2b702 Mon Sep 17 00:00:00 2001 From: pku-xht Date: Thu, 9 Jul 2026 10:20:42 +0800 Subject: [PATCH 02/13] fix review finding: root-portable rm-failure injection in the config-dir test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The best-effort-remove test provoked a real EACCES via a chmod-000 subtree, which only fails for unprivileged users — under root, recursive rm ignores permission bits, deleting the subtree: the existsSync assertion goes red and the swallow branch loses coverage, failing the per-file gate. The rejection is now injected deterministically at the node:fs/promises boundary (rm wrapped with a real-passthrough vi.fn; one test queues a single rejection), the fs-failure boundary being exactly the non-deterministic seam the testing policy sanctions mocking. Everything else in the suite stays on the real filesystem, and the swallow contract stays error-kind agnostic. --- packages/subagent/subagent-process/README.md | 2 +- .../tests/subagent-process.spec.ts | 39 +++++++++++-------- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/packages/subagent/subagent-process/README.md b/packages/subagent/subagent-process/README.md index 597470f52a..e42bb4b611 100644 --- a/packages/subagent/subagent-process/README.md +++ b/packages/subagent/subagent-process/README.md @@ -37,4 +37,4 @@ A per-run isolated config directory for an external CLI child (the target of `CL ## Testing -`tests/subagent-process.spec.ts`: the env scrub and config-dir helpers run against the real process env and real filesystem (including an rm-failure path proving `remove()` never rejects); the exit waits and the dispose ladder run against a scriptable fake child, driving each escalation tier deterministically. The [ACP backend suite](../subagent-acp/README.md) exercises the same ladder against real subprocesses (EOF-cooperative, EOF-ignoring, and SIGTERM-trapping children) end to end. +`tests/subagent-process.spec.ts`: the env scrub and config-dir helpers run against the real process env and real filesystem (the rm-failure path injects its rejection at the fs boundary — a real recursive-rm failure is not portably provokable, and root ignores permission bits); the exit waits and the dispose ladder run against a scriptable fake child, driving each escalation tier deterministically. The [ACP backend suite](../subagent-acp/README.md) exercises the same ladder against real subprocesses (EOF-cooperative, EOF-ignoring, and SIGTERM-trapping children) end to end. diff --git a/packages/subagent/subagent-process/tests/subagent-process.spec.ts b/packages/subagent/subagent-process/tests/subagent-process.spec.ts index c4828d8498..24f2a3a97d 100644 --- a/packages/subagent/subagent-process/tests/subagent-process.spec.ts +++ b/packages/subagent/subagent-process/tests/subagent-process.spec.ts @@ -1,7 +1,7 @@ -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import { EventEmitter } from 'node:events' import { existsSync } from 'node:fs' -import { chmod, mkdir, mkdtemp, rm, stat, writeFile } from 'node:fs/promises' +import { mkdtemp, rm, stat, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import type { ChildProcess } from 'node:child_process' @@ -15,13 +15,24 @@ import { waitForExit, } from '../src/index.ts' +// `rm` is wrapped (real-passthrough by default) so ONE test can inject a +// rejection deterministically. A real recursive-rm failure is not portably +// provokable — permission tricks (a chmod-000 subtree) fail only for +// unprivileged users and are ignored by root — so this is the fs boundary +// the testing policy sanctions mocking; everything else stays the real fs. +vi.mock('node:fs/promises', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, rm: vi.fn(actual.rm) } +}) + /** * Unit tests for the shared out-of-process machinery. The env scrub and the * isolated-config-dir helpers run against the REAL process env and REAL - * filesystem; the exit waits and the dispose ladder run against a scriptable - * fake child so each escalation tier's timing is driven deterministically - * (the ACP backend's suite exercises the same ladder against real - * subprocesses end to end). + * filesystem (one exception: the rm-failure path injects its rejection at the + * mocked fs boundary, see above); the exit waits and the dispose ladder run + * against a scriptable fake child so each escalation tier's timing is driven + * deterministically (the ACP backend's suite exercises the same ladder + * against real subprocesses end to end). */ /** What fells a scripted {@link FakeChild}. */ @@ -287,20 +298,16 @@ describe('createIsolatedConfigDir', () => { expect(existsSync(missing)).toBe(false) }) - it('remove() is best-effort: an rm failure resolves instead of rejecting', async () => { + it('remove() is best-effort: an rm rejection resolves instead of rejecting', async () => { const dir = await createIsolatedConfigDir('dsh-subagent-process-locked-') - const locked = join(dir.path, 'locked') - await mkdir(locked) - await writeFile(join(locked, 'entry'), 'x') - // An unreadable, unwritable non-empty subdir makes recursive rm fail - // (EACCES on readdir/unlink) for a non-root user. - await chmod(locked, 0o000) try { + // The swallow contract is error-kind agnostic; EACCES stands in for the + // family (EBUSY, a vanished mount, …) that best-effort must absorb. + vi.mocked(rm).mockRejectedValueOnce(Object.assign(new Error('EACCES: permission denied'), { code: 'EACCES' })) await expect(dir.remove()).resolves.toBeUndefined() - // rm really did fail — the locked subtree is still there. - expect(existsSync(locked)).toBe(true) + // The injected rejection consumed the only rm call — nothing was deleted. + expect(existsSync(dir.path)).toBe(true) } finally { - await chmod(locked, 0o700) await rm(dir.path, { recursive: true, force: true }) } }) From 2471e2b2bb45d1f8f350571451feda04a4a8786d Mon Sep 17 00:00:00 2001 From: pku-xht Date: Thu, 9 Jul 2026 10:42:34 +0800 Subject: [PATCH 03/13] fix review finding: exitsWithin cleans up its listener and timer on both arms Each timed-out wait used to leave the once('exit') listener from its inner waitForExit attached to the child; the dispose ladder accumulates at most a couple, but in a shared library a caller polling exitsWithin in a loop would pile listeners onto one child (MaxListenersExceededWarning at 11) and retain their closures. The race now owns its wiring: the timeout arm removes the exit listener, the exit arm clears the (still unref'ed) grace timer, and an already-exited child short-circuits true without attaching anything. Tests pin listenerCount('exit') === 0 after every outcome. --- packages/subagent/subagent-process/README.md | 2 +- .../subagent/subagent-process/src/index.ts | 24 ++++++++++++++----- .../tests/subagent-process.spec.ts | 12 ++++++++++ 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/packages/subagent/subagent-process/README.md b/packages/subagent/subagent-process/README.md index e42bb4b611..e6c8e10a96 100644 --- a/packages/subagent/subagent-process/README.md +++ b/packages/subagent/subagent-process/README.md @@ -16,7 +16,7 @@ Spawn-failure capture: a promise that resolves (never rejects) with the child's ### `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 pending timer is `unref()`ed so a grace window never keeps the parent's event loop alive). +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)` diff --git a/packages/subagent/subagent-process/src/index.ts b/packages/subagent/subagent-process/src/index.ts index b54681d3c7..219ed6b083 100644 --- a/packages/subagent/subagent-process/src/index.ts +++ b/packages/subagent/subagent-process/src/index.ts @@ -78,17 +78,29 @@ export function waitForExit(child: ChildProcess): Promise { } /** - * Race the child's exit against a timer. + * Race the child's exit against a timer. Neither outcome leaves anything + * behind on the child: the exit listener is removed on timeout and the timer + * is cleared on exit, so repeated calls (the dispose ladder's tiers, a poll + * loop) never accumulate listeners. * @param child - the child process to watch. * @param ms - the wait window in milliseconds. - * @returns `true` if the child exits within `ms`, `false` on timeout. + * @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 { - return Promise.race([ - waitForExit(child).then(() => true), + if (child.exitCode !== null || child.signalCode !== null) return Promise.resolve(true) + return new Promise((resolve) => { + const onExit = (): void => { + clearTimeout(timer) + resolve(true) + } // `.unref()` so a pending grace timer never keeps the parent's loop alive. - new Promise(resolve => setTimeout(() => { resolve(false) }, ms).unref()), - ]) + const timer = setTimeout(() => { + child.removeListener('exit', onExit) + resolve(false) + }, ms).unref() + child.once('exit', onExit) + }) } /** diff --git a/packages/subagent/subagent-process/tests/subagent-process.spec.ts b/packages/subagent/subagent-process/tests/subagent-process.spec.ts index 24f2a3a97d..d23f075284 100644 --- a/packages/subagent/subagent-process/tests/subagent-process.spec.ts +++ b/packages/subagent/subagent-process/tests/subagent-process.spec.ts @@ -181,15 +181,27 @@ describe('waitForExit / exitsWithin', () => { 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) }) }) From a11000030a14dd382548839894ba49b0568ab801 Mon Sep 17 00:00:00 2001 From: pku-xht Date: Thu, 9 Jul 2026 10:42:41 +0800 Subject: [PATCH 04/13] docs(subagent-acp): point the env-scrub section at its one home MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The scrub pattern and layering semantics live in the dsh-subagent-process README (the fact's home since the extraction); the ACP section restated them in full — two prose copies drift word by word until they disagree (the one-home-per-fact rule in docs/AGENTS.md). The section now links the library and keeps only the backend's own story: which credential enters via config.env and why. --- packages/subagent/subagent-acp/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/subagent/subagent-acp/README.md b/packages/subagent/subagent-acp/README.md index 81bf886067..2cc330aefd 100644 --- a/packages/subagent/subagent-acp/README.md +++ b/packages/subagent/subagent-acp/README.md @@ -57,7 +57,7 @@ A spawn/transport/RPC failure resolves `error` (or `aborted` if a cancel was req ## Environment scrub -Credential-shaped ambient vars (`/KEY|SECRET|TOKEN/i`) are NOT forwarded to the child by default — the parent harness's own secrets must not leak into a spawned process implicitly. The child's OWN credentials are supplied explicitly via `config.env`, layered AFTER the scrub, so an intended `DEEPSEEK_API_KEY` survives while an incidental `AWS_SECRET_ACCESS_KEY` does not. +The child env is built by [`buildChildEnv` from `@deepseek-ai/dsh-subagent-process`](../subagent-process/README.md) — the ambient env minus credential-shaped vars, with `config.env` layered on top after the scrub; the pattern and full semantics live there. For this backend that means the parent harness's own secrets never leak into the spawned agent implicitly, while the child's OWN `DEEPSEEK_API_KEY` is supplied deliberately via `config.env` and survives. ## Testing From c321819053c29468f79adb87dc66c7ff22f37668 Mon Sep 17 00:00:00 2001 From: pku-xht Date: Thu, 9 Jul 2026 13:42:03 +0800 Subject: [PATCH 05/13] rename: @deepseek-ai/dsh-subagent-process -> @deepseek-ai/dsh-subagent-subprocess MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The extracted library's name sat one edit away from @deepseek-ai/dsh-subagent-inprocess (process/inprocess), inviting a typo'd import to silently resolve to the wrong package. subagent-subprocess also reads as the deliberate counterpart to subagent-inprocess (in-process vs. subprocess), matching how the two shared drivers actually differ. Package directory, npm name, module doc, JSDoc module tags, test-file name and its temp-dir prefixes, the subagent-acp import and its Config/tsconfig/package.json references, root tsconfig.json/tsconfig.build.json/knip.json entries, and the packages/subagent group README all renamed together; regenerated docs/module-graph.md and docs/config-catalog.md. Pure rename — no behavior, export, or Config shape changed. --- docs/config-catalog.md | 2 +- docs/module-graph.md | 8 ++++---- knip.json | 2 +- packages/subagent/README.md | 4 ++-- packages/subagent/subagent-acp/README.md | 2 +- packages/subagent/subagent-acp/package.json | 4 ++-- packages/subagent/subagent-acp/src/run.ts | 2 +- .../subagent-acp/tests/subagent-acp.spec.ts | 2 +- packages/subagent/subagent-acp/tsconfig.json | 2 +- .../README.md | 4 ++-- .../package.json | 2 +- .../src/index.ts | 2 +- .../tests/subagent-subprocess.spec.ts} | 16 ++++++++-------- .../tsconfig.json | 0 pnpm-lock.yaml | 16 ++++++++-------- tsconfig.build.json | 2 +- tsconfig.json | 2 +- 17 files changed, 36 insertions(+), 36 deletions(-) rename packages/subagent/{subagent-process => subagent-subprocess}/README.md (86%) rename packages/subagent/{subagent-process => subagent-subprocess}/package.json (93%) rename packages/subagent/{subagent-process => subagent-subprocess}/src/index.ts (99%) rename packages/subagent/{subagent-process/tests/subagent-process.spec.ts => subagent-subprocess/tests/subagent-subprocess.spec.ts} (94%) rename packages/subagent/{subagent-process => subagent-subprocess}/tsconfig.json (100%) diff --git a/docs/config-catalog.md b/docs/config-catalog.md index 34cb543303..642a4d719a 100644 --- a/docs/config-catalog.md +++ b/docs/config-catalog.md @@ -884,4 +884,4 @@ Imported as libraries by other packages; a `cordis.yml` cannot load them. - `@deepseek-ai/dsh-brand` ([`packages/util/brand/src/index.ts`](../packages/util/brand/src/index.ts)) - `@deepseek-ai/dsh-hook-protocol` ([`packages/hooks/hook-protocol/src/index.ts`](../packages/hooks/hook-protocol/src/index.ts)) - `@deepseek-ai/dsh-subagent-inprocess` ([`packages/subagent/subagent-inprocess/src/index.ts`](../packages/subagent/subagent-inprocess/src/index.ts)) -- `@deepseek-ai/dsh-subagent-process` ([`packages/subagent/subagent-process/src/index.ts`](../packages/subagent/subagent-process/src/index.ts)) +- `@deepseek-ai/dsh-subagent-subprocess` ([`packages/subagent/subagent-subprocess/src/index.ts`](../packages/subagent/subagent-subprocess/src/index.ts)) diff --git a/docs/module-graph.md b/docs/module-graph.md index 57fb397e54..c0e64c1d36 100644 --- a/docs/module-graph.md +++ b/docs/module-graph.md @@ -43,8 +43,8 @@ flowchart TD pkg_subagent_acp["subagent-acp"] pkg_subagent_fork["subagent-fork"] pkg_subagent_inprocess["subagent-inprocess"] - pkg_subagent_process["subagent-process"] pkg_subagent_spawn["subagent-spawn"] + pkg_subagent_subprocess["subagent-subprocess"] pkg_tool_subagent["tool-subagent"] end subgraph group_web["packages/web"] @@ -179,7 +179,7 @@ flowchart TD pkg_subagent_acp --> pkg_agent pkg_subagent_acp --> pkg_llm pkg_subagent_acp --> pkg_subagent - pkg_subagent_acp --> pkg_subagent_process + pkg_subagent_acp --> pkg_subagent_subprocess pkg_subagent_inprocess --> pkg_agent pkg_subagent_inprocess --> pkg_llm pkg_subagent_inprocess --> pkg_session @@ -220,7 +220,7 @@ flowchart TD | Package | Group | Depends on | | --- | --- | --- | | [`brand`](../packages/util/brand) | `util` | — | -| [`subagent-process`](../packages/subagent/subagent-process) | `subagent` | — | +| [`subagent-subprocess`](../packages/subagent/subagent-subprocess) | `subagent` | — | | [`acp-snapshot`](../packages/support/acp-snapshot) | `support` | — | | [`app-boot`](../packages/ui/app-boot) | `ui` | — | | [`code-runtime`](../packages/code-runtime/code-runtime) | `code-runtime` | — | @@ -260,7 +260,7 @@ flowchart TD | [`acp`](../packages/ui/acp) | `ui` | [`agent`](../packages/core/agent), [`llm`](../packages/llm/llm), [`session`](../packages/core/session), [`session-persistence`](../packages/session-persistence/session-persistence), [`tools`](../packages/core/tools) | | [`repeat-tool-guard`](../packages/guard/repeat-tool-guard) | `guard` | [`agent`](../packages/core/agent), [`tools`](../packages/core/tools) | | [`agent-core`](../packages/core/agent-core) | `core` | [`agent`](../packages/core/agent), [`agent-loop`](../packages/core/agent-loop), [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm), [`session`](../packages/core/session), [`system-prompt`](../packages/core/system-prompt), [`tool-bash`](../packages/bash/tool-bash), [`tools`](../packages/core/tools) | -| [`subagent-acp`](../packages/subagent/subagent-acp) | `subagent` | [`agent`](../packages/core/agent), [`llm`](../packages/llm/llm), [`subagent`](../packages/subagent/subagent), [`subagent-process`](../packages/subagent/subagent-process) | +| [`subagent-acp`](../packages/subagent/subagent-acp) | `subagent` | [`agent`](../packages/core/agent), [`llm`](../packages/llm/llm), [`subagent`](../packages/subagent/subagent), [`subagent-subprocess`](../packages/subagent/subagent-subprocess) | | [`subagent-inprocess`](../packages/subagent/subagent-inprocess) | `subagent` | [`agent`](../packages/core/agent), [`llm`](../packages/llm/llm), [`session`](../packages/core/session), [`subagent`](../packages/subagent/subagent), [`system-prompt`](../packages/core/system-prompt), [`tools`](../packages/core/tools) | | [`tool-subagent`](../packages/subagent/tool-subagent) | `subagent` | [`agent`](../packages/core/agent), [`llm`](../packages/llm/llm), [`subagent`](../packages/subagent/subagent), [`tools`](../packages/core/tools) | | [`hooks-claude`](../packages/hooks/hooks-claude) | `hooks` | [`agent`](../packages/core/agent), [`hook-protocol`](../packages/hooks/hook-protocol), [`llm`](../packages/llm/llm), [`session`](../packages/core/session), [`subagent`](../packages/subagent/subagent), [`tools`](../packages/core/tools) | diff --git a/knip.json b/knip.json index d4c44e9aa5..ecfd431c1c 100644 --- a/knip.json +++ b/knip.json @@ -70,7 +70,7 @@ "entry": ["tests/**/*.spec.ts", "tests/**/*.e2e.ts", "tests/mock-acp-server.ts"], "project": ["src/**/*.ts", "tests/**/*.ts"] }, - "packages/subagent/subagent-process": { + "packages/subagent/subagent-subprocess": { "entry": ["tests/**/*.spec.ts"], "project": ["src/**/*.ts", "tests/**/*.ts"], "ignoreDependencies": ["cordis"] diff --git a/packages/subagent/README.md b/packages/subagent/README.md index 38c64d22a5..87930167e9 100644 --- a/packages/subagent/README.md +++ b/packages/subagent/README.md @@ -8,10 +8,10 @@ The subagent seam: an agent delegating work to a child agent. Like the [bash](.. | `subagent-inprocess/` | Shared in-process run driver (pure lib; registers nothing) | — | | `subagent-spawn/` | In-process backend: a fresh child agent | (registers on `ctx.subagents`) | | `subagent-fork/` | In-process backend: a child seeded with the parent's completed-turn prefix | (registers on `ctx.subagents`) | -| `subagent-process/` | Shared out-of-process machinery: env scrub, dispose ladder, isolated config dirs (pure lib; registers nothing) | — | +| `subagent-subprocess/` | Shared out-of-process machinery: env scrub, dispose ladder, isolated config dirs (pure lib; registers nothing) | — | | `subagent-acp/` | Out-of-process backend: a child agent in a spawned subprocess, driven over ACP | (registers on `ctx.subagents`) | | `tool-subagent/` | Model-facing `subagent` delegation tool over `ctx.subagents` | (registers on `ctx.tools`) | -The interface lives at `subagent/subagent/`. The in-process `subagent-spawn` / `subagent-fork` backends share the `subagent-inprocess` driver (a pure library — both depend on it, neither on the other), the out-of-process `subagent-acp` backend builds on the `subagent-process` library (the credential env scrub, the dispose ladder, isolated config dirs) and ships alongside them here; the test-only `dsh-subagent-mock` (in [support](../support/README.md)) is separate. All **product** packages except the mock. +The interface lives at `subagent/subagent/`. The in-process `subagent-spawn` / `subagent-fork` backends share the `subagent-inprocess` driver (a pure library — both depend on it, neither on the other), the out-of-process `subagent-acp` backend builds on the `subagent-subprocess` library (the credential env scrub, the dispose ladder, isolated config dirs) and ships alongside them here; the test-only `dsh-subagent-mock` (in [support](../support/README.md)) is separate. All **product** packages except the mock. The proposal and design rationale: [docs/rfc/implemented/feature/2026-06-21-subagent-capability-seam.md](../../docs/rfc/implemented/feature/2026-06-21-subagent-capability-seam.md). diff --git a/packages/subagent/subagent-acp/README.md b/packages/subagent/subagent-acp/README.md index 2cc330aefd..7fb087dcdc 100644 --- a/packages/subagent/subagent-acp/README.md +++ b/packages/subagent/subagent-acp/README.md @@ -57,7 +57,7 @@ A spawn/transport/RPC failure resolves `error` (or `aborted` if a cancel was req ## Environment scrub -The child env is built by [`buildChildEnv` from `@deepseek-ai/dsh-subagent-process`](../subagent-process/README.md) — the ambient env minus credential-shaped vars, with `config.env` layered on top after the scrub; the pattern and full semantics live there. For this backend that means the parent harness's own secrets never leak into the spawned agent implicitly, while the child's OWN `DEEPSEEK_API_KEY` is supplied deliberately via `config.env` and survives. +The child env is built by [`buildChildEnv` from `@deepseek-ai/dsh-subagent-subprocess`](../subagent-subprocess/README.md) — the ambient env minus credential-shaped vars, with `config.env` layered on top after the scrub; the pattern and full semantics live there. For this backend that means the parent harness's own secrets never leak into the spawned agent implicitly, while the child's OWN `DEEPSEEK_API_KEY` is supplied deliberately via `config.env` and survives. ## Testing diff --git a/packages/subagent/subagent-acp/package.json b/packages/subagent/subagent-acp/package.json index 45fc6b072c..e73d861a79 100644 --- a/packages/subagent/subagent-acp/package.json +++ b/packages/subagent/subagent-acp/package.json @@ -25,7 +25,7 @@ "@deepseek-ai/dsh-agent": "^0.0.1", "@deepseek-ai/dsh-llm": "^0.0.1", "@deepseek-ai/dsh-subagent": "^0.0.1", - "@deepseek-ai/dsh-subagent-process": "^0.0.1", + "@deepseek-ai/dsh-subagent-subprocess": "^0.0.1", "cordis": "^4.0.0-rc.6" }, "dependencies": { @@ -36,7 +36,7 @@ "@deepseek-ai/dsh-agent": "workspace:^", "@deepseek-ai/dsh-llm": "workspace:^", "@deepseek-ai/dsh-subagent": "workspace:^", - "@deepseek-ai/dsh-subagent-process": "workspace:^", + "@deepseek-ai/dsh-subagent-subprocess": "workspace:^", "@cordisjs/plugin-loader": "^1.0.0-rc.4", "cordis": "^4.0.0-rc.6" } diff --git a/packages/subagent/subagent-acp/src/run.ts b/packages/subagent/subagent-acp/src/run.ts index a94c923f46..a9fefba27c 100644 --- a/packages/subagent/subagent-acp/src/run.ts +++ b/packages/subagent/subagent-acp/src/run.ts @@ -40,7 +40,7 @@ import { import { AgentId } from '@deepseek-ai/dsh-agent' import type { ContentBlock } from '@deepseek-ai/dsh-llm' import type { SubagentResult, SubagentRun, SubagentStartRequest, SubagentStopReason } from '@deepseek-ai/dsh-subagent' -import { buildChildEnv, disposeChildProcess, spawnFailure } from '@deepseek-ai/dsh-subagent-process' +import { buildChildEnv, disposeChildProcess, spawnFailure } from '@deepseek-ai/dsh-subagent-subprocess' /** * How the client answers a child's `session/request_permission`. The first cut diff --git a/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts b/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts index 6d351c71d0..92c025077a 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-process' +import { buildChildEnv, SENSITIVE_ENV_PATTERN } 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' diff --git a/packages/subagent/subagent-acp/tsconfig.json b/packages/subagent/subagent-acp/tsconfig.json index ab24f60f93..e415ace1de 100644 --- a/packages/subagent/subagent-acp/tsconfig.json +++ b/packages/subagent/subagent-acp/tsconfig.json @@ -27,7 +27,7 @@ "path": "../subagent" }, { - "path": "../subagent-process" + "path": "../subagent-subprocess" } ] } diff --git a/packages/subagent/subagent-process/README.md b/packages/subagent/subagent-subprocess/README.md similarity index 86% rename from packages/subagent/subagent-process/README.md rename to packages/subagent/subagent-subprocess/README.md index e6c8e10a96..ccc68bf31b 100644 --- a/packages/subagent/subagent-process/README.md +++ b/packages/subagent/subagent-subprocess/README.md @@ -1,4 +1,4 @@ -# @deepseek-ai/dsh-subagent-process +# @deepseek-ai/dsh-subagent-subprocess Shared machinery for **out-of-process subagent backends** — providers that spawn an external agent as a child process, such as the [ACP backend](../subagent-acp/README.md). A pure library (no provider, no registration, no Config): what every spawn-a-CLI-child backend needs to keep the parent deployment's credentials out of the child, tear the child down to quiescence, and isolate it from the host user's on-disk CLI state. Design rationale: [the Claude Code / Codex subagent backends RFC](../../../docs/rfc/proposed/feature/2026-07-07-claude-code-and-codex-subagent-backends.md). @@ -37,4 +37,4 @@ A per-run isolated config directory for an external CLI child (the target of `CL ## Testing -`tests/subagent-process.spec.ts`: the env scrub and config-dir helpers run against the real process env and real filesystem (the rm-failure path injects its rejection at the fs boundary — a real recursive-rm failure is not portably provokable, and root ignores permission bits); the exit waits and the dispose ladder run against a scriptable fake child, driving each escalation tier deterministically. The [ACP backend suite](../subagent-acp/README.md) exercises the same ladder against real subprocesses (EOF-cooperative, EOF-ignoring, and SIGTERM-trapping children) end to end. +`tests/subagent-subprocess.spec.ts`: the env scrub and config-dir helpers run against the real process env and real filesystem (the rm-failure path injects its rejection at the fs boundary — a real recursive-rm failure is not portably provokable, and root ignores permission bits); the exit waits and the dispose ladder run against a scriptable fake child, driving each escalation tier deterministically. The [ACP backend suite](../subagent-acp/README.md) exercises the same ladder against real subprocesses (EOF-cooperative, EOF-ignoring, and SIGTERM-trapping children) end to end. diff --git a/packages/subagent/subagent-process/package.json b/packages/subagent/subagent-subprocess/package.json similarity index 93% rename from packages/subagent/subagent-process/package.json rename to packages/subagent/subagent-subprocess/package.json index 218276be44..68f525dd8e 100644 --- a/packages/subagent/subagent-process/package.json +++ b/packages/subagent/subagent-subprocess/package.json @@ -1,5 +1,5 @@ { - "name": "@deepseek-ai/dsh-subagent-process", + "name": "@deepseek-ai/dsh-subagent-subprocess", "description": "Shared out-of-process subagent machinery: credential env scrub, spawn-failure capture, child-exit waits, the EOF-to-SIGTERM-to-SIGKILL dispose ladder, and isolated config dirs (pure lib; registers nothing)", "version": "0.0.1", "private": true, diff --git a/packages/subagent/subagent-process/src/index.ts b/packages/subagent/subagent-subprocess/src/index.ts similarity index 99% rename from packages/subagent/subagent-process/src/index.ts rename to packages/subagent/subagent-subprocess/src/index.ts index 219ed6b083..35d7383456 100644 --- a/packages/subagent/subagent-process/src/index.ts +++ b/packages/subagent/subagent-subprocess/src/index.ts @@ -15,7 +15,7 @@ * periods, a pinned config dir — is a PARAMETER here: defaults belong in each * consuming plugin's Config, per the no-hardcoded-tunables rule. * - * @module @deepseek-ai/dsh-subagent-process + * @module @deepseek-ai/dsh-subagent-subprocess */ import type { ChildProcess } from 'node:child_process' diff --git a/packages/subagent/subagent-process/tests/subagent-process.spec.ts b/packages/subagent/subagent-subprocess/tests/subagent-subprocess.spec.ts similarity index 94% rename from packages/subagent/subagent-process/tests/subagent-process.spec.ts rename to packages/subagent/subagent-subprocess/tests/subagent-subprocess.spec.ts index d23f075284..2766ed0a41 100644 --- a/packages/subagent/subagent-process/tests/subagent-process.spec.ts +++ b/packages/subagent/subagent-subprocess/tests/subagent-subprocess.spec.ts @@ -256,9 +256,9 @@ describe('disposeChildProcess', () => { describe('createIsolatedConfigDir', () => { it('creates a fresh private mkdtemp dir under the OS temp root', async () => { - const dir = await createIsolatedConfigDir('dsh-subagent-process-test-') + const dir = await createIsolatedConfigDir('dsh-subagent-subprocess-test-') try { - expect(dir.path.startsWith(join(tmpdir(), 'dsh-subagent-process-test-'))).toBe(true) + expect(dir.path.startsWith(join(tmpdir(), 'dsh-subagent-subprocess-test-'))).toBe(true) const st = await stat(dir.path) expect(st.isDirectory()).toBe(true) // Private (0700) per the defensive-patterns temp-dir rule. @@ -269,8 +269,8 @@ describe('createIsolatedConfigDir', () => { }) it('creates a distinct dir per call (per-run isolation)', async () => { - const a = await createIsolatedConfigDir('dsh-subagent-process-test-') - const b = await createIsolatedConfigDir('dsh-subagent-process-test-') + const a = await createIsolatedConfigDir('dsh-subagent-subprocess-test-') + const b = await createIsolatedConfigDir('dsh-subagent-subprocess-test-') try { expect(a.path).not.toBe(b.path) } finally { @@ -280,7 +280,7 @@ describe('createIsolatedConfigDir', () => { }) it('remove() deletes a fresh dir recursively and is idempotent', async () => { - const dir = await createIsolatedConfigDir('dsh-subagent-process-test-') + const dir = await createIsolatedConfigDir('dsh-subagent-subprocess-test-') await writeFile(join(dir.path, 'settings.json'), '{}') await dir.remove() expect(existsSync(dir.path)).toBe(false) @@ -289,7 +289,7 @@ describe('createIsolatedConfigDir', () => { }) it('returns a pinned dir verbatim and NEVER removes it', async () => { - const pinned = await mkdtemp(join(tmpdir(), 'dsh-subagent-process-pinned-')) + const pinned = await mkdtemp(join(tmpdir(), 'dsh-subagent-subprocess-pinned-')) try { const dir = await createIsolatedConfigDir('ignored-prefix-', pinned) expect(dir.path).toBe(pinned) @@ -302,7 +302,7 @@ describe('createIsolatedConfigDir', () => { }) it('does not create a missing pinned path (the deployment owns its lifecycle)', async () => { - const missing = join(tmpdir(), `dsh-subagent-process-missing-${process.pid}`) + const missing = join(tmpdir(), `dsh-subagent-subprocess-missing-${process.pid}`) const dir = await createIsolatedConfigDir('ignored-prefix-', missing) expect(dir.path).toBe(missing) expect(existsSync(missing)).toBe(false) @@ -311,7 +311,7 @@ describe('createIsolatedConfigDir', () => { }) it('remove() is best-effort: an rm rejection resolves instead of rejecting', async () => { - const dir = await createIsolatedConfigDir('dsh-subagent-process-locked-') + const dir = await createIsolatedConfigDir('dsh-subagent-subprocess-locked-') try { // The swallow contract is error-kind agnostic; EACCES stands in for the // family (EBUSY, a vanished mount, …) that best-effort must absorb. diff --git a/packages/subagent/subagent-process/tsconfig.json b/packages/subagent/subagent-subprocess/tsconfig.json similarity index 100% rename from packages/subagent/subagent-process/tsconfig.json rename to packages/subagent/subagent-subprocess/tsconfig.json diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 248405f643..023588835a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -635,9 +635,9 @@ importers: '@deepseek-ai/dsh-subagent': specifier: workspace:^ version: link:../subagent - '@deepseek-ai/dsh-subagent-process': + '@deepseek-ai/dsh-subagent-subprocess': specifier: workspace:^ - version: link:../subagent-process + version: link:../subagent-subprocess cordis: specifier: ^4.0.0-rc.6 version: 4.0.0-rc.6(@cordisjs/plugin-include@1.0.4)(@cordisjs/plugin-loader@1.0.0-rc.4) @@ -715,12 +715,6 @@ importers: specifier: ^4.0.0-rc.6 version: 4.0.0-rc.6(@cordisjs/plugin-include@1.0.4)(@cordisjs/plugin-loader@1.0.0-rc.4) - packages/subagent/subagent-process: - devDependencies: - cordis: - specifier: ^4.0.0-rc.6 - version: 4.0.0-rc.6(@cordisjs/plugin-include@1.0.4)(@cordisjs/plugin-loader@1.0.0-rc.4) - packages/subagent/subagent-spawn: dependencies: schemastery: @@ -773,6 +767,12 @@ importers: specifier: ^4.0.0-rc.6 version: 4.0.0-rc.6(@cordisjs/plugin-include@1.0.4)(@cordisjs/plugin-loader@1.0.0-rc.4) + packages/subagent/subagent-subprocess: + devDependencies: + cordis: + specifier: ^4.0.0-rc.6 + version: 4.0.0-rc.6(@cordisjs/plugin-include@1.0.4)(@cordisjs/plugin-loader@1.0.0-rc.4) + packages/subagent/tool-subagent: dependencies: schemastery: diff --git a/tsconfig.build.json b/tsconfig.build.json index c5318e83fe..7fc770fac7 100644 --- a/tsconfig.build.json +++ b/tsconfig.build.json @@ -51,7 +51,7 @@ { "path": "./packages/support/subagent-mock" }, { "path": "./packages/subagent/tool-subagent" }, { "path": "./packages/subagent/subagent-inprocess" }, - { "path": "./packages/subagent/subagent-process" }, + { "path": "./packages/subagent/subagent-subprocess" }, { "path": "./packages/subagent/subagent-spawn" }, { "path": "./packages/subagent/subagent-fork" }, { "path": "./packages/subagent/subagent-acp" }, diff --git a/tsconfig.json b/tsconfig.json index 8882714d50..380d5f72f5 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -62,7 +62,7 @@ { "path": "./packages/support/subagent-mock" }, { "path": "./packages/subagent/tool-subagent" }, { "path": "./packages/subagent/subagent-inprocess" }, - { "path": "./packages/subagent/subagent-process" }, + { "path": "./packages/subagent/subagent-subprocess" }, { "path": "./packages/subagent/subagent-spawn" }, { "path": "./packages/subagent/subagent-fork" }, { "path": "./packages/subagent/subagent-acp" }, From 0ba6d832f80049a6f7c7c89092619296b915b0ac Mon Sep 17 00:00:00 2001 From: pku-xht Date: Thu, 9 Jul 2026 14:19:36 +0800 Subject: [PATCH 06/13] docs: regenerate the module graph on the merged tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The master merge (5309ea54) resolved the module-graph conflict by hand, placing the subagent-subprocess dependency-table row ahead of util/timeout's; the generator's deterministic order (group order, util first) wants them swapped, so the freshness gate (gen-module-graph --check) failed CI's static job. Regenerated on the merged tree — a two-line swap; every other generated catalog was already resolution-fresh (regen-all changed nothing else). --- docs/module-graph.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/module-graph.md b/docs/module-graph.md index 3cb0d6c386..2b606d38db 100644 --- a/docs/module-graph.md +++ b/docs/module-graph.md @@ -229,8 +229,8 @@ flowchart TD | Package | Group | Depends on | | --- | --- | --- | | [`brand`](../packages/util/brand) | `util` | — | -| [`subagent-subprocess`](../packages/subagent/subagent-subprocess) | `subagent` | — | | [`timeout`](../packages/util/timeout) | `util` | — | +| [`subagent-subprocess`](../packages/subagent/subagent-subprocess) | `subagent` | — | | [`acp-snapshot`](../packages/support/acp-snapshot) | `support` | — | | [`app-boot`](../packages/ui/app-boot) | `ui` | — | | [`code-runtime`](../packages/code-runtime/code-runtime) | `code-runtime` | — | From 9d2cf8ce826ee82c2af13f25de12bdf1920dda7e Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Fri, 10 Jul 2026 00:48:27 +0800 Subject: [PATCH 07/13] Add keyless snapshot refresh mode --- docs/testing.md | 8 +- examples/acp-agent/README.md | 2 +- examples/acp-agent/tests/acp.snapshot.ts | 28 +++- package.json | 1 + packages/support/acp-snapshot/README.md | 10 +- packages/support/acp-snapshot/src/suite.ts | 147 +++++++++++++++--- .../support/acp-snapshot/tests/suite.spec.ts | 117 +++++++++++++- vitest.snapshot.config.ts | 8 +- 8 files changed, 278 insertions(+), 43 deletions(-) diff --git a/docs/testing.md b/docs/testing.md index a4a77c9d0c..62989a1096 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -1,17 +1,17 @@ # Testing policy -How this repo tests, tier by tier, and the rules that keep a green suite meaning something. Commands live in the root [AGENTS.md](../AGENTS.md) § Commands; the RFCs linked per tier carry the design rationale. +How this repo tests, tier by tier, and the rules that keep a green suite meaningful. Commands live in root [AGENTS.md](../AGENTS.md); linked RFCs carry the rationale. ## Tiers -- **Unit** (`pnpm run test`): vitest over `packages|examples/*/tests/**/*.spec.ts`, colocated with what they test. Every registry gets an HMR-safety test (dispose the contributing fiber, assert cleanup). Excessive tests are welcome — err toward covering edge cases, error paths, event ordering, and concurrency races; review findings get regression tests (see `packages/core/agent-loop/tests/review-fixes.spec.ts`). +- **Unit** (`pnpm run test`): vitest over `packages|examples/*/tests/**/*.spec.ts`, colocated with what they test. Every registry gets an HMR-safety test (dispose the contributing fiber, assert cleanup). Prefer edge cases, error paths, event ordering, and concurrency races; review findings get regression tests (see `packages/core/agent-loop/tests/review-fixes.spec.ts`). - **Coverage gate** (`pnpm run test:coverage`): the gating run, per-file 100% on `packages/*/*/src`. An uncovered line is often dead code the gate is correctly flagging for deletion, not a missing test to bolt on. Line coverage is necessary, never sufficient — it proves lines ran, not that the feature works as shipped. - **Real-API e2e** (`pnpm run test:e2e`): with-key tests against live provider APIs — the DeepSeek model plus provider-specific smokes that gate on their own keys (`EXA_API_KEY`, `PERPLEXITY_API_KEY`, …); each suite self-skips without its key so keyless CI stays green ([real-API e2e RFC](rfc/implemented/testing/2026-06-19-real-api-e2e-ci.md)). -- **Snapshot** (`pnpm run test:snapshot`): boots the real example subprocess, replays a recorded session keyless, diffs normalized stdout + the re-persisted log against committed goldens ([snapshot RFC](rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md)). Re-record with `pnpm run test:snapshot:record`; reviewing the golden diff is part of the review. System-prompt/tool-schema content is pinned by ONE scenario (`text-turn`) and tokenized in every other fixture, so a prompt or schema edit churns one committed line ([pinned-header RFC](rfc/implemented/testing/2026-07-06-pin-request-header-content-in-one-scenario.md)). +- **Snapshot** (`pnpm run test:snapshot`): boots the real example subprocess, replays a recorded session keyless, diffs normalized stdout + the re-persisted log against committed goldens ([snapshot RFC](rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md)). Use `pnpm run test:snapshot:record` when the model transcript should change; use `pnpm run test:snapshot:refresh` when the committed transcript is still the right mock LLM input and replay goldens need keyless rewrite. Review the golden diff. System-prompt/tool-schema content is pinned by ONE scenario (`text-turn`) and tokenized in every other fixture, so a prompt or schema edit churns one committed line ([pinned-header RFC](rfc/implemented/testing/2026-07-06-pin-request-header-content-in-one-scenario.md)). ## The with-key policy: inference is cheap here -We are DeepSeek — do not ration real-API tests. A no-key test proves the plumbing; only a with-key run proves the agent works against a real model. Write many: real prompts that write files, multi-turn conversations, tool use, cancellation mid-stream. Cheapest and highest-value are **smoke tests** that boot the real example, send one real prompt, and check the world — they catch the "green unit tests, broken product" class that mocks structurally cannot ([postmortem 0001](postmortem/0001-acp-default-export-drops-inject.md)). The self-skip exists only so secretless CI and keyless contributors aren't blocked; it is not a cost signal. Every example ships a keyless smoke and — unless keyless-by-nature — a with-key smoke ([examples/AGENTS.md](../examples/AGENTS.md)). +We are DeepSeek — do not ration real-API tests. A no-key test proves plumbing; only a with-key run proves the agent works against a real model. Write many: file-writing prompts, multi-turn conversations, tool use, cancellation mid-stream. Highest-value are **smoke tests** that boot the real example, send one real prompt, and check the world — they catch the "green unit tests, broken product" class that mocks structurally cannot ([postmortem 0001](postmortem/0001-acp-default-export-drops-inject.md)). The self-skip exists only so secretless CI and keyless contributors aren't blocked; it is not a cost signal. Every example ships a keyless smoke and — unless keyless-by-nature — a with-key smoke ([examples/AGENTS.md](../examples/AGENTS.md)). ## Prefer the real implementation over a mock diff --git a/examples/acp-agent/README.md b/examples/acp-agent/README.md index 5b3c936651..85cb185c82 100644 --- a/examples/acp-agent/README.md +++ b/examples/acp-agent/README.md @@ -33,7 +33,7 @@ The editor sets each session's `cwd` to the project it opens; both the agent's b ## Snapshot tests (record-once / replay-deterministic) -This example is the home of the harness's **snapshot tests** — they boot this server as a real subprocess, drive it with a deterministic input script, and diff its normalized output against committed golden files. The model is made deterministic by `@deepseek-ai/dsh-llm-replay`, a function/namespace plugin that installs an `llm/stream` waterfall listener and short-circuits it, serving model streams reconstructed from a recorded **session JSONL** fixture (`/session.jsonl`) — so replay needs no API key. The fixture IS the persisted session log: its `assistant/chunk` events carry every `StreamChunk`, so grouping them by `(turn, step)` reconstructs each `stream()` call (one model call per loop step). Recording is therefore "run the real agent once and harvest the `.jsonl`". The two failure modes not expressible as logged chunks — a pure throw before any chunk, and cancel/hang — use an optional `/replay.override.json` sidecar (a `ReplayEntry[]` that replaces the derived script). A scenario that needs the agent to operate on existing files ships an optional `/workspace/` directory — the harness copies its contents into the temp cwd before the run (see `workspace-edit`). See [the ACP snapshot tests RFC](../../docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md) for the full design. +This example is the home of the harness's **snapshot tests** — they boot this server as a real subprocess, drive it with a deterministic input script, and diff its normalized output against committed golden files. The model is made deterministic by `@deepseek-ai/dsh-llm-replay`, a function/namespace plugin that installs an `llm/stream` waterfall listener and short-circuits it, serving model streams reconstructed from a recorded **session JSONL** fixture (`/session.jsonl`) — so replay needs no API key. The fixture IS the persisted session log: its `assistant/chunk` events carry every `StreamChunk`, so grouping them by `(turn, step)` reconstructs each `stream()` call (one model call per loop step). Recording is therefore "run the real agent once and harvest the `.jsonl`"; use `pnpm run test:snapshot:record` when the model transcript itself should change, and `pnpm run test:snapshot:refresh` when the committed model transcript is still the right mock input and only the current replay output/goldens need to be rewritten. The two failure modes not expressible as logged chunks — a pure throw before any chunk, and cancel/hang — use an optional `/replay.override.json` sidecar (a `ReplayEntry[]` that replaces the derived script). A scenario that needs the agent to operate on existing files ships an optional `/workspace/` directory — the harness copies its contents into the temp cwd before the run (see `workspace-edit`). See [the ACP snapshot tests RFC](../../docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md) for the full design. ## MVP limitations diff --git a/examples/acp-agent/tests/acp.snapshot.ts b/examples/acp-agent/tests/acp.snapshot.ts index d45ddf8591..7102d8ce88 100644 --- a/examples/acp-agent/tests/acp.snapshot.ts +++ b/examples/acp-agent/tests/acp.snapshot.ts @@ -1,15 +1,16 @@ import { fileURLToPath } from 'node:url' import { dirname, join } from 'node:path' -import { defineAcpSnapshotSuite, type Scenario } from '@deepseek-ai/dsh-acp-snapshot' +import { defineAcpSnapshotSuite, type Scenario, type SnapshotSuiteOptions } from '@deepseek-ai/dsh-acp-snapshot' /** * The acp-agent example's snapshot suite: the scenario table for * `dsh-acp-snapshot`'s suite factory, which owns every compare/guard mechanic - * (golden + re-persisted-log diffs, record write-back, the pinned-header + * (golden + re-persisted-log diffs, record/refresh write-back, the pinned-header * uniformity guard, the fixture guards). Fixtures live under `snapshots//`; - * `pnpm run test:snapshot:record` re-records the `recorded` scenarios against - * the real API. See the package README (packages/support/acp-snapshot) and the - * snapshot RFC, docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md. + * `pnpm run test:snapshot:record` re-records model transcripts against the real + * API; `pnpm run test:snapshot:refresh` rewrites current replay goldens keyless. + * See the package README (packages/support/acp-snapshot) and the snapshot RFC, + * docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md. */ // The dsh-acp-agent bin (the demo:acp entry), this example's cordis.yml, and @@ -26,6 +27,21 @@ const AGENT = { const CODE_MODE_CONFIG = fileURLToPath(new URL('../code-mode.cordis.yml', import.meta.url)) const BOTH_MODE_CONFIG = fileURLToPath(new URL('../both-mode.cordis.yml', import.meta.url)) +function snapshotModeFromEnv(value: string | undefined): SnapshotSuiteOptions['mode'] { + switch (value) { + case undefined: + case '': + case 'replay': + return 'replay' + case 'record': + return 'record' + case 'refresh': + return 'refresh' + default: + throw new Error(`unknown DSH_SNAPSHOT mode: ${value}`) + } +} + const SCENARIOS: Scenario[] = [ { name: 'handshake', hasModelTurn: false, recorded: false }, { name: 'reject-extra-dirs', hasModelTurn: false, recorded: false }, @@ -111,5 +127,5 @@ defineAcpSnapshotSuite({ agent: AGENT, snapshotsDir: join(dirname(fileURLToPath(import.meta.url)), 'snapshots'), scenarios: SCENARIOS, - mode: process.env.DSH_SNAPSHOT === 'record' ? 'record' : 'replay', + mode: snapshotModeFromEnv(process.env.DSH_SNAPSHOT), }) diff --git a/package.json b/package.json index c4cc307893..7f068c7f19 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,7 @@ "test:e2e": "vitest run --config vitest.e2e.config.ts", "test:snapshot": "vitest run --config vitest.snapshot.config.ts", "test:snapshot:record": "DSH_SNAPSHOT=record vitest run --config vitest.snapshot.config.ts --update", + "test:snapshot:refresh": "DSH_SNAPSHOT=refresh vitest run --config vitest.snapshot.config.ts", "check:ci": "tsx scripts/run-gates.ts ci-primary", "check:ci:static": "tsx scripts/run-gates.ts ci-static", "check:ci:lint": "tsx scripts/run-gates.ts ci-lint", diff --git a/packages/support/acp-snapshot/README.md b/packages/support/acp-snapshot/README.md index 209b1a81cb..8ff2fe1413 100644 --- a/packages/support/acp-snapshot/README.md +++ b/packages/support/acp-snapshot/README.md @@ -6,7 +6,7 @@ Three layers, importable separately: - **`runScenario` (harness)** — boots the real agent bin as a subprocess via tsx (unbuilt, Loader path), drives it over ACP JSON-RPC stdio from a deterministic `input.json` script, tees raw stdout for the golden + purity check, and harvests every persisted session JSONL (parent + subagent children, primary-first) after a graceful stdin-EOF shutdown. Parameterized by `AgentUnderTest` (`binScript`, `configPath`, `tsconfigPath` — absolute paths; the subprocess cwd is a temp dir outside the repo). - **Normalizers** — pure functions turning the two captured surfaces into stable text: `normalizeStdout` (JSON-RPC ids → first-seen sequence; UUIDs/cwd → tokens; doubles as the stdout-purity check), `normalizeSessionLog` (times zeroed, `seq` kept), and the composable `scrubRequestHeaders` (header bulk → `{{system}}`/`{{tools}}`, structure kept — [pinned-header RFC](../../../docs/rfc/implemented/testing/2026-07-06-pin-request-header-content-in-one-scenario.md)). -- **`defineAcpSnapshotSuite` (factory)** — registers the whole describe/it tree for a scenario table: per-scenario golden + re-persisted-log compares, record-mode fixture write-back, the per-header-class pin with its live uniformity guard, and the fixture guard block (no orphan scenario dirs, required files present, exactly one pin per class, pinning fixtures well-formed, non-pinning fixtures header-scrubbed). Must be called at vitest collection time. +- **`defineAcpSnapshotSuite` (factory)** — registers the whole describe/it tree for a scenario table: per-scenario golden + re-persisted-log compares, record/refresh fixture write-back, the per-header-class pin with its live uniformity guard, and the fixture guard block (no orphan scenario dirs, required files present, exactly one pin per class, pinning fixtures well-formed, non-pinning fixtures header-scrubbed). Must be called at vitest collection time. A consuming `*.snapshot.ts` is the scenario table plus one factory call: @@ -27,12 +27,16 @@ defineAcpSnapshotSuite({ }, snapshotsDir: join(dirname(fileURLToPath(import.meta.url)), 'snapshots'), scenarios: SCENARIOS, // exactly one entry per header class sets pinsHeader - mode: process.env.DSH_SNAPSHOT === 'record' ? 'record' : 'replay', + mode: process.env.DSH_SNAPSHOT === 'record' + ? 'record' + : process.env.DSH_SNAPSHOT === 'refresh' + ? 'refresh' + : 'replay', }) ``` A scenario booting a differently-composed tree sets its own `configPath` (an overlay whose basename still ends in `cordis.yml`, so the bin's replay swap finds the sibling `*cordis.snapshot.yml`) and, when that composition changes the request header, its own `headerClass` with its own pinning scenario — the acp-agent example's Code Mode scenarios are the template. -The example also ships a `cordis.snapshot.yml` replay overlay next to its `cordis.yml` (the bin swaps them under `DSH_SNAPSHOT=replay` — [single-source replay config RFC](../../../docs/rfc/implemented/testing/2026-07-04-single-source-acp-replay-config.md)); replay fixtures are served by [`dsh-llm-replay`](../llm-replay/README.md), which this package points at via the `DSH_SNAPSHOT_*` env vars it sets on the child. Fixture roles, record/replay semantics, and scenario-table fields are documented on `Scenario` and in the [snapshot RFC](../../../docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md). +The example also ships a `cordis.snapshot.yml` replay overlay next to its `cordis.yml` (the bin swaps them under `DSH_SNAPSHOT=replay` — [single-source replay config RFC](../../../docs/rfc/implemented/testing/2026-07-04-single-source-acp-replay-config.md)); replay fixtures are served by [`dsh-llm-replay`](../llm-replay/README.md), which this package points at via the `DSH_SNAPSHOT_*` env vars it sets on the child. `pnpm run test:snapshot:record` calls the live LLM and rewrites the recorded scenarios' model fixtures; `pnpm run test:snapshot:refresh` stays keyless, runs the replay overlay, and rewrites stdout plus comparable session-log goldens from the committed model scripts. Fixture roles, record/replay/refresh semantics, and scenario-table fields are documented on `Scenario` and in the [snapshot RFC](../../../docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md). Constraints: `suite.ts` imports vitest, so the package is importable only inside a vitest run (the harness and normalizers have no such dependency but ship from the same entry). ACP-specific by design — the harness speaks the SDK's `ClientSideConnection`. Permission round-trips are scriptable: `InputScript.permissionAnswers` is a FIFO queue of option-kind selections (`allow_once`, `reject_once`, …) the client maps to the agent-issued `optionId` at answer time; an absent or exhausted queue answers `cancelled`, and a kind the request never offered rejects the run (the agent is answered `cancelled`, so a tolerant agent cannot absorb the scenario bug). diff --git a/packages/support/acp-snapshot/src/suite.ts b/packages/support/acp-snapshot/src/suite.ts index 53e05d7876..f97cb82c59 100644 --- a/packages/support/acp-snapshot/src/suite.ts +++ b/packages/support/acp-snapshot/src/suite.ts @@ -23,8 +23,11 @@ * * `pnpm run test:snapshot:record` (DSH_SNAPSHOT=record + -u) re-records the * `session.jsonl` fixtures against the real API and refreshes the stdout golden - * in one pass; the caller resolves that env into {@link SnapshotSuiteOptions} - * (env reading stays at the suite edge, not in this library). + * in one pass. `pnpm run test:snapshot:refresh` (DSH_SNAPSHOT=refresh) instead + * replays the committed model scripts keylessly and writes the current stdout + * + persisted-log goldens back without calling a live LLM. The caller resolves + * that env into {@link SnapshotSuiteOptions} (env reading stays at the suite + * edge, not in this library). * * @module @deepseek-ai/dsh-acp-snapshot/suite */ @@ -124,12 +127,13 @@ export interface SnapshotSuiteOptions { /** The scenario table; exactly one entry must set `pinsHeader`. */ scenarios: Scenario[] /** - * `replay` (keyless, the default tier) or `record` (live API; re-records the - * `recorded` scenarios' fixtures and refreshes the vitest goldens under - * `--update`). The caller derives this from `$DSH_SNAPSHOT` — env reading - * stays outside this library. + * `replay` (keyless, the default tier), `record` (live API; re-records the + * `recorded` scenarios' fixtures and refreshes the Vitest goldens under + * `--update`), or `refresh` (keyless replay that rewrites stdout goldens and + * comparable session fixtures from the replay run). The caller derives this + * from `$DSH_SNAPSHOT` — env reading stays outside this library. */ - mode: 'replay' | 'record' + mode: 'replay' | 'record' | 'refresh' } /** @@ -201,6 +205,86 @@ export function headerDeltaCount(rawLog: string): number { .length } +/** A literal string replacement used to carry an existing fixture's volatile value into a refreshed log. */ +export interface FixtureReplacement { + /** The fresh replay-run value to replace. */ + from: string + /** The existing fixture value to keep. */ + to: string +} + +function parseJsonlRecords(text: string): Record[] { + return text.split('\n') + .filter(line => line.trim().length > 0) + .map(line => JSON.parse(line) as Record) +} + +/** + * Build the cross-log id/cwd replacements used by refresh write-back. + * + * @param logs The freshly harvested logs, in fixture order. + * @param fixtures The existing fixture contents, in matching order. + * @returns Literal replacements from fresh volatile values to the fixture's old values. + */ +export function refreshFixtureReplacements(logs: HarvestedLog[], fixtures: string[]): FixtureReplacement[] { + const replacements: FixtureReplacement[] = [] + for (let i = 0; i < logs.length; i++) { + const fresh = parseJsonlRecords((logs[i] as HarvestedLog).content)[0] + const existing = parseJsonlRecords(fixtures[i] ?? '')[0] + for (const field of ['id', 'cwd'] as const) { + const from = fresh?.[field] + const to = existing?.[field] + if (typeof from === 'string' && typeof to === 'string' && from.length > 0 && from !== to) { + replacements.push({ from, to }) + } + } + } + return replacements +} + +function preserveFixtureVolatiles(record: Record, existing: Record | undefined): void { + if (existing === undefined || existing.type !== record.type) return + if (record.type === 'session') { + for (const field of ['id', 'createdAt', 'cwd', 'parentSession', 'seedLength'] as const) { + if (field in record && field in existing) record[field] = existing[field] + } + return + } + if ('time' in record && 'time' in existing) record.time = existing.time + if (record.type !== 'hook/result') return + const data = record.data + const existingData = existing.data + if ( + data !== null && typeof data === 'object' + && existingData !== null && typeof existingData === 'object' + && 'durationMs' in data && 'durationMs' in existingData + ) { + (data as Record).durationMs = (existingData as Record).durationMs + } +} + +/** + * Rewrite a fresh replay-produced log so repeated refreshes do not churn + * volatile fixture fields. Meaningful event payloads come from `fresh`; the + * existing fixture lends session ids, cwd, creation times, event times, and + * hook durations where the record shape still matches. + * + * @param fresh The newly harvested session JSONL. + * @param existing The committed fixture JSONL being refreshed. + * @param replacements Cross-log literal replacements from {@link refreshFixtureReplacements}. + * @returns The stabilized JSONL content to write back. + */ +export function stabilizeRefreshLog(fresh: string, existing: string, replacements: FixtureReplacement[]): string { + let stable = fresh + for (const { from, to } of replacements) stable = stable.split(from).join(to) + const existingRecords = parseJsonlRecords(existing) + const records = parseJsonlRecords(stable) + for (let i = 0; i < records.length; i++) { + preserveFixtureVolatiles(records[i] as Record, existingRecords[i]) + } + return records.map(record => JSON.stringify(record)).join('\n') + '\n' +} + /** * Register the suite: one `describe` per scenario (the golden/log compares and * the header-uniformity guard) plus the fixture guard block (no orphan @@ -215,6 +299,8 @@ export function headerDeltaCount(rawLog: string): number { export function defineAcpSnapshotSuite(options: SnapshotSuiteOptions): void { const { agent, snapshotsDir, scenarios, mode } = options const RECORDING = mode === 'record' + const REFRESHING = mode === 'refresh' + const childMode: 'replay' | 'record' = RECORDING ? 'record' : 'replay' /** The class a scenario's header composition belongs to (see {@link Scenario.headerClass}). */ const classOf = (scenario: Scenario): string => scenario.headerClass ?? 'default' @@ -238,15 +324,18 @@ export function defineAcpSnapshotSuite(options: SnapshotSuiteOptions): void { describe(`snapshot: ${scenario.name}`, () => { // In RECORD mode, only re-run the `recorded` (live-API) scenarios; the // `authored` ones (sidecar-driven errors/cancel) are never re-recorded. + // REFRESH mode is replay-backed and deterministic, so it runs every + // scenario and rewrites the comparable fixtures from that replay run. it.skipIf(RECORDING && !scenario.recorded)('matches the goldens', async () => { const dir = join(snapshotsDir, scenario.name) const input = JSON.parse(await readFile(join(dir, 'input.json'), 'utf8')) as InputScript const overrideFile = join(dir, 'replay.override.json') const workspaceDir = join(dir, 'workspace') const childSessions = scenario.childSessions ?? 0 + const comparesLog = scenario.comparesLog ?? scenario.hasModelTurn const result = await runScenario(input, { agent, - mode, + mode: childMode, fixtureFile: join(dir, 'session.jsonl'), ...existsSync(overrideFile) ? { overrideFile } : {}, // In REPLAY, forward the recorded child fixtures so each subagent session @@ -271,30 +360,47 @@ export function defineAcpSnapshotSuite(options: SnapshotSuiteOptions): void { } // RECORD mode (recorded model scenarios only): persist the freshly-harvested - // logs back to their fixtures — the primary to session.jsonl, each child to - // session..jsonl in harvest order. `--update` refreshes the Vitest - // goldens but NOT these fixtures, so write them here. A non-pinning - // scenario's fixtures are written header-scrubbed, so a re-record can - // never smuggle the full prompt/schema content back into every fixture. + // live logs back to their fixtures. REFRESH mode does the same from a + // keyless replay run for every comparable log, including authored + // scenarios that live record deliberately skips. The primary goes to + // session.jsonl, each child to session..jsonl in harvest order. A + // non-pinning scenario's fixtures are written header-scrubbed, so a + // re-record/refresh can never smuggle the full prompt/schema content + // back into every fixture. const scrub = scenario.pinsHeader === true ? (log: string): string => log : scrubRequestHeaders - if (RECORDING && scenario.recorded && scenario.hasModelTurn) { - expect(result.sessionLogs.length, 'record produced no session log to harvest').toBeGreaterThan(0) + const fixtureFiles = ['session.jsonl', ...Array.from({ length: childSessions }, (_, i) => `session.${i + 1}.jsonl`)] + const existingFixtures = REFRESHING + ? await Promise.all(fixtureFiles.map(file => readFile(join(dir, file), 'utf8'))) + : [] + const replacements = REFRESHING ? refreshFixtureReplacements(result.sessionLogs, existingFixtures) : [] + const writesSessionFixtures = (RECORDING && scenario.recorded && scenario.hasModelTurn) + || (REFRESHING && comparesLog) + if (writesSessionFixtures) { + expect(result.sessionLogs.length, `${mode} produced no session log to harvest`).toBeGreaterThan(0) expect(result.sessionLogs.length, `expected ${childSessions + 1} session logs (parent + children)`) .toBe(childSessions + 1) - await writeFile(join(dir, 'session.jsonl'), scrub((result.sessionLogs[0] as HarvestedLog).content)) + const primary = (result.sessionLogs[0] as HarvestedLog).content + await writeFile(join(dir, 'session.jsonl'), scrub( + REFRESHING ? stabilizeRefreshLog(primary, existingFixtures[0] as string, replacements) : primary, + )) for (let i = 1; i < result.sessionLogs.length; i++) { - await writeFile(join(dir, `session.${i}.jsonl`), scrub((result.sessionLogs[i] as HarvestedLog).content)) + const child = (result.sessionLogs[i] as HarvestedLog).content + await writeFile(join(dir, `session.${i}.jsonl`), scrub( + REFRESHING ? stabilizeRefreshLog(child, existingFixtures[i] as string, replacements) : child, + )) } } - await expect(normalizeStdout(result.rawStdout, ctx)) - .toMatchFileSnapshot(join(dir, 'stdout.golden.jsonl')) + const stdout = normalizeStdout(result.rawStdout, ctx) + if (REFRESHING) { + await writeFile(join(dir, 'stdout.golden.jsonl'), stdout) + } + await expect(stdout).toMatchFileSnapshot(join(dir, 'stdout.golden.jsonl')) // A model turn always produces a log worth comparing; a hook scenario can // produce one without a model turn (a `rejected` turn carrying `hook/*`). - const comparesLog = scenario.comparesLog ?? scenario.hasModelTurn if (comparesLog) { // The harvested logs (primary-first) must match their committed fixtures // 1:1. Each side passes through normalizeSessionLog, scrubbed against ITS @@ -307,7 +413,6 @@ export function defineAcpSnapshotSuite(options: SnapshotSuiteOptions): void { // reason, and config, but not its bulk content (pinned once, in the // `pinsHeader` scenario). expect(result.sessionLogs.length, 'this scenario must persist a session log').toBe(childSessions + 1) - const fixtureFiles = ['session.jsonl', ...Array.from({ length: childSessions }, (_, i) => `session.${i + 1}.jsonl`)] for (let i = 0; i < fixtureFiles.length; i++) { const harvested = scrub((result.sessionLogs[i] as HarvestedLog).content) const fixture = scrub(await readFile(join(dir, fixtureFiles[i] as string), 'utf8')) diff --git a/packages/support/acp-snapshot/tests/suite.spec.ts b/packages/support/acp-snapshot/tests/suite.spec.ts index 0f47010a19..8e79e367f2 100644 --- a/packages/support/acp-snapshot/tests/suite.spec.ts +++ b/packages/support/acp-snapshot/tests/suite.spec.ts @@ -1,11 +1,18 @@ -import { cpSync, mkdtempSync } from 'node:fs' +import { cpSync, mkdtempSync, readFileSync, writeFileSync } from 'node:fs' import { rm } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { fileURLToPath } from 'node:url' import { afterAll, describe, expect, it } from 'vitest' -import { defineAcpSnapshotSuite, type Scenario } from '../src/index.ts' -import { childFixturePaths, fixtureContext, headerDeltaCount, normalizedHeaders } from '../src/suite.ts' +import { defineAcpSnapshotSuite, type HarvestedLog, type Scenario } from '../src/index.ts' +import { + childFixturePaths, + fixtureContext, + headerDeltaCount, + normalizedHeaders, + refreshFixtureReplacements, + stabilizeRefreshLog, +} from '../src/suite.ts' /** * Unit tests for the suite factory, by running it: two synthetic suites over @@ -55,16 +62,40 @@ const RECORD_SCENARIOS: Scenario[] = [ { name: 'rec-skip', hasModelTurn: true, recorded: false, overridden: true }, ] -// Record mode mutates its snapshots dir, so run it on a throwaway copy — -// except under the documented bootstrap knob, which regenerates the committed -// fixtures/goldens in place. +// Record/refresh modes mutate their snapshots dir, so run them on throwaway +// copies — except record's documented bootstrap knob, which regenerates the +// committed record fixtures/goldens in place. const BOOTSTRAP = process.env.ACP_SNAPSHOT_SPEC_BOOTSTRAP === '1' const recordDir = BOOTSTRAP ? RECORD_SRC : mkdtempSync(join(tmpdir(), 'acp-snap-record-suite-')) if (!BOOTSTRAP) cpSync(RECORD_SRC, recordDir, { recursive: true }) +const refreshDir = mkdtempSync(join(tmpdir(), 'acp-snap-refresh-suite-')) +cpSync(REPLAY_DIR, refreshDir, { recursive: true }) +staleRefreshFixtures(refreshDir) afterAll(async () => { if (!BOOTSTRAP) await rm(recordDir, { recursive: true, force: true }) + await rm(refreshDir, { recursive: true, force: true }) }) +function staleRefreshFixtures(dir: string): void { + writeFileSync(join(dir, 'plain-turn', 'stdout.golden.jsonl'), 'stale stdout\n') + + const plainBehaviorFile = join(dir, 'plain-turn', 'behavior.json') + const plainBehavior = JSON.parse(readFileSync(plainBehaviorFile, 'utf8')) as Record + plainBehavior.echoEnv = true + writeFileSync(plainBehaviorFile, `${JSON.stringify(plainBehavior, null, 2)}\n`) + + writeFileSync(join(dir, 'blocked-log', 'session.jsonl'), [ + '{"type":"session","id":"99999999-8888-4777-8666-555555555555","createdAt":13,"cwd":"/rec/blocked-cwd"}', + '{"type":"hook/result","seq":1,"time":13,"data":{"decision":"stale","durationMs":99}}', + '', + ].join('\n')) + writeFileSync(join(dir, 'authored-error', 'session.jsonl'), [ + '{"type":"session","id":"77777777-8888-4777-8666-555555555555","createdAt":13,"cwd":"/rec/error-cwd"}', + '{"type":"turn/end","seq":1,"time":9,"data":{"error":"stale"}}', + '', + ].join('\n')) +} + describe('defineAcpSnapshotSuite: replay mode', () => { defineAcpSnapshotSuite({ agent: AGENT, snapshotsDir: REPLAY_DIR, scenarios: REPLAY_SCENARIOS, mode: 'replay' }) }) @@ -75,6 +106,27 @@ describe('defineAcpSnapshotSuite: record mode', () => { defineAcpSnapshotSuite({ agent: AGENT, snapshotsDir: recordDir, scenarios: RECORD_SCENARIOS, mode: 'record' }) }) +describe('defineAcpSnapshotSuite: refresh mode', () => { + defineAcpSnapshotSuite({ agent: AGENT, snapshotsDir: refreshDir, scenarios: REPLAY_SCENARIOS, mode: 'refresh' }) +}) + +describe('defineAcpSnapshotSuite: refresh write-back', () => { + it('rewrites stdout and comparable logs from a replay-mode child run', () => { + const stdout = readFileSync(join(refreshDir, 'plain-turn', 'stdout.golden.jsonl'), 'utf8') + expect(stdout).not.toContain('stale stdout') + expect(stdout).toContain('env:{\\"mode\\":\\"replay\\"') + expect(stdout).not.toContain('\\"mode\\":\\"refresh\\"') + + const blocked = readFileSync(join(refreshDir, 'blocked-log', 'session.jsonl'), 'utf8') + expect(blocked).toContain('"decision":"block"') + expect(blocked).not.toContain('"decision":"stale"') + + const authored = readFileSync(join(refreshDir, 'authored-error', 'session.jsonl'), 'utf8') + expect(authored).toContain('"error":"model exploded"') + expect(authored).not.toContain('"error":"stale"') + }) +}) + describe('defineAcpSnapshotSuite: registration contract', () => { it("throws when a scenario's header class has no pinning scenario", () => { expect(() => { @@ -175,3 +227,56 @@ describe('headerDeltaCount', () => { expect(headerDeltaCount(`${other}\n`)).toBe(0) }) }) + +describe('refreshFixtureReplacements', () => { + it('maps fresh ids and cwd values to the existing fixture values, skipping non-replacements', () => { + const log = (content: string): HarvestedLog => ({ id: 'diagnostic', createdAt: 1, content }) + const logs = [ + log('{"type":"session","id":"","cwd":"/same"}\n'), + log('{"type":"session","id":"new-parent","cwd":"/new"}\n'), + log('{"type":"session","id":"new-child","cwd":"/new"}\n'), + ] + const fixtures = [ + '{"type":"session","id":"","cwd":"/same"}\n', + '{"type":"session","id":"old-parent","cwd":"/old"}\n', + ] + expect(refreshFixtureReplacements(logs, fixtures)).toEqual([ + { from: 'new-parent', to: 'old-parent' }, + { from: '/new', to: '/old' }, + ]) + }) +}) + +describe('stabilizeRefreshLog', () => { + it('keeps volatile fixture fields while preserving fresh meaningful payloads', () => { + const fresh = [ + '{"type":"session","id":"new-child","createdAt":200,"cwd":"/new","parentSession":"new-parent","seedLength":1}', + '{"type":"hook/result","seq":1,"time":22,"data":{"decision":"block","durationMs":37}}', + '{"type":"turn/end","seq":2,"time":33,"data":{"error":"fresh error"}}', + '{"type":"tool/result","seq":3,"time":44,"data":{"text":"new-parent in /new"}}', + '{"type":"hook/result","seq":4,"time":55,"data":{"decision":"allow","durationMs":5}}', + '', + ].join('\n') + const existing = [ + '{"type":"session","id":"old-child","createdAt":100,"cwd":"/old","parentSession":"old-parent","seedLength":5}', + '{"type":"hook/result","seq":1,"time":11,"data":{"decision":"stale","durationMs":99}}', + '{"type":"turn/end","seq":2,"data":{"error":"stale"}}', + '{"type":"assistant/message","seq":3,"time":12,"data":{"text":"different type"}}', + '{"type":"hook/result","seq":4,"time":13,"data":{"decision":"stale"}}', + '', + ].join('\n') + + expect(stabilizeRefreshLog(fresh, existing, [ + { from: 'new-parent', to: 'old-parent' }, + { from: 'new-child', to: 'old-child' }, + { from: '/new', to: '/old' }, + ])).toBe([ + '{"type":"session","id":"old-child","createdAt":100,"cwd":"/old","parentSession":"old-parent","seedLength":5}', + '{"type":"hook/result","seq":1,"time":11,"data":{"decision":"block","durationMs":99}}', + '{"type":"turn/end","seq":2,"time":33,"data":{"error":"fresh error"}}', + '{"type":"tool/result","seq":3,"time":44,"data":{"text":"old-parent in /old"}}', + '{"type":"hook/result","seq":4,"time":13,"data":{"decision":"allow","durationMs":5}}', + '', + ].join('\n')) + }) +}) diff --git a/vitest.snapshot.config.ts b/vitest.snapshot.config.ts index ecc8d911aa..d0a8ce6a54 100644 --- a/vitest.snapshot.config.ts +++ b/vitest.snapshot.config.ts @@ -7,10 +7,14 @@ import { defineConfig } from 'vitest/config' // normalized stdout transcript + re-persisted log against committed goldens. // `pnpm run test:snapshot:record` (DSH_SNAPSHOT=record + -u) re-records the // fixtures against the real API and refreshes the goldens. +// `pnpm run test:snapshot:refresh` (DSH_SNAPSHOT=refresh) stays keyless: it +// replays the committed model scripts and writes the current stdout/log goldens +// without calling the live LLM. // // Replay loads no .env (it must never reach the network — a recorded fixture -// drives the model). Record reads DEEPSEEK_API_KEY from the env or a gitignored -// repo-root .env, so a contributor with a key only in .env can still record. +// drives the model), and refresh uses that same keyless replay path. Record +// reads DEEPSEEK_API_KEY from the env or a gitignored repo-root .env, so a +// contributor with a key only in .env can still record. if (process.env.DSH_SNAPSHOT === 'record') { try { process.loadEnvFile(new URL('.env', import.meta.url).pathname) From 5f7177a0a983b0eb2733605026ee0d9bb4dcdb53 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Fri, 10 Jul 2026 00:55:47 +0800 Subject: [PATCH 08/13] Stabilize workflow worker coverage wait --- .../tests/workflow-workerthread.spec.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/workflow/workflow-workerthread/tests/workflow-workerthread.spec.ts b/packages/workflow/workflow-workerthread/tests/workflow-workerthread.spec.ts index ecaeed61f6..8d623e8a6f 100644 --- a/packages/workflow/workflow-workerthread/tests/workflow-workerthread.spec.ts +++ b/packages/workflow/workflow-workerthread/tests/workflow-workerthread.spec.ts @@ -17,6 +17,8 @@ function fakeParent(): Agent { /** The vm-context escape hatch, spelled once: real Worker tests use it to make the WORKER misbehave. */ const ESCAPE = "globalThis.constructor.constructor('return process')()" +/** Linux coverage workers can take longer than Vitest's default waitFor timeout to start executing script. */ +const WORKER_CHILD_START_TIMEOUT_MS = 3_000 /** One controllable child run: the test (or auto mode) settles it. */ interface ControlledRun { @@ -572,7 +574,7 @@ describe('dsh-workflow-workerthread', () => { `), parent: fakeParent(), }) - await vi.waitFor(() => { expect(starts).toBe(1) }) + await vi.waitFor(() => { expect(starts).toBe(1) }, { timeout: WORKER_CHILD_START_TIMEOUT_MS }) handle.cancel('stop now') await vi.waitFor(() => { expect(cancelled).toEqual(['stop now']) }, { timeout: 800 }) // The wedged worker's own completion loses to the in-flight cancel. @@ -602,7 +604,7 @@ describe('dsh-workflow-workerthread', () => { `), parent, }) - await vi.waitFor(() => { expect(provider.runs.length).toBe(1) }) + await vi.waitFor(() => { expect(provider.runs.length).toBe(1) }, { timeout: WORKER_CHILD_START_TIMEOUT_MS }) const before = Date.now() await handle.dispose() // Bounded by the grace (plus the terminate), never by the 1.5s spin. From af242dbae56df6b4092c3dd026e12a1682045256 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Fri, 10 Jul 2026 01:04:06 +0800 Subject: [PATCH 09/13] Preserve snapshot refresh seed boundaries --- packages/support/acp-snapshot/src/suite.ts | 2 +- packages/support/acp-snapshot/tests/suite.spec.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/support/acp-snapshot/src/suite.ts b/packages/support/acp-snapshot/src/suite.ts index f97cb82c59..27de939d3d 100644 --- a/packages/support/acp-snapshot/src/suite.ts +++ b/packages/support/acp-snapshot/src/suite.ts @@ -245,7 +245,7 @@ export function refreshFixtureReplacements(logs: HarvestedLog[], fixtures: strin function preserveFixtureVolatiles(record: Record, existing: Record | undefined): void { if (existing === undefined || existing.type !== record.type) return if (record.type === 'session') { - for (const field of ['id', 'createdAt', 'cwd', 'parentSession', 'seedLength'] as const) { + for (const field of ['id', 'createdAt', 'cwd', 'parentSession'] as const) { if (field in record && field in existing) record[field] = existing[field] } return diff --git a/packages/support/acp-snapshot/tests/suite.spec.ts b/packages/support/acp-snapshot/tests/suite.spec.ts index 8e79e367f2..ae01190c01 100644 --- a/packages/support/acp-snapshot/tests/suite.spec.ts +++ b/packages/support/acp-snapshot/tests/suite.spec.ts @@ -271,7 +271,7 @@ describe('stabilizeRefreshLog', () => { { from: 'new-child', to: 'old-child' }, { from: '/new', to: '/old' }, ])).toBe([ - '{"type":"session","id":"old-child","createdAt":100,"cwd":"/old","parentSession":"old-parent","seedLength":5}', + '{"type":"session","id":"old-child","createdAt":100,"cwd":"/old","parentSession":"old-parent","seedLength":1}', '{"type":"hook/result","seq":1,"time":11,"data":{"decision":"block","durationMs":99}}', '{"type":"turn/end","seq":2,"time":33,"data":{"error":"fresh error"}}', '{"type":"tool/result","seq":3,"time":44,"data":{"text":"old-parent in /old"}}', From 62627d7625deee296b564598c3463634c71b4623 Mon Sep 17 00:00:00 2001 From: pku-xht Date: Fri, 10 Jul 2026 16:42:20 +0800 Subject: [PATCH 10/13] fix(subagent-acp): contain onError sink exceptions to keep result from rejecting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit spec.onError is a caller-supplied callback boundary, but the flattening catch invoked it unguarded: a throwing sink rejected the whole async result, breaking the seam's "result never rejects on a child-level failure" contract (and docs/defensive-patterns.md's contain-callback-exceptions rule). The sink's own throw is now swallowed — named as such — while the original child failure still settles as stopReason 'error'. Regression test drives a spawn failure through a throwing sink and asserts result resolves. Same defect as the codex backend's, fixed there on PR #240; this is the symmetric fix for the already-merged ACP backend. --- packages/subagent/subagent-acp/src/run.ts | 9 +++++++- .../subagent-acp/tests/subagent-acp.spec.ts | 22 +++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/packages/subagent/subagent-acp/src/run.ts b/packages/subagent/subagent-acp/src/run.ts index a9fefba27c..9d5b17cb9a 100644 --- a/packages/subagent/subagent-acp/src/run.ts +++ b/packages/subagent/subagent-acp/src/run.ts @@ -89,6 +89,7 @@ export interface AcpRunSpec { * (the seam contract forbids `result` rejecting). The driver calls this with * the original error and the chosen stop reason so the fault is preserved * rather than silently lost; the provider wires it to `ctx.logger.warn`. + * A throw from the sink itself is contained — it cannot reject `result`. * Optional — omitted in a unit test that asserts the stop reason directly. */ onError?: (error: Error, stopReason: SubagentStopReason) => void @@ -336,7 +337,13 @@ export function startAcpRun(request: SubagentStartRequest, spec: AcpRunSpec): Su // (initialize/newSession/prompt transport/RPC errors, or ENOENT), not a // local bug. Flatten to `error` and surface the original via onError so a // real fault is preserved rather than silently lost. - spec.onError?.(toError(error), 'error') + try { + spec.onError?.(toError(error), 'error') + } catch { + // Swallows only the caller-supplied sink's OWN throw: an unguarded + // sink exception would reject `result` and break the contract above. + // The child-level failure being reported still settles as `error`. + } return { output: collectOutput(), stopReason: 'error' } } })() diff --git a/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts b/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts index 92c025077a..4d8a586c63 100644 --- a/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts +++ b/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts @@ -483,6 +483,28 @@ describe('dsh-subagent-acp', () => { await run.dispose() }) + it('resolves error (never rejects) even when the onError sink itself throws', async () => { + // onError is a caller-supplied callback boundary: its own exception must be + // contained, or it would reject `result` and break the seam's "result never + // rejects" contract that the flattening above exists to uphold. + const run = startAcpRun( + { prompt: [{ type: 'text', text: 'p' }], parent: fakeParent }, + { + command: '/nonexistent/acp-agent-binary', + args: [], + cwd: process.cwd(), + permission: 'reject', + env: {}, + disposeEofGraceMs: DEFAULT_DISPOSE_EOF_GRACE_MS, + disposeGraceMs: DEFAULT_DISPOSE_GRACE_MS, + onError: () => { throw new Error('sink boom') }, + }, + ) + const result = await run.result + expect(result.stopReason).toBe('error') + await run.dispose() + }) + it('settles aborted when the child crashes (tears the pipe) AFTER a cancel', async () => { // The child hangs, we cancel, and instead of answering the child exits hard // — the pending prompt RPC rejects. With a cancel already requested, the From 64b4e2ed2db6d5a27c1266b5c8bd97c8532929b1 Mon Sep 17 00:00:00 2001 From: pku-xht Date: Fri, 10 Jul 2026 16:43:41 +0800 Subject: [PATCH 11/13] test(workflow-workerthread): flake-proof the lifecycle spec's waits under CI load MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The spec's 16 vi.waitFor sites used the 1s default timeout to wait for worker-thread startup and child registration — CPU-bound work that blows past 1s on a contended runner. The CI coverage lane (4 vitest workers plus suites that spawn real subprocesses) hit this 3 times across 4 recent PR runs, each a different subset of the cancellation/worker-death tests, each green on rerun. Every wait now goes through a shared helper with a 10s bound, and the file sets a 30s test timeout to make room for it. The one deliberately tight wait keeps its 800ms bound through the helper's override — it proves the host (not the wedged worker's later loop turn) delivered the cancel, so a generous bound would erase what it tests. No behavior under test changed. --- .../tests/workflow-workerthread.spec.ts | 52 +++++++++++++------ 1 file changed, 36 insertions(+), 16 deletions(-) diff --git a/packages/workflow/workflow-workerthread/tests/workflow-workerthread.spec.ts b/packages/workflow/workflow-workerthread/tests/workflow-workerthread.spec.ts index ecaeed61f6..175a82a7b9 100644 --- a/packages/workflow/workflow-workerthread/tests/workflow-workerthread.spec.ts +++ b/packages/workflow/workflow-workerthread/tests/workflow-workerthread.spec.ts @@ -15,6 +15,26 @@ function fakeParent(): Agent { return { id: AgentId('workflow-parent'), options: {} } as unknown as Agent } +// Worker-thread startup is CPU-bound (a fresh thread compiles the runtime on +// every start): on a contended CI runner it regularly blows past vitest's 5s +// default test timeout, observed repeatedly on the coverage lane. +vi.setConfig({ testTimeout: 30_000 }) + +/** + * `vi.waitFor` with a contention-proof timeout: the 1s default flaked + * repeatedly on the CI coverage lane, where worker-thread cold start competes + * with three sibling vitest workers for CPU. Every wait in this file is for + * something that WILL happen (a worker starting, a child registering) — a + * generous bound only removes the flake, it cannot mask a genuine hang (the + * file-wide test timeout above still fences those). + * @param assertion - retried until it stops throwing or the timeout elapses. + * @param timeout - override for a wait that must stay deliberately tight. + * @returns resolves when the assertion passes. + */ +function waitFor(assertion: () => void, timeout = 10_000): Promise { + return vi.waitFor(assertion, { timeout, interval: 50 }) +} + /** The vm-context escape hatch, spelled once: real Worker tests use it to make the WORKER misbehave. */ const ESCAPE = "globalThis.constructor.constructor('return process')()" @@ -316,7 +336,7 @@ describe('dsh-workflow-workerthread', () => { const runEnds: WorkflowResultInfo[] = [] ctx.on('workflow/end', (_info, result) => { runEnds.push(result) }) const handle = ctx.workflows.start({ ...scripted("return await agent('long job')"), parent }) - await vi.waitFor(() => { expect(provider.runs.length).toBe(1) }) + await waitFor(() => { expect(provider.runs.length).toBe(1) }) handle.cancel('user stopped it') const result = await handle.result expect(result.stopReason).toBe('cancelled') @@ -357,7 +377,7 @@ describe('dsh-workflow-workerthread', () => { const controller = new AbortController() const second = ctx.workflows.start({ ...scripted("return await agent('job')"), parent, signal: controller.signal }) - await vi.waitFor(() => { expect(provider.runs.length).toBe(1) }) + await waitFor(() => { expect(provider.runs.length).toBe(1) }) controller.abort() expect((await second.result).stopReason).toBe('cancelled') await second.dispose() @@ -400,7 +420,7 @@ describe('dsh-workflow-workerthread', () => { `), parent, }) - await vi.waitFor(() => { expect(narration).toContain('started') }) + await waitFor(() => { expect(narration).toContain('started') }) handle.cancel('raced the completion') const result = await handle.result expect(result.stopReason).toBe('cancelled') @@ -480,7 +500,7 @@ describe('dsh-workflow-workerthread', () => { }) const result = await handle.result expect(result.stopReason).toBe('completed') - await vi.waitFor(() => { expect(provider.runs.length).toBe(1) }) + await waitFor(() => { expect(provider.runs.length).toBe(1) }) await handle.dispose() // Not a waitFor: by the time dispose() returns, the slow child disposal // must already be complete (host-side registry quiescence). @@ -526,7 +546,7 @@ describe('dsh-workflow-workerthread', () => { expect(result.stopReason).toBe('completed') // BEFORE dispose(): the settlement itself must have aborted the signal — // without it this child would stay live until dispose's terminate. - await vi.waitFor(() => { expect(aborted).toEqual(['workflow settled']) }) + await waitFor(() => { expect(aborted).toEqual(['workflow settled']) }) await handle.dispose() }) @@ -572,9 +592,9 @@ describe('dsh-workflow-workerthread', () => { `), parent: fakeParent(), }) - await vi.waitFor(() => { expect(starts).toBe(1) }) + await waitFor(() => { expect(starts).toBe(1) }) handle.cancel('stop now') - await vi.waitFor(() => { expect(cancelled).toEqual(['stop now']) }, { timeout: 800 }) + await waitFor(() => { expect(cancelled).toEqual(['stop now']) }, 800) // The wedged worker's own completion loses to the in-flight cancel. const result = await handle.result expect(result.stopReason).toBe('cancelled') @@ -602,7 +622,7 @@ describe('dsh-workflow-workerthread', () => { `), parent, }) - await vi.waitFor(() => { expect(provider.runs.length).toBe(1) }) + await waitFor(() => { expect(provider.runs.length).toBe(1) }) const before = Date.now() await handle.dispose() // Bounded by the grace (plus the terminate), never by the 1.5s spin. @@ -625,7 +645,7 @@ describe('dsh-workflow-workerthread', () => { `), parent, }) - await vi.waitFor(() => { expect(provider.runs.length).toBe(1) }) + await waitFor(() => { expect(provider.runs.length).toBe(1) }) const handleDispose = handle.dispose() const result = await handle.result // The script itself settled (the wrapper's own dispose RPC found the @@ -663,7 +683,7 @@ describe('dsh-workflow-workerthread', () => { `), parent, }) - await vi.waitFor(() => { expect(order.filter(entry => entry.startsWith('start:')).length).toBe(2) }) + await waitFor(() => { expect(order.filter(entry => entry.startsWith('start:')).length).toBe(2) }) const fast = provider.runs.find(run => (run.request.prompt[0] as { text?: string }).text === 'fast')! fast.settle(text('fast done')) handle.cancel('stop now') @@ -694,7 +714,7 @@ describe('dsh-workflow-workerthread', () => { ...scripted("await parallel([() => agent('a'), () => agent('b')])\nreturn 'unreachable'"), parent, }) - await vi.waitFor(() => { expect(provider.runs.length).toBe(2) }) + await waitFor(() => { expect(provider.runs.length).toBe(2) }) handle.cancel('user stop') const result = await handle.result expect(result.stopReason).toBe('cancelled') @@ -749,7 +769,7 @@ describe('dsh-workflow-workerthread', () => { // A worker death is a stop reason like any other: workflow/end fires // with the error outcome — for a bus observer it is the only obituary. expect(runEnds).toEqual([{ stopReason: 'error', error: result.error, agentsStarted: 1 }]) - await vi.waitFor(() => { expect(cancelled.length).toBe(1) }) + await waitFor(() => { expect(cancelled.length).toBe(1) }) await handle.dispose() }, 15_000) @@ -770,7 +790,7 @@ describe('dsh-workflow-workerthread', () => { expect(result.stopReason).toBe('error') expect(result.error).toContain('worker blew up') // The reap wound the stray child down (cancel + a CLEAN dispose). - await vi.waitFor(() => { + await waitFor(() => { expect(provider.runs.length).toBe(1) expect(provider.runs[0]!.disposed).toBe(true) }) @@ -802,7 +822,7 @@ describe('dsh-workflow-workerthread', () => { `), parent, }) - await vi.waitFor(() => { expect(order.filter(entry => entry.startsWith('start:')).length).toBe(2) }) + await waitFor(() => { expect(order.filter(entry => entry.startsWith('start:')).length).toBe(2) }) const fast = provider.runs.find(run => (run.request.prompt[0] as { text?: string }).text === 'fast')! fast.settle(text('fast done')) const result = await handle.result @@ -837,7 +857,7 @@ describe('dsh-workflow-workerthread', () => { const result = await handle.result expect(result.stopReason).toBe('error') expect(result.error).toContain('exit code 5') - await vi.waitFor(() => { expect(provider.runs[0]!.disposed).toBe(true) }) + await waitFor(() => { expect(provider.runs[0]!.disposed).toBe(true) }) await handle.dispose() }, 15_000) @@ -855,7 +875,7 @@ describe('dsh-workflow-workerthread', () => { }) const logs: string[] = [] ctx.on('workflow/log', (_info, message) => { logs.push(message) }) - await vi.waitFor(() => { expect(logs).toContain('armed') }) + await waitFor(() => { expect(logs).toContain('armed') }) handle.cancel('stop it') // The grace is deliberately huge: only the worker's own death (exit 3, // unreachable by the cancel — the script ignores hooks) settles this. From dd2f37b80fa1e69403268f02bd64b4576daa01f8 Mon Sep 17 00:00:00 2001 From: pku-xht Date: Fri, 10 Jul 2026 20:48:14 +0800 Subject: [PATCH 12/13] fix(workflow-workerthread): tighten post-result promptness waits back down MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up: the blanket 10s default correctly targets worker-thread cold-start races (starting, first-script-line, async child-registration messages — genuinely CPU-bound under CI contention), but four waits assert something different — that the HOST reacted PROMPTLY to an event that already happened (a settled result, an observed worker death). Those had no cold-start left to wait on, so the generous default just widened the window a real regression could hide in. Verified by injecting a 6s delay into the settle-reap's abort call: the un-overridden helper's test still passed in ~6s. The same mutation now fails in ~1s with the explicit 1000ms override restored on all four sites (the abort-on-settle test's own assertion, the two worker-death cancel/dispose reap checks, and the dispose-ack-race check). The other 12 waits keep the 10s default — they run BEFORE a result is awaited, waiting on the worker to actually start rather than on a host reaction. Doc comment corrected to describe the split instead of claiming every wait is a cold-start race. --- .../tests/workflow-workerthread.spec.ts | 38 +++++++++++++------ 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/packages/workflow/workflow-workerthread/tests/workflow-workerthread.spec.ts b/packages/workflow/workflow-workerthread/tests/workflow-workerthread.spec.ts index 175a82a7b9..75a0b5116e 100644 --- a/packages/workflow/workflow-workerthread/tests/workflow-workerthread.spec.ts +++ b/packages/workflow/workflow-workerthread/tests/workflow-workerthread.spec.ts @@ -21,12 +21,18 @@ function fakeParent(): Agent { vi.setConfig({ testTimeout: 30_000 }) /** - * `vi.waitFor` with a contention-proof timeout: the 1s default flaked - * repeatedly on the CI coverage lane, where worker-thread cold start competes - * with three sibling vitest workers for CPU. Every wait in this file is for - * something that WILL happen (a worker starting, a child registering) — a - * generous bound only removes the flake, it cannot mask a genuine hang (the - * file-wide test timeout above still fences those). + * `vi.waitFor` with a contention-proof default timeout: the 1s default + * flaked repeatedly on the CI coverage lane, where worker-thread cold start + * (CPU-bound — a fresh thread compiles the runtime) competes with three + * sibling vitest workers for CPU. The 10s default is for exactly those + * races — waiting for a worker to start, run its first script line, or + * deliver an async child-registration message to the host. It is NOT for a + * wait that asserts the HOST reacted PROMPTLY to something that already + * happened (a settled result, an observed worker death): those keep an + * explicit tight override below, or the generous default would silently + * accept a multi-second regression in host-side reap latency as passing + * (proven by injecting a 6s delay into one such reap and watching the + * un-overridden version of this helper still pass in ~6s). * @param assertion - retried until it stops throwing or the timeout elapses. * @param timeout - override for a wait that must stay deliberately tight. * @returns resolves when the assertion passes. @@ -545,8 +551,11 @@ describe('dsh-workflow-workerthread', () => { const result = await handle.result expect(result.stopReason).toBe('completed') // BEFORE dispose(): the settlement itself must have aborted the signal — - // without it this child would stay live until dispose's terminate. - await waitFor(() => { expect(aborted).toEqual(['workflow settled']) }) + // without it this child would stay live until dispose's terminate. This + // is a HOST-PROMPTNESS claim, not a cold-start race — a tight explicit + // bound (unlike the file default) so a multi-second reap regression + // cannot pass by outlasting the wait. + await waitFor(() => { expect(aborted).toEqual(['workflow settled']) }, 1000) await handle.dispose() }) @@ -769,7 +778,9 @@ describe('dsh-workflow-workerthread', () => { // A worker death is a stop reason like any other: workflow/end fires // with the error outcome — for a bus observer it is the only obituary. expect(runEnds).toEqual([{ stopReason: 'error', error: result.error, agentsStarted: 1 }]) - await waitFor(() => { expect(cancelled.length).toBe(1) }) + // Result already settled — this is the reap's promptness, not a + // cold-start race; tight explicit bound (see the helper's doc comment). + await waitFor(() => { expect(cancelled.length).toBe(1) }, 1000) await handle.dispose() }, 15_000) @@ -790,10 +801,12 @@ describe('dsh-workflow-workerthread', () => { expect(result.stopReason).toBe('error') expect(result.error).toContain('worker blew up') // The reap wound the stray child down (cancel + a CLEAN dispose). + // Result already settled — this is the reap's promptness, not a + // cold-start race; tight explicit bound (see the helper's doc comment). await waitFor(() => { expect(provider.runs.length).toBe(1) expect(provider.runs[0]!.disposed).toBe(true) - }) + }, 1000) await handle.dispose() }, 15_000) @@ -857,7 +870,10 @@ describe('dsh-workflow-workerthread', () => { const result = await handle.result expect(result.stopReason).toBe('error') expect(result.error).toContain('exit code 5') - await waitFor(() => { expect(provider.runs[0]!.disposed).toBe(true) }) + // Result already settled — this is the reap's promptness (bounded + // above the mock's fixed 300ms dispose delay, not a cold-start race); + // tight explicit bound (see the helper's doc comment). + await waitFor(() => { expect(provider.runs[0]!.disposed).toBe(true) }, 1000) await handle.dispose() }, 15_000) From 53f6a959e2f9721ac676a91bbda58dfd7121a1d9 Mon Sep 17 00:00:00 2001 From: kingwl Date: Fri, 10 Jul 2026 21:15:48 +0800 Subject: [PATCH 13/13] chore(lint): one shared project service; ignore harness-local .claude state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two independent multipliers were pushing bare `pnpm run lint` past node's default heap: 1. parserOptions.project expanded to every package tsconfig plus the root one — each matched config built its OWN ts.Program, and the dev paths map pulls sibling package SOURCES (plus the vendored Cordis stack) into every such program, so resident memory grew as sum-of-closures, not repo size: ~4.6 GB peak for 425 repo files. projectService shares one tsserver-style graph: ~2.0 GB peak, ~28 s → ~14 s wall. 2. `eslint .` traversed .claude/ harness-local state — stale worktree checkouts there carry tens of thousands of additional .ts files (whole-repo copies), roughly tripling the work again even under the project service. Other checkouts are not this one's sources; ignore them like node_modules. (#169 carries the identical ignore line inside its chain; the hunks dedupe on its next rebase.) Type-aware rules verified live under the service: a floating-promise probe still trips no-floating-promises. --- eslint.config.mjs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/eslint.config.mjs b/eslint.config.mjs index cbb696bccb..c62d3e9739 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -22,6 +22,7 @@ export default tseslint.config( '**/lib/**', '**/node_modules/**', '**/.sessions/**', + '.claude/**', // harness-local state (worktrees, skills) — other checkouts, not this one's sources '**/.doc-typecheck-*/**', 'vendor/**', // vendored source keeps upstream style and idioms '**/*.js', @@ -38,7 +39,14 @@ export default tseslint.config( ], languageOptions: { parserOptions: { - project: ['./packages/*/*/tsconfig.json', './tsconfig.json'], + // One shared tsserver-style project service instead of 60+ standalone + // per-package programs: the old `project` glob built every package's + // full dependency closure (sibling sources via the dev `paths` map + + // the vendored Cordis stack) as its own program and kept them all + // resident — ~5 GB peak, an OOM past node's default heap. The service + // resolves each file to its nearest owning tsconfig and shares the + // graph. + projectService: true, tsconfigRootDir: import.meta.dirname, }, }, @@ -89,7 +97,9 @@ export default tseslint.config( ], languageOptions: { parserOptions: { - project: ['./tsconfig.json'], + // Same shared project service as the src block: test files resolve + // through the root tsconfig (its include covers every tests/ tree). + projectService: true, tsconfigRootDir: import.meta.dirname, }, },