Merge remote-tracking branch 'origin/master' into codex/ask-user-question

# Conflicts:
#	docs/config-catalog.md

packages/bash/bash-local/package.json

@@ -23,6 +23,7 @@
   "license": "BSD-3-Clause",
   "peerDependencies": {
     "@deepseek-ai/dsh-bash": "^0.0.1",
+    "@deepseek-ai/dsh-timeout": "^0.0.1",
     "cordis": "^4.0.0-rc.6"
   },
   "dependencies": {
@@ -30,6 +31,7 @@
   },
   "devDependencies": {
     "@deepseek-ai/dsh-bash": "workspace:^",
+    "@deepseek-ai/dsh-timeout": "workspace:^",
     "cordis": "^4.0.0-rc.6"
   }
 }

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

@@ -18,6 +18,7 @@ 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 { clampTimeout, deadline, timeoutOf } from '@deepseek-ai/dsh-timeout'
 import { DEFAULT_GRACE_MS, runBash } from './run.ts'
 import type { RunInternals, RunningBash } from './run.ts'
 
@@ -114,8 +115,12 @@ export class LocalBashExecutor extends BashExecutor {
    * values and never re-default.
    */
   resolve(request: BashExecRequest): BashExecSpec {
-    if (request.timeoutMs !== undefined) assertPositiveFinite('request.timeoutMs', request.timeoutMs)
-    const timeoutMs = Math.min(request.timeoutMs ?? this.config.timeoutMs, this.config.maxTimeoutMs)
+    const timeoutMs = clampTimeout(
+      request.timeoutMs,
+      this.config.timeoutMs,
+      this.config.maxTimeoutMs,
+      'bash-local: request.timeoutMs',
+    )
     return {
       command: request.command,
       workdir: request.workdir ?? this.config.cwd ?? process.cwd(),
@@ -132,29 +137,39 @@ export class LocalBashExecutor extends BashExecutor {
   }
 
   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.
+    using d = deadline(spec.signal, spec.timeoutMs, 'BASH_TIMEOUT')
     const outcome = await runBash({
       command: spec.command,
       cwd: spec.workdir,
-      timeoutMs: spec.timeoutMs,
       maxOutputBytes: this.config.maxOutputBytes,
       graceMs: this.config.graceMs,
-      signal: spec.signal,
+      signal: d.signal,
       stdin: spec.stdin,
       env: spec.env,
     }, this.internals).done
-    return { ...outcome, timeoutMs: spec.timeoutMs }
+    // 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.
+    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). spec.timeoutMs is ignored
-    // here by design.
+    // 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).
     const running = runBash({
       command: spec.command,
       cwd: spec.workdir,
-      timeoutMs: 0,
       maxOutputBytes: this.config.maxOutputBytes,
       graceMs: this.config.graceMs,
       signal: spec.signal,
@@ -174,8 +189,10 @@ export class LocalBashExecutor extends BashExecutor {
       stdoutOffset: 0,
       stderrOffset: 0,
       done: running.done.then((outcome) => {
-        // Abort-killed tasks report as killed, not completed.
-        if (task.status === 'running') task.status = outcome.aborted ? 'killed' : 'completed'
+        // 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)

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

@@ -6,6 +6,12 @@
  * 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
@@ -71,13 +77,17 @@ export function childEnv(extra?: Record<string, string>): NodeJS.ProcessEnv {
 export interface SpawnSpec {
   command: string
   cwd: string
-  /** Kill the process group after this many milliseconds. 0 = no timeout. */
-  timeoutMs: number
   /** Per-stream in-memory cap; overflow spills to disk (tail kept in memory). */
   maxOutputBytes: number
   /** Grace period between the SIGTERM and the SIGKILL escalation on a kill. */
   graceMs: number
-  /** Abort signal — kills the process group when fired. */
+  /**
+   * Abort signal — kills the process group when it fires. The executor owns
+   * timing: `run()` passes a fused timeout/cancel deadline signal (see
+   * `@deepseek-ai/dsh-timeout`), `start()` passes the bare upstream signal.
+   * runBash only listens and kills; it does NOT classify why (the executor
+   * reads the signal's reason afterward).
+   */
   signal?: AbortSignal | undefined
   /**
    * Bytes to write to the child's stdin, then close it. Absent (or empty)
@@ -94,12 +104,15 @@ export interface SpawnSpec {
   env?: Record<string, string> | undefined
 }
 
-/** Raw outcome of one closed process (before result shaping). */
+/**
+ * Raw outcome of one closed process (before result shaping). Deliberately
+ * carries NO timeout/cancel classification: runBash kills on abort but does not
+ * decide why — the executor's `run()`/`start()` reads the deadline signal it
+ * owns to classify `timedOut`/`aborted` (see the package README).
+ */
 export interface SpawnOutcome {
   exitCode: number | null
   signal: NodeJS.Signals | null
-  timedOut: boolean
-  aborted: boolean
   stdout: CollectedOutput
   stderr: CollectedOutput
 }
@@ -343,9 +356,6 @@ export function runBash(spec: SpawnSpec, internals: RunInternals = {}): RunningB
   child.stdout.on('data', (chunk: Buffer) => { stdout.push(chunk) })
   child.stderr.on('data', (chunk: Buffer) => { stderr.push(chunk) })
 
-  let timedOut = false
-  let aborted = false
-  let killTimer: NodeJS.Timeout | undefined
   let graceTimer: NodeJS.Timeout | undefined
 
   // pid is undefined when the spawn itself fails (bad cwd, missing binary);
@@ -358,17 +368,12 @@ export function runBash(spec: SpawnSpec, internals: RunInternals = {}): RunningB
     graceTimer = setTimeout(() => { killGroup(pid, 'SIGKILL') }, spec.graceMs)
   }
 
-  if (spec.timeoutMs > 0) {
-    killTimer = setTimeout(() => {
-      timedOut = true
-      kill()
-    }, spec.timeoutMs)
-  }
-
-  const onAbort = (): void => {
-    aborted = true
-    kill()
-  }
+  // 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.
+  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
@@ -401,14 +406,11 @@ export function runBash(spec: SpawnSpec, internals: RunInternals = {}): RunningB
       resolve({
         exitCode,
         signal,
-        timedOut,
-        aborted,
         stdout: stdout.finalize(),
         stderr: stderr.finalize(),
       })
     })
     function cleanup(): void {
-      if (killTimer !== undefined) clearTimeout(killTimer)
       if (graceTimer !== undefined) clearTimeout(graceTimer)
       spec.signal?.removeEventListener('abort', onAbort)
     }

packages/bash/bash-local/tests/executor.spec.ts

@@ -103,6 +103,8 @@ describe('LocalBashExecutor.run', () => {
     const { bash } = await setup({ timeoutMs: 60_000 })
     const result = await bash.run(bash.resolve({ command: 'sleep 60', timeoutMs: 100 }))
     expect(result.timedOut).toBe(true)
+    // Mutually exclusive: a timeout classifies as timedOut, never also aborted.
+    expect(result.aborted).toBe(false)
     expect(result.timeoutMs).toBe(100)
   })
 
@@ -113,6 +115,20 @@ describe('LocalBashExecutor.run', () => {
     setTimeout(() => { controller.abort() }, 50)
     const result = await pending
     expect(result.aborted).toBe(true)
+    // Mutually exclusive: an upstream cancel classifies as aborted, never also timedOut.
+    expect(result.timedOut).toBe(false)
+  })
+
+  it('classifies a self-killed command as neither timed out nor aborted', async () => {
+    // The command kills itself (SIGTERM) with no timeout and no upstream abort:
+    // the deadline signal never fires, so both classifications are false — the
+    // fused-signal classification reports the cause that cut the command short,
+    // and here nothing the executor owns did.
+    const { bash } = await setup({ timeoutMs: 60_000 })
+    const result = await bash.run(bash.resolve({ command: 'kill -TERM $$' }))
+    expect(result.signal).toBe('SIGTERM')
+    expect(result.timedOut).toBe(false)
+    expect(result.aborted).toBe(false)
   })
 
   it('rejects on spawn failure (bad workdir)', async () => {

packages/bash/bash-local/tests/run.spec.ts

@@ -26,7 +26,6 @@ function spec(command: string, overrides: Partial<Parameters<typeof runBash>[0]>
   return {
     command,
     cwd: process.cwd(),
-    timeoutMs: 0,
     maxOutputBytes: 64_000,
     graceMs: 3_000,
     ...overrides,
@@ -75,8 +74,6 @@ describe('runBash', () => {
     const result = await runBash(spec('echo hello')).done
     expect(result.exitCode).toBe(0)
     expect(result.signal).toBeNull()
-    expect(result.timedOut).toBe(false)
-    expect(result.aborted).toBe(false)
     expect(result.stdout.text).toBe('hello\n')
     expect(result.stdout.truncated).toBe(false)
     expect(result.stderr.text).toBe('')
@@ -111,11 +108,16 @@ describe('runBash', () => {
     expect(result.stdout.text.trim()).toMatch(/\/tmp$/)
   })
 
-  it('kills with SIGTERM on timeout', async () => {
+  it('kills the process group with SIGTERM when the signal fires', async () => {
+    // runBash owns no timer: it kills on abort. The executor drives the timeout
+    // by firing this signal via a deadline (see executor.spec.ts); here we
+    // assert the kill itself lands as SIGTERM.
+    const controller = new AbortController()
     const start = Date.now()
-    const result = await runBash(spec('sleep 60', { timeoutMs: 100 })).done
+    const running = runBash(spec('sleep 60', { signal: controller.signal }))
+    setTimeout(() => { controller.abort('deadline') }, 100)
+    const result = await running.done
     expect(Date.now() - start).toBeLessThan(5_000)
-    expect(result.timedOut).toBe(true)
     expect(result.signal).toBe('SIGTERM')
     expect(result.exitCode).toBeNull()
   })
@@ -147,7 +149,6 @@ describe('runBash', () => {
     const running = runBash(spec('sleep 60', { signal: controller.signal }))
     setTimeout(() => { controller.abort('user cancelled') }, 50)
     const result = await running.done
-    expect(result.aborted).toBe(true)
     expect(result.signal).toBe('SIGTERM')
   })
 
@@ -224,7 +225,6 @@ describe('stdin and extra env (set by in-process plugins)', () => {
     const big = 'x'.repeat(1024 * 1024)
     const result = await runBash(spec('exit 7', { stdin: big })).done
     expect(result.exitCode).toBe(7)
-    expect(result.aborted).toBe(false)
   })
 })
 
@@ -352,11 +352,11 @@ describe('abort edge cases', () => {
       .toThrow(/aborted before spawn: aborted/)
   })
 
-  it('reports an externally self-killed command without the timeout marker', async () => {
+  it('reports the terminating signal of an externally self-killed command', async () => {
+    // runBash reports the raw signal; whether it counts as timeout/cancel is the
+    // executor's classification (a self-kill is neither) — see executor.spec.ts.
     const result = await runBash(spec('kill -TERM $$')).done
     expect(result.signal).toBe('SIGTERM')
-    expect(result.timedOut).toBe(false)
-    expect(result.aborted).toBe(false)
   })
 })
 
@@ -409,10 +409,9 @@ describe('review fixes: env scrubbing and spill hardening', () => {
 
   it('honors AbortSignal on background-style runs (no timeout)', async () => {
     const controller = new AbortController()
-    const running = runBash(spec('sleep 60', { timeoutMs: 0, signal: controller.signal }))
+    const running = runBash(spec('sleep 60', { signal: controller.signal }))
     setTimeout(() => { controller.abort() }, 50)
     const result = await running.done
-    expect(result.aborted).toBe(true)
     expect(result.signal).toBe('SIGTERM')
   })
 })

packages/bash/bash-local/tsconfig.json

@@ -20,6 +20,9 @@
     {
       "path": "../../util/brand"
     },
+    {
+      "path": "../../util/timeout"
+    },
     {
       "path": "../../bash/bash"
     }