From bd1805c637cf2d3ea704d0077384cf58a7ae5cf1 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Mon, 20 Jul 2026 23:44:07 +0800 Subject: [PATCH] fix(tools): close cancellation review gaps --- ...on-through-tool-capability-seams.i18n.yaml | 2 +- ...lation-through-tool-capability-seams.zh.md | 12 +++---- docs/cookbook/adding-a-tool.i18n.yaml | 4 +-- docs/cookbook/adding-a-tool.md | 2 +- docs/cookbook/adding-a-tool.zh.md | 2 +- packages/bash/tool-bash/src/index.ts | 2 ++ packages/bash/tool-bash/tests/tools.spec.ts | 32 +++++++++++++++++-- 7 files changed, 43 insertions(+), 13 deletions(-) diff --git a/.agents/notes/proposed/architecture/2026-07-19-required-cancellation-through-tool-capability-seams.i18n.yaml b/.agents/notes/proposed/architecture/2026-07-19-required-cancellation-through-tool-capability-seams.i18n.yaml index b298484d69..bbb2b78989 100644 --- a/.agents/notes/proposed/architecture/2026-07-19-required-cancellation-through-tool-capability-seams.i18n.yaml +++ b/.agents/notes/proposed/architecture/2026-07-19-required-cancellation-through-tool-capability-seams.i18n.yaml @@ -3,4 +3,4 @@ # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write 2026-07-19-required-cancellation-through-tool-capability-seams.md: c2cfb09f27222136965058695e9b6b706ac688a9 -2026-07-19-required-cancellation-through-tool-capability-seams.zh.md: b17480e8844f7200d3c100f5f02f69dfad83ad91 +2026-07-19-required-cancellation-through-tool-capability-seams.zh.md: f7a1d303212dfab6da27feba2d6e7195ea07bd50 diff --git a/.agents/notes/proposed/architecture/2026-07-19-required-cancellation-through-tool-capability-seams.zh.md b/.agents/notes/proposed/architecture/2026-07-19-required-cancellation-through-tool-capability-seams.zh.md index b17480e884..f7a1d30321 100644 --- a/.agents/notes/proposed/architecture/2026-07-19-required-cancellation-through-tool-capability-seams.zh.md +++ b/.agents/notes/proposed/architecture/2026-07-19-required-cancellation-through-tool-capability-seams.zh.md @@ -8,7 +8,7 @@ Status: proposed 已经实现的[工具注册表取消契约](../../implemented/architecture/2026-07-19-cooperative-tool-cancellation.md)让每个工具主体中的 `exec.signal` 成为必填值，但许多由工具主体调用的异步能力接口仍接受可选信号。因此，工具可以满足自身类型，却在下一次同进程调用时意外丢失取消。 -这项缺口会沿调用链传递。文件系统工具可能调用路径解析和 I/O，Web 工具可能调用提供方，Bash 工具可能调用执行器，组合工具可能启动或等待任务、子智能体或工作流。只要某个控制工具所持有工作的等待操作允许省略信号，TypeScript 就无法证明取消仍能到达拥有副作用的边界。 +这项缺口会沿调用链传递。文件系统工具可能调用路径解析和 I/O，Web 工具可能调用提供方，Bash 工具可能调用执行器，组合工具可能启动或等待任务、subagent 或工作流。只要某个控制工具所持有工作的等待操作允许省略信号，TypeScript 就无法证明取消仍能到达拥有副作用的边界。 要求仓库中所有异步函数都携带信号会过度扩张。有些操作无法从工具到达，有些同步查询不会等待或持有持续工作，而明确分离的工作在刻意交接后已经拥有新的所有者。 @@ -18,13 +18,13 @@ Status: proposed 每个直接调用方提供自己持有的信号，或从自身必填的操作上下文继续传递信号。实现可以派生子截止时间或取消作用域，但派生信号在委托期间仍须与上游信号关联。能力实现不得生成永不中止信号、使用环境式异步本地取消，也不得仅为重复类型化同进程契约而在运行时校验 `AbortSignal`。 -迁移首先从每个第一方 `ToolDefinition.execute()` 出发，清点其等待的能力调用；随后把每个内聚的接口、实现和使用方接缝连同测试与生成的 API 文档一起修改。文件系统、Bash 与任务、Web 与提供方、工作流与子智能体、代码运行时等能力族可以通过独立 PR 迁移，以保持每项变更可审查；但根据仓库的预发布原则，已经迁移的接口不得保留可选兼容重载。 +迁移首先从每个第一方 `ToolDefinition.execute()` 出发，清点其等待的能力调用；随后把每个内聚的接口、实现和使用方接缝连同测试与生成的 API 文档一起修改。文件系统、Bash 与任务、Web 与提供方、工作流与 subagent、代码运行时等能力族可以通过独立 PR 迁移，以保持每项变更可审查；但根据仓库的预发布原则，已经迁移的接口不得保留可选兼容重载。 ### 范围边界 本提议包含完成或取消仍属于当前工具生命周期的异步能力操作，包括所有权交接前的启动操作、前台执行、读写、提供方请求、等待，以及工具会等待的清理或释放操作。 -本提议不包含同步注册表查询、可用性检查、schema 渲染、参数分类，以及其他无法保留异步工作的操作。明确交接所有权后的分离工作也不在范围内：任务、工作流、worker 或子智能体成功发布给新的生命周期所有者后，其分离生命周期由新所有者的控制器管理。发起启动的操作在交接提交前仍须接收调用方信号；之后若另一次工具调用等待该分离工作，则必须使用该次调用自己的信号。 +本提议不包含同步注册表查询、可用性检查、schema 渲染、参数分类，以及其他无法保留异步工作的操作。明确交接所有权后的分离工作也不在范围内：任务、工作流、worker 或 subagent 成功发布给新的生命周期所有者后，其分离生命周期由新所有者的控制器管理。发起启动的操作在交接提交前仍须接收调用方信号；之后若另一次工具调用等待该分离工作，则必须使用该次调用自己的信号。 若外部协议本身允许省略取消，解析器、配置、模型与工具 JSON、持久化与文件格式、worker、进程或线协议输入仍可保留可选取消。所属边界必须先把该输入解析为必填的同进程信号，再调用已经迁移的能力接缝。 @@ -34,7 +34,7 @@ Status: proposed **通过 lint 规则或回调检查强制传递。** 不予采纳，因为语法检查无法可靠识别所有权、派生信号、抽象层或正确的完全停稳行为。必填接口参数可以在 TypeScript 能检查每个调用方的位置表达契约。 -**把 `ToolRunContext` 传入所有能力。** 不予采纳，因为能力需要的是取消，而不是工具身份、智能体状态或上下文延后功能。传递更大的上下文会让可复用服务耦合到工具注册表，也会掩盖狭窄接缝。 +**把 `ToolRunContext` 传入所有能力。** 不予采纳，因为能力需要的是取消，而不是工具身份、agent 状态或上下文延后功能。传递更大的上下文会让可复用服务耦合到工具注册表，也会掩盖狭窄接缝。 **使用环境式异步本地信号。** 不予采纳，因为隐藏传递会让所有权和分离交接难以审计，使测试复杂化，并可能让调用静默绑定到错误的生命周期。 @@ -50,11 +50,11 @@ Status: proposed - 派生截止时间和包装层作用域仍与调用方信号关联，集成测试证明取消到达副作用所有者，且等待的工作完全停稳。 - 同步查询和明确交接后的分离工作不受这项要求约束；存在歧义时，需要记录并测试所有权转换。 - 只有真实的无类型边界才添加运行时校验，不得重复校验 TypeScript 已要求的字段或参数。 -- 每次内聚迁移后，顶层 typecheck、覆盖率、快照、文档、模块图、构建、hygiene、演示和构建产物门禁全部通过。 +- 每次内聚迁移后，顶层类型检查、覆盖率、快照、文档、模块图、构建、hygiene、演示和构建产物门禁全部通过。 ## 风险 -**传递性影响范围较大。** 一个必填参数可能同时暴露大量直接调用方。应按内聚能力族迁移，并把 typecheck 失败作为完整的调用方清单。 +**传递性影响范围较大。** 一个必填参数可能同时暴露大量直接调用方。应按内聚能力族迁移，并把类型检查失败作为完整的调用方清单。 **错误划分分离工作。** 过早排除启动操作可能在发布提交前就让工作脱离控制；永久要求父信号又可能让已完成工具取消合法分离的工作。每次交接都需要明确提交点、新所有者、回滚行为和完全停稳的失败路径。 diff --git a/docs/cookbook/adding-a-tool.i18n.yaml b/docs/cookbook/adding-a-tool.i18n.yaml index f60aab5cd7..8dc6c89aea 100644 --- a/docs/cookbook/adding-a-tool.i18n.yaml +++ b/docs/cookbook/adding-a-tool.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 -adding-a-tool.md: d5ea48542fb439ab1e7f3d2648a4593477d73cf2 -adding-a-tool.zh.md: c989e521f95c69b5b666f8e464d2cd157aa2f634 +adding-a-tool.md: 9e8fa1287c854f33f62a4c6a1ed93adfccc19471 +adding-a-tool.zh.md: be4e0800036ac9bde949a11d8a35e49cfb92efd7 diff --git a/docs/cookbook/adding-a-tool.md b/docs/cookbook/adding-a-tool.md index d5ea48542f..9e8fa1287c 100644 --- a/docs/cookbook/adding-a-tool.md +++ b/docs/cookbook/adding-a-tool.md @@ -25,7 +25,7 @@ export function apply(ctx: Context) { async execute(args, exec) { // args is TYPED from the schema: { path: string; limit?: number } // exec carries immutable identity + token; signal is the operational field - return [{ type: 'text', text: await readFile(args.path, 'utf8') }] + return [{ type: 'text', text: await readFile(args.path, { encoding: 'utf8', signal: exec.signal }) }] }, })) } diff --git a/docs/cookbook/adding-a-tool.zh.md b/docs/cookbook/adding-a-tool.zh.md index c989e521f9..be4e080003 100644 --- a/docs/cookbook/adding-a-tool.zh.md +++ b/docs/cookbook/adding-a-tool.zh.md @@ -25,7 +25,7 @@ export function apply(ctx: Context) { async execute(args, exec) { // args is TYPED from the schema: { path: string; limit?: number } // exec carries immutable identity + token; signal is the operational field - return [{ type: 'text', text: await readFile(args.path, 'utf8') }] + return [{ type: 'text', text: await readFile(args.path, { encoding: 'utf8', signal: exec.signal }) }] }, })) } diff --git a/packages/bash/tool-bash/src/index.ts b/packages/bash/tool-bash/src/index.ts index da9ad8f0dc..7d1fccde10 100644 --- a/packages/bash/tool-bash/src/index.ts +++ b/packages/bash/tool-bash/src/index.ts @@ -422,6 +422,8 @@ export function apply(ctx: Context, config: Config = {}): void { if (tasks === undefined) { throw new Error('background tasks unavailable: load @deepseek-ai/dsh-tasks and @deepseek-ai/dsh-tool-tasks') } + // The caller owns cancellation until TaskService commits detached ownership. + if (exec.signal.aborted) return [] // Task preflight finishes before the starter can spawn a process. const id = tasks.start({ kind: 'bash', diff --git a/packages/bash/tool-bash/tests/tools.spec.ts b/packages/bash/tool-bash/tests/tools.spec.ts index 1e1ea37d24..9c9e06bb31 100644 --- a/packages/bash/tool-bash/tests/tools.spec.ts +++ b/packages/bash/tool-bash/tests/tools.spec.ts @@ -7,7 +7,7 @@ import { CallId } from '@deepseek-ai/dsh-llm' import { BashExecutor } from '@deepseek-ai/dsh-bash' import type { BashExecRequest, BashExecSpec, BashProcess, BashProcessRead, BashRunResult } from '@deepseek-ai/dsh-bash' import SystemPrompt from '@deepseek-ai/dsh-system-prompt' -import ToolRegistry, { TOOL_ABORTED_BEFORE_DISPATCH } from '@deepseek-ai/dsh-tools' +import ToolRegistry, { TOOL_ABORTED, TOOL_ABORTED_BEFORE_DISPATCH } from '@deepseek-ai/dsh-tools' import AgentRegistry from '@deepseek-ai/dsh-agent' import type { Agent } from '@deepseek-ai/dsh-agent' import SessionStore, { SessionId } from '@deepseek-ai/dsh-session' @@ -181,7 +181,11 @@ async function setupSandboxed(withApproval = false) { return { ctx, bash: ctx.bash as RecordingSandboxExecutor } } -function sandboxAgent(mode?: 'read-only' | 'workspace-write' | 'danger-full-access', ctx?: Context): Agent { +function sandboxAgent( + mode?: 'read-only' | 'workspace-write' | 'danger-full-access', + ctx?: Context, + onAppend?: (type: string) => void, +): Agent { const events: Array<{ type: string; data?: Record }> = [{ type: 'turn/start' }] if (mode !== undefined) events.push({ type: 'sandbox/mode', data: { mode } }) const id = SessionId('sandbox-session') @@ -195,6 +199,7 @@ function sandboxAgent(mode?: 'read-only' | 'workspace-write' | 'danger-full-acce append: (type: string, data: Record) => { const event = { type, data } events.push(event) + onAppend?.(type) return event }, }, @@ -600,6 +605,29 @@ describe('sandbox escalation through the generic task producer', () => { expect(bash.modes).toEqual(['workspace-write', 'workspace-write']) }) + it('does not publish detached work when cancellation follows the escalation grant', async () => { + const { ctx, bash } = await setupSandboxed(true) + const controller = new AbortController() + const agent = sandboxAgent(undefined, ctx, (type) => { + if (type === 'approval/decided') controller.abort() + }) + ctx.agents.register(agent) + ctx.on('approval/request', () => Promise.resolve('allowed-once')) + const start = vi.spyOn(bash, 'start') + + const result = await ctx.tools.execute({ + callId: CallId('cancelled-escalation-background'), + name: 'bash', + arguments: { ...escalate, run_in_background: true }, + agent, + signal: controller.signal, + }) + + expect(result.error).toEqual({ name: 'AbortError', code: TOOL_ABORTED }) + expect(text(result)).toBe('Error: tool call aborted') + expect(start).not.toHaveBeenCalled() + }) + it('uses the session override for ordinary calls and evaluates widening against it', async () => { const { ctx, bash } = await setupSandboxed(true) const agent = sandboxAgent('workspace-write')