diff --git a/docs/config-catalog.md b/docs/config-catalog.md index 46aa0ea5d6..3d54b2fcb2 100644 --- a/docs/config-catalog.md +++ b/docs/config-catalog.md @@ -1102,4 +1102,5 @@ 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-subprocess` ([`packages/subagent/subagent-subprocess/src/index.ts`](../packages/subagent/subagent-subprocess/src/index.ts)) - `@deepseek-ai/dsh-timeout` ([`packages/util/timeout/src/index.ts`](../packages/util/timeout/src/index.ts)) diff --git a/docs/module-graph.md b/docs/module-graph.md index 064a154f2e..accb46d8a6 100644 --- a/docs/module-graph.md +++ b/docs/module-graph.md @@ -46,6 +46,7 @@ flowchart TD pkg_subagent_fork["subagent-fork"] pkg_subagent_inprocess["subagent-inprocess"] pkg_subagent_spawn["subagent-spawn"] + pkg_subagent_subprocess["subagent-subprocess"] pkg_tool_subagent["tool-subagent"] end subgraph group_web["packages/web"] @@ -239,6 +240,7 @@ flowchart TD pkg_subagent_acp --> pkg_agent pkg_subagent_acp --> pkg_llm pkg_subagent_acp --> pkg_subagent + pkg_subagent_acp --> pkg_subagent_subprocess pkg_subagent_inprocess --> pkg_agent pkg_subagent_inprocess --> pkg_llm pkg_subagent_inprocess --> pkg_session @@ -291,6 +293,7 @@ flowchart TD | --- | --- | --- | | [`brand`](../packages/util/brand) | `util` | — | | [`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` | — | @@ -340,7 +343,7 @@ flowchart TD | [`repeat-tool-guard`](../packages/guard/repeat-tool-guard) | `guard` | [`agent`](../packages/core/agent), [`tools`](../packages/core/tools) | | [`tool-workflow`](../packages/workflow/tool-workflow) | `workflow` | [`agent`](../packages/core/agent), [`llm`](../packages/llm/llm), [`system-prompt`](../packages/core/system-prompt), [`tools`](../packages/core/tools), [`workflow`](../packages/workflow/workflow) | | [`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-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/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/eslint.config.mjs b/eslint.config.mjs index 500f42f4de..c62d3e9739 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -39,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, }, }, @@ -90,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, }, }, diff --git a/examples/acp-agent/README.md b/examples/acp-agent/README.md index c78943284d..e3d8542efd 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/knip.json b/knip.json index 296894e877..cf43b90e34 100644 --- a/knip.json +++ b/knip.json @@ -89,6 +89,11 @@ "entry": ["tests/**/*.spec.ts", "tests/**/*.e2e.ts", "tests/mock-acp-server.ts"], "project": ["src/**/*.ts", "tests/**/*.ts"] }, + "packages/subagent/subagent-subprocess": { + "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/package.json b/package.json index d3c05dae78..0fc57b7db5 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/subagent/README.md b/packages/subagent/README.md index 6da8ee42f5..87930167e9 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-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), 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-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 81bf886067..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 -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-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 2c55051da9..e73d861a79 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-subprocess": "^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-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 d7631f4d8d..9d5b17cb9a 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-subprocess' /** * How the client answers a child's `session/request_permission`. The first cut @@ -88,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 @@ -110,31 +112,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 +173,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 +213,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[] = [] @@ -380,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' } } })() @@ -393,33 +356,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..4d8a586c63 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-subprocess' 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 @@ -482,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 diff --git a/packages/subagent/subagent-acp/tsconfig.json b/packages/subagent/subagent-acp/tsconfig.json index 3c06fef150..e415ace1de 100644 --- a/packages/subagent/subagent-acp/tsconfig.json +++ b/packages/subagent/subagent-acp/tsconfig.json @@ -25,6 +25,9 @@ }, { "path": "../subagent" + }, + { + "path": "../subagent-subprocess" } ] } diff --git a/packages/subagent/subagent-subprocess/README.md b/packages/subagent/subagent-subprocess/README.md new file mode 100644 index 0000000000..ccc68bf31b --- /dev/null +++ b/packages/subagent/subagent-subprocess/README.md @@ -0,0 +1,40 @@ +# @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). + +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 race cleans up after itself on both outcomes — the pending timer is `unref()`ed and cleared on exit, the exit listener removed on timeout — so repeated calls (the dispose ladder's tiers, a poll loop) never accumulate listeners on the child. + +### `disposeChildProcess(child, graces)` + +The three-tier dispose ladder. Resolves only once the child has ACTUALLY exited — quiescence reached, not merely requested (see [defensive patterns](../../../docs/defensive-patterns.md)): + +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-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-subprocess/package.json b/packages/subagent/subagent-subprocess/package.json new file mode 100644 index 0000000000..68f525dd8e --- /dev/null +++ b/packages/subagent/subagent-subprocess/package.json @@ -0,0 +1,30 @@ +{ + "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, + "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-subprocess/src/index.ts b/packages/subagent/subagent-subprocess/src/index.ts new file mode 100644 index 0000000000..35d7383456 --- /dev/null +++ b/packages/subagent/subagent-subprocess/src/index.ts @@ -0,0 +1,219 @@ +/** + * 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-subprocess + */ + +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. 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` (immediately if it is + * already gone), `false` on timeout. + */ +export function exitsWithin(child: ChildProcess, ms: number): Promise { + if (child.exitCode !== null || child.signalCode !== null) return Promise.resolve(true) + return new Promise((resolve) => { + const onExit = (): void => { + clearTimeout(timer) + resolve(true) + } + // `.unref()` so a pending grace timer never keeps the parent's loop alive. + const timer = setTimeout(() => { + child.removeListener('exit', onExit) + resolve(false) + }, ms).unref() + child.once('exit', onExit) + }) +} + +/** + * 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-subprocess/tests/subagent-subprocess.spec.ts b/packages/subagent/subagent-subprocess/tests/subagent-subprocess.spec.ts new file mode 100644 index 0000000000..2766ed0a41 --- /dev/null +++ b/packages/subagent/subagent-subprocess/tests/subagent-subprocess.spec.ts @@ -0,0 +1,326 @@ +import { describe, expect, it, vi } from 'vitest' +import { EventEmitter } from 'node:events' +import { existsSync } from 'node:fs' +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' +import { + buildChildEnv, + createIsolatedConfigDir, + disposeChildProcess, + exitsWithin, + SENSITIVE_ENV_PATTERN, + spawnFailure, + waitForExit, +} from '../src/index.ts' + +// `rm` is wrapped (real-passthrough by default) so ONE test can inject a +// 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 (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}. */ +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 immediately for an already-exited child (no listener attached)', async () => { + const fake = new FakeChild() + fake.exitCode = 0 + await expect(exitsWithin(asChild(fake), 1000)).resolves.toBe(true) + expect(fake.listenerCount('exit')).toBe(0) + }) + + it('exitsWithin resolves true when the child exits inside the window', async () => { + const fake = new FakeChild({ diesOn: 'SIGTERM', delayMs: 5 }) + fake.kill('SIGTERM') + await expect(exitsWithin(asChild(fake), 1000)).resolves.toBe(true) + // The once-listener fired and the grace timer was cleared — nothing lingers. + expect(fake.listenerCount('exit')).toBe(0) + }) + + it('exitsWithin resolves false on timeout for a child that never exits', async () => { + const fake = new FakeChild() // nothing short of SIGKILL fells it; no signal sent + await expect(exitsWithin(asChild(fake), 20)).resolves.toBe(false) + // The timeout arm removed its exit listener: repeated waits (a poll loop, + // the ladder's tiers) never accumulate listeners on the same child. + expect(fake.listenerCount('exit')).toBe(0) + }) +}) + +describe('disposeChildProcess', () => { + it('returns immediately for an already-exited child (no EOF, no signals)', async () => { + const fake = new FakeChild() + 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-subprocess-test-') + try { + 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. + 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-subprocess-test-') + const b = await createIsolatedConfigDir('dsh-subagent-subprocess-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-subprocess-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-subprocess-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-subprocess-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 rejection resolves instead of rejecting', async () => { + 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. + vi.mocked(rm).mockRejectedValueOnce(Object.assign(new Error('EACCES: permission denied'), { code: 'EACCES' })) + await expect(dir.remove()).resolves.toBeUndefined() + // The injected rejection consumed the only rm call — nothing was deleted. + expect(existsSync(dir.path)).toBe(true) + } finally { + await rm(dir.path, { recursive: true, force: true }) + } + }) +}) diff --git a/packages/subagent/subagent-subprocess/tsconfig.json b/packages/subagent/subagent-subprocess/tsconfig.json new file mode 100644 index 0000000000..749cb0208e --- /dev/null +++ b/packages/subagent/subagent-subprocess/tsconfig.json @@ -0,0 +1,11 @@ +{ + "extends": "../../../tsconfig.base.json", + "compilerOptions": { + "rootDir": "src", + "outDir": "lib/types" + }, + "include": [ + "src" + ], + "references": [] +} diff --git a/packages/support/acp-snapshot/README.md b/packages/support/acp-snapshot/README.md index ff1738f462..002dbf8a19 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). Session config options are scriptable too: the `setConfigOption` step switches a knob over `session/set_config_option`, and `setConfigOptionExpectError` asserts the bridge rejects an unknown id or out-of-vocabulary value (the error frame stays in the transcript). diff --git a/packages/support/acp-snapshot/src/suite.ts b/packages/support/acp-snapshot/src/suite.ts index 0508b37e13..6e0058ebce 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 */ @@ -133,12 +136,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' } /** @@ -210,6 +214,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'] 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 @@ -224,6 +308,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' @@ -247,15 +333,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 @@ -280,30 +369,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 @@ -316,7 +422,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..ae01190c01 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":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"}}', + '{"type":"hook/result","seq":4,"time":13,"data":{"decision":"allow","durationMs":5}}', + '', + ].join('\n')) + }) +}) diff --git a/packages/workflow/workflow-workerthread/tests/workflow-workerthread.spec.ts b/packages/workflow/workflow-workerthread/tests/workflow-workerthread.spec.ts index 38a5435b69..75a0b5116e 100644 --- a/packages/workflow/workflow-workerthread/tests/workflow-workerthread.spec.ts +++ b/packages/workflow/workflow-workerthread/tests/workflow-workerthread.spec.ts @@ -15,6 +15,32 @@ 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 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. + */ +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 +342,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 +383,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 +426,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 +506,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). @@ -525,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 vi.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() }) @@ -572,9 +601,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 +631,7 @@ describe('dsh-workflow-workerthread', () => { `), parent, }) - await vi.waitFor(() => { expect(provider.runs.length).toBe(1) }, { timeout: 10_000 }) + 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 +654,7 @@ describe('dsh-workflow-workerthread', () => { `), parent, }) - await vi.waitFor(() => { expect(provider.runs.length).toBe(1) }, { timeout: 10_000 }) + 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 +692,7 @@ describe('dsh-workflow-workerthread', () => { `), parent, }) - await vi.waitFor(() => { expect(order.filter(entry => entry.startsWith('start:')).length).toBe(2) }, { timeout: 10_000 }) + 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 +723,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 +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 vi.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) @@ -770,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). - await vi.waitFor(() => { + // 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) @@ -802,7 +835,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 +870,10 @@ 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) }) + // 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) @@ -855,7 +891,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. diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index c22e799340..7a2e9b13fc 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -781,6 +781,9 @@ importers: '@deepseek-ai/dsh-subagent': specifier: workspace:^ version: link:../subagent + '@deepseek-ai/dsh-subagent-subprocess': + specifier: workspace:^ + 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) @@ -910,6 +913,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 09a2795a88..1622d7f2eb 100644 --- a/tsconfig.build.json +++ b/tsconfig.build.json @@ -59,6 +59,7 @@ { "path": "./packages/support/subagent-mock" }, { "path": "./packages/subagent/tool-subagent" }, { "path": "./packages/subagent/subagent-inprocess" }, + { "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 cf630b244c..5597c8c082 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -70,6 +70,7 @@ { "path": "./packages/support/subagent-mock" }, { "path": "./packages/subagent/tool-subagent" }, { "path": "./packages/subagent/subagent-inprocess" }, + { "path": "./packages/subagent/subagent-subprocess" }, { "path": "./packages/subagent/subagent-spawn" }, { "path": "./packages/subagent/subagent-fork" }, { "path": "./packages/subagent/subagent-acp" }, 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)