Sfoglia il codice sorgente

fix(tools): close cancellation review gaps

Tianyi Cui 2 mesi fa
parent
commit
bd1805c637

+ 1 - 1
.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

+ 6 - 6
.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 失败作为完整的调用方清单。
+**传递性影响范围较大。** 一个必填参数可能同时暴露大量直接调用方。应按内聚能力族迁移,并把类型检查失败作为完整的调用方清单。
 
 **错误划分分离工作。** 过早排除启动操作可能在发布提交前就让工作脱离控制;永久要求父信号又可能让已完成工具取消合法分离的工作。每次交接都需要明确提交点、新所有者、回滚行为和完全停稳的失败路径。
 

+ 2 - 2
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

+ 1 - 1
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 }) }]
     },
   }))
 }

+ 1 - 1
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 }) }]
     },
   }))
 }

+ 2 - 0
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',

+ 30 - 2
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<string, unknown> }> = [{ 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<string, unknown>) => {
         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<ApprovalOutcome>('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')