mirror of
https://github.com/claude-code-best/claude-code.git
synced 2026-06-23 00:35:51 +00:00
fix: 严格对齐 ACP 协议实现到 stable v1 规范
对照 /Users/konghayao/code/knowledgebase/origin/acp 规范审计并修复 53 条合规性
发现(critical 5 / major 17 / minor 20 / nit 11),完整审计报告见
docs/acp-compliance-audit.md。
Agent 端 (src/services/acp/agent.ts):
- initialize() 补齐 authMethods,promptCapabilities.image 降级为 false(声明与
实现脱节,按 initialization.mdx 不声明的 capability 视为不支持)
- sessionCapabilities.fork 移至 _meta.claudeCode.forkSession(fork 在
meta.unstable.json 中,避免在 stable sessionCapabilities 中暴露 unstable 特性)
- unstable_resumeSession 传 replay:false,不再通过 session/update 重放历史
(session-setup.mdx:239 明确禁止)
- PromptResponse.usage 移至 _meta.claudeCode.usage
(extensibility.mdx:39 禁止在 spec 类型根添加自定义字段)
- 空字符串 prompt 改为显式 throw(不再误返 end_turn)
Bridge (src/services/acp/bridge.ts):
- 删除全部 usage_update discriminator(不在 stable v1 schema 中)
- 显式映射 refusal stop_reason(之前误报 end_turn)
- max_tokens / isError 检查互斥
- Read/Write/Edit/Glob 路径全部绝对化(协议规定路径 MUST 绝对)
- 补全 resource_link / resource ContentBlock 渲染
Permissions (src/services/acp/permissions.ts):
- 补齐 reject_always PermissionOption(schema 规定的四个 option 之一)
- checkTerminalOutput 优先检查标准 clientCapabilities.terminal,
回退到 _meta.terminal_output
- 新增 onPermissionCancelled 回调:cancelled permission outcome →
StopReason::Cancelled(schema.json:629)
- ExitPlanMode cancelled 分支补上 toolUseID 字段
PromptConversion (src/services/acp/promptConversion.ts):
- resource 分支处理 BlobResource(之前静默丢弃 blob 内容)
acp-link 代理 (packages/acp-link/src/):
- WS 协议从专有 {type, payload} 改造为标准 JSON-RPC 2.0
(transports.mdx:52 要求自定义 transport MUST 保留 JSON-RPC 消息格式),
同时向后兼容旧 envelope
- 实现 $/cancel_request 处理
- 使用 JSON-RPC 标准错误码 -32700 / -32600 / -32601 / -32602 / -32603
- capability / agentInfo / protocolVersion 完整透传
验证:bun run precheck 全部通过(tsc 零错误、biome ci 零警告、5841/5841 测试通过);
ACP 专项测试 221/221 通过。独立 verification agent 抽查全部 PASS。
已知暂缓项(审计文档附录 B/C):
- §3.5 traceparent/trace-context 传播(QueryEngine 无 header hook)
- §5.2 terminal/create 完整生命周期(P1,非阻断,需新 RPC 流程)
- §4.2 in_progress tool_call status(SHOULD 级)
- §8.8/8.9/8.14 stale types.ts(不在 owner 分配集合,runtime 已修正)
Co-Authored-By: glm-5.2 <zai-org@claude-code-best.win>
This commit is contained in:
@@ -299,6 +299,91 @@ describe('toolInfoFromToolUse', () => {
|
||||
])
|
||||
})
|
||||
|
||||
test('Read with relative file_path and cwd → locations resolved to absolute', () => {
|
||||
// Audit §5.5: ToolCallLocation.path MUST be absolute. A relative input
|
||||
// path is resolved against the session cwd before being emitted.
|
||||
const info = toolInfoFromToolUse(
|
||||
{
|
||||
name: 'Read',
|
||||
id: 'x',
|
||||
input: { file_path: 'src/main.ts' },
|
||||
},
|
||||
false,
|
||||
'/Users/test/project',
|
||||
)
|
||||
expect(info.locations).toEqual([
|
||||
{ path: '/Users/test/project/src/main.ts', line: 1 },
|
||||
])
|
||||
})
|
||||
|
||||
test('Write with relative file_path and cwd → diff path resolved absolute', () => {
|
||||
// Audit §5.5: Diff.path MUST be absolute.
|
||||
const info = toolInfoFromToolUse(
|
||||
{
|
||||
name: 'Write',
|
||||
id: 'x',
|
||||
input: { file_path: 'rel/file.txt', content: 'hi' },
|
||||
},
|
||||
false,
|
||||
'/Users/test/project',
|
||||
)
|
||||
expect(info.content).toEqual([
|
||||
{
|
||||
type: 'diff',
|
||||
path: '/Users/test/project/rel/file.txt',
|
||||
oldText: null,
|
||||
newText: 'hi',
|
||||
},
|
||||
])
|
||||
expect(info.locations).toEqual([
|
||||
{ path: '/Users/test/project/rel/file.txt' },
|
||||
])
|
||||
})
|
||||
|
||||
test('Edit with relative file_path and cwd → diff path resolved absolute', () => {
|
||||
// Audit §5.5: Diff.path MUST be absolute.
|
||||
const info = toolInfoFromToolUse(
|
||||
{
|
||||
name: 'Edit',
|
||||
id: 'x',
|
||||
input: {
|
||||
file_path: 'rel/edit.txt',
|
||||
old_string: 'a',
|
||||
new_string: 'b',
|
||||
},
|
||||
},
|
||||
false,
|
||||
'/Users/test/project',
|
||||
)
|
||||
expect(info.content).toEqual([
|
||||
{
|
||||
type: 'diff',
|
||||
path: '/Users/test/project/rel/edit.txt',
|
||||
oldText: 'a',
|
||||
newText: 'b',
|
||||
},
|
||||
])
|
||||
expect(info.locations).toEqual([
|
||||
{ path: '/Users/test/project/rel/edit.txt' },
|
||||
])
|
||||
})
|
||||
|
||||
test('Glob with relative path and cwd → locations resolved absolute', () => {
|
||||
// Audit §5.5: ToolCallLocation.path MUST be absolute. Title keeps the raw
|
||||
// input for display, but the emitted location is resolved against cwd.
|
||||
const info = toolInfoFromToolUse(
|
||||
{
|
||||
name: 'Glob',
|
||||
id: 'x',
|
||||
input: { pattern: '*.ts', path: 'src' },
|
||||
},
|
||||
false,
|
||||
'/Users/test/project',
|
||||
)
|
||||
expect(info.title).toBe('Find `src` `*.ts`')
|
||||
expect(info.locations).toEqual([{ path: '/Users/test/project/src' }])
|
||||
})
|
||||
|
||||
// ── WebSearch ─────────────────────────────────────────────────
|
||||
|
||||
test('WebSearch with allowed/blocked domains', () => {
|
||||
@@ -543,6 +628,91 @@ describe('toolUpdateFromToolResult', () => {
|
||||
)
|
||||
expect(result.title).toBe('Exited Plan Mode')
|
||||
})
|
||||
|
||||
test('renders resource_link content as ACP ResourceLink (audit §7.3)', () => {
|
||||
const result = toolUpdateFromToolResult(
|
||||
{
|
||||
content: [
|
||||
{
|
||||
type: 'resource_link',
|
||||
uri: 'file:///tmp/spec.md',
|
||||
name: 'Spec',
|
||||
mimeType: 'text/markdown',
|
||||
},
|
||||
],
|
||||
is_error: false,
|
||||
tool_use_id: 't1',
|
||||
},
|
||||
{ name: 'SomeTool', id: 't1' },
|
||||
)
|
||||
expect(result.content).toEqual([
|
||||
{
|
||||
type: 'content',
|
||||
content: {
|
||||
type: 'resource_link',
|
||||
uri: 'file:///tmp/spec.md',
|
||||
name: 'Spec',
|
||||
mimeType: 'text/markdown',
|
||||
},
|
||||
},
|
||||
])
|
||||
})
|
||||
|
||||
test('resource_link without name falls back to uri (audit §7.3)', () => {
|
||||
const result = toolUpdateFromToolResult(
|
||||
{
|
||||
content: [{ type: 'resource_link', uri: 'file:///tmp/x.md' }],
|
||||
is_error: false,
|
||||
tool_use_id: 't1',
|
||||
},
|
||||
{ name: 'SomeTool', id: 't1' },
|
||||
)
|
||||
expect(result.content).toEqual([
|
||||
{
|
||||
type: 'content',
|
||||
content: {
|
||||
type: 'resource_link',
|
||||
uri: 'file:///tmp/x.md',
|
||||
name: 'file:///tmp/x.md',
|
||||
mimeType: undefined,
|
||||
},
|
||||
},
|
||||
])
|
||||
})
|
||||
|
||||
test('renders resource content as ACP EmbeddedResource (audit §7.3)', () => {
|
||||
const result = toolUpdateFromToolResult(
|
||||
{
|
||||
content: [
|
||||
{
|
||||
type: 'resource',
|
||||
resource: {
|
||||
uri: 'file:///tmp/readme.md',
|
||||
mimeType: 'text/markdown',
|
||||
text: '# Hello',
|
||||
},
|
||||
},
|
||||
],
|
||||
is_error: false,
|
||||
tool_use_id: 't1',
|
||||
},
|
||||
{ name: 'SomeTool', id: 't1' },
|
||||
)
|
||||
expect(result.content).toEqual([
|
||||
{
|
||||
type: 'content',
|
||||
content: {
|
||||
type: 'resource',
|
||||
resource: {
|
||||
uri: 'file:///tmp/readme.md',
|
||||
mimeType: 'text/markdown',
|
||||
text: '# Hello',
|
||||
blob: undefined,
|
||||
},
|
||||
},
|
||||
},
|
||||
])
|
||||
})
|
||||
})
|
||||
|
||||
// ── toolUpdateFromEditToolResponse ─────────────────────────────────
|
||||
@@ -650,6 +820,56 @@ describe('toolUpdateFromEditToolResponse', () => {
|
||||
}),
|
||||
).toEqual({})
|
||||
})
|
||||
|
||||
test('resolves relative filePath against cwd (audit §5.5)', () => {
|
||||
// ToolCallLocation.path / Diff.path MUST be absolute.
|
||||
const result = toolUpdateFromEditToolResponse(
|
||||
{
|
||||
filePath: 'rel/file.ts',
|
||||
structuredPatch: [
|
||||
{
|
||||
oldStart: 1,
|
||||
oldLines: 1,
|
||||
newStart: 1,
|
||||
newLines: 1,
|
||||
lines: ['-old', '+new'],
|
||||
},
|
||||
],
|
||||
},
|
||||
'/Users/test/project',
|
||||
)
|
||||
expect(result).toEqual({
|
||||
content: [
|
||||
{
|
||||
type: 'diff',
|
||||
path: '/Users/test/project/rel/file.ts',
|
||||
oldText: 'old',
|
||||
newText: 'new',
|
||||
},
|
||||
],
|
||||
locations: [{ path: '/Users/test/project/rel/file.ts', line: 1 }],
|
||||
})
|
||||
})
|
||||
|
||||
test('keeps absolute filePath unchanged when cwd provided', () => {
|
||||
const result = toolUpdateFromEditToolResponse(
|
||||
{
|
||||
filePath: '/abs/file.ts',
|
||||
structuredPatch: [
|
||||
{
|
||||
oldStart: 1,
|
||||
oldLines: 1,
|
||||
newStart: 1,
|
||||
newLines: 1,
|
||||
lines: ['-old', '+new'],
|
||||
},
|
||||
],
|
||||
},
|
||||
'/Users/test/project',
|
||||
)
|
||||
expect(result.content![0]).toMatchObject({ path: '/abs/file.ts' })
|
||||
expect(result.locations![0]).toMatchObject({ path: '/abs/file.ts' })
|
||||
})
|
||||
})
|
||||
|
||||
// ── markdownEscape ─────────────────────────────────────────────────
|
||||
@@ -945,7 +1165,10 @@ describe('forwardSessionUpdates', () => {
|
||||
expect(update.rawInput).not.toBe(input)
|
||||
})
|
||||
|
||||
test('sends usage_update on result message with correct tokens', async () => {
|
||||
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
|
||||
// PromptResponse return value so callers can include them via _meta.
|
||||
const conn = makeConn()
|
||||
const msgs: SDKMessage[] = [
|
||||
{
|
||||
@@ -973,9 +1196,19 @@ describe('forwardSessionUpdates', () => {
|
||||
expect(result.usage).toBeDefined()
|
||||
expect(result.usage!.inputTokens).toBe(100)
|
||||
expect(result.usage!.outputTokens).toBe(50)
|
||||
const calls = (conn.sessionUpdate as ReturnType<typeof mock>).mock.calls
|
||||
const usageUpdate = calls.find(
|
||||
(c: unknown[]) =>
|
||||
((c[0] as Record<string, Record<string, unknown>>).update ?? {})[
|
||||
'sessionUpdate'
|
||||
] === 'usage_update',
|
||||
)
|
||||
expect(usageUpdate).toBeUndefined()
|
||||
})
|
||||
|
||||
test('sends usage_update with context window from modelUsage', async () => {
|
||||
test('does not emit usage_update even when modelUsage reports context window', async () => {
|
||||
// Context-window resolution still runs internally (so PromptResponse can
|
||||
// surface it), but no usage_update notification is sent for v1 compliance.
|
||||
const conn = makeConn()
|
||||
const msgs: SDKMessage[] = [
|
||||
{
|
||||
@@ -1023,18 +1256,10 @@ describe('forwardSessionUpdates', () => {
|
||||
'sessionUpdate'
|
||||
] === 'usage_update',
|
||||
)
|
||||
expect(usageUpdate).toBeDefined()
|
||||
expect(
|
||||
(
|
||||
(usageUpdate![0] as Record<string, unknown>).update as Record<
|
||||
string,
|
||||
unknown
|
||||
>
|
||||
).size,
|
||||
).toBe(1000000)
|
||||
expect(usageUpdate).toBeUndefined()
|
||||
})
|
||||
|
||||
test('sends usage_update with prefix-matched modelUsage', async () => {
|
||||
test('prefix-matches modelUsage without emitting usage_update', async () => {
|
||||
const conn = makeConn()
|
||||
const msgs: SDKMessage[] = [
|
||||
{
|
||||
@@ -1082,18 +1307,125 @@ describe('forwardSessionUpdates', () => {
|
||||
'sessionUpdate'
|
||||
] === 'usage_update',
|
||||
)
|
||||
expect(usageUpdate).toBeDefined()
|
||||
expect(
|
||||
(
|
||||
(usageUpdate![0] as Record<string, unknown>).update as Record<
|
||||
string,
|
||||
unknown
|
||||
>
|
||||
).size,
|
||||
).toBe(2000000)
|
||||
expect(usageUpdate).toBeUndefined()
|
||||
})
|
||||
|
||||
test('resets usage on compact_boundary', async () => {
|
||||
test('maps refusal stop_reason to ACP refusal stop reason', async () => {
|
||||
// Audit §3.3: a safety refusal must surface as StopReason::refusal rather
|
||||
// than being misreported as end_turn.
|
||||
const conn = makeConn()
|
||||
const msgs: SDKMessage[] = [
|
||||
{
|
||||
type: 'result',
|
||||
subtype: 'success',
|
||||
is_error: false,
|
||||
result: '',
|
||||
stop_reason: 'refusal',
|
||||
} as unknown as SDKMessage,
|
||||
]
|
||||
const result = await forwardSessionUpdates(
|
||||
's1',
|
||||
makeStream(msgs),
|
||||
conn,
|
||||
new AbortController().signal,
|
||||
{},
|
||||
)
|
||||
expect(result.stopReason).toBe('refusal')
|
||||
})
|
||||
|
||||
test('success with max_tokens stop_reason maps to max_tokens when not error', async () => {
|
||||
// Audit §3.3/§3.4: success + max_tokens + no error surfaces max_tokens.
|
||||
const conn = makeConn()
|
||||
const msgs: SDKMessage[] = [
|
||||
{
|
||||
type: 'result',
|
||||
subtype: 'success',
|
||||
is_error: false,
|
||||
result: '',
|
||||
stop_reason: 'max_tokens',
|
||||
} as unknown as SDKMessage,
|
||||
]
|
||||
const result = await forwardSessionUpdates(
|
||||
's1',
|
||||
makeStream(msgs),
|
||||
conn,
|
||||
new AbortController().signal,
|
||||
{},
|
||||
)
|
||||
expect(result.stopReason).toBe('max_tokens')
|
||||
})
|
||||
|
||||
test('success with max_tokens stop_reason falls back to end_turn when isError', async () => {
|
||||
// Audit §3.3: in the success branch, isError acts as a last-resort
|
||||
// override to end_turn per the merged fix diff.
|
||||
const conn = makeConn()
|
||||
const msgs: SDKMessage[] = [
|
||||
{
|
||||
type: 'result',
|
||||
subtype: 'success',
|
||||
is_error: true,
|
||||
result: '',
|
||||
stop_reason: 'max_tokens',
|
||||
} as unknown as SDKMessage,
|
||||
]
|
||||
const result = await forwardSessionUpdates(
|
||||
's1',
|
||||
makeStream(msgs),
|
||||
conn,
|
||||
new AbortController().signal,
|
||||
{},
|
||||
)
|
||||
expect(result.stopReason).toBe('end_turn')
|
||||
})
|
||||
|
||||
test('maps error_during_execution with max_tokens stop_reason', async () => {
|
||||
// Audit §3.4: error_during_execution branch must preserve max_tokens even
|
||||
// when isError is set (mutually exclusive branches).
|
||||
const conn = makeConn()
|
||||
const msgs: SDKMessage[] = [
|
||||
{
|
||||
type: 'result',
|
||||
subtype: 'error_during_execution',
|
||||
is_error: true,
|
||||
result: '',
|
||||
stop_reason: 'max_tokens',
|
||||
} as unknown as SDKMessage,
|
||||
]
|
||||
const result = await forwardSessionUpdates(
|
||||
's1',
|
||||
makeStream(msgs),
|
||||
conn,
|
||||
new AbortController().signal,
|
||||
{},
|
||||
)
|
||||
expect(result.stopReason).toBe('max_tokens')
|
||||
})
|
||||
|
||||
test('maps error_during_execution without max_tokens to end_turn', async () => {
|
||||
const conn = makeConn()
|
||||
const msgs: SDKMessage[] = [
|
||||
{
|
||||
type: 'result',
|
||||
subtype: 'error_during_execution',
|
||||
is_error: true,
|
||||
result: '',
|
||||
stop_reason: 'end_turn',
|
||||
} as unknown as SDKMessage,
|
||||
]
|
||||
const result = await forwardSessionUpdates(
|
||||
's1',
|
||||
makeStream(msgs),
|
||||
conn,
|
||||
new AbortController().signal,
|
||||
{},
|
||||
)
|
||||
expect(result.stopReason).toBe('end_turn')
|
||||
})
|
||||
|
||||
test('compact_boundary emits completion message without usage_update', async () => {
|
||||
// After audit §4.1, compact_boundary still sends the "Compacting completed."
|
||||
// agent_message_chunk but no longer emits the unstable usage_update
|
||||
// notification.
|
||||
const conn = makeConn()
|
||||
const msgs: SDKMessage[] = [
|
||||
{ type: 'system', subtype: 'compact_boundary' } as unknown as SDKMessage,
|
||||
@@ -1112,15 +1444,14 @@ describe('forwardSessionUpdates', () => {
|
||||
'sessionUpdate'
|
||||
] === 'usage_update',
|
||||
)
|
||||
expect(usageCall).toBeDefined()
|
||||
expect(
|
||||
(
|
||||
(usageCall![0] as Record<string, unknown>).update as Record<
|
||||
string,
|
||||
unknown
|
||||
>
|
||||
).used,
|
||||
).toBe(0)
|
||||
expect(usageCall).toBeUndefined()
|
||||
const messageCall = calls.find(
|
||||
(c: unknown[]) =>
|
||||
((c[0] as Record<string, Record<string, unknown>>).update ?? {})[
|
||||
'sessionUpdate'
|
||||
] === 'agent_message_chunk',
|
||||
)
|
||||
expect(messageCall).toBeDefined()
|
||||
})
|
||||
|
||||
test('ignores unknown message types without crashing', async () => {
|
||||
|
||||
Reference in New Issue
Block a user