Files

T

Tianyi Cui 3672cd25b4 feat(subprocess): migrate lsp-local, subagent-acp, and the env scrubs onto the seam

Review direction (tianyicui, PR #660): in a stacked PR, change all other
process-running places to use the new service.

- lsp-local: LspConnection spawns through ctx.subprocess (piped protocol
  streams + a no-spill collected stderr tail); its private process-tree
  helpers (POSIX group signalling, Windows taskkill, liveness polling) are
  deleted in favor of the seam's handle verbs, and its buildChildEnv now
  rides scrubbedParentEnv (LSP children also stop inheriting stale DSH_*).
  The plugin injects 'subprocess'; compositions/tests mount
  dsh-subprocess-local.
- subagent-acp: the ACP child spawns through the seam (piped ndjson streams,
  inherited stderr); spawn failure surfaces through done-rejection into the
  same startup race; disposal is handle.dispose with the plugin's configured
  graces. dsh-subagent-subprocess is DELETED — its dispose ladder and scrub
  are the seam's, and the isolated-config-dir helper had no consumer.
- mcp-client, pty-local, sdk-helper: adopt scrubbedParentEnv as the one
  scrub definition (their spawns stay put by ownership: the MCP SDK and
  node-pty own those calls; the SDK wizard runs outside any composition).
- Coverage: per-file 100% over every touched src file, with each v8 ignore
  carrying a platform or contract reason; new suites cover stdio
  dispositions, the dispose ladder tiers, injected-win32 tree semantics,
  waitForExit, settled-kill/terminate no-ops, and spawn-failure disposal.
- Docs: consumer-migration Agent Note (en; zh follows in this PR), seam note
  updated in place, subprocess.md rewritten for the reshaped vocabulary
  (type-equiv re-registered), READMEs and SERVICE_ROLES updated, taskkill
  added to knip ignoreBinaries.

2026-07-26 15:27:59 +08:00

6.3 KiB

Raw Blame History

Agent Note: The subprocess seam goes Node-shaped and every eligible spawner rides it

Status: implemented

English | 中文

Problem

The subprocess seam shipped shaped for exactly one consumer family: batch-collected stdout/stderr, batch stdin, a single escalating kill(). That was deliberate scope control, and its own note records "migrate the other spawn sites" as rejected-for-now. Review on the introducing PR reversed that deferral: the stacked follow-up should reshape the interface toward Node's API and move the remaining process-running places onto the service. The remaining spawners each carried a private copy of some slice of the same mechanics — lsp-local had its own detached-tree signalling (POSIX group + Windows taskkill + liveness polling), subagent-subprocess had the dispose ladder and its own scrub, mcp-client and pty-local and the SDK helper each had a third/fourth/fifth copy of the credential scrub — and none of it was swappable or centrally testable.

Decision

The seam's vocabulary is now Node-shaped, and every spawner that can ride the service does:

Per-stream stdio dispositions on SubprocessSpawnSpec: 'pipe' (the raw Readable/Writable, for consumer-owned protocol framing), 'inherit' (diagnostics to the parent's stream), and collect mode { maxBytes, spill? } — the original bounded tail-keep shape, with the spill file now optional so a diagnostic tail (a language server's stderr) buffers without touching disk. stdin is 'ignore', 'pipe', or { data } (write-and-close batch).
SubprocessOutcome carries exit facts only (Node's close-event vocabulary); collected output stays readable through handle.collected after settlement (spill fds seal at the settle boundary), so batch and streaming callers share one access path and nothing is copied into the outcome.
Tree-scoped termination, split Node-style: kill(signal?) sends one signal and is a no-op after settlement; terminate() owns the SIGTERM→grace→SIGKILL escalation (and serves the spec's abort signal); waitForExit() polls tree liveness (POSIX group probe; direct-child boundary on Windows); dispose(graces) is the cooperative stdin-EOF→SIGTERM→SIGKILL ladder absorbed from subagent-subprocess, memoized per handle. Windows tree termination (taskkill /T, injectable) moved in from lsp-local, so tree semantics are platform-correct for every consumer.
One scrub definition: scrubbedParentEnv()/SENSITIVE_ENV_PATTERN live on the seam. Spawners that cannot route the spawn itself through the service — pty-local (node-pty owns the fork) and mcp-client (the MCP SDK owns the transport spawn) — import the function, so environment policy is single-sourced even where process ownership is not; the SDK helper's scrubEnvironment() defaults through it as well.

Migrations landed with the reshape: bash-local/bash-sandbox (collect modes + batch stdin; the bash kill() maps to terminate() so task_kill keeps escalation semantics), lsp-local (piped protocol streams + a no-spill collected stderr tail; LspConnection takes the seam's spawn function; its private tree-op helpers deleted), subagent-acp (piped ndjson streams + inherited stderr; spawn failure surfaces through done rejection into the same startup race; disposal is handle.dispose with the plugin's configured graces). dsh-subagent-subprocess is deleted — the dispose ladder and scrub are the seam's; the unused isolated-config-dir helper died with it (no consumer existed).

Compositions mounting lsp-local or subagent-acp now load dsh-subprocess-local (the plugins inject 'subprocess'); the acp/lsp test fixtures gained the row.

Alternatives considered

Keep the batch-only seam and let stream consumers stay bespoke. The introducing note's position, rejected by review: it leaves three private copies of tree signalling and five of the scrub, and any future runner (containerized executor, remote process host) would have to pick which private copy to fork. The Node-shaped dispositions cover all three observed stream shapes without widening the outcome type or buffering piped streams.

A single stdio: 'pipe' | 'inherit' | 'collect' mode for all three streams at once. Rejected: real consumers mix modes per stream (lsp: pipe/pipe/collect; acp: pipe/pipe/inherit; bash: data/collect/collect). Per-stream dispositions are exactly Node's shape and avoid a second spawn call for the mixed cases.

Migrate pty-local and mcp-client spawns too. Rejected on ownership grounds, not scope: node-pty's fork() allocates the terminal itself, and the MCP SDK's StdioClientTransport spawns internally — neither call site is ours to route. They adopt the shared scrub (the part that is policy), and their READMEs say why the spawn stays put.

Migrate the test-support launchers (acp-snapshot, loader-smoke) and the SDK package-manager runner. Rejected: the support packages are deliberately dependency-light test infrastructure that must not depend on product seams, and the SDK wizard's stdio: 'inherit'-with-redirect semantics plus its out-of-composition lifecycle (no cordis context at all) make the service a poor fit; it shares the scrub instead.

Consequences

Bought: one implementation of tree signalling, escalation, the dispose ladder, bounded collection, and the scrub, tested once in dsh-subprocess-local's suites (including injected-platform Windows coverage that lsp-local's private copy never had); lsp-local and subagent-acp shed their process plumbing and their children now survive plugin reloads and die with composition teardown like bash's; a whole package (dsh-subagent-subprocess) is gone. The seam README's "one consumer family" limitation is retired.

Cost: the seam is wider — three stdio modes and four termination verbs instead of one of each — so a future backend implements more surface; the compositions for lsp-local/subagent-acp each carry the subprocess row now; and SubprocessOutcome no longer carries output, a breaking shape change inside the still-unreleased stack (the PR2 layer was updated in place rather than shimmed, per the pre-release stance). pty-local/mcp-client/SDK/test-support spawns remain outside the service by ownership, with the scrub as the shared floor.

6.3 KiB Raw Blame History