Fix timer bounds and ACP teardown ownership

2026-08-05 05:24:23 +08:00
parent 075b6b0756
commit 0ccd847ef3
36 changed files with 130 additions and 96 deletions
--- a/packages/subagent/subagent-acp/README.i18n.yaml
+++ b/packages/subagent/subagent-acp/README.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 packages/subagent/subagent-acp/README.md
-README.md: efcc77c442714a83631d009efb712fa7b8f5dfa0
-README.zh.md: 216d66ed5d259c1bf2ea2383356df97be3e5ced1
+README.md: ead942668f19c44f552d3feb556c39c13b656dea
+README.zh.md: e1204ff62f23c8586852111687574a60b0bb7302
--- a/packages/subagent/subagent-acp/README.md
+++ b/packages/subagent/subagent-acp/README.md
@@ -14,7 +14,7 @@ The returned run id is minted in the parent namespace. The child server's sessio

 After publication, the provider sends the prompt and collects streamed `agent_message_chunk` text into `SubagentResult.output`. A prompt/transport failure resolves with `stopReason: 'error'`, or `aborted` when the required request signal or disposal requested cancellation.

-`dispose()` is idempotent. It removes the signal listener, requests ACP cancellation when possible, then runs this backend's own teardown ladder (`disposeAcpChild`) over the seam's verbs: close stdin and wait `disposeEofGraceMs` for cooperative quiescence, then the handle's `terminate()` escalation (SIGTERM, the spawn grace, SIGKILL — Windows force-terminates directly), then a bounded whole-tree exit wait that rejects if survivors remain. Every run uses a fresh process; process pooling is not implemented.
+`dispose()` is idempotent. It removes the signal listener, requests ACP cancellation when possible, then runs this backend's own teardown ladder (`disposeAcpChild`) over the seam's verbs: close stdin and wait `disposeEofGraceMs` for cooperative quiescence, then invoke the handle's `terminate()` escalation (SIGTERM, the spawn grace, SIGKILL — Windows force-terminates directly) and await the subprocess owner's whole-tree exit proof. Every run uses a fresh process; process pooling is not implemented.

 ## Capabilities and context

@@ -30,8 +30,8 @@ ACP advertises no start-time capabilities because this process cannot enforce th
 | `cwd` | parent session cwd | Working-directory override for the child process and its ACP session; must be non-empty, a relative value resolves against the harness launch directory at load, and the result must name a directory the harness can enter. |
 | `permission` | `reject` | Auto-answer permission requests by rejecting or choosing the first allow-shaped option. |
 | `env` | `{}` | Explicit child environment layered over a credential-scrubbed parent environment. |
-| `disposeEofGraceMs` | `6000` | Grace after stdin EOF before platform termination. |
-| `disposeGraceMs` | `3000` | Exit-confirmation grace after termination; POSIX also waits this long after SIGTERM before SIGKILL. |
+| `disposeEofGraceMs` | `6000` | Positive grace after stdin EOF before platform termination; it cannot exceed [`MAX_TIMER_DELAY_MS`](../../util/timeout/README.md). |
+| `disposeGraceMs` | `3000` | Positive POSIX grace after SIGTERM before SIGKILL (Windows force-terminates directly); it cannot exceed [`MAX_TIMER_DELAY_MS`](../../util/timeout/README.md). |

 ```yaml
 - id: subagent-acp
@@ -57,7 +57,7 @@ ACP advertises no start-time capabilities because this process cannot enforce th

 ## Process boundary

-The child spawns through the [`dsh-subprocess`](../../subprocess/subprocess/README.md) seam: credential-shaped ambient variables and ambient `DSH_*` names are removed by the shared scrub, then explicit `config.env` values merge after it (an intended `DEEPSEEK_API_KEY` survives, and a `DSH_*` deployment fact such as `DSH_PERMISSION_MODE` reaches the child the same way — the scrub drops only its stale ambient namesake), stderr is inherited to the parent's own stream, and disposal runs the seam's cooperative stdin-EOF→SIGTERM→SIGKILL ladder with this plugin's configured graces. The ACP wire is the real serialization boundary; same-process subagent values are not defensively cloned.
+The child spawns through the [`dsh-subprocess`](../../subprocess/subprocess/README.md) seam: credential-shaped ambient variables and ambient `DSH_*` names are removed by the shared scrub, then explicit `config.env` values merge after it (an intended `DEEPSEEK_API_KEY` survives, and a `DSH_*` deployment fact such as `DSH_PERMISSION_MODE` reaches the child the same way — the scrub drops only its stale ambient namesake), stderr is inherited to the parent's own stream, and disposal applies this plugin's EOF window before the subprocess-owned SIGTERM→SIGKILL escalation and whole-tree join. The ACP wire is the real serialization boundary; same-process subagent values are not defensively cloned.

 The package has no default export. Cordis loader unwrapping would otherwise hide the named `inject` metadata; see [postmortem 0001](../../../docs/postmortem/0001-acp-default-export-drops-inject.md).

--- a/packages/subagent/subagent-acp/README.zh.md
+++ b/packages/subagent/subagent-acp/README.zh.md
@@ -14,7 +14,7 @@ ACP（Agent Client Protocol）提供方会在全新的子进程中运行每个 s

 发布后，提供方发送提示词，并把流式 `agent_message_chunk` 文本收集到 `SubagentResult.output`。提示词/传输失败会以 `stopReason: 'error'` 兑现；如果必需的请求信号或 dispose（资源释放）请求了取消，则以 `aborted` 兑现。

-`dispose()` 是幂等的。它会移除信号监听器，在可行时请求 ACP 取消，然后经由该 seam 的动词运行本后端自有的拆卸阶梯（`disposeAcpChild`）：先关闭 stdin 并等待 `disposeEofGraceMs` 让子进程协作式完全停稳，再触发句柄的 `terminate()` 升级（SIGTERM、spawn 宽限期、SIGKILL——Windows 直接强制终止），最后进行有界的整树退出等待；若仍有存活进程，则拒绝。每次运行都使用全新进程；尚未实现进程池。
+`dispose()` 是幂等的。它会移除信号监听器，在可行时请求 ACP 取消，然后经由该 seam 的动词运行本后端自有的拆卸阶梯（`disposeAcpChild`）：先关闭 stdin 并等待 `disposeEofGraceMs` 让子进程协作式完全停稳，再触发句柄的 `terminate()` 升级（SIGTERM、spawn 宽限期、SIGKILL——Windows 直接强制终止），并等待子进程责任方给出整棵进程树的退出证明。每次运行都使用全新进程；尚未实现进程池。

 ## 能力与上下文

@@ -30,8 +30,8 @@ ACP 不声明任何启动时能力，因为当前进程无法强制执行远程
 | `cwd` | 父会话 cwd | 子进程及其 ACP 会话的工作目录覆盖值；不得为空。相对值会在加载时以 harness 启动目录为基准解析，结果必须指向 harness 可以进入的目录。 |
 | `permission` | `reject` | 自动回答权限请求：拒绝，或选择第一个允许形态的选项。 |
 | `env` | `{}` | 显式子进程环境，叠加到已清理凭据的父进程环境之上。 |
-| `disposeEofGraceMs` | `6000` | stdin EOF 之后、平台终止之前的宽限时间。 |
-| `disposeGraceMs` | `3000` | 终止后的退出确认宽限时间；POSIX 在 SIGTERM 后、SIGKILL 前也会等待同样时长。 |
+| `disposeEofGraceMs` | `6000` | stdin EOF 之后、平台终止之前的宽限时间须为正值，且不得大于 [`MAX_TIMER_DELAY_MS`](../../util/timeout/README.md)。 |
+| `disposeGraceMs` | `3000` | POSIX 在 SIGTERM 后、SIGKILL 前的宽限时间（Windows 直接强制终止），须为正值且不得大于 [`MAX_TIMER_DELAY_MS`](../../util/timeout/README.md)。 |

 ```yaml
 - id: subagent-acp
@@ -57,7 +57,7 @@ ACP 不声明任何启动时能力，因为当前进程无法强制执行远程

 ## 进程边界

-子进程经由 [`dsh-subprocess`](../../subprocess/subprocess/README.md) seam spawn：共享的凭据清除先移除疑似凭据的环境变量和环境中已有的 `DSH_*` 名称，显式 `config.env` 值在清除之后合并（有意转发的 `DEEPSEEK_API_KEY` 会保留下来，`DSH_PERMISSION_MODE` 这类 `DSH_*` 部署事实也以同样的方式到达子进程——清除只丢弃其陈旧的同名环境值），stderr 会继承到父进程自身的流，dispose 则以本插件配置的宽限期运行该 seam 的协作式 stdin EOF→SIGTERM→SIGKILL 阶梯。ACP 协议格式（wire format）是真正的序列化边界；同进程 subagent 值不会为防御目的而克隆。
+子进程经由 [`dsh-subprocess`](../../subprocess/subprocess/README.md) seam spawn：共享的凭据清除先移除疑似凭据的环境变量和环境中已有的 `DSH_*` 名称，显式 `config.env` 值在清除之后合并（有意转发的 `DEEPSEEK_API_KEY` 会保留下来，`DSH_PERMISSION_MODE` 这类 `DSH_*` 部署事实也以同样的方式到达子进程——清除只丢弃其陈旧的同名环境值），stderr 会继承到父进程自身的流，dispose 则先应用本插件的 EOF 时间窗，再由子进程责任方执行 SIGTERM→SIGKILL 升级并等待整棵进程树退出。ACP 协议格式（wire format）是真正的序列化边界；同进程 subagent 值不会为防御目的而克隆。

 本包（package）没有默认导出。否则 Cordis loader 的解包会隐藏具名 `inject` 元数据；见[事故复盘（postmortem）0001](../../../docs/postmortem/0001-acp-default-export-drops-inject.md)。

--- a/packages/subagent/subagent-acp/package.json
+++ b/packages/subagent/subagent-acp/package.json
@@ -33,6 +33,7 @@
    "@deepseek-ai/dsh-session": "^0.0.1",
    "@deepseek-ai/dsh-subagent": "^0.0.1",
    "@deepseek-ai/dsh-subprocess": "^0.0.1",
+    "@deepseek-ai/dsh-timeout": "^0.0.1",
    "cordis": "^4.0.0-rc.7"
  },
  "dependencies": {
@@ -49,6 +50,7 @@
    "@deepseek-ai/dsh-subagent": "workspace:^",
    "@deepseek-ai/dsh-subprocess": "workspace:^",
    "@deepseek-ai/dsh-subprocess-local": "workspace:^",
+    "@deepseek-ai/dsh-timeout": "workspace:^",
    "cordis": "^4.0.0-rc.7"
  }
 }
--- a/packages/subagent/subagent-acp/src/index.ts
+++ b/packages/subagent/subagent-acp/src/index.ts
@@ -17,6 +17,7 @@ import type {
  SubagentProvider,
  SubagentStartRequest,
 } from '@deepseek-ai/dsh-subagent'
+import { MAX_TIMER_DELAY_MS } from '@deepseek-ai/dsh-timeout'
 import { type AcpRunSpec, DEFAULT_DISPOSE_EOF_GRACE_MS, DEFAULT_DISPOSE_GRACE_MS, type PermissionPolicy, startAcpRun } from './run.ts'

 export const name = 'subagent-acp'
@@ -54,10 +55,11 @@ export interface Config {
  /**
   * Grace period (ms) for the child's EOF-driven quiesce on dispose — its
   * window to flush persistence and tear down its own nested subprocesses
-   * before the parent escalates to a signal.
+   * before the parent escalates to a signal. Must not exceed
+   * `MAX_TIMER_DELAY_MS`.
   */
  disposeEofGraceMs?: number
-  /** Termination confirmation window (ms), including forced exit on every platform. */
+  /** Termination-escalation grace (ms); must not exceed `MAX_TIMER_DELAY_MS`. */
  disposeGraceMs?: number
 }

@@ -72,10 +74,10 @@ export const Config: z<Config> = z.object({
  disposeGraceMs: z.number().default(DEFAULT_DISPOSE_GRACE_MS),
 })

-/** A dispose grace must be a positive finite number (it bounds the teardown wait). */
+/** A dispose grace must fit the single Node timer that owns its teardown tier. */
 function assertPositiveFinite(name: string, value: number): void {
-  if (!Number.isFinite(value) || value <= 0) {
-    throw new Error(`subagent-acp: ${name} must be a positive finite number`)
+  if (!Number.isFinite(value) || value <= 0 || value > MAX_TIMER_DELAY_MS) {
+    throw new Error(`subagent-acp: ${name} must be a positive finite number no greater than ${MAX_TIMER_DELAY_MS}`)
  }
 }

--- a/packages/subagent/subagent-acp/src/run.ts
+++ b/packages/subagent/subagent-acp/src/run.ts
@@ -62,9 +62,9 @@ export interface AcpRunSpec {
   */
  disposeEofGraceMs: number
  /**
-   * Termination confirmation window (ms) in {@link SubagentRun.dispose}; POSIX applies it after
-   * `SIGTERM` and `SIGKILL`, while Windows applies it after direct forced termination. The plugin
-   * fills this from its `disposeGraceMs` config.
+   * Termination-escalation grace (ms) in {@link SubagentRun.dispose}; POSIX
+   * waits this long after `SIGTERM` before `SIGKILL`, while Windows
+   * force-terminates directly. The plugin fills it from `disposeGraceMs`.
   */
  disposeGraceMs: number
  /**
@@ -105,14 +105,12 @@ async function treeExitsWithin(child: SubprocessHandle, ms: number): Promise<boo
 * Cooperative teardown ladder for an out-of-process agent, over the seam's
 * public verbs; resolves only at whole-tree quiescence: stdin EOF (the child's
 * window to flush persistence and reap its own descendants), then the
- * terminate() escalation (SIGTERM → spec grace → SIGKILL), then a bounded
- * confirmation wait.
+ * terminate() escalation (SIGTERM → spec grace → SIGKILL) and its
+ * whole-tree exit proof.
 * @param child - the spawned ACP child's handle.
 * @param eofGraceMs - tier-1 window after stdin EOF.
- * @param graceMs - confirmation window after the escalation's SIGKILL.
- * @throws when the tree still has not exited `graceMs` after forced termination.
 */
-export async function disposeAcpChild(child: SubprocessHandle, eofGraceMs: number, graceMs: number): Promise<void> {
+export async function disposeAcpChild(child: SubprocessHandle, eofGraceMs: number): Promise<void> {
  // A spawn failure has no process to tear down; observe the rejection so
  // disposal in a finally block cannot surface it as unhandled.
  if (child.pid <= 0) {
@@ -121,13 +119,10 @@ export async function disposeAcpChild(child: SubprocessHandle, eofGraceMs: numbe
  }
  child.stdin?.end()
  if (await treeExitsWithin(child, eofGraceMs)) return
-  // terminate() sends SIGTERM now and SIGKILL after the spawn spec's grace
-  // (this plugin passes disposeGraceMs there), so the bound covers both the
-  // escalation window and an equal confirmation window after the SIGKILL.
+  // terminate() owns the bounded SIGTERM→SIGKILL timer. Its unbounded wait is
+  // the process owner's exit proof, not a second derived grace that can overflow.
  child.terminate()
-  if (!(await treeExitsWithin(child, graceMs * 2))) {
-    throw new Error('ACP child process tree did not exit within its dispose windows')
-  }
+  await child.waitForExit()
 }

 /**
@@ -235,7 +230,7 @@ export async function startAcpRun(request: SubagentStartRequest, spec: AcpRunSpe

  // Startup rollback and the published handle share one process teardown.
  let processDisposal: Promise<void> | undefined
-  const disposeProcess = (): Promise<void> => (processDisposal ??= disposeAcpChild(child, spec.disposeEofGraceMs, spec.disposeGraceMs))
+  const disposeProcess = (): Promise<void> => (processDisposal ??= disposeAcpChild(child, spec.disposeEofGraceMs))

  // Accumulate the child's streamed assistant text — the SubagentResult output.
  const output: string[] = []
--- a/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts
+++ b/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts
@@ -7,6 +7,7 @@ import { join, resolve } from 'node:path'
 import { fileURLToPath } from 'node:url'
 import SubagentService from '@deepseek-ai/dsh-subagent'
 import type { Agent } from '@deepseek-ai/dsh-agent'
+import { MAX_TIMER_DELAY_MS } from '@deepseek-ai/dsh-timeout'
 import * as acp from '../src/index.ts'
 import { acpStopReason, acpContentText, DEFAULT_DISPOSE_EOF_GRACE_MS, DEFAULT_DISPOSE_GRACE_MS, disposeAcpChild, startAcpRun, toAcpPrompt, type AcpRunSpec } from '../src/run.ts'
 import LocalSubprocessService from '@deepseek-ai/dsh-subprocess-local'
@@ -147,7 +148,7 @@ describe('disposeAcpChild (the backend-owned teardown ladder over seam verbs)',

  it('tier 1: a cooperative child exits on stdin EOF without any signal', async () => {
    const child = bash('read -r line; exit 0')
-    await disposeAcpChild(child, 5_000, 200)
+    await disposeAcpChild(child, 5_000)
    const outcome = await child.done
    expect(outcome.exitCode).toBe(0)
    expect(outcome.signal).toBeNull()
@@ -155,7 +156,7 @@ describe('disposeAcpChild (the backend-owned teardown ladder over seam verbs)',

  it('tier 2: an EOF-deaf child dies by the terminate escalation (SIGTERM)', async () => {
    const child = bash('sleep 60')
-    await disposeAcpChild(child, 100, 5_000)
+    await disposeAcpChild(child, 100)
    const outcome = await child.done
    expect(outcome.signal).toBe('SIGTERM')
  })
@@ -166,30 +167,11 @@ describe('disposeAcpChild (the backend-owned teardown ladder over seam verbs)',
    while (!child.collected.stdout!.readFrom(0).text.includes('armed')) {
      await new Promise(resolve => setTimeout(resolve, 10))
    }
-    await disposeAcpChild(child, 50, 2_000)
+    await disposeAcpChild(child, 50)
    const outcome = await child.done
    expect(outcome.signal).toBe('SIGKILL')
  })

-  it('throws when the tree survives even the escalation window', async () => {
-    // A handle whose tree never exits (waitForExit only ever aborts): the
-    // ladder must fail loud instead of resolving over survivors. Built as a
-    // stub because the ladder composes only public verbs.
-    const never: Parameters<typeof disposeAcpChild>[0] = {
-      pid: 1,
-      stdin: undefined,
-      stdout: undefined,
-      stderr: undefined,
-      collected: {},
-      done: new Promise(() => {}),
-      terminate: () => {},
-      waitForExit: (signal?: AbortSignal) => new Promise((resolve) => {
-        signal?.addEventListener('abort', () => { resolve(false) }, { once: true })
-      }),
-    }
-    await expect(disposeAcpChild(never, 20, 20)).rejects.toThrow(/did not exit within its dispose windows/)
-  })
-
  it('observes a spawn-level rejection and returns without a process to reap', async () => {
    const child = spawnSubprocess({
      argv: ['bash', '-c', 'true'],
@@ -197,7 +179,7 @@ describe('disposeAcpChild (the backend-owned teardown ladder over seam verbs)',
      stdio: { stdin: 'ignore', stdout: { maxBytes: 1000 }, stderr: { maxBytes: 1000 } },
      graceMs: 200,
    })
-    await expect(disposeAcpChild(child, 1_000, 1_000)).resolves.toBeUndefined()
+    await expect(disposeAcpChild(child, 1_000)).resolves.toBeUndefined()
    await expect(child.done).rejects.toThrow()
  })
 })
@@ -721,13 +703,20 @@ describe('dsh-subagent-acp', () => {
    }
  })

-  it('rejects a non-positive dispose grace at load', async () => {
-    for (const bad of [{ disposeEofGraceMs: 0 }, { disposeGraceMs: -1 }, { disposeEofGraceMs: Number.NaN }]) {
+  it('rejects a dispose grace outside the Node timer range at load', async () => {
+    for (const bad of [
+      { disposeEofGraceMs: 0 },
+      { disposeGraceMs: -1 },
+      { disposeEofGraceMs: Number.NaN },
+      { disposeGraceMs: Number.POSITIVE_INFINITY },
+      { disposeEofGraceMs: MAX_TIMER_DELAY_MS + 1 },
+      { disposeGraceMs: MAX_TIMER_DELAY_MS + 1 },
+    ]) {
      const ctx = new Context()
      await ctx.plugin(SubagentService)
      await ctx.plugin(LocalSubprocessService)
      await expect(ctx.plugin(acp, { providerName: 'acp', command: 'true', args: [], permission: 'reject', env: {}, ...bad }))
-        .rejects.toThrow(/subagent-acp: dispose(?:Eof)?GraceMs must be a positive finite number/)
+        .rejects.toThrow(new RegExp(`subagent-acp: dispose(?:Eof)?GraceMs must be a positive finite number no greater than ${MAX_TIMER_DELAY_MS}`))
      await ctx.fiber.dispose()
    }
  })
--- a/packages/subagent/subagent-acp/tsconfig.json
+++ b/packages/subagent/subagent-acp/tsconfig.json
@@ -29,6 +29,9 @@
    {
      "path": "../../subprocess/subprocess"
    },
+    {
+      "path": "../../util/timeout"
+    },
    {
      "path": "../../support/loader-smoke"
    },
--- a/packages/subagent/subagent-codex/package.json
+++ b/packages/subagent/subagent-codex/package.json
@@ -40,6 +40,7 @@
    "schemastery": "^3.18.0"
  },
  "devDependencies": {
+    "@cordisjs/plugin-loader": "^1.0.0-rc.5",
    "@deepseek-ai/dsh-agent": "workspace:^",
    "@deepseek-ai/dsh-invariants": "workspace:^",
    "@deepseek-ai/dsh-llm": "workspace:^",