feat(subprocess): reshape the seam Node-ward for multi-consumer use

Review direction (tianyicui, PR #660): make the interface closer to Node's
API so the other process-running places can adopt it. The spec gains
per-stream stdio dispositions — 'pipe' (raw Readable/Writable for protocol
streams), 'inherit' (diagnostics to the parent), and collect mode ({maxBytes,
spill?} — the old bounded tail-keep shape, now with spill optional for
diagnostic tails). SubprocessOutcome carries exit facts only; collected
output stays readable through handle.collected after settlement (spill fds
are sealed at the settle boundary). The handle grows Node-style kill(signal)
(single signal, tree-scoped, no-op after settlement), terminate() (the
SIGTERM→grace→SIGKILL escalation, also driven by the spec signal),
waitForExit() (tree liveness, not just the direct child), and dispose()
(the cooperative stdin-EOF→SIGTERM→SIGKILL ladder from subagent-subprocess,
graces caller-supplied). Tree semantics are platform-correct: POSIX detached
groups with direct-child fallback; Windows taskkill /T with an injectable
runner. scrubbedParentEnv/SENSITIVE_ENV_PATTERN move to the seam as the one
shared scrub definition.

bash-local maps its config onto collect modes and batch stdin and reads
results through the collected readers; its kill() maps to terminate() so
task_kill keeps escalation semantics.

packages/subprocess/subprocess/src/index.ts

@@ -1,14 +1,17 @@
 /**
- * The subprocess seam (`ctx.subprocess`): spawn fully-specified
- * commands into managed process groups with bounded, spill-backed output and
- * escalated kills. Command defaulting, shell semantics, deadlines, and
- * presentation belong to consumers — the bash executor seam is the owning
+ * The subprocess seam (`ctx.subprocess`): spawn fully-specified commands into
+ * managed process trees with Node-shaped stdio dispositions — raw pipes for
+ * protocol streams, inherit for diagnostics, bounded spill-backed collection
+ * for batch output — plus tree-scoped signalling and a cooperative dispose
+ * ladder. Command defaulting, shell semantics, deadlines, framing, and
+ * presentation belong to consumers; the bash executor seam is the owning
  * template. The local implementation lives in
  * `@deepseek-ai/dsh-subprocess-local`.
  * @module @deepseek-ai/dsh-subprocess
  */
 
 import { Context, Service } from 'cordis'
+import { DSH_ENV_PREFIX } from './types.ts'
 import type { SubprocessHandle, SubprocessSpawnSpec } from './types.ts'
 
 export { DSH_ENV_PREFIX } from './types.ts'
@@ -16,13 +19,48 @@ export type {
   CollectedOutput,
   DshEnvironment,
   DshEnvironmentKey,
+  SubprocessCollect,
+  SubprocessCollectedOutputs,
+  SubprocessDisposeGraces,
   SubprocessHandle,
   SubprocessOutcome,
+  SubprocessOutputMode,
   SubprocessOutputRead,
   SubprocessOutputReader,
   SubprocessSpawnSpec,
+  SubprocessStdinMode,
+  SubprocessStdio,
 } from './types.ts'
 
+/**
+ * Credential-shaped environment names are NOT forwarded to children (the
+ * harness's own `DEEPSEEK_API_KEY`/secrets must not leak into a spawned
+ * process implicitly). One heuristic for every in-repo spawner; a
+ * deliberately supplied entry survives because explicit env layers merge
+ * after the scrub.
+ */
+export const SENSITIVE_ENV_PATTERN = /KEY|SECRET|TOKEN/i
+
+/**
+ * The ambient parent environment minus credential-shaped names and minus all
+ * `DSH_*` names — the canonical base every harness child starts from. `PATH`,
+ * `HOME`, locale, and proxy variables survive, so child CLIs run normally;
+ * harness identity never leaks implicitly (a child that needs current `DSH_*`
+ * facts receives them through {@link SubprocessSpawnSpec.dshEnv}, and a
+ * deliberately forwarded credential goes through an explicit env layer, which
+ * merges after this scrub). Exported as a plain function so spawners that
+ * cannot route through the service (node-pty backends, SDK-managed
+ * transports) share the one scrub definition.
+ * @returns a fresh environment object safe to hand to a child spawn.
+ */
+export function scrubbedParentEnv(): Record<string, string> {
+  const env: Record<string, string> = {}
+  for (const [key, value] of Object.entries(process.env)) {
+    if (value !== undefined && !SENSITIVE_ENV_PATTERN.test(key) && !key.startsWith(DSH_ENV_PREFIX)) env[key] = value
+  }
+  return env
+}
+
 declare module 'cordis' {
   interface Context {
     subprocess: SubprocessService
@@ -37,13 +75,17 @@ declare module 'cordis' {
  *
  * Implementations must honor these semantics:
  * - {@link spawn} returns immediately with a live handle; `done` resolves at
- *   process close and rejects only for spawn-level failures.
- * - Output readers are offset-based and non-consuming, so independent readers
- *   never consume one another's output; lossy reads report truncation and the
- *   spill file holding the complete stream when one exists.
- * - {@link SubprocessHandle.kill} and the spec's abort signal escalate
- *   SIGTERM→grace→SIGKILL across the whole process group.
- * - Disposal kills all still-running managed processes and awaits their exit.
+ *   process close with exit facts and rejects only for spawn-level failures.
+ * - Collect-mode readers are offset-based and non-consuming, so independent
+ *   readers never consume one another's output; lossy reads report truncation
+ *   and the spill file holding the complete stream when one exists. Piped
+ *   streams are handed to the caller raw and never buffered here.
+ * - {@link SubprocessHandle.kill} signals without escalation,
+ *   {@link SubprocessHandle.terminate} (and the spec's abort signal) escalates
+ *   SIGTERM→grace→SIGKILL, and {@link SubprocessHandle.dispose} runs the
+ *   cooperative EOF-first ladder — all tree-scoped on every platform.
+ * - Disposal of the service terminates all still-running managed processes
+ *   and awaits their exit.
  */
 export abstract class SubprocessService extends Service {
   constructor(ctx: Context) {
@@ -53,8 +95,8 @@ export abstract class SubprocessService extends Service {
   /**
    * Start one managed child process from a fully-specified spec; this seam
    * applies no defaults.
-   * @param spec - argv, directory, limits, grace, cancellation, and environment.
-   * @returns the live process handle (readers, kill, outcome promise).
+   * @param spec - argv, directory, stdio dispositions, grace, cancellation, and environment.
+   * @returns the live process handle (streams/readers, signalling, outcome promise).
    */
   abstract spawn(spec: SubprocessSpawnSpec): SubprocessHandle
 }

packages/subprocess/subprocess/src/types.ts

@@ -1,11 +1,14 @@
 /**
- * Vocabulary for the subprocess seam: fully-specified spawn requests,
- * bounded output with spill recovery, and live process handles. Command
- * defaulting, shell semantics, and presentation belong to consumers such as
- * the bash executor seam.
+ * Vocabulary for the subprocess seam: fully-specified spawn requests with
+ * Node-shaped per-stream stdio modes, bounded collected output with spill
+ * recovery, raw piped streams, and tree-scoped termination. Command
+ * defaulting, shell semantics, protocol framing, and presentation belong to
+ * consumers such as the bash executor seam.
  * @module dsh-subprocess/types
  */
 
+import type { Readable, Writable } from 'node:stream'
+
 /** Namespace prefix reserved for DeepSeek Harness-managed child environment facts. */
 export const DSH_ENV_PREFIX = 'DSH_' as const
 
@@ -26,60 +29,97 @@ export interface CollectedOutput {
 }
 
 /**
- * A fully-specified spawn request. This seam applies no defaults: every limit
- * and directory is explicit, so the caller's own config — not a hidden
- * subprocess-service default — decides them (the `dsh-bash` request/spec split
- * is the owning template).
+ * stdin disposition. `'ignore'` leaves fd 0 on `/dev/null`; `'pipe'` exposes
+ * {@link SubprocessHandle.stdin} for the caller's ongoing protocol writes;
+ * `{ data }` writes the bytes and closes (the batch shape).
+ */
+export type SubprocessStdinMode = 'ignore' | 'pipe' | { readonly data: string }
+
+/**
+ * Bounded in-memory collection for one output stream, with an optional
+ * full-stream spill file. Omitting `spill` keeps only the in-memory tail —
+ * the diagnostic-tail shape (a language server's stderr); including it makes
+ * the complete stream recoverable up to its cap (the bash tool shape).
+ */
+export interface SubprocessCollect {
+  /** In-memory cap in bytes; overflow keeps the TAIL. */
+  maxBytes: number
+  /** Full-stream spill file; absent disables spilling entirely. */
+  spill?: {
+    /** Whole-stream byte cap; a larger stream discards its now-incomplete spill. */
+    maxBytes: number
+  }
+}
+
+/**
+ * stdout/stderr disposition. `'pipe'` exposes the raw `Readable` for the
+ * caller's protocol decoding; `'inherit'` passes the parent's descriptor
+ * through (child diagnostics land on the harness's own stream); a
+ * {@link SubprocessCollect} object buffers boundedly with offset-based reads.
+ */
+export type SubprocessOutputMode = 'pipe' | 'inherit' | SubprocessCollect
+
+/** Per-stream stdio dispositions, all explicit — this seam applies no defaults. */
+export interface SubprocessStdio {
+  stdin: SubprocessStdinMode
+  stdout: SubprocessOutputMode
+  stderr: SubprocessOutputMode
+}
+
+/**
+ * A fully-specified spawn request. This seam applies no defaults: every
+ * disposition, limit, and directory is explicit, so the caller's own config —
+ * not a hidden subprocess-service default — decides them (the `dsh-bash`
+ * request/spec split is the owning template).
  */
 export interface SubprocessSpawnSpec {
   /** Executable and arguments; `argv[0]` is the program. Never shell-interpreted here. */
   argv: readonly string[]
   /** Working directory for the child. */
   cwd: string
-  /** Stdout in-memory cap; overflow spills to disk (tail kept in memory). */
-  stdoutMaxBytes: number
-  /** Stderr in-memory cap; overflow spills to disk (tail kept in memory). */
-  stderrMaxBytes: number
-  /** Per-stream spill-file cap; larger streams retain only their in-memory tail. */
-  maxSpillBytes: number
-  /** Grace period for kill escalation and for inherited pipes after process exit. */
+  /** Per-stream stdio dispositions. */
+  stdio: SubprocessStdio
+  /**
+   * Grace period in milliseconds for the {@link SubprocessHandle.terminate}
+   * escalation and for draining still-open collected pipes after the process
+   * exits (an inherited descriptor held by a surviving descendant cannot hold
+   * the outcome open indefinitely).
+   */
   graceMs: number
   /**
-   * Abort signal — kills the process group when it fires. The caller owns
-   * deadlines and cause classification; this seam only reacts to the abort.
+   * Abort signal — starts the terminate escalation on the process tree when
+   * it fires. The caller owns deadlines and cause classification; this seam
+   * only reacts to the abort.
    */
   signal?: AbortSignal | undefined
   /**
-   * Bytes to write to the child's stdin, then close it. Absent (or empty)
-   * leaves stdin closed/empty.
-   */
-  stdin?: string | undefined
-  /**
-   * Ordinary environment entries merged after the implementation's credential
-   * scrub. `DSH_*` names are rejected and belong in {@link dshEnv}.
+   * Ordinary environment entries merged onto the implementation's scrubbed
+   * parent base (see `scrubbedParentEnv`). `DSH_*` names are rejected and
+   * belong in {@link dshEnv}; a deliberately forwarded credential-shaped
+   * entry survives because this layer merges after the scrub.
    */
   env?: Record<string, string> | undefined
   /**
-   * Harness-owned `DSH_*` variables for this execution. Implementations
-   * discard ambient `DSH_*` entries before merging this snapshot, so an
-   * unavailable current fact cannot inherit a stale value from the harness
-   * process, and reject non-`DSH_*` names supplied through this channel.
+   * Harness-owned `DSH_*` variables for this execution. The scrubbed base has
+   * already discarded ambient `DSH_*` entries, so an unavailable current fact
+   * cannot inherit a stale value from the harness process; non-`DSH_*` names
+   * on this channel are rejected.
    */
   dshEnv?: DshEnvironment | undefined
 }
 
 /**
- * Raw outcome of one closed process. Deliberately carries NO timeout or
- * cancellation classification: the service kills on abort but does not decide
- * why — the caller reads the signal it owns to classify causes.
+ * Exit facts of one closed process — Node's `close`-event vocabulary.
+ * Deliberately carries NO timeout or cancellation classification (the caller
+ * reads the signal it owns to classify causes) and NO output: collected
+ * streams stay readable through {@link SubprocessHandle.collected} after
+ * settlement, so batch and streaming callers share one access path.
  */
 export interface SubprocessOutcome {
   /** Exit code; null when the process died from a signal. */
   exitCode: number | null
   /** Terminating signal (e.g. 'SIGTERM'); null on normal exit. */
   signal: NodeJS.Signals | null
-  stdout: CollectedOutput
-  stderr: CollectedOutput
 }
 
 /** One incremental {@link SubprocessOutputReader.readFrom} read. */
@@ -95,9 +135,11 @@ export interface SubprocessOutputRead {
 }
 
 /**
- * Cursor-free incremental access to one live output stream. Offsets are
+ * Cursor-free incremental access to one collected output stream. Offsets are
  * whole-stream byte coordinates owned by the caller, so independent readers
- * cannot consume one another's output.
+ * cannot consume one another's output; `readFrom(0)` after settlement is the
+ * batch result (`lossy` then means the in-memory tail lost its head — the
+ * {@link CollectedOutput.truncated} fact).
  */
 export interface SubprocessOutputReader {
   /**
@@ -110,19 +152,87 @@ export interface SubprocessOutputReader {
   readFrom(fromByte: number): SubprocessOutputRead
 }
 
+/** Offset-based readers for the streams spawned in collect mode. */
+export interface SubprocessCollectedOutputs {
+  /** Present iff stdout is a {@link SubprocessCollect}. */
+  readonly stdout?: SubprocessOutputReader
+  /** Present iff stderr is a {@link SubprocessCollect}. */
+  readonly stderr?: SubprocessOutputReader
+}
+
 /**
- * A live child process. `kill()` starts the group SIGTERM→grace→SIGKILL
- * escalation; buffered output remains readable after exit.
+ * The two grace periods of the cooperative dispose ladder
+ * ({@link SubprocessHandle.dispose}). Consumers carry them as defaulted,
+ * validated Config fields, so teardown timing is deployment-tunable and this
+ * seam hardcodes nothing.
+ */
+export interface SubprocessDisposeGraces {
+  /**
+   * Tier-1 window (ms): after stdin EOF, how long the child gets to quiesce
+   * ON ITS OWN — flush durable state, tear down its own descendants — before
+   * escalation to platform termination. Usually WIDER than
+   * {@link SubprocessDisposeGraces.graceMs}: a cooperative child's EOF-driven
+   * teardown may itself wait on a signal-trapping grandchild plus a final
+   * flush.
+   */
+  eofGraceMs: number
+  /**
+   * Termination confirmation window (ms): POSIX applies it after `SIGTERM`
+   * and again after `SIGKILL`; Windows applies it after the forced tree
+   * termination.
+   */
+  graceMs: number
+}
+
+/**
+ * A live child process rooted in its own process tree. Collected output
+ * remains readable after exit; piped streams belong to the caller.
+ *
+ * Termination is tree-scoped everywhere: POSIX signals the detached process
+ * group (falling back to the direct child when the group is gone), Windows
+ * terminates the tree via `taskkill /T`, so helper processes cannot outlive
+ * the handle unnoticed.
  */
 export interface SubprocessHandle {
-  /** Process id (group leader); -1 when the spawn itself failed. */
+  /** Process id (tree root); -1 when the spawn itself failed. */
   readonly pid: number
-  /** Live stdout reader (also readable after exit). */
-  readonly stdout: SubprocessOutputReader
-  /** Live stderr reader (also readable after exit). */
-  readonly stderr: SubprocessOutputReader
-  /** Resolves when the process closes; rejects only for spawn-level failures. */
+  /** The child's stdin, present iff spawned with `stdin: 'pipe'`. */
+  readonly stdin: Writable | undefined
+  /** The child's raw stdout, present iff spawned with `stdout: 'pipe'`. */
+  readonly stdout: Readable | undefined
+  /** The child's raw stderr, present iff spawned with `stderr: 'pipe'`. */
+  readonly stderr: Readable | undefined
+  /** Offset-based readers for collect-mode streams (also readable after exit). */
+  readonly collected: SubprocessCollectedOutputs
+  /** Resolves at process close with exit facts; rejects only for spawn-level failures. */
   readonly done: Promise<SubprocessOutcome>
-  /** Begin SIGTERM→grace→SIGKILL on the process group. Idempotent. */
-  kill(): void
+  /**
+   * Send one signal to the process tree, Node-style — no escalation, no
+   * timers. A no-op after the outcome has settled (the pid may be reused).
+   * @param signal - the signal to deliver (default `SIGTERM`; Windows
+   *   force-terminates the tree for any value).
+   */
+  kill(signal?: NodeJS.Signals): void
+  /**
+   * Begin the SIGTERM → `graceMs` → SIGKILL escalation on the process tree
+   * (Windows force-terminates immediately). Idempotent; also triggered by the
+   * spec's abort signal.
+   */
+  terminate(): void
+  /**
+   * Wait until the process tree has exited — the tree, not just the direct
+   * child, so a still-running helper is observable before teardown returns.
+   * @param signal - optional bound for the wait.
+   * @returns `true` when the tree exited, `false` when the signal aborted first.
+   */
+  waitForExit(signal?: AbortSignal): Promise<boolean>
+  /**
+   * Tear the child down to quiescence, resolving only after exit: close stdin
+   * (when this handle owns a piped one) and allow cooperative flush for
+   * `eofGraceMs`, then SIGTERM with a `graceMs` window (POSIX), then forced
+   * tree termination with a final bounded `graceMs` wait.
+   * @param graces - the ladder's two windows, from the consumer's Config.
+   * @throws when the child still has not exited `graceMs` after the forced tier.
+   */
+  dispose(graces: SubprocessDisposeGraces): Promise<void>
 }

packages/subprocess/subprocess/tests/service.spec.ts

@@ -1,7 +1,7 @@
 import { describe, expect, it } from 'vitest'
 import { Context } from 'cordis'
-import { SubprocessService } from '@deepseek-ai/dsh-subprocess'
-import type { SubprocessHandle, SubprocessOutputRead, SubprocessSpawnSpec } from '@deepseek-ai/dsh-subprocess'
+import { scrubbedParentEnv, SubprocessService } from '@deepseek-ai/dsh-subprocess'
+import type { SubprocessDisposeGraces, SubprocessHandle, SubprocessOutputRead, SubprocessSpawnSpec } from '@deepseek-ai/dsh-subprocess'
 
 /**
  * Minimal concrete service: a hand-built handle. The seam is spawn-only —
@@ -11,18 +11,20 @@ import type { SubprocessHandle, SubprocessOutputRead, SubprocessSpawnSpec } from
 class StubSubprocessService extends SubprocessService {
   spawn(spec: SubprocessSpawnSpec): SubprocessHandle {
     const read: SubprocessOutputRead = { text: '', nextOffset: 0, lossy: false }
-    let killed = false
+    const collected = spec.stdio.stdout !== 'pipe' && spec.stdio.stdout !== 'inherit'
+      ? { stdout: { readFrom: () => read } }
+      : {}
     return {
       pid: spec.argv.length,
-      stdout: { readFrom: () => read },
-      stderr: { readFrom: () => read },
-      done: Promise.resolve({
-        exitCode: killed ? null : 0,
-        signal: null,
-        stdout: { text: 'ok', truncated: false },
-        stderr: { text: '', truncated: false },
-      }),
-      kill: () => { killed = true },
+      stdin: undefined,
+      stdout: undefined,
+      stderr: undefined,
+      collected,
+      done: Promise.resolve({ exitCode: 0, signal: null }),
+      kill: () => {},
+      terminate: () => {},
+      waitForExit: () => Promise.resolve(true),
+      dispose: (_graces: SubprocessDisposeGraces) => Promise.resolve(),
     }
   }
 }
@@ -34,22 +36,40 @@ describe('SubprocessService seam', () => {
     const handle = ctx.subprocess.spawn({
       argv: ['true'],
       cwd: '/stub',
-      stdoutMaxBytes: 1,
-      stderrMaxBytes: 1,
-      maxSpillBytes: 1,
+      stdio: { stdin: 'ignore', stdout: { maxBytes: 1 }, stderr: 'inherit' },
       graceMs: 1,
     })
     expect(handle.pid).toBe(1)
-    expect(handle.stdout.readFrom(0)).toEqual({ text: '', nextOffset: 0, lossy: false })
+    expect(handle.collected.stdout!.readFrom(0)).toEqual({ text: '', nextOffset: 0, lossy: false })
     handle.kill()
+    handle.terminate()
+    await expect(handle.waitForExit()).resolves.toBe(true)
+    await expect(handle.dispose({ eofGraceMs: 1, graceMs: 1 })).resolves.toBeUndefined()
     const outcome = await handle.done
-    expect(outcome.stdout.text).toBe('ok')
+    expect(outcome.exitCode).toBe(0)
   })
 
-  it('loading a second implementation throws (one processes service per context — cordis standard)', async () => {
+  it('loading a second implementation throws (one subprocess service per context — cordis standard)', async () => {
     const ctx = new Context()
     await ctx.plugin(StubSubprocessService)
-    class SecondManager extends StubSubprocessService {}
-    await expect(ctx.plugin(SecondManager)).rejects.toThrow(/service "subprocess" has been registered/)
+    class SecondService extends StubSubprocessService {}
+    await expect(ctx.plugin(SecondService)).rejects.toThrow(/service "subprocess" has been registered/)
+  })
+
+  it('scrubbedParentEnv drops credential-shaped and DSH_ names but keeps PATH', () => {
+    process.env.DSH_SCRUB_PROBE = 'stale'
+    process.env.SCRUB_PROBE_TOKEN = 'secret'
+    process.env.SCRUB_PROBE_PLAIN = 'visible'
+    try {
+      const env = scrubbedParentEnv()
+      expect(env.DSH_SCRUB_PROBE).toBeUndefined()
+      expect(env.SCRUB_PROBE_TOKEN).toBeUndefined()
+      expect(env.SCRUB_PROBE_PLAIN).toBe('visible')
+      expect(env.PATH).toBeDefined()
+    } finally {
+      delete process.env.DSH_SCRUB_PROBE
+      delete process.env.SCRUB_PROBE_TOKEN
+      delete process.env.SCRUB_PROBE_PLAIN
+    }
   })
 })