From 79a28ad6d98988ab6e0eac7b71dd7cf2662998b5 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sun, 26 Jul 2026 16:50:36 +0800 Subject: [PATCH] fix(subprocess): tree-scoped escalation and byte-exact tails (Codex round 1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (A1) terminate()/dispose()/service teardown keyed on direct-child settlement could leak a TERM-trapping descendant that outlived the leader (Codex reproduced it with a disowned trap-SIGTERM helper). kill()/terminate() now gate on tree liveness instead of outcome settlement; the SIGKILL escalation timer survives settle (unref'd, re-probing the tree); dispose's tier quiescence is whole-tree exit via a bounded waitForExit; the service's live set releases handles only when their tree is gone, and its teardown awaits tree exit. Three new suites pin the survivor scenarios end to end (terminate, dispose, service teardown). (A2) the escalation branch is now real tested behavior — its ignore is gone; the one remaining signalTree guard ignore states why it is unreachable through the handle verbs. (A3) docs contradictions fixed: the impl README's stale POSIX-only bullet now states the contained best-effort Windows tree story; the lsp-local README no longer claims taskkill failures stay visible (containment + the tree-liveness wait is the actual contract); the architecture tables (en+zh) list all three consumer families. (B1) OutputCollector keeps a byte-exact tail across uneven chunk boundaries (trim the head chunk instead of dropping it whole) — the LSP diagnostic-tail contract; pinned by a cross-chunk test. (B2) the subagent-acp coverage ignore is narrowed to exactly the never-settling success arm. --- docs/architecture.i18n.yaml | 4 +- docs/architecture.md | 2 +- docs/architecture.zh.md | 2 +- packages/lsp/lsp-local/README.md | 2 +- packages/subagent/subagent-acp/src/run.ts | 12 +- .../subprocess/subprocess-local/README.md | 2 +- .../subprocess/subprocess-local/package.json | 2 + .../subprocess/subprocess-local/src/index.ts | 18 ++- .../subprocess/subprocess-local/src/spawn.ts | 146 ++++++++++-------- .../subprocess-local/tests/spawn.spec.ts | 90 +++++++++-- .../subprocess/subprocess-local/tsconfig.json | 3 + pnpm-lock.yaml | 3 + 12 files changed, 195 insertions(+), 91 deletions(-) diff --git a/docs/architecture.i18n.yaml b/docs/architecture.i18n.yaml index 5c9c706d6c..0c67fdd66e 100644 --- a/docs/architecture.i18n.yaml +++ b/docs/architecture.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write -architecture.md: 9c4c9fc12a51c9c49d02a7aa9c3633ae7c95e4fe -architecture.zh.md: bc6adef969f60e7a7522a1877d29b2f90fc0c322 +architecture.md: 8334153482843f26defa8d175042a1349d6c9eca +architecture.zh.md: bb874654496bc9d131b1d7529c3585ed4682c0cf diff --git a/docs/architecture.md b/docs/architecture.md index 9c4c9fc12a..8334153482 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -28,7 +28,7 @@ Harnesses are [Cordis](cordis-primer.md) contexts with package-contributed servi | `ctx.llm` | [`llm/`](../packages/llm/README.md) | adapter registry and streaming model calls | | `ctx.tokenMeter` | [`llm/token-meter`](../packages/llm/token-meter/README.md) | singleton replay-aware request/surface pressure | | `ctx.bash` | [`bash/`](../packages/bash/README.md) | foreground/background command execution | -| `ctx.subprocess` | [`subprocess/`](../packages/subprocess/README.md) | managed child-process groups under the bash executors | +| `ctx.subprocess` | [`subprocess/`](../packages/subprocess/README.md) | managed child-process trees for the bash executors, the LSP host, and the ACP subagent backend | | `ctx.pty` | [`pty/`](../packages/pty/README.md) | owner-scoped persistent terminal sessions | | `ctx.sandbox` | [`sandbox/`](../packages/sandbox/README.md) | same-world process confinement (argv wrapping, per-call policy) | | `ctx.sandboxPolicy` | [`sandbox/`](../packages/sandbox/README.md) | shared sandbox policy home | diff --git a/docs/architecture.zh.md b/docs/architecture.zh.md index bc6adef969..bb87465449 100644 --- a/docs/architecture.zh.md +++ b/docs/architecture.zh.md @@ -28,7 +28,7 @@ | `ctx.llm` | [`llm/`](../packages/llm/README.md) | 适配器注册表和模型流式调用 | | `ctx.tokenMeter` | [`llm/token-meter`](../packages/llm/token-meter/README.md) | 感知回放的单实例请求压力和会话表面压力 | | `ctx.bash` | [`bash/`](../packages/bash/README.md) | 前台和后台命令执行 | -| `ctx.subprocess` | [`subprocess/`](../packages/subprocess/README.md) | bash 执行器之下受管理的子进程组 | +| `ctx.subprocess` | [`subprocess/`](../packages/subprocess/README.md) | 供 bash 执行器、LSP host 与 ACP subagent 后端使用的受管子进程树 | | `ctx.pty` | [`pty/`](../packages/pty/README.md) | 按 owner 隔离的持久化终端会话 | | `ctx.sandbox` | [`sandbox/`](../packages/sandbox/README.md) | 同一执行环境内的进程限制（argv 包装、逐调用策略） | | `ctx.sandboxPolicy` | [`sandbox/`](../packages/sandbox/README.md) | 共享沙箱策略归属点 | diff --git a/packages/lsp/lsp-local/README.md b/packages/lsp/lsp-local/README.md index 7c6c05b7df..3b7147f830 100644 --- a/packages/lsp/lsp-local/README.md +++ b/packages/lsp/lsp-local/README.md @@ -10,7 +10,7 @@ Namespace plugin (`name` / `inject` / `Config` / `apply`, no default export). - Lazily single-flights one server process per `(server id, canonical workspace realpath)`. A live server error is not replayed; if the selected pooled transport fails before or during a read-only query, the provider awaits its disposal and retries that query once on a fresh process. - Uses a compatibility-first **transient-open** sequence per query: canonicalize and read the source with Node APIs, `textDocument/didOpen` (version 1, full text), the requested request, then `textDocument/didClose` in `finally`. A failed or canceled `didOpen` write terminates the instance before the pool can reuse it. Documents close after each call, so the first version needs no `didChange`, content cache, or document LRU. - Serializes each source-read/open/query/close lifecycle through one abortable per-workspace queue so queued calls read current source only when their turn starts; distinct workspaces run in parallel. -- After protocol shutdown fails, terminates the server's descendant tree through POSIX process-group signaling or synchronous Windows `taskkill /T /F`. Windows suppresses only taskkill's already-absent-tree result; command, permission, and other tree-kill failures remain visible. +- After protocol shutdown fails, terminates the server's descendant tree through the subprocess seam (POSIX process-group signaling; Windows `taskkill /T /F`). Tree-kill delivery is contained like every group signal — it races server exit — and quiescence is confirmed by the handle's tree-liveness wait rather than by the kill's own outcome. - Reads sources through Node filesystem APIs in the subprocess's host namespace — NOT `ctx.fs`, and emits no `fs/observed`: only the LSP result is model-visible, so a query does not satisfy read-before-write policy. ## Configuration diff --git a/packages/subagent/subagent-acp/src/run.ts b/packages/subagent/subagent-acp/src/run.ts index aa9cd4f492..85f1b9fdde 100644 --- a/packages/subagent/subagent-acp/src/run.ts +++ b/packages/subagent/subagent-acp/src/run.ts @@ -180,10 +180,14 @@ export async function startAcpRun(request: SubagentStartRequest, spec: AcpRunSpe } /* v8 ignore stop */ // Spawn-level failure surfaces as `done` rejecting into the startup race; a - // clean exit must never win it, so the success arm parks forever. - /* v8 ignore start -- the success arm's never-settling executor is intentionally empty. */ - const spawnFailed: Promise = child.done.then(() => new Promise(() => {}), (err: unknown) => Promise.reject(toError(err))) - /* v8 ignore stop */ + // clean exit must never win it, so the success arm parks forever. (The ACP + // connection observing its streams closing bounds a child that exits + // without speaking the protocol.) + const spawnFailed: Promise = child.done.then( + /* v8 ignore next -- the success arm's never-settling executor is intentionally empty. */ + () => new Promise(() => {}), + (err: unknown) => Promise.reject(toError(err)), + ) spawnFailed.catch(() => { /* observed by the startup race; never unhandled */ }) // Startup rollback and the published handle share one process teardown. diff --git a/packages/subprocess/subprocess-local/README.md b/packages/subprocess/subprocess-local/README.md index eccd842396..c8aa39cc7b 100644 --- a/packages/subprocess/subprocess-local/README.md +++ b/packages/subprocess/subprocess-local/README.md @@ -20,7 +20,7 @@ No direct invalidation; the named consumers own any request-prefix changes. ## Known Limitations and Deferred Work -- **POSIX-only** — detached process groups, group kills, and SIGTERM→SIGKILL escalation are hardcoded; Windows is unsupported. +- **Windows tree support is best-effort and untested in CI** — termination routes through `taskkill /PID /T /F` with all outcomes contained (absent tree, races, missing binary), and liveness falls back to the direct-child boundary; the suites cover the routing through an injected runner only, and `packages/subprocess/*` is excluded from the Windows test matrix. - **The credential scrub is a name heuristic** — `*KEY*`/`*SECRET*`/`*TOKEN*` only; differently-named secrets (e.g. `*PASSWORD*`) pass through, and a whitelist for over-scrubbed vars is noted future work. - **Completed spill files are not deleted** — bounded full-output recovery files (and the private per-process spill dir) accumulate under the OS tmpdir until something external cleans them; oversize incomplete spills are discarded and deletion is attempted immediately, but a cleanup failure can leave a bounded file behind. diff --git a/packages/subprocess/subprocess-local/package.json b/packages/subprocess/subprocess-local/package.json index 72ff50c422..871b4cfac6 100644 --- a/packages/subprocess/subprocess-local/package.json +++ b/packages/subprocess/subprocess-local/package.json @@ -29,11 +29,13 @@ "peerDependencies": { "@deepseek-ai/dsh-invariants": "^0.0.1", "@deepseek-ai/dsh-subprocess": "^0.0.1", + "@deepseek-ai/dsh-timeout": "^0.0.1", "cordis": "^4.0.0-rc.7" }, "devDependencies": { "@deepseek-ai/dsh-invariants": "workspace:^", "@deepseek-ai/dsh-subprocess": "workspace:^", + "@deepseek-ai/dsh-timeout": "workspace:^", "cordis": "^4.0.0-rc.7" } } diff --git a/packages/subprocess/subprocess-local/src/index.ts b/packages/subprocess/subprocess-local/src/index.ts index d76e5cf410..d71db5622a 100644 --- a/packages/subprocess/subprocess-local/src/index.ts +++ b/packages/subprocess/subprocess-local/src/index.ts @@ -28,13 +28,14 @@ export class LocalSubprocessService extends SubprocessService { constructor(ctx: Context) { super(ctx) ctx.effect(() => async () => { - // Terminate (escalating), then await closure so even a TERM-trapping - // child cannot outlive the fiber. + // Terminate (escalating), then await WHOLE-TREE exit — not just the + // direct child's settlement — so even a TERM-trapping descendant cannot + // outlive the fiber. const pending: Promise[] = [] for (const handle of this.live) { handle.terminate() // Spawn-failure rejections already settled and left the live set. - pending.push(handle.done.catch(() => {})) + pending.push(handle.done.catch(() => {}).then(() => handle.waitForExit())) } this.live.clear() await Promise.all(pending) @@ -44,10 +45,13 @@ export class LocalSubprocessService extends SubprocessService { spawn(spec: SubprocessSpawnSpec): SubprocessHandle { const handle = spawnSubprocess(spec, this.internals) this.live.add(handle) - handle.done.then( - () => { this.live.delete(handle) }, - () => { this.live.delete(handle) }, - ) + // Release ownership only once the whole TREE is gone, not at direct-child + // settlement — a TERM-trapping helper that outlives the leader must stay + // owned so teardown can still escalate it. For the common no-survivor + // case waitForExit resolves immediately after settlement. + const release = (): Promise => + handle.waitForExit().then(() => { this.live.delete(handle) }) + handle.done.then(release, release) return handle } } diff --git a/packages/subprocess/subprocess-local/src/spawn.ts b/packages/subprocess/subprocess-local/src/spawn.ts index 99c527ace6..212c4cb53e 100644 --- a/packages/subprocess/subprocess-local/src/spawn.ts +++ b/packages/subprocess/subprocess-local/src/spawn.ts @@ -14,7 +14,8 @@ import { randomBytes } from 'node:crypto' import { closeSync, mkdtempSync, openSync, unlinkSync, writeSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' -import { setImmediate as yieldToEventLoop } from 'node:timers/promises' +import { setTimeout as sleepMs } from 'node:timers/promises' +import { deadline } from '@deepseek-ai/dsh-timeout' import { DSH_ENV_PREFIX, scrubbedParentEnv } from '@deepseek-ai/dsh-subprocess' import type { CollectedOutput, @@ -62,6 +63,14 @@ export interface SpawnInternals { platform?: NodeJS.Platform } +/** Timeout code marking a dispose-ladder tier bound (vs an external abort). */ +const DISPOSE_TIER_TIMEOUT = 'SUBPROCESS_DISPOSE_TIER' + +/** Liveness-poll cadence for tree-exit waits; unref'd so an abandoned wait cannot hold the parent's loop open. */ +function sleepTick(): Promise { + return sleepMs(15, undefined, { ref: false }) +} + let spillCounter = 0 let defaultSpillDir: string | undefined @@ -118,19 +127,20 @@ export class OutputCollector { if (!this.spillDisabled && (overflows || this.spillFd !== undefined)) this.spillAll(chunk) this.chunks.push(chunk) this.bytes += chunk.length - while (this.bytes > this.maxBytes && this.chunks.length > 1) { - // Drop whole chunks from the head; pipe chunks are small (≤64KiB), so - // the retained tail tracks the cap closely enough for a model-facing - // truncation boundary. (length > 1 was just checked — shift() returns.) - const head = this.chunks.shift() as Buffer - this.bytes -= head.length - this.dropped = true - } - if (this.bytes > this.maxBytes && this.chunks.length === 1) { - // A single chunk larger than the cap: keep its tail. - const only = this.chunks[0] as Buffer - this.chunks[0] = only.subarray(only.length - this.maxBytes) - this.bytes = this.maxBytes + while (this.bytes > this.maxBytes) { + const head = this.chunks[0] as Buffer + const excess = this.bytes - this.maxBytes + if (head.length <= excess) { + // Drop the whole head chunk (length ≥ 1 is guaranteed while over cap). + this.chunks.shift() + this.bytes -= head.length + } else { + // Trim the head so the retained window is byte-exact at the cap — a + // diagnostic tail (an LSP server's stderr) must hold the LAST + // maxBytes regardless of how the stream was chunked. + this.chunks[0] = head.subarray(excess) + this.bytes -= excess + } this.dropped = true } } @@ -281,6 +291,7 @@ function signalTree( taskkill(pid) return } + /* v8 ignore next -- kill/terminate gate on treeAlive(), which is false for pid -1; this guard protects direct callers only. */ if (pid <= 0) return try { process.kill(-pid, sig) @@ -352,21 +363,50 @@ export function spawnSubprocess(spec: SubprocessSpawnSpec, internals: SpawnInter // Failed spawns use pid -1 so signalling remains a no-op. const pid = child.pid ?? -1 + /** Whether the detached tree's root (or POSIX group) is still alive. */ + const treeAlive = (): boolean => { + if (pid <= 0) return false + if (platform === 'win32') { + // Windows has no group-liveness probe; the direct child's exit is the + // observable boundary (taskkill /T already took the tree with it). + return child.exitCode === null && child.signalCode === null + } + try { + process.kill(-pid, 0) + return true + } catch (error) { + const code = (error as NodeJS.ErrnoException).code + /* v8 ignore next 2 -- POSIX reports an absent group as ESRCH; child-reaping timing + makes observing the other arm platform-dependent. */ + if (code === 'ESRCH') return false + /* v8 ignore start -- EPERM and non-POSIX negative-pid failures are platform defenses; CI runs + tree-lifecycle tests on POSIX hosts where absence reports ESRCH. */ + if (code === 'EPERM') return true + return child.exitCode === null && child.signalCode === null + /* v8 ignore stop */ + } + } + const kill = (sig: NodeJS.Signals = 'SIGTERM'): void => { - // After settlement the tree is gone and the pid may be reused; callers - // commonly kill() in a finally, so this must not re-signal. - if (settled) return + // Guard on TREE liveness, not outcome settlement: a TERM-trapping helper + // can outlive the settled direct child and must stay signalable, while a + // fully-dead tree (possible pid reuse) must not be re-signalled from a + // caller's finally block. + if (!treeAlive()) return signalTree(platform, pid, sig, child, taskkill) } const terminate = (): void => { if (graceTimer !== undefined) return // escalation already in flight - if (settled) return + if (!treeAlive()) return signalTree(platform, pid, 'SIGTERM', child, taskkill) + // The escalation must survive direct-child settlement — the leader dying + // does not mean the tree died — so the timer is unref'd rather than + // cleared at settle, and re-checks tree liveness before force-killing. graceTimer = setTimeout(() => { - /* v8 ignore next -- the timer is cleared at settlement; only an in-flight fire racing the close event sees settled=true. */ - if (!settled) signalTree(platform, pid, 'SIGKILL', child, taskkill) + if (treeAlive()) signalTree(platform, pid, 'SIGKILL', child, taskkill) }, spec.graceMs) + graceTimer.unref() } // The caller owns timeout classification; this layer only reacts to abort. @@ -408,75 +448,51 @@ export function spawnSubprocess(spec: SubprocessSpawnSpec, internals: SpawnInter }) child.on('close', settle) function cleanup(): void { - if (graceTimer !== undefined) clearTimeout(graceTimer) + // graceTimer deliberately NOT cleared: the SIGKILL escalation must be + // able to reach tree survivors after the direct child settles. if (pipeDrainTimer !== undefined) clearTimeout(pipeDrainTimer) spec.signal?.removeEventListener('abort', onAbort) } }) - /** Whether the detached tree's root (or POSIX group) is still alive. */ - const treeAlive = (): boolean => { - if (pid <= 0) return false - if (platform === 'win32') { - // Windows has no group-liveness probe; the direct child's exit is the - // observable boundary (taskkill /T already took the tree with it). - return child.exitCode === null && child.signalCode === null - } - try { - process.kill(-pid, 0) - return true - } catch (error) { - const code = (error as NodeJS.ErrnoException).code - /* v8 ignore next -- POSIX reports an absent group as ESRCH; child-reaping timing - makes observing the other arm platform-dependent. */ - if (code === 'ESRCH') return false - /* v8 ignore start -- EPERM and non-POSIX negative-pid failures are platform defenses; CI runs - tree-lifecycle tests on POSIX hosts where absence reports ESRCH. */ - if (code === 'EPERM') return true - return child.exitCode === null && child.signalCode === null - /* v8 ignore stop */ - } - } - const waitForExit = async (signal?: AbortSignal): Promise => { while (treeAlive()) { if (signal?.aborted) return false - await yieldToEventLoop() + await sleepTick() } return true } - /** Race settlement against a timer without leaving listeners or live timers behind. */ - const settlesWithin = async (ms: number): Promise => { - if (settled) return true - // The executor runs synchronously, so the timer is assigned before the race. - let timer!: NodeJS.Timeout - const timeout = new Promise((resolve) => { - // `.unref()` so a pending grace timer never keeps the parent's loop alive. - timer = setTimeout(() => { resolve(false) }, ms) - timer.unref() - }) - try { - return await Promise.race([done.then(() => true, () => true), timeout]) - } finally { - clearTimeout(timer) - } + /** + * Wait, bounded, for whole-tree exit — the dispose ladder's quiescence test. + * Tree liveness, not direct-child settlement: a TERM-trapping helper that + * outlives the leader must hold the ladder on its tier until it exits. + */ + const treeExitsWithin = async (ms: number): Promise => { + using bound = deadline(undefined, ms, DISPOSE_TIER_TIMEOUT) + return await waitForExit(bound.signal) } let disposal: Promise | undefined const dispose = (graces: SubprocessDisposeGraces): Promise => (disposal ??= (async () => { + // A spawn failure has no process to tear down; observe the rejection so + // disposal in a finally block cannot surface it as unhandled. + if (pid <= 0) { + await done.catch(() => {}) + return + } // 1. Close a piped stdin and allow cooperative teardown and flush. if (stdinMode === 'pipe') child.stdin?.end() - if (await settlesWithin(graces.eofGraceMs)) return + if (await treeExitsWithin(graces.eofGraceMs)) return // 2. POSIX gets a catchable graceful signal; Windows taskkill force-terminates. if (platform !== 'win32') { kill('SIGTERM') - if (await settlesWithin(graces.graceMs)) return + if (await treeExitsWithin(graces.graceMs)) return } // 3. Force-kill the tree and await a bounded exit edge. kill('SIGKILL') - if (!(await settlesWithin(graces.graceMs))) { - throw new Error(`child process did not exit within ${graces.graceMs}ms after forced termination`) + if (!(await treeExitsWithin(graces.graceMs))) { + throw new Error(`child process tree did not exit within ${graces.graceMs}ms after forced termination`) } })()) diff --git a/packages/subprocess/subprocess-local/tests/spawn.spec.ts b/packages/subprocess/subprocess-local/tests/spawn.spec.ts index ecff00d229..38d4a970bd 100644 --- a/packages/subprocess/subprocess-local/tests/spawn.spec.ts +++ b/packages/subprocess/subprocess-local/tests/spawn.spec.ts @@ -349,6 +349,19 @@ describe('OutputCollector', () => { expect(readFileSync(out.spillPath!, 'utf8')).toBe('0123456789abcdef') }) + it('retains a byte-exact tail across uneven chunk boundaries', () => { + // The old whole-chunk drop could under-retain; a diagnostic tail must be + // exactly the LAST maxBytes regardless of chunking. + const collector = new OutputCollector(10, undefined, 'exact-tail', spillDir) + collector.push(Buffer.from('aaaa')) + collector.push(Buffer.from('bbbbbb')) + collector.push(Buffer.from('cc')) + const out = collector.finalize() + expect(out.text).toBe('aabbbbbbcc') + expect(Buffer.byteLength(out.text)).toBe(10) + expect(out.truncated).toBe(true) + }) + it('readFrom returns increments and flags lossy reads', () => { const collector = new OutputCollector(10, 100, 'test', spillDir) collector.push(Buffer.from('aaaaa')) @@ -439,16 +452,18 @@ describe('killGroup', () => { expect(() => { killGroup(running.pid, 'SIGTERM') }).not.toThrow() }) - it('handle.kill() after settlement signals nothing and starts no grace timer', async () => { - // Cleanup code commonly kills handles in a finally; after settlement the - // group is gone and the pid may be reused, so a late kill must be inert - // (no signal to a possibly-recycled pgid, no referenced timer delaying exit). + it('handle.kill() after the tree died delivers no termination signal', async () => { + // Cleanup code commonly kills handles in a finally; once the tree is gone + // the pid may be reused, so a late kill must deliver nothing (the + // liveness PROBE — signal 0 — is the only process.kill allowed). const running = spawnSubprocess(spec('true')) await running.done + await running.waitForExit() const spy = vi.spyOn(process, 'kill') try { running.kill() - expect(spy).not.toHaveBeenCalled() + const delivered = spy.mock.calls.filter(([, sig]) => sig !== 0) + expect(delivered).toEqual([]) } finally { spy.mockRestore() } @@ -575,6 +590,59 @@ describe('waitForExit', () => { }) }) +describe('tree-survivor escalation (terminate/dispose reach helpers the leader left behind)', () => { + it('terminate() SIGKILLs a TERM-trapping descendant after the direct child settles', async () => { + // The leader spawns a TERM-trapping helper with all stdio detached from + // the collected pipes, then exits: the helper holds the GROUP alive while + // the direct child settles. The escalation must still reach it. + const pidFile = join(spillDir, `survivor-${Date.now()}.pid`) + const running = spawnSubprocess(spec( + `bash -c 'trap "" TERM; echo $$ > ${pidFile}; sleep 60' >/dev/null 2>&1 & disown; wait_placeholder=; exit 0`, + { graceMs: 300 }, + )) + const helper = await waitForPidFile(pidFile) + await running.done // direct child settled; helper survives in the group + expect(() => process.kill(helper, 0)).not.toThrow() + + running.terminate() // SIGTERM (trapped) → grace → SIGKILL the group + await expect(running.waitForExit()).resolves.toBe(true) + await waitGone(helper) + }) + + it('dispose() holds each tier on whole-tree exit, not direct-child settlement', async () => { + const pidFile = join(spillDir, `survivor-dispose-${Date.now()}.pid`) + const running = spawnSubprocess(spec( + `bash -c 'trap "" TERM; echo $$ > ${pidFile}; sleep 60' >/dev/null 2>&1 & disown; exit 0`, + { graceMs: 200 }, + )) + const helper = await waitForPidFile(pidFile) + await running.done + expect(() => process.kill(helper, 0)).not.toThrow() + + await running.dispose({ eofGraceMs: 100, graceMs: 300 }) + // The ladder only returns once the WHOLE tree is gone. + expect(() => process.kill(helper, 0)).toThrow() + }) + + it('service teardown awaits tree survivors, not just handle settlement', async () => { + const { Context } = await import('cordis') + const { default: LocalSubprocessService } = await import('@deepseek-ai/dsh-subprocess-local') + const ctx = new Context() + const fiber = await ctx.plugin(LocalSubprocessService) + ;(ctx.subprocess as InstanceType).internals = { spillDir } + const pidFile = join(spillDir, `survivor-svc-${Date.now()}.pid`) + const running = ctx.subprocess.spawn(spec( + `bash -c 'trap "" TERM; echo $$ > ${pidFile}; sleep 60' >/dev/null 2>&1 & disown; exit 0`, + { graceMs: 200 }, + )) + const helper = await waitForPidFile(pidFile) + await running.done + await fiber.dispose() + // Teardown itself waited for the survivor to die. + expect(() => process.kill(helper, 0)).toThrow() + }) +}) + describe('coverage seams', () => { it('taskkillProcessTree ignores non-positive pids and contains a missing binary', () => { expect(() => { taskkillProcessTree(-1) }).not.toThrow() @@ -604,13 +672,15 @@ describe('coverage seams', () => { expect(running.collected.stderr!.readFrom(0).text).toBe('err\n') }) - it('terminate() after settlement is a no-op', async () => { + it('terminate() after the tree died delivers no termination signal', async () => { const running = spawnSubprocess(spec('true')) await running.done + await running.waitForExit() const spy = vi.spyOn(process, 'kill') try { running.terminate() - expect(spy).not.toHaveBeenCalled() + const delivered = spy.mock.calls.filter(([, sig]) => sig !== 0) + expect(delivered).toEqual([]) } finally { spy.mockRestore() } @@ -622,13 +692,15 @@ describe('coverage seams', () => { await expect(running.waitForExit()).resolves.toBe(true) }) - it('dispose() on an already-settled handle returns without signalling', async () => { + it('dispose() on an already-exited tree returns without delivering a signal', async () => { const running = spawnSubprocess(spec('true')) await running.done + await running.waitForExit() const spy = vi.spyOn(process, 'kill') try { await running.dispose({ eofGraceMs: 50, graceMs: 50 }) - expect(spy).not.toHaveBeenCalled() + const delivered = spy.mock.calls.filter(([, sig]) => sig !== 0) + expect(delivered).toEqual([]) } finally { spy.mockRestore() } diff --git a/packages/subprocess/subprocess-local/tsconfig.json b/packages/subprocess/subprocess-local/tsconfig.json index 5a8dea211b..5272a4f78d 100644 --- a/packages/subprocess/subprocess-local/tsconfig.json +++ b/packages/subprocess/subprocess-local/tsconfig.json @@ -17,6 +17,9 @@ { "path": "../subprocess" }, + { + "path": "../../util/timeout" + }, { "path": "../../support/invariants" } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index d85d124931..c3be946fc0 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -3699,6 +3699,9 @@ importers: '@deepseek-ai/dsh-subprocess': specifier: workspace:^ version: link:../subprocess + '@deepseek-ai/dsh-timeout': + specifier: workspace:^ + version: link:../../util/timeout cordis: specifier: ^4.0.0-rc.7 version: 4.0.0-rc.7(@cordisjs/plugin-include@1.0.4)(@cordisjs/plugin-loader@1.0.0-rc.5)