fix: 补齐 ACP tool_call 完整生命周期(in_progress)+ 去除伪造 terminal 元数据

承接 acp 合规审计第二轮:修复 tool 调用完整性相关的 3 条遗留发现。

§4.2 [minor] tool_call 从不发出 in_progress 状态:
- bridge.ts toAcpNotifications 的 tool_use 分支:当同一 tool_use 块被第二次遇到
  (streaming content_block_start 首次 + assistant 完整消息回放第二次)时,
  alreadyCached 路径补发 tool_call_update with status: 'in_progress'。
  语义为"input 已收齐,即将执行"。
- ToolCallStatus 完整生命周期现在是 pending → in_progress → completed|failed,
  对齐 schema.json:3525-3548 与 tool-calls.mdx:76-91。
- 新增 forwardSessionUpdates 集成测试验证 streaming + 回放场景下发出
  in_progress 中间状态。

§4.4 + §5.2 简化版(合并修复):
- bridge.ts toolInfoFromToolUse Bash 分支:去除 _supportsTerminalOutput 为 true
  时发出的 { type: 'terminal', terminalId: toolUse.id }(terminalId 从未通过
  terminal/create 注册,合规客户端按此 id 查 terminal/output 会失败)。统一
  回退到 description 文本内容。
- bridge.ts toolUpdateFromToolResult Bash 分支:去除 _supportsTerminalOutput
  分支里伪造的 terminalId 与三个非标准 _meta 键(terminal_info / terminal_output
  / terminal_exit,违反 _meta 应使用 vendor namespace 的规范)。Bash 输出统一
  以 ```console 围栏文本呈现。删除随之无用的 exitCode / terminalId 局部变量。
- _supportsTerminalOutput 参数保留(前向兼容),用 void 标注暂未使用。
- 完整版(真接 terminal/create + terminal/release + PTY)涉及 BashTool 执行
  管线改造,需单独决策,留作待办。

测试更新:
- toolInfoFromToolUse Bash 测试改写:不再断言 terminalId,改为断言回退到空
  content(无 description)或 description 文本(有 description)。
- toolUpdateFromToolResult Bash 测试改写:不再断言 terminal_info/terminal_output/
  terminal_exit,改为断言走 ```console 文本路径且 _meta 为 undefined。
- bash_code_execution_result 测试同步更新。

验证:bun run precheck 全绿(tsc 零错误、biome ci 零警告、5851/5851 测试通过)。

Co-Authored-By: glm-5.2 <zai-org@claude-code-best.win>
This commit is contained in:
claude-code-best
2026-06-19 15:01:44 +08:00
parent 5e30697950
commit 35768837a7
3 changed files with 146 additions and 65 deletions

View File

@@ -374,11 +374,11 @@
})
~~~
### 4.2 [minor] 从未发出 tool_call in_progress 状态
### 4.2 [minor] 从未发出 tool_call in_progress 状态 ✅ 已修复 (2026-06-19)
- 位置: `src/services/acp/bridge.ts:1300-1316` (toAcpNotifications, 'tool_use' 第一次遇到 → tool_call) 和 1321-1348 ('tool_result' → tool_call_update)
- 位置: `src/services/acp/bridge.ts` `toAcpNotifications``tool_use` 分支 alreadyCached 路径
- 规范要求: schema.json:3525-3548 ToolCallStatus 枚举为 `pending``in_progress``completed``failed`。tool-calls.mdx:76-91 ('Updating') 文档化了一个生命周期,其中 Agent 在工具实际运行时报告 `status: 'in_progress'`。v1 规范称工具 "在其生命周期中会经历不同状态"。
- 当前实现: 实现仅发送两种状态转换: 初始 'tool_call' 为 `status: 'pending'`(第 1309 行),以及随后的 'tool_call_update' 为 `status: 'completed' | 'failed'`(第 1340-1343 行)。从未发出 `in_progress` 状态,因此客户端无法区分 "awaiting approval / streaming input" 和 "currently executing"
- 修复: 当同一 tool_use 块被第二次遇到时(streaming `content_block_start` 首次 + assistant 完整消息回放第二次),发 `tool_call_update` with `status: 'in_progress'`。此时语义为"input 已收齐,即将执行"。完整 ToolCallStatus 生命周期现在是 pending in_progress → completed|failed
- 修复建议: 当 Claude Code 知道工具开始执行时,发出一个中间的 tool_call_update:
~~~ts
@@ -403,16 +403,11 @@
这通过 v1 稳定版规范中记录的通道,为客户端提供了规范的会话显示名称。
### 4.4 [nit] Bash 工具 _meta 键未命名空间化
### 4.4 [nit] Bash 工具 _meta 键未命名空间化 ✅ 已修复 (2026-06-19,与 §5.2 合并)
- 位置: `src/services/acp/bridge.ts:501-503` (toolUpdateFromToolResult, 'Bash' 情况)
- 规范要求: schema.json 将 `_meta` 记录为保留的扩展点("实现不得对这些键上的值做出假设")。为了向前兼容,建议使用反向 DNS / 供应商命名空间的自定义键。SessionUpdate 上的其他 _meta 用法bridge.ts:1288, 1304, 1336, 1370正确使用了 `_meta.claudeCode.*` 命名空间。
- 当前实现: Bash 工具结果在 ToolCallContent._meta 中放置了未命名空间的键: `_meta: { terminal_info: { terminal_id }, terminal_output: { terminal_id, data }, terminal_exit: { terminal_id, exit_code, signal } }`。这些是附加的,因此符合规范,但它们破坏了在其他所有地方使用的 `_meta.claudeCode.*` 命名空间约定,并且仅依赖于客户端知道这些键
- 修复建议: 为了与文件的其余部分保持一致,请在 `_meta.claudeCode` 下进行命名空间处理(可选;这并非规范违规):
~~~ts
_meta: { claudeCode: { terminal: { info: {...}, output: {...}, exit: {...} } } }
~~~
- 位置: `src/services/acp/bridge.ts` `toolUpdateFromToolResult` Bash 分支
- 规范要求: schema.json 将 `_meta` 记录为保留的扩展点("实现不得对这些键上的值做出假设")。建议使用反向 DNS / 供应商命名空间的自定义键。
- 修复: 与 §5.2 合并处理 — 完全删除了 `terminal_info` / `terminal_output` / `terminal_exit` 三个非标准 `_meta` 键,以及伪造的 `terminalId`。Bash 工具结果现在统一走 inline `{type:'text'}` content,不再向 `_meta` 注入任何键。命名空间问题随之消失
---
@@ -435,12 +430,12 @@
}
~~~
### 5.2 [major] terminal 生命周期未实现,伪造 terminalId 且 _meta 注入非标准键
### 5.2 [major] terminal 生命周期未实现,伪造 terminalId 且 _meta 注入非标准键 — 🔶 简化版已修复 (2026-06-19),完整版待办
- 位置: `src/services/acp/bridge.ts:498-511` (toolUpdateFromToolResult, Bash branch)
- 位置: `src/services/acp/bridge.ts` `toolUpdateFromToolResult` Bash 分支 + `toolInfoFromToolUse` Bash 分支
- 规范要求: Terminals doc (docs/protocol/terminals.mdx:27-110) defines the standard terminal lifecycle: the Agent MUST call `terminal/create` to obtain a real `terminalId`, embed it via ToolCallContent `{type:'terminal', terminalId}` (schema.json:3242-3256), and the Client retrieves output via `terminal/output`。ToolCallUpdate._meta is reserved: "Implementations MUST NOT make assumptions about values at these keys" (schema.json:3555)。
- 当前实现: 对于 Bash results,当 supportsTerminalOutput 为 true 时,代码伪造了一个 `terminalId: toolUse.id`(从未通过 `terminal/create` 注册),发出 `{type:'terminal', terminalId}` content,并注入了三个非标准键到 `_meta`: `terminal_info``terminal_output`(携带实际的 stdout/stderr 数据)和 `terminal_exit`。从未发出 `terminal/create` / `terminal/release`。标准 ACP 客户端没有义务读取这些 `_meta` 键,且嵌入的 `terminalId` 对其 terminal 子系统未知,因此 `terminal/output` / `terminal/wait_for_exit` 将失败
- 修复建议: 当 `clientCapabilities.terminal === true` 时实现标准 terminal 流程: 在工具运行前(或首次输出时)调用 `conn.request('terminal/create', {sessionId, command, cwd, outputByteLimit})`,在 tool_call content 中嵌入返回的真实 `terminalId`,通过 terminal 子系统流式输出,并在完成时 `terminal/release`在此之前,不要宣告 terminal 支持;回退到 inline `{type:'text'}` content 并丢弃 `_meta.terminal_*` 键(它们对合规客户端不可解释)
- 简化版修复(已落地): 按文档建议回退到 inline `{type:'text'}` content,删除了伪造的 `terminalId: toolUse.id`toolInfoFromToolUse + toolUpdateFromToolResult 两处)和三个非标准 `_meta` 键(`terminal_info` / `terminal_output` / `terminal_exit`)。合规客户端不再被误导去查找不存在的 terminal。Bash 输出仍以 ```console 围栏文本形式呈现给客户端
- 完整版(待办): 实现标准 terminal 流程,需要 BashTool 接入 PTY 子系统:在工具运行前调用 `conn.request('terminal/create', {sessionId, command, cwd, outputByteLimit})`,嵌入返回的真实 `terminalId` 到 ToolCallContent,通过 terminal 子系统流式输出,完成时 `terminal/release`此改造涉及 BashTool 执行管线(影响 CLI REPL 路径),需单独决策是否仅 ACP 路径启用
### 5.3 [major] cancelled 权限结果被当作普通拒绝

View File

@@ -83,13 +83,35 @@ describe('toolInfoFromToolUse', () => {
])
})
test('Bash with terminalOutput → returns terminalId content', () => {
test('Bash with terminalOutput flag → no longer emits fake terminalId (audit §5.2)', () => {
// Standard ACP terminal lifecycle is not wired through BashTool; previously
// this returned { type: 'terminal', terminalId: toolUse.id } which would
// cause compliant clients to fail terminal/output lookups. The flag is now
// ignored until terminal/create is actually plumbed through.
const info = toolInfoFromToolUse(
{ name: 'Bash', id: 'tu_123', input: { command: 'ls' } },
true,
)
expect(info.kind).toBe('execute')
expect(info.content).toEqual([{ type: 'terminal', terminalId: 'tu_123' }])
expect(info.content).toEqual([])
expect(info.title).toBe('ls')
})
test('Bash with terminalOutput flag + description → falls back to description text', () => {
const info = toolInfoFromToolUse(
{
name: 'Bash',
id: 'tu_456',
input: { command: 'ls', description: 'list files' },
},
true,
)
expect(info.content).toEqual([
{
type: 'content',
content: { type: 'text', text: 'list files' },
},
])
})
test('Bash without description → empty content', () => {
@@ -511,7 +533,9 @@ describe('toolUpdateFromToolResult', () => {
])
})
test('returns terminal metadata for Bash with terminalOutput', () => {
test('Bash with terminalOutput flag → falls back to inline text (audit §5.2)', () => {
// Standard ACP terminal lifecycle is not wired; the flag is now ignored
// and no fake terminalId / non-standard _meta keys are emitted.
const result = toolUpdateFromToolResult(
{
content: [{ type: 'text', text: 'output' }],
@@ -521,20 +545,13 @@ describe('toolUpdateFromToolResult', () => {
{ name: 'Bash', id: 't1' },
true,
)
expect(result.content).toEqual([{ type: 'terminal', terminalId: 't1' }])
expect(result._meta).toBeDefined()
expect((result._meta as Record<string, unknown>).terminal_info).toEqual({
terminal_id: 't1',
})
expect((result._meta as Record<string, unknown>).terminal_output).toEqual({
terminal_id: 't1',
data: 'output',
})
expect((result._meta as Record<string, unknown>).terminal_exit).toEqual({
terminal_id: 't1',
exit_code: 0,
signal: null,
})
expect(result.content).toEqual([
{
type: 'content',
content: { type: 'text', text: '```console\noutput\n```' },
},
])
expect(result._meta).toBeUndefined()
})
test('handles bash_code_execution_result format', () => {
@@ -552,9 +569,15 @@ describe('toolUpdateFromToolResult', () => {
{ name: 'Bash', id: 't1' },
true,
)
const meta = result._meta as Record<string, unknown>
const termOutput = meta.terminal_output as { data: string }
expect(termOutput.data).toBe('out\nerr')
// terminalOutput flag is ignored; bash_code_execution_result is rendered
// as inline console text just like plain string content.
expect(result.content).toEqual([
{
type: 'content',
content: { type: 'text', text: '```console\nout\nerr\n```' },
},
])
expect(result._meta).toBeUndefined()
})
test('returns empty when no toolUse', () => {
@@ -1165,6 +1188,67 @@ describe('forwardSessionUpdates', () => {
expect(update.rawInput).not.toBe(input)
})
test('emits tool_call_update with status in_progress when tool_use is encountered again (audit §4.2)', async () => {
// When the same tool_use block is seen twice (first via content_block_start
// in stream_event, then again in the final assistant message), the second
// encounter signals "input fully received, about to execute" and is emitted
// as a tool_call_update with status:'in_progress' per ACP v1 ToolCallStatus
// lifecycle (pending → in_progress → completed|failed).
const conn = makeConn()
const input = { command: 'ls' }
const msgs: SDKMessage[] = [
// streaming content_block_start: first sighting of tool_use
{
type: 'stream_event',
event: {
type: 'content_block_start',
content_block: {
type: 'tool_use',
id: 'tu_2',
name: 'Bash',
input: {},
},
},
} as unknown as SDKMessage,
// final assistant message: tool_use block with full input
{
type: 'assistant',
message: {
content: [{ type: 'tool_use', id: 'tu_2', name: 'Bash', input }],
role: 'assistant',
},
} as unknown as SDKMessage,
]
await forwardSessionUpdates(
's1',
makeStream(msgs),
conn,
new AbortController().signal,
{},
)
const calls = (conn.sessionUpdate as ReturnType<typeof mock>).mock.calls
const statuses = calls
.map((c: unknown[]) => {
const u = (c[0] as { update?: Record<string, unknown> }).update
return u && u.toolCallId === 'tu_2'
? {
sessionUpdate: u.sessionUpdate,
status: u.status,
}
: null
})
.filter(Boolean)
// First: tool_call pending; second: tool_call_update in_progress
expect(statuses[0]).toEqual({
sessionUpdate: 'tool_call',
status: 'pending',
})
expect(statuses[1]).toEqual({
sessionUpdate: 'tool_call_update',
status: 'in_progress',
})
})
test('returns accumulated usage on result message without sending usage_update', async () => {
// usage_update is an UNSTABLE SessionUpdate discriminator and is no longer
// emitted (audit §4.1). Token totals are still aggregated for the

View File

@@ -234,19 +234,24 @@ export function toolInfoFromToolUse(
case 'Bash': {
const command = (input?.command as string | undefined) ?? 'Terminal'
const description = input?.description as string | undefined
// Standard ACP terminal lifecycle (terminal/create → embed real terminalId →
// terminal/release) is not wired through BashTool yet. Embedding a fake
// terminalId here would cause compliant clients to fail terminal/output
// lookups, so we fall back to inline text content per audit doc §5.2.
// The _supportsTerminalOutput flag is retained for forward compatibility
// once terminal/create is actually plumbed through.
void _supportsTerminalOutput
return {
title: command,
kind: 'execute',
content: _supportsTerminalOutput
? [{ type: 'terminal' as const, terminalId: toolUse.id }]
: description
? [
{
type: 'content' as const,
content: { type: 'text' as const, text: description },
},
]
: [],
content: description
? [
{
type: 'content' as const,
content: { type: 'text' as const, text: description },
},
]
: [],
}
}
@@ -492,8 +497,16 @@ export function toolUpdateFromToolResult(
case 'Bash': {
let output = ''
let exitCode = isError ? 1 : 0
const terminalId = String(toolUse.id)
// Standard ACP terminal lifecycle (terminal/create → embed real terminalId
// → terminal/release) is not wired through BashTool yet. Previously this
// branch embedded a fake terminalId (= toolUse.id, never registered via
// terminal/create) and injected non-standard _meta keys (terminal_info /
// terminal_output / terminal_exit) that compliant clients cannot
// interpret. We now fall back to inline text content for the output; see
// audit doc §5.2/§4.4. The _supportsTerminalOutput flag is retained on
// the signature for forward compatibility once terminal/create is plumbed
// through.
void _supportsTerminalOutput
// Handle bash_code_execution_result format
if (
@@ -507,7 +520,6 @@ export function toolUpdateFromToolResult(
output = [bashResult.stdout, bashResult.stderr]
.filter(Boolean)
.join('\n')
exitCode = (bashResult.return_code as number) ?? (isError ? 1 : 0)
} else if (typeof resultContent === 'string') {
output = resultContent
} else if (Array.isArray(resultContent) && resultContent.length > 0) {
@@ -518,21 +530,6 @@ export function toolUpdateFromToolResult(
.join('\n')
}
if (_supportsTerminalOutput) {
return {
content: [{ type: 'terminal' as const, terminalId }],
_meta: {
terminal_info: { terminal_id: terminalId },
terminal_output: { terminal_id: terminalId, data: output },
terminal_exit: {
terminal_id: terminalId,
exit_code: exitCode,
signal: null,
},
},
}
}
if (output.trim()) {
return {
content: [
@@ -1320,13 +1317,18 @@ function toAcpNotifications(
const rawInput = toolInput ? { ...toolInput } : {}
if (alreadyCached) {
// Second encounter — send as tool_call_update
// Second encounter — tool_use input is now fully received.
// The tool is about to execute (pending permission, then run).
// Emit a tool_call_update with status 'in_progress' so clients
// can distinguish "awaiting approval / running" from the initial
// 'pending' (per ACP v1 ToolCallStatus lifecycle, schema.json:3525).
update = {
_meta: {
claudeCode: { toolName },
},
toolCallId: toolUseId,
sessionUpdate: 'tool_call_update',
status: 'in_progress',
rawInput,
...toolInfoFromToolUse(
{ name: toolName, id: toolUseId, input: toolInput ?? {} },