From a49ef7581fbc15341e1c34bc7549d51981dda6c7 Mon Sep 17 00:00:00 2001 From: pku-xht Date: Wed, 5 Aug 2026 04:21:29 +0800 Subject: [PATCH] Fix Codex wire frames and bound grace timers --- ...code-and-codex-subagent-backends.i18n.yaml | 4 +- ...claude-code-and-codex-subagent-backends.md | 2 +- ...ude-code-and-codex-subagent-backends.zh.md | 2 +- docs/config-catalog.md | 2 +- .../core-data-structures/subprocess.i18n.yaml | 6 +- docs/core-data-structures/subprocess.md | 9 +-- docs/core-data-structures/subprocess.zh.md | 9 +-- docs/module-graph.md | 6 +- .../subagent/subagent-codex/README.i18n.yaml | 4 +- packages/subagent/subagent-codex/README.md | 2 +- packages/subagent/subagent-codex/README.zh.md | 2 +- packages/subagent/subagent-codex/package.json | 2 + packages/subagent/subagent-codex/src/index.ts | 6 ++ packages/subagent/subagent-codex/src/wire.ts | 31 +++------- .../tests/subagent-codex.spec.ts | 13 ++++ .../subagent/subagent-codex/tsconfig.json | 3 + .../subprocess/subprocess-local/package.json | 2 + .../subprocess/subprocess-local/src/spawn.ts | 60 ++++--------------- .../subprocess-local/tests/spawn.spec.ts | 45 +++----------- .../subprocess/subprocess-local/tsconfig.json | 3 + .../subprocess/subprocess/README.i18n.yaml | 4 +- packages/subprocess/subprocess/README.md | 2 +- packages/subprocess/subprocess/README.zh.md | 2 +- packages/subprocess/subprocess/src/types.ts | 9 +-- pnpm-lock.yaml | 6 ++ 25 files changed, 100 insertions(+), 136 deletions(-) diff --git a/.agents/notes/proposed/feature/2026-08-04-claude-code-and-codex-subagent-backends.i18n.yaml b/.agents/notes/proposed/feature/2026-08-04-claude-code-and-codex-subagent-backends.i18n.yaml index 9331bac1a3..0045a33382 100644 --- a/.agents/notes/proposed/feature/2026-08-04-claude-code-and-codex-subagent-backends.i18n.yaml +++ b/.agents/notes/proposed/feature/2026-08-04-claude-code-and-codex-subagent-backends.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 .agents/notes/proposed/feature/2026-08-04-claude-code-and-codex-subagent-backends.md -2026-08-04-claude-code-and-codex-subagent-backends.md: 1908eb3466fd6ae6cd14f74e70366d2c7b4c977f -2026-08-04-claude-code-and-codex-subagent-backends.zh.md: 6d173133a308be2613cc71b99c5dfd695d18f337 +2026-08-04-claude-code-and-codex-subagent-backends.md: 0afeae6269fcff588461dd58221c376a257c1f1b +2026-08-04-claude-code-and-codex-subagent-backends.zh.md: c754f5f436fd76b85bd15e45f9673d8bfe61cce6 diff --git a/.agents/notes/proposed/feature/2026-08-04-claude-code-and-codex-subagent-backends.md b/.agents/notes/proposed/feature/2026-08-04-claude-code-and-codex-subagent-backends.md index 1908eb3466..0afeae6269 100644 --- a/.agents/notes/proposed/feature/2026-08-04-claude-code-and-codex-subagent-backends.md +++ b/.agents/notes/proposed/feature/2026-08-04-claude-code-and-codex-subagent-backends.md @@ -35,7 +35,7 @@ fixed tool → shared subagent service → product provider → official product ## Codex provider -`@deepseek-ai/dsh-subagent-codex` registers the fixed `codex` provider and always starts `codex app-server --stdio` from `PATH`. Its public configuration contains only an explicit `env` overlay and a positive finite `disposeGraceMs`. Installation, login, `CODEX_HOME`, model selection, base URL, sandbox, approval policy, and product-session settings remain native Codex or deployment responsibilities. +`@deepseek-ai/dsh-subagent-codex` registers the fixed `codex` provider and always starts `codex app-server --stdio` from `PATH`. Its public configuration contains only an explicit `env` overlay and a positive finite `disposeGraceMs` no greater than the repository's shared `MAX_TIMER_DELAY_MS`. Installation, login, `CODEX_HOME`, model selection, base URL, sandbox, approval policy, and product-session settings remain native Codex or deployment responsibilities. Before publication, the provider validates a non-empty text-only task, starts the managed app-server in the parent workspace, completes `initialize` → `initialized`, and creates an `ephemeral: true` thread. The published run owns exactly one `turn/start`; its thread and turn ids remain private and are never persisted in the parent Session. diff --git a/.agents/notes/proposed/feature/2026-08-04-claude-code-and-codex-subagent-backends.zh.md b/.agents/notes/proposed/feature/2026-08-04-claude-code-and-codex-subagent-backends.zh.md index 6d173133a3..c754f5f436 100644 --- a/.agents/notes/proposed/feature/2026-08-04-claude-code-and-codex-subagent-backends.zh.md +++ b/.agents/notes/proposed/feature/2026-08-04-claude-code-and-codex-subagent-backends.zh.md @@ -35,7 +35,7 @@ fixed tool → shared subagent service → product provider → official product ## Codex 提供方 -`@deepseek-ai/dsh-subagent-codex` 注册固定的 `codex` 提供方，并始终启动 `codex app-server --stdio`，该命令从 `PATH` 解析。其公开配置仅包含显式的 `env` 覆盖项和须为正有限值的 `disposeGraceMs`。安装、登录、`CODEX_HOME`、模型选择、基础 URL、沙箱、审批策略和产品会话设置仍由 Codex 原生机制或部署环境负责。 +`@deepseek-ai/dsh-subagent-codex` 注册固定的 `codex` 提供方，并始终启动 `codex app-server --stdio`，该命令从 `PATH` 解析。其公开配置仅包含显式的 `env` 覆盖项和须为正有限值的 `disposeGraceMs`，且后者不得大于仓库共享的 `MAX_TIMER_DELAY_MS`。安装、登录、`CODEX_HOME`、模型选择、基础 URL、沙箱、审批策略和产品会话设置仍由 Codex 原生机制或部署环境负责。 发布前，提供方会验证非空的纯文本任务，在父级工作区中启动受管的 app-server，完成 `initialize` → `initialized` 握手，并创建一个 `ephemeral: true` 线程。已发布的运行只拥有一次 `turn/start`；其线程 ID 与轮次 ID 保持私有，绝不会持久化到父会话。 diff --git a/docs/config-catalog.md b/docs/config-catalog.md index 2c1dd2a33a..85114bca5a 100644 --- a/docs/config-catalog.md +++ b/docs/config-catalog.md @@ -1580,7 +1580,7 @@ export interface Config { } ``` -Source: [`packages/subagent/subagent-codex/src/index.ts:29`](../packages/subagent/subagent-codex/src/index.ts) +Source: [`packages/subagent/subagent-codex/src/index.ts:30`](../packages/subagent/subagent-codex/src/index.ts) ## `@deepseek-ai/dsh-subagent-dsh-sdk` diff --git a/docs/core-data-structures/subprocess.i18n.yaml b/docs/core-data-structures/subprocess.i18n.yaml index b85701557b..98b8f06a56 100644 --- a/docs/core-data-structures/subprocess.i18n.yaml +++ b/docs/core-data-structures/subprocess.i18n.yaml @@ -1,6 +1,6 @@ # Bilingual-pair consistency record (docs/i18n/README.md): the git blob hash of each # 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 -subprocess.md: 922e7ad0ee8b5c0dbcd0a6a4553c9d2a580f3ee2 -subprocess.zh.md: 5befdcdfc9b0e1d2a9adc825b177c90e53269def +# pnpm run verify-translation-pairing --write docs/core-data-structures/subprocess.md +subprocess.md: 9e1a73e0b807347f6c87ab5a589b7bb4b763df0e +subprocess.zh.md: 769d59e6610ffe2114984797b4b84afafc472262 diff --git a/docs/core-data-structures/subprocess.md b/docs/core-data-structures/subprocess.md index 922e7ad0ee..9e1a73e0b8 100644 --- a/docs/core-data-structures/subprocess.md +++ b/docs/core-data-structures/subprocess.md @@ -101,10 +101,11 @@ interface SubprocessSpawnSpec { /** Per-stream stdio dispositions. */ stdio: SubprocessStdio /** - * Grace period in milliseconds for the {@link SubprocessHandle.terminate} - * escalation and for draining still-open collected pipes after the process - * exits (an inherited descriptor held by a surviving descendant cannot hold - * the outcome open indefinitely). + * Positive finite grace period in milliseconds, no greater than + * `MAX_TIMER_DELAY_MS`, for the {@link SubprocessHandle.terminate} escalation + * and for draining still-open collected pipes after the process exits (an + * inherited descriptor held by a surviving descendant cannot hold the + * outcome open indefinitely). */ graceMs: number /** diff --git a/docs/core-data-structures/subprocess.zh.md b/docs/core-data-structures/subprocess.zh.md index 5befdcdfc9..769d59e661 100644 --- a/docs/core-data-structures/subprocess.zh.md +++ b/docs/core-data-structures/subprocess.zh.md @@ -101,10 +101,11 @@ interface SubprocessSpawnSpec { /** Per-stream stdio dispositions. */ stdio: SubprocessStdio /** - * Grace period in milliseconds for the {@link SubprocessHandle.terminate} - * escalation and for draining still-open collected pipes after the process - * exits (an inherited descriptor held by a surviving descendant cannot hold - * the outcome open indefinitely). + * Positive finite grace period in milliseconds, no greater than + * `MAX_TIMER_DELAY_MS`, for the {@link SubprocessHandle.terminate} escalation + * and for draining still-open collected pipes after the process exits (an + * inherited descriptor held by a surviving descendant cannot hold the + * outcome open indefinitely). */ graceMs: number /** diff --git a/docs/module-graph.md b/docs/module-graph.md index fc6cb07cb4..39693da6a7 100644 --- a/docs/module-graph.md +++ b/docs/module-graph.md @@ -343,6 +343,7 @@ flowchart TD pkg_storage_sqlite --> pkg_storage pkg_subprocess_local --> pkg_invariants pkg_subprocess_local --> pkg_subprocess + pkg_subprocess_local --> pkg_timeout pkg_typert_loader --> pkg_invariants pkg_typert_loader --> pkg_typert_registry pkg_llm_deepseek --> pkg_credentials @@ -1009,6 +1010,7 @@ flowchart TD pkg_subagent_codex --> pkg_session pkg_subagent_codex --> pkg_subagent pkg_subagent_codex --> pkg_subprocess + pkg_subagent_codex --> pkg_timeout pkg_subagent_fork --> pkg_agent pkg_subagent_fork --> pkg_invariants pkg_subagent_fork --> pkg_session @@ -1124,7 +1126,7 @@ flowchart TD | [`storage-domain`](../packages/storage/storage-domain) | `storage` | [`invariants`](../packages/support/invariants), [`storage`](../packages/storage/storage) | | [`storage-json`](../packages/storage/storage-json) | `storage` | [`invariants`](../packages/support/invariants), [`storage`](../packages/storage/storage) | | [`storage-sqlite`](../packages/storage/storage-sqlite) | `storage` | [`invariants`](../packages/support/invariants), [`storage`](../packages/storage/storage) | -| [`subprocess-local`](../packages/subprocess/subprocess-local) | `subprocess` | [`invariants`](../packages/support/invariants), [`subprocess`](../packages/subprocess/subprocess) | +| [`subprocess-local`](../packages/subprocess/subprocess-local) | `subprocess` | [`invariants`](../packages/support/invariants), [`subprocess`](../packages/subprocess/subprocess), [`timeout`](../packages/util/timeout) | | [`typert-loader`](../packages/typert/loader) | `typert` | [`invariants`](../packages/support/invariants), [`typert-registry`](../packages/typert/registry) | | [`llm-deepseek`](../packages/llm/llm-deepseek) | `llm` | [`credentials`](../packages/credentials/credentials), [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm), [`settings`](../packages/settings/settings), [`timeout`](../packages/util/timeout) | | [`llm-pi-ai`](../packages/llm/llm-pi-ai) | `llm` | [`credentials`](../packages/credentials/credentials), [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm), [`settings`](../packages/settings/settings), [`timeout`](../packages/util/timeout) | @@ -1254,7 +1256,7 @@ flowchart TD | [`sdk-protocol`](../packages/sdk/sdk-protocol) | `sdk` | [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm), [`session`](../packages/core/session), [`subagent`](../packages/subagent/subagent) | | [`tool-ralph`](../packages/workflow/tool-ralph) | `workflow` | [`agent`](../packages/core/agent), [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm), [`subagent`](../packages/subagent/subagent), [`system-prompt`](../packages/core/system-prompt), [`tools`](../packages/core/tools), [`workflow`](../packages/workflow/workflow) | | [`workflow-workerthread`](../packages/workflow/workflow-workerthread) | `workflow` | [`agent`](../packages/core/agent), [`brand`](../packages/util/brand), [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm), [`session`](../packages/core/session), [`subagent`](../packages/subagent/subagent), [`tools`](../packages/core/tools), [`workflow`](../packages/workflow/workflow) | -| [`subagent-codex`](../packages/subagent/subagent-codex) | `subagent` | [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm), [`sdk-protocol`](../packages/sdk/sdk-protocol), [`session`](../packages/core/session), [`subagent`](../packages/subagent/subagent), [`subprocess`](../packages/subprocess/subprocess) | +| [`subagent-codex`](../packages/subagent/subagent-codex) | `subagent` | [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm), [`sdk-protocol`](../packages/sdk/sdk-protocol), [`session`](../packages/core/session), [`subagent`](../packages/subagent/subagent), [`subprocess`](../packages/subprocess/subprocess), [`timeout`](../packages/util/timeout) | | [`subagent-fork`](../packages/subagent/subagent-fork) | `subagent` | [`agent`](../packages/core/agent), [`invariants`](../packages/support/invariants), [`session`](../packages/core/session), [`subagent`](../packages/subagent/subagent), [`subagent-inprocess`](../packages/subagent/subagent-inprocess) | | [`subagent-spawn`](../packages/subagent/subagent-spawn) | `subagent` | [`invariants`](../packages/support/invariants), [`subagent`](../packages/subagent/subagent), [`subagent-inprocess`](../packages/subagent/subagent-inprocess) | | [`jsonrpc`](../packages/ui/jsonrpc) | `ui` | [`agent`](../packages/core/agent), [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm), [`llm-deepseek`](../packages/llm/llm-deepseek), [`scope`](../packages/core/scope), [`sdk-protocol`](../packages/sdk/sdk-protocol), [`session`](../packages/core/session), [`subagent`](../packages/subagent/subagent) | diff --git a/packages/subagent/subagent-codex/README.i18n.yaml b/packages/subagent/subagent-codex/README.i18n.yaml index a40167b79a..97c2b9f705 100644 --- a/packages/subagent/subagent-codex/README.i18n.yaml +++ b/packages/subagent/subagent-codex/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-codex/README.md -README.md: 1dde57e10e27786ae06d395c7088976bf6f60ece -README.zh.md: cb56281d9018e7a400ceb770e31da8a60d10d54f +README.md: c25ee90edf8972da66448fe84cb659b0aec79e6f +README.zh.md: 10c8fcc47a9ab04bca983857bd44ede265c23435 diff --git a/packages/subagent/subagent-codex/README.md b/packages/subagent/subagent-codex/README.md index 1dde57e10e..c25ee90edf 100644 --- a/packages/subagent/subagent-codex/README.md +++ b/packages/subagent/subagent-codex/README.md @@ -23,7 +23,7 @@ The provider advertises no optional start-time capabilities and reports `inherit | Key | Default | Meaning | |---|---|---| | `env` | `{}` | Explicit child environment layered over the subprocess seam's credential-scrubbed parent environment. | -| `disposeGraceMs` | `3000` | Positive finite grace in milliseconds between the shared process-tree owner's termination tiers; disposal then waits for whole-tree exit. | +| `disposeGraceMs` | `3000` | Positive finite grace in milliseconds, no greater than [`MAX_TIMER_DELAY_MS`](../../util/timeout/README.md), between the shared process-tree owner's termination tiers; disposal then waits for whole-tree exit. | Production resolves `codex` from `PATH` and uses the host's native Codex configuration and authentication. The plugin does not install Codex, select a model, create `CODEX_HOME`, log in, or probe a version. Credential-shaped ambient variables are removed by the subprocess seam, so an API key intended for the child must be supplied explicitly in `env`; ordinary ambient values such as `PATH` and `HOME` remain available unless overridden. diff --git a/packages/subagent/subagent-codex/README.zh.md b/packages/subagent/subagent-codex/README.zh.md index cb56281d90..10c8fcc47a 100644 --- a/packages/subagent/subagent-codex/README.zh.md +++ b/packages/subagent/subagent-codex/README.zh.md @@ -23,7 +23,7 @@ | 配置键 | 默认值 | 含义 | |---|---|---| | `env` | `{}` | 显式指定的子进程环境，叠加在由子进程 seam 清除凭证后的父环境之上。 | -| `disposeGraceMs` | `3000` | 共享进程树责任方各终止层级之间的宽限期，单位为毫秒且须为正有限值；随后资源释放会等待整棵进程树退出。 | +| `disposeGraceMs` | `3000` | 共享进程树责任方各终止层级之间的宽限期，单位为毫秒且须为正有限值，并不得大于仓库共享的 [`MAX_TIMER_DELAY_MS`](../../util/timeout/README.md)；随后资源释放会等待整棵进程树退出。 | 生产环境会从 `PATH` 中解析 `codex`，并使用宿主机原生的 Codex 配置与身份验证。本插件不安装 Codex、不选择模型、不创建 `CODEX_HOME`、不执行登录，也不探测版本。子进程 seam 会移除具有凭证特征的环境变量，因此供子进程使用的 API 密钥必须在 `env` 中显式提供；除非被覆盖，`PATH` 和 `HOME` 等普通环境变量值仍然可用。 diff --git a/packages/subagent/subagent-codex/package.json b/packages/subagent/subagent-codex/package.json index 10bf7ee5f4..cc1d016225 100644 --- a/packages/subagent/subagent-codex/package.json +++ b/packages/subagent/subagent-codex/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": { @@ -48,6 +49,7 @@ "@deepseek-ai/dsh-subagent": "workspace:^", "@deepseek-ai/dsh-subprocess": "workspace:^", "@deepseek-ai/dsh-subprocess-local": "workspace:^", + "@deepseek-ai/dsh-timeout": "workspace:^", "@openai/codex": "0.146.0", "cordis": "^4.0.0-rc.7" } diff --git a/packages/subagent/subagent-codex/src/index.ts b/packages/subagent/subagent-codex/src/index.ts index 00fe95d817..09ece5e22e 100644 --- a/packages/subagent/subagent-codex/src/index.ts +++ b/packages/subagent/subagent-codex/src/index.ts @@ -8,6 +8,7 @@ import type { Context } from 'cordis' import z from 'schemastery' +import { MAX_TIMER_DELAY_MS } from '@deepseek-ai/dsh-timeout' import { assertPositiveFinite, NO_START_CAPABILITIES, @@ -85,5 +86,10 @@ export function apply(ctx: Context, config: Config): void { 'disposeGraceMs', resolved.disposeGraceMs, ) + if (resolved.disposeGraceMs > MAX_TIMER_DELAY_MS) { + throw new Error( + `subagent-codex: disposeGraceMs must be no greater than ${MAX_TIMER_DELAY_MS}`, + ) + } ctx.subagents.registerProvider(new CodexProvider(ctx, resolved)) } diff --git a/packages/subagent/subagent-codex/src/wire.ts b/packages/subagent/subagent-codex/src/wire.ts index f933c1a04b..460eeae877 100644 --- a/packages/subagent/subagent-codex/src/wire.ts +++ b/packages/subagent/subagent-codex/src/wire.ts @@ -14,22 +14,6 @@ import { JsonRpcLineTransport } from '@deepseek-ai/dsh-sdk-protocol' type JsonObject = Record -interface Deferred { - readonly promise: Promise - readonly resolve: (value: T) => void - readonly reject: (reason?: unknown) => void -} - -function deferred(): Deferred { - let resolve!: (value: T) => void - let reject!: (reason?: unknown) => void - const promise = new Promise((settle, fail) => { - resolve = settle - reject = fail - }) - return { promise, resolve, reject } -} - function object(value: unknown, label: string): JsonObject { if (value === null || typeof value !== 'object' || Array.isArray(value)) { throw new Error(`subagent-codex: app-server returned invalid ${label}`) @@ -98,11 +82,11 @@ async function raceAbort(pending: Promise, signal: AbortSignal): Promise() + private readonly fatal = Promise.withResolvers() private threadId: string | undefined private turnId: string | undefined private pendingTurnId: string | undefined - private turnCompleted: Deferred | undefined + private turnCompleted: PromiseWithResolvers | undefined private readonly earlyTurnNotifications: Array<{ readonly method: string readonly params: JsonObject @@ -193,7 +177,7 @@ export class CodexAppServerWire { signal: AbortSignal, cancelled: () => boolean, ): Promise { - const completion = deferred() + const completion = Promise.withResolvers() this.turnCompleted = completion const threadId = this.threadId as string const response = object(await this.guarded(this.transport.request('turn/start', { @@ -340,7 +324,8 @@ export class CodexAppServerWire { private handleNotification(method: string, params: JsonObject): void { if (method === 'turn/started') { - if (params.threadId !== this.threadId) return + const threadId = string(params.threadId, 'turn/started thread id') + if (threadId !== this.threadId) return const turn = object(params.turn, 'turn/started turn') if (this.turnCompleted !== undefined && this.turnId === undefined) { this.observePendingTurnId(string(turn.id, 'turn/started turn id')) @@ -348,7 +333,8 @@ export class CodexAppServerWire { return } if (method === 'item/completed') { - if (params.threadId !== this.threadId) return + const threadId = string(params.threadId, 'item/completed thread id') + if (threadId !== this.threadId) return const id = string(params.turnId, 'item/completed turn id') if (this.turnId === undefined) { if (this.turnCompleted !== undefined) { @@ -373,7 +359,8 @@ export class CodexAppServerWire { return } if (method !== 'turn/completed') return - if (params.threadId !== this.threadId) return + const threadId = string(params.threadId, 'turn/completed thread id') + if (threadId !== this.threadId) return const turn = object(params.turn, 'turn/completed turn') const id = string(turn.id, 'turn/completed turn id') const turnCompleted = this.turnCompleted diff --git a/packages/subagent/subagent-codex/tests/subagent-codex.spec.ts b/packages/subagent/subagent-codex/tests/subagent-codex.spec.ts index 181ddb919e..01cc28e59e 100644 --- a/packages/subagent/subagent-codex/tests/subagent-codex.spec.ts +++ b/packages/subagent/subagent-codex/tests/subagent-codex.spec.ts @@ -6,6 +6,7 @@ import type { Agent } from '@deepseek-ai/dsh-agent' import type { InvariantInstaller } from '@deepseek-ai/dsh-invariants' import type { ContentBlock } from '@deepseek-ai/dsh-llm' import SubagentService from '@deepseek-ai/dsh-subagent' +import { MAX_TIMER_DELAY_MS } from '@deepseek-ai/dsh-timeout' import type { SubprocessHandle, SubprocessOutcome, @@ -294,6 +295,8 @@ describe('task admission and package contracts', () => { await expect(ctx.plugin(codex, { disposeGraceMs })) .rejects.toThrow('disposeGraceMs must be a positive finite number') } + await expect(ctx.plugin(codex, { disposeGraceMs: MAX_TIMER_DELAY_MS + 1 })) + .rejects.toThrow(`disposeGraceMs must be no greater than ${MAX_TIMER_DELAY_MS}`) await ctx.fiber.dispose() }) @@ -513,6 +516,16 @@ describe('CodexAppServerWire', () => { } }) + it('fails closed when terminal notification params are not an object', async () => { + const { child, wire } = await initializeWire() + const result = wire.runTurn(['task'], new AbortController().signal, () => false) + const turnStart = await child.peer.nextMethod('turn/start') + child.peer.respond(turnStart, { turn: { id: 'turn-1' } }) + child.peer.send({ method: 'turn/completed', params: null }) + await expect(result).rejects.toThrow('invalid turn/completed thread id') + wire.close() + }) + it('keeps an unsupported request authoritative over an early terminal in the same chunk', async () => { const { child, wire } = await initializeWire() const result = wire.runTurn(['task'], new AbortController().signal, () => false) diff --git a/packages/subagent/subagent-codex/tsconfig.json b/packages/subagent/subagent-codex/tsconfig.json index 6034bf5fbe..b9f33967ba 100644 --- a/packages/subagent/subagent-codex/tsconfig.json +++ b/packages/subagent/subagent-codex/tsconfig.json @@ -35,6 +35,9 @@ { "path": "../../subprocess/subprocess" }, + { + "path": "../../util/timeout" + }, { "path": "../../support/invariants" } 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/spawn.ts b/packages/subprocess/subprocess-local/src/spawn.ts index 932daa2c59..fda32be42a 100644 --- a/packages/subprocess/subprocess-local/src/spawn.ts +++ b/packages/subprocess/subprocess-local/src/spawn.ts @@ -15,6 +15,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { setTimeout as sleepMs } from 'node:timers/promises' import { scrubbedParentEnv } from '@deepseek-ai/dsh-subprocess' +import { MAX_TIMER_DELAY_MS } from '@deepseek-ai/dsh-timeout' import type { CollectedOutput, SubprocessCollect, @@ -55,47 +56,6 @@ function sleepTick(): Promise { return sleepMs(15) } -/** Largest delay Node schedules without collapsing it to one millisecond. */ -const MAX_TIMER_DELAY_MS = 2_147_483_647n - -/** - * Schedule a positive finite millisecond delay across as many Node-safe timer - * segments as necessary. Fractional milliseconds round up so a grace never - * expires earlier than configured. - * @param delayMs - positive finite delay in milliseconds. - * @param callback - work to run after the complete delay. - * @returns a handle that cancels the active segment and all future segments. - */ -export function scheduleFiniteTimeout( - delayMs: number, - callback: () => void, -): { cancel(): void } { - let remaining = BigInt(Math.ceil(delayMs)) - let timer: ReturnType | undefined - const arm = (): void => { - const chunk = remaining > MAX_TIMER_DELAY_MS - ? MAX_TIMER_DELAY_MS - : remaining - remaining -= chunk - timer = setTimeout(() => { - timer = undefined - if (remaining === 0n) { - callback() - } else { - arm() - } - }, Number(chunk)) - } - arm() - return { - cancel(): void { - if (timer === undefined) return - clearTimeout(timer) - timer = undefined - }, - } -} - let spillCounter = 0 let defaultSpillDir: string | undefined @@ -339,8 +299,12 @@ function signalTree( * @param spec - fully resolved argv, cwd, stdio, grace, cancellation, environment. * @param internals - test-only spill-directory, platform, and taskkill overrides. * @returns live subprocess handle. + * @throws when `graceMs` cannot be represented by one Node timer. */ export function spawnSubprocess(spec: SubprocessSpawnSpec, internals: SpawnInternals = {}): SubprocessHandle { + if (!Number.isFinite(spec.graceMs) || spec.graceMs <= 0 || spec.graceMs > MAX_TIMER_DELAY_MS) { + throw new Error(`subprocess graceMs must be a positive finite number no greater than ${MAX_TIMER_DELAY_MS}`) + } const spillDir = internals.spillDir ?? privateSpillDir() const platform = internals.platform ?? process.platform const taskkill = internals.taskkill ?? taskkillProcessTree @@ -382,7 +346,7 @@ export function spawnSubprocess(spec: SubprocessSpawnSpec, internals: SpawnInter const stdoutCollector = collectStream(outMode, child.stdout, 'stdout') const stderrCollector = collectStream(errMode, child.stderr, 'stderr') - let graceTimer: ReturnType | undefined + let graceTimer: ReturnType | undefined let treeExitObserved = false let treeExitObservation: Promise | undefined let settled = false @@ -426,7 +390,7 @@ export function spawnSubprocess(spec: SubprocessSpawnSpec, internals: SpawnInter treeExitObservation ??= (async () => { while (treeAlive()) await sleepTick() treeExitObserved = true - graceTimer?.cancel() + if (graceTimer !== undefined) clearTimeout(graceTimer) graceTimer = undefined })() return treeExitObservation @@ -457,7 +421,7 @@ export function spawnSubprocess(spec: SubprocessSpawnSpec, internals: SpawnInter // kill() re-probes tree liveness before force-killing. It stays ref'd: // the pending SIGKILL is a commitment, and a parent exiting before it // fires would orphan a trapped survivor. Self-bounds at graceMs. - graceTimer = scheduleFiniteTimeout(spec.graceMs, () => { kill('SIGKILL') }) + graceTimer = setTimeout(() => { kill('SIGKILL') }, spec.graceMs) } // The caller owns timeout classification; this layer only reacts to abort. @@ -472,7 +436,7 @@ export function spawnSubprocess(spec: SubprocessSpawnSpec, internals: SpawnInter } const done = new Promise((resolve, reject) => { - let pipeDrainTimer: ReturnType | undefined + let pipeDrainTimer: ReturnType | undefined const settle = (exitCode: number | null, signal: NodeJS.Signals | null): void => { if (settled) return settled = true @@ -495,15 +459,15 @@ export function spawnSubprocess(spec: SubprocessSpawnSpec, internals: SpawnInter // A surviving descendant that inherited a pipe must not hold the // outcome open indefinitely: after exit, the same bounded grace that // governs kills also bounds the close wait. - pipeDrainTimer = scheduleFiniteTimeout(spec.graceMs, () => { + pipeDrainTimer = setTimeout(() => { settle(exitCode, signal) - }) + }, spec.graceMs) }) child.on('close', settle) function cleanup(): void { // graceTimer deliberately NOT cleared: the SIGKILL escalation must be // able to reach tree survivors after the direct child settles. - pipeDrainTimer?.cancel() + if (pipeDrainTimer !== undefined) clearTimeout(pipeDrainTimer) spec.signal?.removeEventListener('abort', onAbort) } }) diff --git a/packages/subprocess/subprocess-local/tests/spawn.spec.ts b/packages/subprocess/subprocess-local/tests/spawn.spec.ts index ad2fc0f30a..34157f8fc0 100644 --- a/packages/subprocess/subprocess-local/tests/spawn.spec.ts +++ b/packages/subprocess/subprocess-local/tests/spawn.spec.ts @@ -5,11 +5,11 @@ import { describe, expect, it, vi } from 'vitest' import { killGroup, OutputCollector, - scheduleFiniteTimeout, spawnSubprocess, taskkillProcessTree, } from '../src/spawn.ts' import type { SubprocessHandle, SubprocessOutputReader } from '@deepseek-ai/dsh-subprocess' +import { MAX_TIMER_DELAY_MS } from '@deepseek-ai/dsh-timeout' const { failNextClose, failNextUnlink } = vi.hoisted(() => ({ failNextClose: { value: false }, @@ -107,31 +107,15 @@ async function waitForPidFile(path: string, timeoutMs = 5_000): Promise throw new Error(`pid file ${path} was not written after ${timeoutMs}ms`) } -describe('scheduleFiniteTimeout', () => { - it('rounds fractions up, chains Node-safe segments, and cancels idempotently', async () => { - vi.useFakeTimers() - try { - const fired = vi.fn() - const chained = scheduleFiniteTimeout(2_147_483_647.25, fired) - await vi.advanceTimersByTimeAsync(2_147_483_647) - expect(fired).not.toHaveBeenCalled() - await vi.advanceTimersByTimeAsync(1) - expect(fired).toHaveBeenCalledOnce() - chained.cancel() - - const cancelled = vi.fn() - const timer = scheduleFiniteTimeout(0.25, cancelled) - timer.cancel() - timer.cancel() - await vi.advanceTimersByTimeAsync(1) - expect(cancelled).not.toHaveBeenCalled() - } finally { - vi.useRealTimers() - } - }) -}) - describe('spawnSubprocess', () => { + it.each([0, -1, Number.NaN, Number.POSITIVE_INFINITY, MAX_TIMER_DELAY_MS + 1])( + 'rejects an invalid grace before spawning: %s', + (graceMs) => { + expect(() => spawnSubprocess(spec('true', { graceMs }))) + .toThrow(`subprocess graceMs must be a positive finite number no greater than ${MAX_TIMER_DELAY_MS}`) + }, + ) + it('captures stdout on success', async () => { const result = await finish(spawnSubprocess(spec('echo hello'))) expect(result.exitCode).toBe(0) @@ -194,17 +178,6 @@ describe('spawnSubprocess', () => { expect(result.signal).toBe('SIGKILL') }) - it('cancels a larger-than-Node escalation timer once SIGTERM removes the tree', async () => { - const running = spawnSubprocess(spec('echo ready; sleep 60', { - graceMs: Number.MAX_VALUE, - })) - await waitForStdout(running, 'ready\n') - running.terminate() - const result = await running.done - expect(result.signal).toBe('SIGTERM') - await expect(running.waitForExit()).resolves.toBe(true) - }) - it('cancels escalation when the terminated group vanishes before collected pipes drain', async () => { const pidFile = join(spillDir, `escaped-pipe-holder-${Date.now()}.pid`) const graceMs = 160 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/packages/subprocess/subprocess/README.i18n.yaml b/packages/subprocess/subprocess/README.i18n.yaml index 64f64d65ed..dd8908dc7d 100644 --- a/packages/subprocess/subprocess/README.i18n.yaml +++ b/packages/subprocess/subprocess/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/subprocess/subprocess/README.md -README.md: c360437bf2b2b95734f55f6aec46b0cecffb9260 -README.zh.md: dac459a6ed1b92c2354bf0a2cc4e0c23e824154f +README.md: a8061e67a5346fce785f3c9fb27c8f885bf99921 +README.zh.md: 0e6a7e9192eb7028ea951681f5e013f8012a4121 diff --git a/packages/subprocess/subprocess/README.md b/packages/subprocess/subprocess/README.md index c360437bf2..a8061e67a5 100644 --- a/packages/subprocess/subprocess/README.md +++ b/packages/subprocess/subprocess/README.md @@ -7,7 +7,7 @@ The subprocess seam (`ctx.subprocess`). The abstract `SubprocessService` exposes ## Contract - `spawn(spec)` returns immediately with a live handle; `done` resolves at process close with exit facts (`SubprocessOutcome` carries no output and no cause classification) and rejects only for spawn-level failures. -- The spec is fully explicit — argv, cwd, per-stream stdio dispositions, grace — because deployment-varying defaults belong to the calling seam's config, not to a hidden subprocess-service default (the `dsh-bash` request/spec split is the owning template). `argv` is never shell-interpreted; a consumer that wants a shell passes `['bash', '-c', command]` itself. +- The spec is fully explicit — argv, cwd, per-stream stdio dispositions, grace — because deployment-varying defaults belong to the calling seam's config, not to a hidden subprocess-service default (the `dsh-bash` request/spec split is the owning template). Grace must be positive, finite, and no greater than [`MAX_TIMER_DELAY_MS`](../../util/timeout/README.md), so the implementation can represent it with one Node timer instead of accepting a value that Node collapses to one millisecond. `argv` is never shell-interpreted; a consumer that wants a shell passes `['bash', '-c', command]` itself. - Stdio is Node-shaped per stream: `'pipe'` hands the caller the raw stream for its own protocol framing (LSP JSON-RPC, ACP ndjson), `'inherit'` passes the parent descriptor through for diagnostics, and collect mode (`{ maxBytes, spill? }`) buffers a bounded tail with an optional full-stream spill file. Collect readers take whole-stream byte offsets and never consume, so independent readers cannot steal one another's deltas; a read whose offset slid out of the in-memory tail is `lossy` and points at the spill file when one exists. Collected output stays readable after settlement. - Termination is tree-scoped on every platform (POSIX detached groups with direct-child fallback; Windows `taskkill /T`): `terminate()` — the only termination verb — escalates SIGTERM→grace→SIGKILL (idempotent, driven by the spec's abort signal too, a no-op once the tree is gone), and `waitForExit(signal?)` observes whole-tree liveness so a consumer-owned teardown ladder holds each tier on real quiescence — the manager reacts but never classifies why (callers own deadlines, teardown ladders, and cause classification). - `scrubbedParentEnv()` / `SENSITIVE_ENV_PATTERN` are the one shared scrub definition: ambient credential-shaped and `DSH_*` names are dropped, and the spec's explicit `env` merges after the scrub with no namespace validation — a deliberately forwarded credential or a current `DSH_*` fact survives precisely because it is an explicit caller opt-in, while the stale ambient namesake never reaches the child. Spawners that cannot route through the service (node-pty backends, SDK-managed transports) import the scrub. diff --git a/packages/subprocess/subprocess/README.zh.md b/packages/subprocess/subprocess/README.zh.md index dac459a6ed..0e6a7e9192 100644 --- a/packages/subprocess/subprocess/README.zh.md +++ b/packages/subprocess/subprocess/README.zh.md @@ -7,7 +7,7 @@ ## 契约 - `spawn(spec)` 立即返回一个活动句柄；`done` 在进程关闭时以退出事实 resolve（`SubprocessOutcome` 不携带输出，也不携带原因分类），仅在 spawn 层面失败时 reject。 -- spec 完全显式（argv、cwd、按流划分的 stdio 处置方式（disposition）、宽限期），因为随部署变化的默认值属于调用方 seam 的配置，而不属于某个隐藏的子进程默认值（`dsh-bash` 的 request/spec 拆分是这条规则的所属模板）。`argv` 绝不经过 shell 解释；需要 shell 的消费方自行传入 `['bash', '-c', command]`。 +- spec 完全显式（argv、cwd、按流划分的 stdio 处置方式（disposition）、宽限期），因为随部署变化的默认值属于调用方 seam 的配置，而不属于某个隐藏的子进程默认值（`dsh-bash` 的 request/spec 拆分是这条规则的所属模板）。宽限期须为正有限值，且不得大于 [`MAX_TIMER_DELAY_MS`](../../util/timeout/README.md)，这样实现便可用一个 Node 定时器表示它，而不会接受会被 Node 折叠为 1 毫秒的值。`argv` 绝不经过 shell 解释；需要 shell 的消费方自行传入 `['bash', '-c', command]`。 - stdio 按流采用 Node 风格：`'pipe'` 把原始流交给调用方做自己的协议分帧（LSP 的 JSON-RPC、ACP（Agent Client Protocol）的 ndjson），`'inherit'` 直通父进程描述符以承载诊断输出，收集模式（collect）`{ maxBytes, spill? }` 则缓冲一段有界尾部，外加可选的完整流 spill 文件。收集模式的读取器接受全流字节偏移量且从不消费，因此独立的读取器不会抢走彼此的增量；偏移量滑出内存尾部窗口的读取标记为 `lossy`，并在 spill 文件存在时指向它。收集到的输出在结算后仍可读取。 - 终止在每个平台上都以进程树为范围（POSIX 用 detached 进程组并以直接子进程回退；Windows 用 `taskkill /T`）：`terminate()`（唯一的终止动词）执行 SIGTERM→宽限期→SIGKILL 升级（幂等，也由 spec 的 abort 信号驱动，进程树消亡后为空操作）；`waitForExit(signal?)` 观察整棵进程树的存活状态，使消费方自有的拆卸阶梯能在真正完全停稳后才进入下一层。管理器只响应中止，但绝不判定原因（deadline、拆卸阶梯与原因分类归调用方所有）。 - `scrubbedParentEnv()` / `SENSITIVE_ENV_PATTERN` 是唯一一份共享的环境清理定义：环境中形似凭据的名称与 `DSH_*` 名称都会被丢弃，spec 的显式 `env` 在清除之后合并且不做命名空间校验——有意转发的凭据或当前 `DSH_*` 事实之所以能保留下来，正因为它是调用方的显式选择，而陈旧的同名环境值永远到不了子进程。无法把 spawn 路由到该服务的进程启动方（node-pty 后端、由 SDK 管理的传输层）改为导入环境清理函数。 diff --git a/packages/subprocess/subprocess/src/types.ts b/packages/subprocess/subprocess/src/types.ts index fdfc44b3c2..3dbe401b75 100644 --- a/packages/subprocess/subprocess/src/types.ts +++ b/packages/subprocess/subprocess/src/types.ts @@ -80,10 +80,11 @@ export interface SubprocessSpawnSpec { /** Per-stream stdio dispositions. */ stdio: SubprocessStdio /** - * Grace period in milliseconds for the {@link SubprocessHandle.terminate} - * escalation and for draining still-open collected pipes after the process - * exits (an inherited descriptor held by a surviving descendant cannot hold - * the outcome open indefinitely). + * Positive finite grace period in milliseconds, no greater than + * `MAX_TIMER_DELAY_MS`, for the {@link SubprocessHandle.terminate} escalation + * and for draining still-open collected pipes after the process exits (an + * inherited descriptor held by a surviving descendant cannot hold the + * outcome open indefinitely). */ graceMs: number /** diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index c10c897164..a817239885 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -5119,6 +5119,9 @@ importers: '@deepseek-ai/dsh-subprocess-local': specifier: workspace:^ version: link:../../subprocess/subprocess-local + '@deepseek-ai/dsh-timeout': + specifier: workspace:^ + version: link:../../util/timeout '@openai/codex': specifier: 0.146.0 version: 0.146.0 @@ -5460,6 +5463,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: link:../../../vendor/cordis