From 32f829c4e6a3004268afd26dce8684e3555d83d7 Mon Sep 17 00:00:00 2001 From: pku-xht Date: Tue, 4 Aug 2026 22:18:35 +0800 Subject: [PATCH] fix(subagent): complete product provider lifecycle --- ...6-06-21-subagent-capability-seam.i18n.yaml | 4 +- .../2026-06-21-subagent-capability-seam.md | 8 +- .../2026-06-21-subagent-capability-seam.zh.md | 8 +- .../2026-06-22-acp-subagent-backend.i18n.yaml | 4 +- .../2026-06-22-acp-subagent-backend.md | 4 +- .../2026-06-22-acp-subagent-backend.zh.md | 4 +- docs/module-graph.md | 7 + .../subagent-claude-code/README.i18n.yaml | 4 +- .../subagent/subagent-claude-code/README.md | 2 +- .../subagent-claude-code/README.zh.md | 2 +- .../subagent/subagent-claude-code/src/run.ts | 69 ++---- .../tests/subagent-claude-code.spec.ts | 201 ++++++++++-------- .../subagent-claude-code/tsconfig.json | 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/src/run.ts | 29 +-- .../tests/subagent-codex.spec.ts | 79 ++----- .../subagent/subagent/src/out-of-process.ts | 70 ++---- scripts/run-gates.spec.ts | 6 + scripts/run-gates.ts | 2 + 21 files changed, 216 insertions(+), 301 deletions(-) diff --git a/.agents/notes/implemented/feature/2026-06-21-subagent-capability-seam.i18n.yaml b/.agents/notes/implemented/feature/2026-06-21-subagent-capability-seam.i18n.yaml index f8b82ebfbf..80508d16b4 100644 --- a/.agents/notes/implemented/feature/2026-06-21-subagent-capability-seam.i18n.yaml +++ b/.agents/notes/implemented/feature/2026-06-21-subagent-capability-seam.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/implemented/feature/2026-06-21-subagent-capability-seam.md -2026-06-21-subagent-capability-seam.md: 5b9b018df151f0d734b54cfdd4dacfd09058f7d7 -2026-06-21-subagent-capability-seam.zh.md: 49571288e35abb1369c16abd5c77a81dd3212a12 +2026-06-21-subagent-capability-seam.md: 35fe7b7aaf02d9d55012e3285b3f5a58bc76cde8 +2026-06-21-subagent-capability-seam.zh.md: 221335859cec104a55136201e4923d783d616e86 diff --git a/.agents/notes/implemented/feature/2026-06-21-subagent-capability-seam.md b/.agents/notes/implemented/feature/2026-06-21-subagent-capability-seam.md index 5b9b018df1..35fe7b7aaf 100644 --- a/.agents/notes/implemented/feature/2026-06-21-subagent-capability-seam.md +++ b/.agents/notes/implemented/feature/2026-06-21-subagent-capability-seam.md @@ -4,7 +4,7 @@ Status: implemented English | [中文](2026-06-21-subagent-capability-seam.zh.md) -> The full seam is shipped: the `dsh-subagent` interface and `dsh-tool-subagent` consumer; the two in-process backends (`dsh-subagent-spawn`, `dsh-subagent-fork`); the nested-agent snapshot infrastructure ([per-session snapshot replay](../testing/2026-06-22-subagent-snapshot-replay.md)); and the out-of-process `dsh-subagent-acp` backend ([its Agent Note](2026-06-22-acp-subagent-backend.md)). +> The full seam is shipped: the `dsh-subagent` interface and `dsh-tool-subagent` consumer; the two in-process backends (`dsh-subagent-spawn`, `dsh-subagent-fork`); the nested-agent snapshot infrastructure ([per-session snapshot replay](../testing/2026-06-22-subagent-snapshot-replay.md)); and the out-of-process ACP, Codex, and Claude Code backends ([ACP Agent Note](2026-06-22-acp-subagent-backend.md), [product-provider Agent Note](2026-08-04-claude-code-and-codex-subagent-backends.md)). ## Problem @@ -14,8 +14,8 @@ The distinctive requirement — the one that shapes the whole design — is that - **in-process** — a child concrete `Agent` on the same `Context` (the cheapest, and nearly free given the existing agent factory); - **ACP** — act as an ACP *client* driving another agent process (which can be another instance of ourselves); -- **Codex app-server** — a current one-shot sibling that applies the same named-provider seam to the official product process ([product-provider Agent Note](../../implemented/feature/2026-08-04-claude-code-and-codex-subagent-backends.md)); -- later: **A2A** and the **Claude Code Agent SDK** — the same out-of-process "start a child, prompt it, settle, cancel" shape; the Claude sibling remains in the product-provider proposal. +- **Codex app-server and Claude Code Agent SDK** — current one-shot siblings that apply the same named-provider seam to official product processes ([product-provider Agent Note](2026-08-04-claude-code-and-codex-subagent-backends.md)); +- later: **A2A** using the same out-of-process "start a child, prompt it, settle, cancel" shape. ## Alternatives considered @@ -35,6 +35,8 @@ A new package group `packages/subagent/`: | `@deepseek-ai/dsh-subagent-spawn` | implementation: a fresh in-process child via `ctx.agents.create` | | `@deepseek-ai/dsh-subagent-fork` | implementation: an in-process child seeded with a snapshot of the parent's log | | `@deepseek-ai/dsh-subagent-acp` | implementation: an ACP client driving a configured child process | +| `@deepseek-ai/dsh-subagent-codex` | implementation: a one-shot official Codex app-server process | +| `@deepseek-ai/dsh-subagent-claude-code` | implementation: a one-shot official Claude Code process through the Agent SDK | | `@deepseek-ai/dsh-tool-subagent` | consumer: the model-facing `subagent` tool over `ctx.subagents` | ### The primitive: async `start → SubagentRun` diff --git a/.agents/notes/implemented/feature/2026-06-21-subagent-capability-seam.zh.md b/.agents/notes/implemented/feature/2026-06-21-subagent-capability-seam.zh.md index 49571288e3..221335859c 100644 --- a/.agents/notes/implemented/feature/2026-06-21-subagent-capability-seam.zh.md +++ b/.agents/notes/implemented/feature/2026-06-21-subagent-capability-seam.zh.md @@ -4,7 +4,7 @@ Status: implemented [English](2026-06-21-subagent-capability-seam.md) | 中文 -> 完整 seam 已交付：`dsh-subagent` 接口与 `dsh-tool-subagent` 消费方；两个进程内后端（`dsh-subagent-spawn`、`dsh-subagent-fork`）；嵌套 agent 快照基础设施（[逐会话快照回放](../testing/2026-06-22-subagent-snapshot-replay.md)）；以及进程外后端 `dsh-subagent-acp`（[其 Agent Note](2026-06-22-acp-subagent-backend.md)）。 +> 完整 seam 已交付：`dsh-subagent` 接口与 `dsh-tool-subagent` 消费方；两个进程内后端（`dsh-subagent-spawn`、`dsh-subagent-fork`）；嵌套 agent 快照基础设施（[逐会话快照回放](../testing/2026-06-22-subagent-snapshot-replay.md)）；以及进程外的 ACP、Codex 与 Claude Code 后端（[ACP Agent Note](2026-06-22-acp-subagent-backend.md)、[产品提供方 Agent Note](2026-08-04-claude-code-and-codex-subagent-backends.md)）。 ## 问题 @@ -14,8 +14,8 @@ harness 有一个长期搁置的 seam 用于 **subagent**：一个 agent（智 - **进程内**：在同一个 `Context` 上创建一个具体的子 `Agent`（最廉价，且鉴于现有 agent 工厂几乎零成本）； - **ACP**：作为 ACP *客户端*驱动另一个 agent 进程（可以是自身的另一个实例）； -- **Codex app-server**：当前的一次性兄弟提供方，将同一个命名提供方 seam 应用于官方产品进程（[产品提供方 Agent Note](../../implemented/feature/2026-08-04-claude-code-and-codex-subagent-backends.md)）； -- 后续：**A2A** 与 **Claude Code Agent SDK**——两者采用同样的进程外形态：「启动子 agent、发送提示词、结算、取消」；Claude 兄弟提供方仍在产品提供方提案中。 +- **Codex app-server 与 Claude Code Agent SDK**：当前的一次性兄弟提供方，将同一个命名提供方 seam 应用于官方产品进程（[产品提供方 Agent Note](2026-08-04-claude-code-and-codex-subagent-backends.md)）； +- 后续：**A2A**，采用同样的进程外形态：「启动子 agent、发送提示词、结算、取消」。 ## 曾考虑的替代方案 @@ -35,6 +35,8 @@ bash seam（[能力 seam](../architecture/2026-06-13-capability-seams.md)）在 | `@deepseek-ai/dsh-subagent-spawn` | 实现：通过 `ctx.agents.create` 创建全新的进程内子 agent | | `@deepseek-ai/dsh-subagent-fork` | 实现：用父 agent 日志快照初始化的进程内子 agent | | `@deepseek-ai/dsh-subagent-acp` | 实现：作为 ACP 客户端驱动已配置的子进程 | +| `@deepseek-ai/dsh-subagent-codex` | 实现：一次性官方 Codex app-server 进程 | +| `@deepseek-ai/dsh-subagent-claude-code` | 实现：通过 Agent SDK 运行的一次性官方 Claude Code 进程 | | `@deepseek-ai/dsh-tool-subagent` | 消费方：基于 `ctx.subagents` 的面向模型的 `subagent` 工具 | ### 原语：异步 `start → SubagentRun` diff --git a/.agents/notes/implemented/feature/2026-06-22-acp-subagent-backend.i18n.yaml b/.agents/notes/implemented/feature/2026-06-22-acp-subagent-backend.i18n.yaml index 54ba7268df..58325b9b0e 100644 --- a/.agents/notes/implemented/feature/2026-06-22-acp-subagent-backend.i18n.yaml +++ b/.agents/notes/implemented/feature/2026-06-22-acp-subagent-backend.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/implemented/feature/2026-06-22-acp-subagent-backend.md -2026-06-22-acp-subagent-backend.md: d839ab6f75d8a518c9bc850894d1c3c5ffdbed92 -2026-06-22-acp-subagent-backend.zh.md: e9027e282bf351890643e0545b01fe00287375a6 +2026-06-22-acp-subagent-backend.md: c994ebfa69649bb9e79d3aa389a0e13c178131c7 +2026-06-22-acp-subagent-backend.zh.md: e3c651f752e93b9ba8298b7fa52f83c33ed71a2e diff --git a/.agents/notes/implemented/feature/2026-06-22-acp-subagent-backend.md b/.agents/notes/implemented/feature/2026-06-22-acp-subagent-backend.md index d839ab6f75..c994ebfa69 100644 --- a/.agents/notes/implemented/feature/2026-06-22-acp-subagent-backend.md +++ b/.agents/notes/implemented/feature/2026-06-22-acp-subagent-backend.md @@ -57,6 +57,6 @@ Persistent-process pooling (reuse a warm child across runs) is a performance opt Every run pays a fresh subprocess (spawn + `initialize` + `newSession`). The parent surfaces only the child's final answer: `session/update` thoughts and tool-call cards are consumed and dropped, and permission prompts never reach a human — the configured policy answers them. The child's environment is credential-scrubbed by default, so its own model key is supplied explicitly via `config.env`. -## Future providers +## Product-provider siblings -The [Codex app-server provider](../../implemented/feature/2026-08-04-claude-code-and-codex-subagent-backends.md) now applies the same out-of-process spawn/prompt/settle/cancel boundary as a sibling registered by name. A2A and the Claude Code Agent SDK remain future sibling transports; the ACP backend proves that the common seam supports the boundary without owning their private protocols. +The [Codex app-server and Claude Code Agent SDK providers](2026-08-04-claude-code-and-codex-subagent-backends.md) apply the same out-of-process spawn/prompt/settle/cancel boundary as siblings registered by name. A2A remains a future sibling transport; the ACP backend proves that the common seam supports this boundary without owning product-private protocols. diff --git a/.agents/notes/implemented/feature/2026-06-22-acp-subagent-backend.zh.md b/.agents/notes/implemented/feature/2026-06-22-acp-subagent-backend.zh.md index e9027e282b..e3c651f752 100644 --- a/.agents/notes/implemented/feature/2026-06-22-acp-subagent-backend.zh.md +++ b/.agents/notes/implemented/feature/2026-06-22-acp-subagent-backend.zh.md @@ -57,6 +57,6 @@ ACP `StopReason` → harness `SubagentStopReason`：`end_turn`→`completed`、` 每次运行都要付出一个全新子进程的代价（spawn + `initialize` + `newSession`）。父进程仅暴露子 agent 的最终回答：`session/update` 中的思考和工具调用卡片被消费后丢弃，权限提示从不到达人类——由配置的策略应答。子进程环境默认经过凭证清洗，因此其自身的模型密钥需通过 `config.env` 显式提供。 -## 后续提供方 +## 兄弟产品提供方 -[Codex app-server 提供方](../../implemented/feature/2026-08-04-claude-code-and-codex-subagent-backends.md)已将同样的进程外启动/提示词/结算/取消边界应用于按名称注册的兄弟提供方。A2A 与 Claude Code Agent SDK 仍是未来的兄弟传输方式；ACP 后端证明了通用 seam 能够支持该边界，而无需负责它们的私有协议。 +[Codex app-server 与 Claude Code Agent SDK 提供方](2026-08-04-claude-code-and-codex-subagent-backends.md)作为按名称注册的兄弟提供方，采用同样的进程外启动/提示词/结算/取消边界。A2A 仍是未来的兄弟传输方式；ACP 后端证明了通用 seam 能够支持这项边界，而无需负责产品私有协议。 diff --git a/docs/module-graph.md b/docs/module-graph.md index 35f832154b..341655c57f 100644 --- a/docs/module-graph.md +++ b/docs/module-graph.md @@ -65,6 +65,7 @@ flowchart TD subgraph group_subagent["packages/subagent"] pkg_subagent["subagent"] pkg_subagent_acp["subagent-acp"] + pkg_subagent_claude_code["subagent-claude-code"] pkg_subagent_codex["subagent-codex"] pkg_subagent_dsh_sdk["subagent-dsh-sdk"] pkg_subagent_fork["subagent-fork"] @@ -892,6 +893,11 @@ flowchart TD pkg_subagent_acp --> pkg_session pkg_subagent_acp --> pkg_subagent pkg_subagent_acp --> pkg_subprocess + pkg_subagent_claude_code --> pkg_invariants + pkg_subagent_claude_code --> pkg_llm + pkg_subagent_claude_code --> pkg_session + pkg_subagent_claude_code --> pkg_subagent + pkg_subagent_claude_code --> pkg_subprocess pkg_subagent_inprocess --> pkg_agent pkg_subagent_inprocess --> pkg_invariants pkg_subagent_inprocess --> pkg_llm @@ -1218,6 +1224,7 @@ flowchart TD | [`tool-tasks`](../packages/tasks/tool-tasks) | `tasks` | [`agent`](../packages/core/agent), [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm), [`retention`](../packages/util/retention), [`system-prompt`](../packages/core/system-prompt), [`tasks`](../packages/tasks/tasks), [`tools`](../packages/core/tools) | | [`tool-workflow`](../packages/workflow/tool-workflow) | `workflow` | [`agent`](../packages/core/agent), [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm), [`system-prompt`](../packages/core/system-prompt), [`tools`](../packages/core/tools), [`workflow`](../packages/workflow/workflow) | | [`subagent-acp`](../packages/subagent/subagent-acp) | `subagent` | [`agent`](../packages/core/agent), [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm), [`session`](../packages/core/session), [`subagent`](../packages/subagent/subagent), [`subprocess`](../packages/subprocess/subprocess) | +| [`subagent-claude-code`](../packages/subagent/subagent-claude-code) | `subagent` | [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm), [`session`](../packages/core/session), [`subagent`](../packages/subagent/subagent), [`subprocess`](../packages/subprocess/subprocess) | | [`subagent-inprocess`](../packages/subagent/subagent-inprocess) | `subagent` | [`agent`](../packages/core/agent), [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm), [`sandbox-policy`](../packages/sandbox/sandbox-policy), [`session`](../packages/core/session), [`subagent`](../packages/subagent/subagent), [`system-prompt`](../packages/core/system-prompt), [`tools`](../packages/core/tools), [`user-approval`](../packages/ui/user-approval) | | [`tool-subagent`](../packages/subagent/tool-subagent) | `subagent` | [`agent`](../packages/core/agent), [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm), [`subagent`](../packages/subagent/subagent), [`tasks`](../packages/tasks/tasks), [`tools`](../packages/core/tools) | | [`tool-subagent-control`](../packages/subagent/tool-subagent-control) | `subagent` | [`invariants`](../packages/support/invariants), [`llm`](../packages/llm/llm), [`session`](../packages/core/session), [`session-query`](../packages/session-query/session-query), [`subagent`](../packages/subagent/subagent), [`tools`](../packages/core/tools) | diff --git a/packages/subagent/subagent-claude-code/README.i18n.yaml b/packages/subagent/subagent-claude-code/README.i18n.yaml index ac60e83f76..63570333f1 100644 --- a/packages/subagent/subagent-claude-code/README.i18n.yaml +++ b/packages/subagent/subagent-claude-code/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-claude-code/README.md -README.md: facaae300eeb8907076182a129aa216863dec8ec -README.zh.md: 75627cb54032edde07ec1ba5e21a058768515e29 +README.md: e19b119e355953a388ec4fca6a2db511e7eb43da +README.zh.md: 6d3b8719329691681582abd91b82abd167e89f6d diff --git a/packages/subagent/subagent-claude-code/README.md b/packages/subagent/subagent-claude-code/README.md index facaae300e..e19b119e35 100644 --- a/packages/subagent/subagent-claude-code/README.md +++ b/packages/subagent/subagent-claude-code/README.md @@ -27,7 +27,7 @@ The provider advertises no optional start-time capabilities and reports `inherit | Key | Default | Meaning | |---|---|---| | `env` | `{}` | Explicit SDK/CLI environment layered over the shared credential-scrubbed parent environment. | -| `disposeGraceMs` | `3000` | Positive finite process-tree termination grace in milliseconds; the final exit proof is bounded at twice this value. | +| `disposeGraceMs` | `3000` | Positive finite grace in milliseconds between the shared process-tree owner's termination tiers; disposal then waits for whole-tree exit. | Production uses the Claude Code CLI supplied by `@anthropic-ai/claude-agent-sdk` and the host's native settings and authentication. The plugin does not install another CLI, select a model, create a product home, log in, or probe an account. Credential-shaped ambient variables are removed before the explicit `env` overlay is applied, so an API key or endpoint intended for the child must be supplied there; ordinary ambient values such as `PATH` and `HOME` remain available unless overridden. diff --git a/packages/subagent/subagent-claude-code/README.zh.md b/packages/subagent/subagent-claude-code/README.zh.md index 75627cb540..6d3b871932 100644 --- a/packages/subagent/subagent-claude-code/README.zh.md +++ b/packages/subagent/subagent-claude-code/README.zh.md @@ -27,7 +27,7 @@ SDK 接收由文本块原样拼接成的任务。提供方会完整迭代 SDK | 配置键 | 默认值 | 含义 | |---|---|---| | `env` | `{}` | 显式指定的 SDK/CLI 环境，叠加在由共享机制清除凭证后的父环境之上。 | -| `disposeGraceMs` | `3000` | 进程树终止宽限期，须为正有限值，单位为毫秒；最终退出确认的等待时间上限为该值的两倍。 | +| `disposeGraceMs` | `3000` | 共享进程树责任方各终止层级之间的宽限期，单位为毫秒且须为正有限值；随后资源释放会等待整棵进程树退出。 | 生产环境使用 `@anthropic-ai/claude-agent-sdk` 提供的 Claude Code CLI，以及宿主机原生设置与身份验证。本插件不安装另一份 CLI、不选择模型、不创建产品主目录、不执行登录，也不探测账户。具有凭证特征的环境变量会在显式 `env` 覆盖生效前被清除，因此供子进程使用的 API 密钥或端点必须在该配置中显式提供；除非被覆盖，`PATH` 和 `HOME` 等普通环境变量仍然可用。 diff --git a/packages/subagent/subagent-claude-code/src/run.ts b/packages/subagent/subagent-claude-code/src/run.ts index dbb2af3593..2ad305f155 100644 --- a/packages/subagent/subagent-claude-code/src/run.ts +++ b/packages/subagent/subagent-claude-code/src/run.ts @@ -18,9 +18,9 @@ import { import type { ContentBlock } from '@deepseek-ai/dsh-llm' import { SessionId } from '@deepseek-ai/dsh-session' import { - doubledGraceWindow, settleRunResult, subprocessRunHandle, + thrownError, type SubagentResult, type SubagentRun, type SubagentStartRequest, @@ -39,32 +39,20 @@ import { /** Default POSIX grace between subprocess termination tiers. */ export const DEFAULT_DISPOSE_GRACE_MS = 3_000 -type QueryFactory = (params: { - prompt: string - options: Options -}) => Query - /** Fully resolved inputs for one official Claude Agent SDK query. */ export interface ClaudeCodeRunSpec { /** Parent Session workspace supplied to the SDK and real CLI. */ readonly cwd: string /** Explicit deployment/test environment layered after shared scrubbing. */ readonly env: Record - /** Subprocess termination grace and final tree-exit bound. */ + /** Subprocess termination grace passed to the shared process-tree owner. */ readonly disposeGraceMs: number /** Shared subprocess service spawn operation. */ readonly spawn: (spec: SubprocessSpawnSpec) => SubprocessHandle - /** Official query entrypoint; replaced only by package-local unit tests. */ - readonly query?: QueryFactory /** Diagnostic sink for a post-publication error flattened into a result. */ readonly onError?: (error: Error, stopReason: SubagentStopReason) => void } -function thrown(value: unknown): Error { - /* v8 ignore next -- SDK and subprocess failures reject with Error. */ - return value instanceof Error ? value : new Error(String(value)) -} - /** * Validate and preserve the one-shot task before crossing the SDK boundary. * @param prompt - task content accepted from the shared subagent service. @@ -110,18 +98,15 @@ export function successfulResult(message: SDKResultMessage): string { * Consume the complete SDK stream and require one strict success plus normal * iterator completion. * @param query - published official SDK query. - * @param setOutput - captures the candidate result for error diagnostics. * @returns the completed shared result. */ export async function consumeClaudeQuery( query: AsyncIterable, - setOutput: (output: ContentBlock[]) => void, ): Promise { let answer: string | undefined for await (const message of query) { if (message.type !== 'result') continue answer = successfulResult(message) - setOutput([{ type: 'text', text: answer }]) } if (answer === undefined) { throw new Error('subagent-claude-code: Claude Code ended without a result') @@ -137,48 +122,30 @@ export async function consumeClaudeQuery( * the subprocess owner to prove it is gone. * @param query - official SDK query, when creation reached that point. * @param child - shared-service handle that owns the CLI process tree. - * @param graceMs - termination grace used to bound final exit observation. */ export async function disposeClaudeCodeChild( query: Pick | undefined, child: SubprocessHandle, - graceMs: number, ): Promise { const failures: Error[] = [] - let treeExited = child.pid <= 0 try { query?.close() } catch (error: unknown) { - failures.push(thrown(error)) + failures.push(thrownError(error)) } if (child.pid > 0) { child.terminate() - const exitWindow = doubledGraceWindow(graceMs) try { - treeExited = await child.waitForExit(exitWindow.signal) - if (!treeExited) { - failures.push(new Error( - 'subagent-claude-code: Claude Code process tree did not exit within its dispose window', - )) - } + await child.waitForExit() } catch (error: unknown) { - failures.push(thrown(error)) - } finally { - exitWindow.cancel() + failures.push(thrownError(error)) } } - if (treeExited) { - try { - await child.done - } catch (error: unknown) { - failures.push(thrown(error)) - } - } else { - // The bounded tree observation owns teardown completion. Keep a later - // direct-child spawn failure observed without turning that bound into an - // unbounded wait. - void child.done.catch(() => {}) + try { + await child.done + } catch (error: unknown) { + failures.push(thrownError(error)) } const firstFailure = failures[0] @@ -244,7 +211,7 @@ export async function startClaudeCodeRun( let child: SubprocessHandle | undefined let query: Query | undefined try { - query = (spec.query ?? officialQuery)({ + query = officialQuery({ prompt, options: claudeQueryOptions(spec, controller, (captured) => { child = captured @@ -264,10 +231,10 @@ export async function startClaudeCodeRun( requestCancel() if (child !== undefined) { try { - await disposeClaudeCodeChild(query, child, spec.disposeGraceMs) + await disposeClaudeCodeChild(query, child) } catch (disposeError: unknown) { throw new AggregateError( - [thrown(error), thrown(disposeError)], + [thrownError(error), thrownError(disposeError)], 'subagent-claude-code: startup failed and CLI cleanup also failed', ) } @@ -276,7 +243,7 @@ export async function startClaudeCodeRun( query.close() } catch (disposeError: unknown) { throw new AggregateError( - [thrown(error), thrown(disposeError)], + [thrownError(error), thrownError(disposeError)], 'subagent-claude-code: startup failed and query cleanup also failed', ) } @@ -285,17 +252,14 @@ export async function startClaudeCodeRun( if (cancelledBeforeCleanup || request.signal.aborted) { throw new Error('subagent-claude-code: request was aborted before SDK startup') } - throw thrown(error) + throw thrownError(error) } - let output: ContentBlock[] = [] const publishedQuery = query const publishedChild = child const result = settleRunResult({ - attempt: () => consumeClaudeQuery(publishedQuery, (value) => { - output = value - }), - collectOutput: () => output, + attempt: () => consumeClaudeQuery(publishedQuery), + collectOutput: () => [], cancelled: () => controller.signal.aborted, onError: spec.onError, signal: request.signal, @@ -311,7 +275,6 @@ export async function startClaudeCodeRun( teardown: () => disposeClaudeCodeChild( publishedQuery, publishedChild, - spec.disposeGraceMs, ), }) } diff --git a/packages/subagent/subagent-claude-code/tests/subagent-claude-code.spec.ts b/packages/subagent/subagent-claude-code/tests/subagent-claude-code.spec.ts index 1f02914163..8c8158fb28 100644 --- a/packages/subagent/subagent-claude-code/tests/subagent-claude-code.spec.ts +++ b/packages/subagent/subagent-claude-code/tests/subagent-claude-code.spec.ts @@ -1,5 +1,6 @@ import { PassThrough } from 'node:stream' import type { + Options, Query, SDKMessage, SDKResultMessage, @@ -7,7 +8,15 @@ import type { } from '@anthropic-ai/claude-agent-sdk' import { Context } from 'cordis' import Loader from '@cordisjs/plugin-loader' -import { afterEach, describe, expect, it, type Mock, vi } from 'vitest' +import { + afterEach, + beforeEach, + describe, + expect, + it, + type Mock, + vi, +} from 'vitest' import type { Agent } from '@deepseek-ai/dsh-agent' import type { InvariantInstaller } from '@deepseek-ai/dsh-invariants' import type { ContentBlock } from '@deepseek-ai/dsh-llm' @@ -35,6 +44,18 @@ import { type ClaudeCodeRunSpec, } from '../src/run.ts' +type QueryFactory = (params: { + prompt: string + options: Options +}) => Query + +const queryMock = vi.hoisted(() => vi.fn()) + +vi.mock('@anthropic-ai/claude-agent-sdk', async importOriginal => ({ + ...await importOriginal(), + query: queryMock, +})) + const fakeParent = { id: 'parent', session: { header: { cwd: process.cwd() } }, @@ -56,7 +77,6 @@ interface FakeChildOptions { readonly stdin?: PassThrough | undefined readonly stdout?: PassThrough | undefined readonly exitOnTerminate?: boolean - readonly waitForExitResult?: boolean readonly waitForExitError?: Error readonly doneError?: Error } @@ -103,9 +123,6 @@ function fakeChild(options: FakeChildOptions = {}): FakeChild { if (options.waitForExitError !== undefined) { throw options.waitForExitError } - if (options.waitForExitResult !== undefined) { - return options.waitForExitResult - } if (exited) return true if (signal === undefined) { await done.catch(() => {}) @@ -218,7 +235,7 @@ interface FakeRun { readonly query: Query readonly close: ReturnType readonly spawnSpecs: SubprocessSpawnSpec[] - readonly options: Array>[0]['options']> + readonly options: Options[] readonly spec: ClaudeCodeRunSpec } @@ -239,16 +256,28 @@ function fakeRun( spawnSpecs.push(spawnSpec) return child.handle }, - query: (params) => { - options.push(params.options) - params.options.spawnClaudeCodeProcess!(sdkSpawnOptions()) - return query - }, } + queryMock.mockImplementation((params) => { + options.push(params.options) + params.options.spawnClaudeCodeProcess!(sdkSpawnOptions()) + return query + }) return { child, query, close, spawnSpecs, options, spec } } +beforeEach(() => { + queryMock.mockImplementation(({ options }) => { + options.spawnClaudeCodeProcess!(sdkSpawnOptions({ + cwd: options.cwd!, + env: options.env!, + signal: options.abortController!.signal, + })) + return queryFrom([]) + }) +}) + afterEach(() => { + queryMock.mockReset() vi.restoreAllMocks() vi.unstubAllEnvs() }) @@ -523,25 +552,17 @@ describe('query options and result mapping', () => { }) it('consumes the complete stream and keeps the latest strict success', async () => { - const outputs: ContentBlock[][] = [] const query = queryFrom([ { type: 'system', subtype: 'init' } as SDKMessage, success('first'), success('last'), ]) - await expect(consumeClaudeQuery(query, (output) => { - outputs.push(output) - })).resolves.toEqual({ + await expect(consumeClaudeQuery(query)).resolves.toEqual({ output: [{ type: 'text', text: 'last' }], stopReason: 'completed', }) - expect(outputs).toEqual([ - [{ type: 'text', text: 'first' }], - [{ type: 'text', text: 'last' }], - ]) await expect(consumeClaudeQuery( queryFrom([{ type: 'system', subtype: 'init' } as SDKMessage]), - () => {}, )).rejects.toThrow('ended without a result') }) }) @@ -596,14 +617,14 @@ describe('run publication, cancellation, and settlement', () => { } }) - it('preserves candidate output when iteration fails after a result', async () => { + it('fails closed when iteration rejects after a result', async () => { const fixture = fakeRun( [success('partial final')], new Error('iterator boom'), ) const run = await startClaudeCodeRun(request(), fixture.spec) await expect(run.result).resolves.toEqual({ - output: [{ type: 'text', text: 'partial final' }], + output: [], stopReason: 'error', }) await run.dispose() @@ -635,14 +656,14 @@ describe('run publication, cancellation, and settlement', () => { env: {}, disposeGraceMs: 5, spawn: () => children[index++]!.handle, - query: ({ prompt, options }) => { - controllers.push(options.abortController!) - options.spawnClaudeCodeProcess!(sdkSpawnOptions()) - return prompt === 'wait' - ? waitingQuery(options.abortController!.signal) - : queryFrom([success('second answer')]) - }, } + queryMock.mockImplementation(({ prompt, options }) => { + controllers.push(options.abortController!) + options.spawnClaudeCodeProcess!(sdkSpawnOptions()) + return prompt === 'wait' + ? waitingQuery(options.abortController!.signal) + : queryFrom([success('second answer')]) + }) const firstAbort = new AbortController() const first = await startClaudeCodeRun( request([{ type: 'text', text: 'wait' }], firstAbort.signal), @@ -667,6 +688,33 @@ describe('run publication, cancellation, and settlement', () => { await Promise.all([first.dispose(), second.dispose()]) }) + it('keeps local cancellation authoritative when the SDK iterator ends normally', async () => { + const parentAbort = new AbortController() + const child = fakeChild() + async function* stream(): AsyncGenerator { + yield success('candidate answer') + parentAbort.abort(new Error('parent cancelled at iterator completion')) + } + queryMock.mockImplementation(({ options }) => { + options.spawnClaudeCodeProcess!(sdkSpawnOptions()) + return Object.assign(stream(), { close: vi.fn() }) as unknown as Query + }) + const run = await startClaudeCodeRun( + request(undefined, parentAbort.signal), + { + cwd: '/workspace', + env: {}, + disposeGraceMs: 5, + spawn: () => child.handle, + }, + ) + await expect(run.result).resolves.toEqual({ + output: [], + stopReason: 'aborted', + }) + await run.dispose() + }) + it('rejects pre-abort and every incomplete startup transaction', async () => { const preAborted = new AbortController() preAborted.abort() @@ -678,32 +726,36 @@ describe('run publication, cancellation, and settlement', () => { expect(unused.options).toEqual([]) const noChildClose = vi.fn() + queryMock.mockImplementationOnce( + () => queryFrom([], undefined, noChildClose), + ) await expect(startClaudeCodeRun(request(), { ...unused.spec, - query: () => queryFrom([], undefined, noChildClose), })).rejects.toThrow('did not publish a controllable') expect(noChildClose).toHaveBeenCalledOnce() const closeFailure = vi.fn(() => { throw new Error('close boom') }) + queryMock.mockImplementationOnce( + () => queryFrom([], undefined, closeFailure), + ) const noChild = startClaudeCodeRun(request(), { ...unused.spec, - query: () => queryFrom([], undefined, closeFailure), }) await expect(noChild).rejects.toBeInstanceOf(AggregateError) const startupAbort = new AbortController() const abortedChild = fakeChild() const abortedClose = vi.fn() + queryMock.mockImplementationOnce(({ options }) => { + options.spawnClaudeCodeProcess!(sdkSpawnOptions()) + startupAbort.abort(new Error('startup cancelled')) + return queryFrom([], undefined, abortedClose) + }) const abortedDuringStartup = startClaudeCodeRun( request(undefined, startupAbort.signal), { ...unused.spec, spawn: () => abortedChild.handle, - query: ({ options }) => { - options.spawnClaudeCodeProcess!(sdkSpawnOptions()) - startupAbort.abort(new Error('startup cancelled')) - return queryFrom([], undefined, abortedClose) - }, }, ) await expect(abortedDuringStartup) @@ -711,27 +763,27 @@ describe('run publication, cancellation, and settlement', () => { expect(abortedClose).toHaveBeenCalledOnce() expect(abortedChild.terminate).toHaveBeenCalledOnce() + queryMock.mockImplementationOnce(() => { + throw new Error('query failed before resource creation') + }) await expect(startClaudeCodeRun(request(), { ...unused.spec, - query: () => { - throw new Error('query failed before resource creation') - }, })).rejects.toThrow('query failed before resource creation') const spawned = fakeChild() const spawnSpecs: SubprocessSpawnSpec[] = [] let factoryController: AbortController | undefined + queryMock.mockImplementationOnce(({ options }) => { + factoryController = options.abortController + options.spawnClaudeCodeProcess!(sdkSpawnOptions()) + throw new Error('query construction failed') + }) const factoryFailure = startClaudeCodeRun(request(), { ...unused.spec, spawn: (spawnSpec) => { spawnSpecs.push(spawnSpec) return spawned.handle }, - query: ({ options }) => { - factoryController = options.abortController - options.spawnClaudeCodeProcess!(sdkSpawnOptions()) - throw new Error('query construction failed') - }, }) await expect(factoryFailure).rejects.toThrow('query construction failed') expect(spawnSpecs).toHaveLength(1) @@ -749,76 +801,45 @@ describe('run publication, cancellation, and settlement', () => { }) }) -describe('bounded query and process disposal', () => { +describe('query and process disposal', () => { it('closes the query, terminates the tree, and waits for direct-child outcome', async () => { const child = fakeChild() const close = vi.fn() - await disposeClaudeCodeChild({ close }, child.handle, 5) + await disposeClaudeCodeChild({ close }, child.handle) expect(close).toHaveBeenCalledOnce() expect(child.terminate).toHaveBeenCalledOnce() expect(child.waitForExit).toHaveBeenCalledOnce() + expect(child.waitForExit).toHaveBeenCalledWith() await expect(child.handle.done).resolves.toEqual({ exitCode: 0, signal: null, }) }) - it('accepts fractional and larger-than-Node grace windows', async () => { - for (const graceMs of [0.25, Number.MAX_VALUE]) { - const child = fakeChild() - await expect(disposeClaudeCodeChild( - { close: vi.fn() }, - child.handle, - graceMs, - )).resolves.toBeUndefined() - const signal = child.waitForExit.mock.calls[0]?.[0] - expect(signal?.aborted).toBe(false) - } - }) - - it('chains a doubled grace window beyond one Node timer segment', async () => { - vi.useFakeTimers() - try { - const child = fakeChild({ exitOnTerminate: false }) - const disposal = disposeClaudeCodeChild( - { close: vi.fn() }, - child.handle, - 1_073_741_823.75, - ) - const rejected = expect(disposal) - .rejects.toThrow('did not exit within its dispose window') - await vi.advanceTimersByTimeAsync(2_147_483_647) - await vi.advanceTimersByTimeAsync(1) - await rejected - } finally { - vi.useRealTimers() - } - }) - - it('does not turn a missed tree-exit bound into an unbounded done wait', async () => { - const child = fakeChild({ - exitOnTerminate: false, - waitForExitResult: false, - }) - await expect(disposeClaudeCodeChild( + it('does not finish disposal before the managed tree exits', async () => { + const child = fakeChild({ exitOnTerminate: false }) + let disposed = false + const disposal = disposeClaudeCodeChild( { close: vi.fn() }, child.handle, - 5, - )).rejects.toThrow('did not exit within its dispose window') - child.fail(new Error('late direct-child failure')) + ).then(() => { + disposed = true + }) await nextTask() + expect(disposed).toBe(false) + child.settle() + await disposal + expect(disposed).toBe(true) }) it('reports wait, close, and direct-child failures without skipping cleanup', async () => { const waitFailure = fakeChild({ - exitOnTerminate: false, waitForExitError: new Error('wait boom'), }) const closeFailure = vi.fn(() => { throw new Error('close boom') }) await expect(disposeClaudeCodeChild( { close: closeFailure }, waitFailure.handle, - 5, )).rejects.toBeInstanceOf(AggregateError) expect(waitFailure.terminate).toHaveBeenCalledOnce() @@ -829,7 +850,6 @@ describe('bounded query and process disposal', () => { await expect(disposeClaudeCodeChild( { close: vi.fn() }, doneFailure.handle, - 5, )).rejects.toThrow('spawn boom') const both = fakeChild({ @@ -839,7 +859,6 @@ describe('bounded query and process disposal', () => { await expect(disposeClaudeCodeChild( { close: () => { throw new Error('close boom') } }, both.handle, - 5, )).rejects.toBeInstanceOf(AggregateError) }) }) diff --git a/packages/subagent/subagent-claude-code/tsconfig.json b/packages/subagent/subagent-claude-code/tsconfig.json index 72e5f73fec..61e81d7fcc 100644 --- a/packages/subagent/subagent-claude-code/tsconfig.json +++ b/packages/subagent/subagent-claude-code/tsconfig.json @@ -9,6 +9,12 @@ "src/**/*.ts" ], "references": [ + { + "path": "../../../vendor/cordis" + }, + { + "path": "../../../vendor/schemastery" + }, { "path": "../../llm/llm" }, diff --git a/packages/subagent/subagent-codex/README.i18n.yaml b/packages/subagent/subagent-codex/README.i18n.yaml index 3e8e805c88..c3d4da77bf 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: ce1c66427b562c08af06320f012f28b9e125ac45 -README.zh.md: bef47586db77c70bec741629d8579ba0e2efba1e +README.md: d7293a0ef37e4ec0f0cf983c254f9e22f830fcd8 +README.zh.md: 110953312162e146f01ef037a40d2f70b136850c diff --git a/packages/subagent/subagent-codex/README.md b/packages/subagent/subagent-codex/README.md index ce1c66427b..d7293a0ef3 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 process-tree termination grace in milliseconds; the final exit proof is bounded at twice this value. | +| `disposeGraceMs` | `3000` | Positive finite grace in milliseconds 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 bef47586db..1109533121 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` | 共享进程树责任方各终止层级之间的宽限期，单位为毫秒且须为正有限值；随后资源释放会等待整棵进程树退出。 | 生产环境会从 `PATH` 中解析 `codex`，并使用宿主机原生的 Codex 配置与身份验证。本插件不安装 Codex、不选择模型、不创建 `CODEX_HOME`、不执行登录，也不探测版本。子进程 seam 会移除具有凭证特征的环境变量，因此供子进程使用的 API 密钥必须在 `env` 中显式提供；除非被覆盖，`PATH` 和 `HOME` 等普通环境变量值仍然可用。 diff --git a/packages/subagent/subagent-codex/src/run.ts b/packages/subagent/subagent-codex/src/run.ts index b07bf9dc6b..27d01f6ac6 100644 --- a/packages/subagent/subagent-codex/src/run.ts +++ b/packages/subagent/subagent-codex/src/run.ts @@ -11,9 +11,9 @@ import { randomUUID } from 'node:crypto' import type { ContentBlock } from '@deepseek-ai/dsh-llm' import { SessionId } from '@deepseek-ai/dsh-session' import { - doubledGraceWindow, settleRunResult, subprocessRunHandle, + thrownError, type SubagentResult, type SubagentRun, type SubagentStartRequest, @@ -31,7 +31,7 @@ export interface CodexRunSpec { readonly cwd: string /** Explicit deployment/test environment layered after the shared scrub. */ readonly env: Record - /** Subprocess termination grace and final tree-exit bound. */ + /** Subprocess termination grace passed to the shared process-tree owner. */ readonly disposeGraceMs: number /** Shared subprocess service spawn operation. */ readonly spawn: (spec: SubprocessSpawnSpec) => SubprocessHandle @@ -39,11 +39,6 @@ export interface CodexRunSpec { readonly onError?: (error: Error, stopReason: SubagentStopReason) => void } -function thrown(value: unknown): Error { - /* v8 ignore next -- typed subprocess/wire failures reject with Error. */ - return value instanceof Error ? value : new Error(String(value)) -} - /** * Validate and preserve the one-shot task before crossing the process seam. * @param prompt - task content accepted from the shared subagent service. @@ -71,12 +66,10 @@ export function textTask(prompt: readonly ContentBlock[]): string[] { * subprocess owner to prove it is gone. * @param wire - private app-server protocol connection. * @param child - shared-service handle that owns the process tree. - * @param graceMs - termination grace used to bound final exit observation. */ export async function disposeCodexChild( wire: CodexAppServerWire, child: SubprocessHandle, - graceMs: number, ): Promise { wire.close() if (child.pid <= 0) { @@ -89,14 +82,7 @@ export async function disposeCodexChild( // A concurrently closed stdin does not change tree ownership below. } child.terminate() - const exitWindow = doubledGraceWindow(graceMs) - try { - if (!(await child.waitForExit(exitWindow.signal))) { - throw new Error('subagent-codex: app-server process tree did not exit within its dispose window') - } - } finally { - exitWindow.cancel() - } + await child.waitForExit() await child.done } @@ -127,15 +113,14 @@ export async function startCodexRun( child.stdout as NonNullable, child.stdin as NonNullable, ) - const disposeProcess = (): Promise => - disposeCodexChild(wire, child, spec.disposeGraceMs) + const disposeProcess = (): Promise => disposeCodexChild(wire, child) const processFailure: Promise = child.done.then( outcome => Promise.reject(new Error( 'subagent-codex: app-server exited before the run settled ' + `(code ${String(outcome.exitCode)}, signal ${String(outcome.signal)})`, )), - (error: unknown) => Promise.reject(thrown(error)), + (error: unknown) => Promise.reject(thrownError(error)), ) // A normal post-result dispose also closes the process. Keep that expected // late rejection observed after the result race has already settled. @@ -160,14 +145,14 @@ export async function startCodexRun( await disposeProcess() } catch (disposeError: unknown) { throw new AggregateError( - [thrown(error), thrown(disposeError)], + [thrownError(error), thrownError(disposeError)], 'subagent-codex: startup failed and app-server cleanup also failed', ) } if (runAbort.signal.aborted) { throw new Error('subagent-codex: request was aborted before app-server startup') } - throw thrown(error) + throw thrownError(error) } const collectOutput = (): ContentBlock[] => wire.collectOutput() diff --git a/packages/subagent/subagent-codex/tests/subagent-codex.spec.ts b/packages/subagent/subagent-codex/tests/subagent-codex.spec.ts index 8e6c7ebd51..7d550d42cb 100644 --- a/packages/subagent/subagent-codex/tests/subagent-codex.spec.ts +++ b/packages/subagent/subagent-codex/tests/subagent-codex.spec.ts @@ -91,7 +91,6 @@ class ProtocolPeer { interface FakeChildOptions { readonly pid?: number readonly exitOnTerminate?: boolean - readonly waitForExitResult?: boolean readonly doneError?: Error } @@ -134,9 +133,6 @@ function fakeChild(options: FakeChildOptions = {}): FakeChild { if (options.exitOnTerminate !== false) settle() }) const waitForExit = vi.fn(async (signal?: AbortSignal) => { - if (options.waitForExitResult !== undefined) { - return options.waitForExitResult - } if (exited) return true if (signal === undefined) { await done.catch(() => {}) @@ -964,19 +960,6 @@ describe('run lifecycle and quiescence', () => { expect(child.terminate).toHaveBeenCalledTimes(1) }) - it('reports both startup and rollback failures', async () => { - const child = fakeChild({ waitForExitResult: false, exitOnTerminate: false }) - const starting = startCodexRun( - request(), - runSpec(child, { disposeGraceMs: 1 }), - ) - const initialize = await child.peer.nextMethod('initialize') - child.peer.respond(initialize, { userAgent: '' }) - await expect(starting).rejects.toThrow( - 'startup failed and app-server cleanup also failed', - ) - }) - it('keeps overlapping runs isolated', async () => { const first = fakeChild() const second = fakeChild() @@ -1047,41 +1030,25 @@ describe('disposeCodexChild', () => { const child = fakeChild() const wire = new CodexAppServerWire(child.handle.stdout!, child.handle.stdin!) const end = vi.spyOn(child.toChild, 'end') - await disposeCodexChild(wire, child.handle, 100) + await disposeCodexChild(wire, child.handle) expect(end).toHaveBeenCalled() expect(child.terminate).toHaveBeenCalledTimes(1) expect(child.waitForExit).toHaveBeenCalledTimes(1) + expect(child.waitForExit).toHaveBeenCalledWith() }) - it('accepts fractional and larger-than-Node grace windows', async () => { - for (const graceMs of [0.25, Number.MAX_VALUE]) { - const child = fakeChild() - const wire = new CodexAppServerWire(child.handle.stdout!, child.handle.stdin!) - await expect(disposeCodexChild(wire, child.handle, graceMs)) - .resolves.toBeUndefined() - const signal = vi.mocked(child.waitForExit).mock.calls[0]?.[0] - expect(signal?.aborted).toBe(false) - } - }) - - it('chains a doubled grace window beyond one Node timer segment', async () => { - vi.useFakeTimers() - try { - const child = fakeChild({ exitOnTerminate: false }) - const wire = new CodexAppServerWire(child.handle.stdout!, child.handle.stdin!) - const disposal = disposeCodexChild( - wire, - child.handle, - 1_073_741_823.75, - ) - const rejected = expect(disposal) - .rejects.toThrow('did not exit within its dispose window') - await vi.advanceTimersByTimeAsync(2_147_483_647) - await vi.advanceTimersByTimeAsync(1) - await rejected - } finally { - vi.useRealTimers() - } + it('does not finish disposal before the managed tree exits', async () => { + const child = fakeChild({ exitOnTerminate: false }) + const wire = new CodexAppServerWire(child.handle.stdout!, child.handle.stdin!) + let disposed = false + const disposal = disposeCodexChild(wire, child.handle).then(() => { + disposed = true + }) + await new Promise((resolve) => { setImmediate(resolve) }) + expect(disposed).toBe(false) + child.settle() + await disposal + expect(disposed).toBe(true) }) it('contains a concurrently closed stdin error', async () => { @@ -1090,7 +1057,7 @@ describe('disposeCodexChild', () => { vi.spyOn(child.toChild, 'end').mockImplementation(() => { throw new Error('already closed') }) - await expect(disposeCodexChild(wire, child.handle, 100)) + await expect(disposeCodexChild(wire, child.handle)) .resolves.toBeUndefined() }) @@ -1100,34 +1067,26 @@ describe('disposeCodexChild', () => { doneError: new Error('spawn failed'), }) const wire = new CodexAppServerWire(child.handle.stdout!, child.handle.stdin!) - await expect(disposeCodexChild(wire, child.handle, 100)) + await expect(disposeCodexChild(wire, child.handle)) .resolves.toBeUndefined() expect(child.terminate).not.toHaveBeenCalled() expect(child.waitForExit).not.toHaveBeenCalled() }) - it('fails when the tree misses the release window or done rejects', async () => { - { - const child = fakeChild({ - exitOnTerminate: false, - }) - const wire = new CodexAppServerWire(child.handle.stdout!, child.handle.stdin!) - await expect(disposeCodexChild(wire, child.handle, 1)) - .rejects.toThrow('did not exit within its dispose window') - } + it('reports direct-child observer failure and accepts absent stdin', async () => { { const child = fakeChild({ doneError: new Error('close observer failed'), }) const wire = new CodexAppServerWire(child.handle.stdout!, child.handle.stdin!) - await expect(disposeCodexChild(wire, child.handle, 1)) + await expect(disposeCodexChild(wire, child.handle)) .rejects.toThrow('close observer failed') } { const child = fakeChild() const handle = { ...child.handle, stdin: undefined } const wire = new CodexAppServerWire(child.handle.stdout!, child.handle.stdin!) - await expect(disposeCodexChild(wire, handle, 1)).resolves.toBeUndefined() + await expect(disposeCodexChild(wire, handle)).resolves.toBeUndefined() } }) }) diff --git a/packages/subagent/subagent/src/out-of-process.ts b/packages/subagent/subagent/src/out-of-process.ts index 86b0772283..eac2125897 100644 --- a/packages/subagent/subagent/src/out-of-process.ts +++ b/packages/subagent/subagent/src/out-of-process.ts @@ -42,49 +42,6 @@ export function assertPositiveFinite(prefix: string, name: string, value: number } } -/** Largest delay Node schedules without collapsing it to one millisecond. */ -const MAX_TIMER_DELAY_MS = 2_147_483_647n - -/** - * Bound final exit observation at twice a positive finite grace without - * narrowing public provider config to Node's single-timer integer range. - * @param graceMs - the already validated positive finite termination grace. - * @returns a cancellable abort signal for the doubled observation window. - */ -export function doubledGraceWindow(graceMs: number): { - readonly signal: AbortSignal - readonly cancel: () => void -} { - const whole = Math.floor(graceMs) - let remaining = BigInt(whole) * 2n - + BigInt(Math.ceil((graceMs - whole) * 2)) - const controller = new AbortController() - 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) { - controller.abort() - } else { - arm() - } - }, Number(chunk)) - } - arm() - return { - signal: controller.signal, - cancel: () => { - if (timer === undefined) return - clearTimeout(timer) - timer = undefined - }, - } -} - /** * Whether `path` names an existing directory the harness can ENTER. The * search-permission probe matters: `statSync().isDirectory()` is true for a @@ -162,8 +119,12 @@ export function resolveChildCwd(prefix: string, configured: string | undefined, return assertUsableCwd(prefix, 'parent session cwd', parentCwd) } -/** Normalize an unknown thrown value to an Error (the catch binding is `unknown`). */ -function toError(value: unknown): Error { +/** + * Normalize an unknown thrown value to an Error. + * @param value - the unknown catch binding. + * @returns the original Error or a defensive Error wrapper. + */ +export function thrownError(value: unknown): Error { // The rejecting surfaces (wire clients, spawn failures) only throw // `Error`s; the `String(value)` arm is a defensive fallback for a non-Error // throw the typed surfaces cannot produce. @@ -175,9 +136,9 @@ function toError(value: unknown): Error { export interface RunResultSettlement { /** The turn attempt (typically racing local cancellation); returns the terminal result. */ attempt: () => Promise - /** Snapshot of the child output streamed so far (a partial answer survives failure). */ + /** Snapshot the provider exposes when cancellation or failure wins settlement. */ collectOutput: () => ContentBlock[] - /** Whether local cancellation settled (an in-flight rejection then reads as `aborted`). */ + /** Whether local cancellation settled before the attempt's outcome is observed. */ cancelled: () => boolean /** Diagnostic sink for a failure flattened to a stop reason; a throw from it is contained. */ onError?: ((error: Error, stopReason: SubagentStopReason) => void) | undefined @@ -189,22 +150,25 @@ export interface RunResultSettlement { /** * Settle an out-of-process run result under the seam contract: `result` never - * rejects after publication. A rejection from the attempt resolves as - * `aborted` when cancellation already settled locally, else it is flattened - * to `stopReason: 'error'` through the contained diagnostic sink; the abort - * listener is removed on every path. + * rejects after publication. A normally completed or rejected attempt resolves + * as `aborted` when cancellation already settled locally; another rejection is + * flattened to `stopReason: 'error'` through the contained diagnostic sink. + * The abort listener is removed on every path. * @param parts - the attempt, output snapshot, cancellation state, sink, and signal wiring. * @returns the terminal result (never a rejection). */ export async function settleRunResult(parts: RunResultSettlement): Promise { try { - return await parts.attempt() + const result = await parts.attempt() + return parts.cancelled() + ? { output: parts.collectOutput(), stopReason: 'aborted' } + : result } catch (error: unknown) { // Cover a rejection already queued when cancellation arrives. if (parts.cancelled()) return { output: parts.collectOutput(), stopReason: 'aborted' } // Flatten post-publication transport failures while preserving diagnostics. try { - parts.onError?.(toError(error), 'error') + parts.onError?.(thrownError(error), 'error') } catch { // The diagnostic sink cannot reject the run result. } diff --git a/scripts/run-gates.spec.ts b/scripts/run-gates.spec.ts index d7fb7e1b13..84eeeb10bb 100644 --- a/scripts/run-gates.spec.ts +++ b/scripts/run-gates.spec.ts @@ -189,6 +189,12 @@ describe('Node 24 lane ownership', () => { expect(subject.find(item => item.id === 'doc-typecheck')?.env).toEqual({ DSH_DOC_TYPECHECK_USE_BUILD_OUTPUT: '1', }) + expect(subject.find(item => item.id === 'built-bin-smoke')?.args).toEqual( + expect.arrayContaining([ + 'packages/subagent/subagent-codex/tests/loader-composition.e2e.ts', + 'packages/subagent/subagent-claude-code/tests/loader-composition.e2e.ts', + ]), + ) expect(subject.find(item => item.id === 'web-snapshot')).toMatchObject({ displayCommand: 'DSH_SNAPSHOT=replay pnpm run test:web:built', env: { DSH_SNAPSHOT: 'replay' }, diff --git a/scripts/run-gates.ts b/scripts/run-gates.ts index 74d90a547d..51bdf51f0e 100644 --- a/scripts/run-gates.ts +++ b/scripts/run-gates.ts @@ -598,6 +598,8 @@ function builtBinSmokeGate(needs: string[] = ['build']): Gate { 'packages/examples/cli-demo/tests/built-bin.e2e.ts', 'packages/examples/acp-demo/tests/built-bin.e2e.ts', 'packages/ui/jsonrpc/tests/built-scope-carrier.e2e.ts', + 'packages/subagent/subagent-codex/tests/loader-composition.e2e.ts', + 'packages/subagent/subagent-claude-code/tests/loader-composition.e2e.ts', // The worker-entry packages' built bundles: the only automated proof // that lib/index.js resolves its sibling lib/worker.cjs under plain node // (the e2e lane runs unbuilt, so these files self-skip there).