Merge remote-tracking branch 'origin/master' into codex/rfc-subagent-background-tasks

Adopts #185 (dsh-timeout: clampTimeout/deadline/timeoutOf drive bash
run() timeout classification; runBash loses its own timer) and #108
(ask_user_question) across the task-runtime rework: bash-local keeps
the BashProcess handle shape with master's deadline mechanics, tool
catalogs/expectations carry both the task_* and ask-user tools, and
generated docs are regenerated on the union.

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

@@ -19,6 +19,7 @@ import { Context } from 'cordis'
 import z from 'schemastery'
 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'
 
@@ -107,8 +108,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(),
@@ -122,29 +127,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): BashProcess {
     // No timeout for background processes (matches Claude Code, which
     // detaches the timeout when backgrounding); callers stop them via the
     // handle's 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.
+    // for background runs too (runBash wires it to the group kill). No
+    // deadline is created here, so spec.timeoutMs is ignored by design —
+    // background processes 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,
@@ -160,8 +175,10 @@ export class LocalBashExecutor extends BashExecutor {
       exitCode: null,
       signal: null,
       done: running.done.then((outcome) => {
-        // Abort-killed processes report as killed, not completed.
-        if (proc.status === 'running') proc.status = outcome.aborted ? 'killed' : 'completed'
+        // Abort-killed processes 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 (proc.status === 'running') proc.status = spec.signal?.aborted === true ? 'killed' : 'completed'
         proc.exitCode = outcome.exitCode
         proc.signal = outcome.signal
         this.live.delete(proc)

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)
     }