Merge origin/master into codex/truncated-design

packages/bash/bash-local/src/index.ts

@@ -1,30 +1,18 @@
 /**
- * `LocalBashExecutor`: the local-subprocess implementation of the
- * `@deepseek-ai/dsh-bash` executor seam. Spawns `bash -c` per call in its
- * own process group (see `./run.ts` for the plumbing and the agent-tool
- * survey notes), tracks background tasks, and kills everything on dispose.
- *
- * TODO(permissions/sandbox): execution policy does NOT belong here — use
- * the `tools/pre-execute` deny/ask gate (see docs/architecture.md
- * § Extending The Harness) or implement a sandboxing `BashExecutor`.
- * Reference points:
- * Claude Code wraps commands in sandbox-exec/bubblewrap; Codex applies
- * seatbelt/landlock plus an execpolicy prefix-rule engine.
- *
+ * Local-subprocess implementation of the bash executor seam. Each command runs
+ * as `bash -c` in its own process group; disposal kills and joins live groups.
+ * Execution policy belongs in `tools/pre-execute` or a sandboxing executor.
  * @module @deepseek-ai/dsh-bash-local
  */
 
 import { Context } from 'cordis'
 import z from 'schemastery'
-import { BashExecutor, BashTaskId } from '@deepseek-ai/dsh-bash'
-import type { BashExecRequest, BashExecSpec, BashRunResult, BashTask, BashTaskRead, OwnerToken } from '@deepseek-ai/dsh-bash'
+import { BashExecutor } from '@deepseek-ai/dsh-bash'
+import type { BashExecRequest, BashExecSpec, BashProcess, BashProcessRead, BashRunResult } from '@deepseek-ai/dsh-bash'
 import { clampTimeout, deadline, timeoutOf } from '@deepseek-ai/dsh-timeout'
 import { DEFAULT_GRACE_MS, runBash } from './run.ts'
 import type { RunInternals, RunningBash } from './run.ts'
 
-export { DEFAULT_GRACE_MS, ENV_OVERRIDES, killGroup, OutputCollector, runBash } from './run.ts'
-export type { RunInternals, RunningBash, SpawnOutcome, SpawnSpec } from './run.ts'
-
 /** Plugin config (all optional — `static Config` supplies the defaults). */
 export interface Config {
   /** Default working directory for commands (default: process.cwd()). */
@@ -48,20 +36,9 @@ function assertPositiveFinite(name: string, value: number): void {
   }
 }
 
-interface TrackedTask extends BashTask {
-  running: RunningBash
-  /** Whole-stream byte offsets already delivered via {@link LocalBashExecutor.readOutput}. */
-  stdoutOffset: number
-  stderrOffset: number
-  /** Opaque owner token from the {@link BashExecSpec} (the consumer's isolation key). */
-  owner: OwnerToken | undefined
-}
-
 /**
- * Local-subprocess bash executor. Defaults follow the agent-tool survey
- * consensus: 120s default / 600s max timeout (Claude Code, OpenCode), 64KB
- * in-memory output with full-stream spill files (pi, OpenCode),
- * process-group SIGTERM→SIGKILL kills with a 3s grace (OpenCode).
+ * Local bash executor with bounded output, spill files, and process-group
+ * `SIGTERM` to `SIGKILL` escalation.
  */
 export class LocalBashExecutor extends BashExecutor {
   static Config: z<Config> = z.object({
@@ -72,8 +49,8 @@ export class LocalBashExecutor extends BashExecutor {
     graceMs: z.number().default(DEFAULT_GRACE_MS),
   })
 
-  private tasks = new Map<BashTaskId, TrackedTask>()
-  private nextTaskId = 1
+  /** Live processes retained only so disposal can kill and join them. */
+  private live = new Map<BashProcess, RunningBash>()
   /** Test seam: spill knobs forwarded to runBash. */
   internals: RunInternals = {}
 
@@ -82,27 +59,21 @@ export class LocalBashExecutor extends BashExecutor {
 
   constructor(ctx: Context, config: Config) {
     super(ctx)
-    // schemastery (static Config) has already filled the defaulted fields;
-    // the cast records that runtime fact for exactOptionalPropertyTypes.
+    // Schemastery fills these fields before construction; the type does not encode that step.
     this.config = config as ResolvedConfig
     assertPositiveFinite('timeoutMs', this.config.timeoutMs)
     assertPositiveFinite('maxTimeoutMs', this.config.maxTimeoutMs)
     assertPositiveFinite('maxOutputBytes', this.config.maxOutputBytes)
     assertPositiveFinite('graceMs', this.config.graceMs)
     ctx.effect(() => async () => {
-      // Kill every live process group and WAIT for the processes to close so
-      // nothing outlives the fiber (HMR safety) — a TERM-trapping child is
-      // held until the SIGKILL escalation lands. The base class already
-      // silenced listeners, so these kills complete without notices.
+      // Await closure so even a TERM-trapping child cannot outlive the fiber.
       const pending: Promise<void>[] = []
-      for (const task of this.tasks.values()) {
-        if (task.status === 'running') {
-          task.status = 'killed'
-          task.running.kill()
-          pending.push(task.done)
-        }
+      for (const [proc, running] of this.live) {
+        proc.status = 'killed'
+        running.kill()
+        pending.push(proc.done)
       }
-      this.tasks.clear()
+      this.live.clear()
       await Promise.all(pending)
     }, 'local bash teardown')
   }
@@ -129,24 +100,16 @@ export class LocalBashExecutor extends BashExecutor {
       timeoutMs,
       stdoutMaxBytes,
       ...request.signal ? { signal: request.signal } : {},
-      // Carry stdin/env through verbatim — optional, no config default (absent
-      // means none). env merges AFTER the scrub in run.ts.
+      // Explicit environment values are merged after credential scrubbing in run.ts.
       ...request.stdin !== undefined ? { stdin: request.stdin } : {},
       ...request.env !== undefined ? { env: request.env } : {},
-      // Carry the owner through verbatim (required-but-nullable on the spec):
-      // the executor never interprets it — the consumer's access policy does.
-      owner: request.owner,
-      // Carry a sandbox-mode override through verbatim: this executor never
-      // confines, so the field is inert here (the seam contract) — a
-      // sandboxing subclass overrides resolve() to stamp its default instead.
+      // Local execution carries this override for sandboxing subclasses.
       sandboxMode: request.sandboxMode,
     }
   }
 
   async run(spec: BashExecSpec): Promise<BashRunResult> {
-    // One fused deadline drives both the timeout and upstream cancellation;
-    // runBash listens on d.signal and runs the SIGTERM→grace→SIGKILL kill.
-    // `using` clears the timer across the awaited process lifetime.
+    // One deadline combines timeout and upstream cancellation; disposal clears its timer.
     using d = deadline(spec.signal, spec.timeoutMs, 'BASH_TIMEOUT')
     const outcome = await runBash({
       command: spec.command,
@@ -158,23 +121,14 @@ export class LocalBashExecutor extends BashExecutor {
       stdin: spec.stdin,
       env: spec.env,
     }, this.internals).done
-    // Classify the FIRST abort reason: a BASH_TIMEOUT TimeoutReason means our
-    // timeout cut the command short; any other abort — an upstream cancel, or a
-    // foreign (outer) deadline's timeout under nesting — is aborted. Scoping to
-    // our own code keeps a nested outer deadline from reading as our timeout.
-    // Mutually exclusive by construction — the fused signal reports one cause.
+    // Only this executor's timeout reason counts as timedOut; outer deadlines count as aborts.
     const timedOut = timeoutOf(d.signal, 'BASH_TIMEOUT') !== undefined
     const aborted = d.signal.aborted && !timedOut
     return { ...outcome, timedOut, aborted, timeoutMs: spec.timeoutMs }
   }
 
-  start(spec: BashExecSpec): BashTask {
-    // No timeout for background tasks (matches Claude Code, which detaches
-    // the timeout when backgrounding); callers stop tasks via kill() — or
-    // via spec.signal, which the seam contract honors for background runs
-    // too (runBash wires it to the group kill). No deadline is created here,
-    // so spec.timeoutMs is ignored by design — background tasks stay
-    // timeout-free (see the timeout-library RFC).
+  start(spec: BashExecSpec): BashProcess {
+    // Background runs ignore timeoutMs; callers stop them through kill() or spec.signal.
     const running = runBash({
       command: spec.command,
       cwd: spec.workdir,
@@ -186,94 +140,66 @@ export class LocalBashExecutor extends BashExecutor {
       env: spec.env,
     }, this.internals)
 
-    const id = BashTaskId(`bash-${this.nextTaskId++}`)
-    const task: TrackedTask = {
-      id,
-      command: spec.command,
+    let stdoutOffset = 0
+    let stderrOffset = 0
+    const proc: BashProcess = {
       status: 'running',
       exitCode: null,
       signal: null,
-      owner: spec.owner,
-      running,
-      stdoutOffset: 0,
-      stderrOffset: 0,
       done: running.done.then((outcome) => {
-        // Abort-killed tasks report as killed, not completed. Background runs
-        // forward only the upstream signal (no timeout), so its aborted state
-        // is the authoritative "was this cancelled" signal.
-        if (task.status === 'running') task.status = spec.signal?.aborted === true ? 'killed' : 'completed'
-        task.exitCode = outcome.exitCode
-        task.signal = outcome.signal
-        this.notifyTaskDone(task)
+        // Any signal termination is killed, including a command signaling itself.
+        if (proc.status === 'running') {
+          proc.status = spec.signal?.aborted === true || outcome.signal !== null ? 'killed' : 'completed'
+        }
+        proc.exitCode = outcome.exitCode
+        proc.signal = outcome.signal
+        this.onProcessDone(proc, running.stderr.readFrom(0).text)
+        this.live.delete(proc)
       }, (error: unknown) => {
-        // Spawn-level failure (bad workdir, …): the task never ran. String()
-        // suffices — runBash only rejects with Error instances.
-        task.status = 'killed'
-        task.running.stderr.push(Buffer.from(`spawn failed: ${String(error)}`))
-        this.notifyTaskDone(task)
+        // Background spawn failures settle as killed and surface through the read path.
+        proc.status = 'killed'
+        running.stderr.push(Buffer.from(`spawn failed: ${String(error)}`))
+        this.onProcessDone(proc, running.stderr.readFrom(0).text)
+        this.live.delete(proc)
       }),
-    }
-    this.tasks.set(id, task)
-    return task
-  }
+      readOutput: (): BashProcessRead => {
+        const out = running.stdout.readFrom(stdoutOffset)
+        const err = running.stderr.readFrom(stderrOffset)
+        stdoutOffset = out.nextOffset
+        stderrOffset = err.nextOffset
 
-  get(id: BashTaskId): BashTask | undefined {
-    return this.tasks.get(id)
+        // Single newline between sections: stdout chunks usually end with one
+        // already; add it only when missing.
+        const separator = out.text.length > 0 && !out.text.endsWith('\n') ? '\n' : ''
+        const delta = out.text
+          + (err.text.length > 0 ? `${separator}[stderr]\n${err.text}` : '')
+        return {
+          delta,
+          lossy: out.lossy || err.lossy,
+          ...out.spillPath !== undefined ? { stdoutSpillPath: out.spillPath } : {},
+          ...err.spillPath !== undefined ? { stderrSpillPath: err.spillPath } : {},
+        }
+      },
+      kill: (): boolean => {
+        if (proc.status !== 'running') return false
+        proc.status = 'killed'
+        running.kill()
+        return true
+      },
+    }
+    this.live.set(proc, running)
+    return proc
   }
 
   /**
-   * Full collected stderr of a tracked task from stream start (bounded by the
-   * in-memory cap; bytes only in the spill file are not re-read). A protected
-   * seam for subclasses that classify a settled task's outcome — reading here
-   * does NOT advance the consumer's {@link readOutput} cursor. An unknown id
-   * (a task already dropped by disposal) reads as empty.
+   * Settlement hook for subclasses that attach execution facts to a process.
+   * Called after exit facts or spawn-failure output are stamped and before
+   * {@link BashProcess.done} resolves. The base implementation is intentionally
+   * empty.
+   * @param _proc - the settled process handle.
+   * @param _stderr - the process's retained stderr tail used by subclasses for settlement classification.
    */
-  protected collectedStderr(id: BashTaskId): string {
-    const task = this.tasks.get(id)
-    return task === undefined ? '' : task.running.stderr.readFrom(0).text
-  }
-
-  ownerOf(id: BashTaskId): OwnerToken | undefined {
-    // Unknown id and known-but-ownerless both read as undefined — the consumer
-    // treats undefined as "open" and a truly unknown id fails at readOutput/kill.
-    return this.tasks.get(id)?.owner
-  }
-
-  list(): BashTask[] {
-    return [...this.tasks.values()]
-  }
-
-  readOutput(id: BashTaskId): BashTaskRead {
-    const task = this.tasks.get(id)
-    if (!task) throw new Error(`unknown bash task "${id}"`)
-
-    const out = task.running.stdout.readFrom(task.stdoutOffset)
-    const err = task.running.stderr.readFrom(task.stderrOffset)
-    task.stdoutOffset = out.nextOffset
-    task.stderrOffset = err.nextOffset
-
-    // Single newline between sections: stdout chunks usually end with one
-    // already; add it only when missing.
-    const separator = out.text.length > 0 && !out.text.endsWith('\n') ? '\n' : ''
-    const delta = out.text
-      + (err.text.length > 0 ? `${separator}[stderr]\n${err.text}` : '')
-    return {
-      task,
-      delta,
-      lossy: out.lossy || err.lossy,
-      ...out.spillPath !== undefined ? { stdoutSpillPath: out.spillPath } : {},
-      ...err.spillPath !== undefined ? { stderrSpillPath: err.spillPath } : {},
-    }
-  }
-
-  kill(id: BashTaskId): boolean {
-    const task = this.tasks.get(id)
-    if (!task) throw new Error(`unknown bash task "${id}"`)
-    if (task.status !== 'running') return false
-    task.status = 'killed'
-    task.running.kill()
-    return true
-  }
+  protected onProcessDone(_proc: BashProcess, _stderr: string): void {}
 }
 
 export default LocalBashExecutor

packages/bash/bash-local/src/run.ts

@@ -1,23 +1,7 @@
 /**
- * Process plumbing for the local bash executor: spawn, output collection
- * with tail-keep + spill-to-disk truncation, and process-group kill with
- * SIGTERM→SIGKILL escalation.
- *
- * Everything here is deliberately free of Cordis concepts so it can be unit
- * tested in isolation; `LocalBashExecutor` owns lifecycle and configuration.
- *
- * runBash owns NO timing: it kills the process group when its `spec.signal`
- * fires and does not distinguish a timeout from a cancel. The executor fuses
- * timeout + upstream cancellation into that one signal via
- * `@deepseek-ai/dsh-timeout`'s `deadline`, and classifies the outcome from the
- * signal afterward — the timing/classification half is shared, the kill is not.
- *
- * Design notes (surveyed against Claude Code, OpenCode, Codex, and pi — see
- * the package README): spawn-per-call with `detached: true` so the child
- * leads its own process group; kills target the group (`kill(-pid)`) so
- * pipelines and subshells die with the parent. SIGTERM first, SIGKILL after a
- * grace period (OpenCode's escalation; Codex/pi jump straight to SIGKILL).
- *
+ * Process plumbing for the local bash executor: detached process-group spawn,
+ * tail-keep output with spill files, and SIGTERM→SIGKILL escalation. This layer
+ * reacts to an abort signal; the executor owns deadlines and classifies causes.
  * @module dsh-bash-local/run
  */
 
@@ -50,18 +34,9 @@ export const ENV_OVERRIDES = {
 export const SENSITIVE_ENV_PATTERN = /KEY|SECRET|TOKEN/i
 
 /**
- * `process.env` minus credential-shaped vars, plus the model-friendly
- * overrides, plus any caller-supplied `extra` entries.
+ * Build a child environment by scrubbing credential-shaped ambient variables,
+ * applying model-friendly overrides, then merging trusted caller entries last.
  *
- * Layering matters: the scrub drops `process.env` credentials, then
- * `ENV_OVERRIDES` forces the model-friendly terminal vars, then `extra` is
- * merged LAST so an explicit caller entry wins even when its name matches the
- * scrub pattern (the scrub is the control that stops the HARNESS's ambient
- * credentials leaking into a spawned command; a caller that explicitly sets a
- * var named a value it already holds, not that ambient secret). `extra` is set
- * by in-process plugins (the hooks bridges), not the model — `dsh-tool-bash`
- * builds its request from named fields only and does not forward model input
- * here (see its README, § "The tool builds its request from named args only").
  * @param extra - caller-supplied entries merged last; an explicit entry wins even against the scrub and the overrides.
  * @returns the environment to hand to `spawn` for the child process.
  */
@@ -211,27 +186,6 @@ export class OutputCollector {
     writeSync(this.spillFd, chunk)
   }
 
-  // TODO(snapshot-scope): `snapshot()` has one internal caller (`finalize()` at
-  // the bottom of this file) and `totalBytes` is read only by a test. The live
-  // background-poll path goes through `readFrom()`, so inline snapshot() into
-  // finalize() and drop or privatize the totalBytes getter.
-  /**
-   * Read the collected tail without finalizing (the final-result snapshot).
-   * @returns the retained tail text, the truncation flag, and the spill path when one was created.
-   */
-  snapshot(): CollectedOutput {
-    return {
-      text: Buffer.concat(this.chunks).toString('utf8'),
-      truncated: this.dropped,
-      ...this.spillFile !== undefined ? { spillPath: this.spillFile } : {},
-    }
-  }
-
-  /** Total bytes ever pushed (including bytes dropped from memory). */
-  get totalBytes(): number {
-    return this.total
-  }
-
   /**
    * Incremental read in whole-stream byte coordinates: returns everything
    * pushed since `fromByte`. When `fromByte` has already slid out of the
@@ -264,26 +218,24 @@ export class OutputCollector {
       try {
         closeSync(this.spillFd)
       } catch {
-        // close can surface delayed writeback failures (for example EIO/ENOSPC)
-        // after writeSync appeared to succeed. Keep finalize total so runBash's
-        // close handler still resolves, but stop advertising a spill file that
-        // may be missing its tail.
+        // A delayed writeback failure makes the spill unreliable; keep finalize
+        // total but stop advertising that file.
         this.spillFile = undefined
       }
       this.spillFd = undefined
     }
-    return this.snapshot()
+    return {
+      text: Buffer.concat(this.chunks).toString('utf8'),
+      truncated: this.dropped,
+      ...this.spillFile !== undefined ? { spillPath: this.spillFile } : {},
+    }
   }
 }
 
 /**
- * Send `sig` to the process GROUP led by `pid` (requires the child to have
- * been spawned with `detached: true`). NEVER throws: kills race process exit
- * by design (ESRCH), and the other failure modes (EPERM from setuid
- * children, …) fire inside timer callbacks where a throw would crash the
- * host process — a kill that cannot be delivered is reported by the process
- * NOT dying, which callers already handle via escalation/timeouts. No-op for
- * non-positive pids (spawn never started a process).
+ * Send `sig` to a detached process group. Never throws: delivery races process
+ * exit and may run in a timer callback, so failures are contained and a
+ * non-positive pid is a no-op.
  * @param pid - the group leader's pid; non-positive means the spawn failed and the call is a no-op.
  * @param sig - the signal to deliver to the whole group.
  */
@@ -313,24 +265,13 @@ export interface RunningBash {
 }
 
 /**
- * Spawn `bash -c <command>` in its own process group and collect output.
- *
- * Outcome semantics: the returned promise REJECTS only for spawn-level
- * failures (bad cwd → ENOENT, missing binary, pre-aborted signal); every
- * runtime outcome — nonzero exit, timeout kill, abort kill, signal death —
- * RESOLVES with a {@link SpawnOutcome} describing what happened, so callers
- * shape one consistent report for the model.
- *
- * XXX(stateful-shell): per the agent-tool survey there are two proven
- * stateful designs worth revisiting — Claude Code persists ONLY cwd between
- * calls (captures `pwd -P` after each command), and Codex keeps whole PTY
- * exec sessions addressable via session ids + stdin writes. We deliberately
- * spawn a fresh non-login `bash -c` per call for determinism (no rc files,
- * no inherited shell state); revisit when real workflows demand it.
- * @param spec - the fully-resolved run (command, cwd, limits); no defaulting happens here.
- * @param internals - test-only knobs; omitted fields fall back to the private per-process spill dir.
- * @returns the live handle: pid, the two live collectors, the outcome promise, and `kill()`.
+ * Spawn one isolated `bash -c` process group and collect its output.
+ * Runtime exits resolve as {@link SpawnOutcome}; only spawn failures reject.
+ * @param spec - fully resolved command, cwd, limits, and cancellation.
+ * @param internals - test-only process and spill-directory overrides.
+ * @returns live process handle and outcome promise.
  */
+// XXX(stateful-shell): evaluate persistent cwd or PTY sessions when workflows require shell state.
 export function runBash(spec: SpawnSpec, internals: RunInternals = {}): RunningBash {
   const spillDir = internals.spillDir ?? privateSpillDir()
 
@@ -338,16 +279,7 @@ export function runBash(spec: SpawnSpec, internals: RunInternals = {}): RunningB
     throw new Error(`aborted before spawn: ${String(spec.signal.reason ?? 'aborted')}`)
   }
 
-  // stdin is a pipe ONLY when the caller supplied bytes; with none it is `ignore`
-  // (fd 0 → /dev/null) — the exact pre-seam default. This matters: a spawn pipe
-  // and /dev/null are NOT observationally identical (node's pipe is an AF_UNIX
-  // socket, so a command that probes stdin's type — `test -c /dev/stdin`, `stat
-  // /proc/self/fd/0` — sees a char device vs a socket), so the no-stdin path
-  // (every model-driven call) must keep /dev/null rather than regress to a socket.
-  // Two LITERAL `stdio` tuples (not one variable tuple): only a literal lets the
-  // typed `spawn` overload infer non-null stdout/stderr, which the
-  // `ChildProcessByStdio` annotation captures (stdin `Writable | null`; stdout/
-  // stderr the non-null `Readable` the collectors attach to without a cast).
+  // Keep absent stdin as /dev/null; literal tuples preserve non-null output types.
   const env = childEnv(spec.env)
   const child: ChildProcessByStdio<Writable | null, Readable, Readable> = spec.stdin !== undefined
     ? spawn('bash', ['-c', spec.command], { cwd: spec.cwd, env, stdio: ['pipe', 'pipe', 'pipe'], detached: true })
@@ -360,8 +292,7 @@ export function runBash(spec: SpawnSpec, internals: RunInternals = {}): RunningB
 
   let graceTimer: NodeJS.Timeout | undefined
 
-  // pid is undefined when the spawn itself fails (bad cwd, missing binary);
-  // the 'error' handler rejects `done` and kills become no-ops via pid -1.
+  // Failed spawns use pid -1 so kill remains a no-op.
   const pid = child.pid ?? -1
 
   const kill = (): void => {
@@ -370,27 +301,11 @@ export function runBash(spec: SpawnSpec, internals: RunInternals = {}): RunningB
     graceTimer = setTimeout(() => { killGroup(pid, 'SIGKILL') }, spec.graceMs)
   }
 
-  // runBash owns no timer: the executor's `run()` fuses timeout+cancel into one
-  // deadline signal (`@deepseek-ai/dsh-timeout`) and passes it here; we only
-  // listen and run the SIGTERM→grace→SIGKILL kill. Whether the abort was a
-  // timeout or an upstream cancel is classified by the executor from that
-  // signal, not tracked here.
+  // The executor owns timeout classification; this layer only reacts to abort.
   const onAbort = (): void => { kill() }
   spec.signal?.addEventListener('abort', onAbort, { once: true })
 
-  // Write stdin and close it, but ONLY when the caller supplied bytes — with no
-  // stdin, fd 0 is `ignore` (/dev/null) and `child.stdin` is null. The error
-  // handler must exist whenever we write: an unhandled 'error' on the stream
-  // would throw and crash the host. We swallow the error rather than reject
-  // `done`, and that is correct for ANY stdin-write error, not just the common
-  // one — the stdin write is BEST-EFFORT, while the command's authoritative
-  // outcome is its exit code + captured output, which the `close` handler reports
-  // regardless of whether the write landed. The expected case is EPIPE (the child
-  // exited without reading, so closing our end of a still-full pipe fails); a rare
-  // non-EPIPE pipe fault means the command ran with incomplete stdin, and it
-  // surfaces that itself through its own exit/output (e.g. a hook that gets
-  // truncated JSON errors out) — rejecting here would instead discard that real
-  // output and turn it into an opaque infrastructure error, which is worse.
+  // Stdin writes are best-effort; process exit and captured output remain authoritative.
   if (child.stdin !== null) {
     child.stdin.on('error', () => { /* stdin write is best-effort; outcome rides on exit/output. */ })
     child.stdin.end(spec.stdin)
@@ -398,8 +313,7 @@ export function runBash(spec: SpawnSpec, internals: RunInternals = {}): RunningB
 
   const done = new Promise<SpawnOutcome>((resolve, reject) => {
     child.on('error', (error) => {
-      // Spawn-level failure (ENOENT cwd, EACCES, …): no close event with
-      // meaningful output follows; clean up and reject.
+      // No meaningful close outcome follows a spawn failure.
       cleanup()
       reject(error)
     })