From d7c8671b0df5dc62fb4e395604e8da949beb5009 Mon Sep 17 00:00:00 2001 From: jianYanZhiX7 Date: Sat, 19 Sep 2026 19:14:55 +0800 Subject: [PATCH 1/2] =?UTF-8?q?fix:=20ACP=20=E8=BD=AC=E5=8F=91=E4=B8=8D?= =?UTF-8?q?=E5=86=8D=E4=B8=A2=E5=BC=83=E4=B8=8A=E6=B8=B8=20API=20=E9=94=99?= =?UTF-8?q?=E8=AF=AF=E6=B6=88=E6=81=AF?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 上游流被截断且重试预算耗尽后，claude-code 会构造一条合成错误消息 （model 为 、带 isApiErrorMessage 标记）并写入会话记录。 但该消息在 ACP 转发时被静默丢弃，客户端只收到空回合 （usage used=0 + stopReason=end_turn），故障不可见。 根因有两处： 1. normalizeMessage 构造 SDKMessage 时未复制 isApiErrorMessage 字段， 转发层无从区分"合成错误消息"与"文本已经流式下发的正常回合"。 2. assistantMessageToAcpNotifications 在 streamingActive 为真时过滤 text/thinking 块以去重。原注释假设合成消息的 content 是字符串， 实际 createAssistantAPIErrorMessage 产出的是数组 [{type:'text', text}]，因此错误文本被一并滤掉， contentToProcess 为空后直接返回空数组。 改动： - queryHelpers.ts: 透传 isApiErrorMessage 字段 - notifications.ts: 合成错误消息旁路 streamingActive 过滤，并修正失实注释 - types.ts: BridgeAssistantMessage 补充该字段声明 - bridge.test.ts: 新增 2 项回归测试，覆盖完整 forwardSessionUpdates 链路 验证： - bun test src/services/acp/__tests__/bridge.test.ts: 98 通过 / 0 失败 - bun run precheck: 类型 / lint / 测试全部通过 - 反向验证：撤销转发层修复后，新增用例精确失败，恢复后通过 --- src/services/acp/__tests__/bridge.test.ts | 88 +++++++++++++++++++++++ src/services/acp/bridge/notifications.ts | 24 ++++--- src/services/acp/bridge/types.ts | 3 + src/utils/queryHelpers.ts | 3 + 4 files changed, 110 insertions(+), 8 deletions(-) diff --git a/src/services/acp/__tests__/bridge.test.ts b/src/services/acp/__tests__/bridge.test.ts index 76e82bd20b..cc13d6193f 100644 --- a/src/services/acp/__tests__/bridge.test.ts +++ b/src/services/acp/__tests__/bridge.test.ts @@ -1871,3 +1871,91 @@ describe('replayHistoryMessages — message-id (RFD)', () => { expect(typeof chunkCall!.messageId).toBe('string') }) }) + +// ── API error messages must survive streaming ──────────────────── +// +// A synthetic API error message (isApiErrorMessage) carries its text only in +// `message.content`. No stream_event ever emitted that text, so the +// streamingActive duplicate filter must not drop it — otherwise the failure +// reaches the client as an empty turn with no error surface. + +describe('forwardSessionUpdates — API error messages', () => { + test('emits agent_message_chunk for API error text while streaming is active', async () => { + const conn = makeConn() + const msgs: SDKMessage[] = [ + { + type: 'stream_event', + parent_tool_use_id: null, + event: { + type: 'content_block_delta', + delta: { type: 'text_delta', text: 'partial answer' }, + }, + }, + { + type: 'assistant', + parent_tool_use_id: null, + isApiErrorMessage: true, + message: { + model: '', + role: 'assistant', + content: [ + { + type: 'text', + text: 'API Error: OpenAI stream ended without returning any data', + }, + ], + }, + } as unknown as SDKMessage, + ] + await forwardSessionUpdates( + 's1', + makeStream(msgs), + conn, + new AbortController().signal, + {}, + ) + const calls = (conn.sessionUpdate as ReturnType).mock.calls + const texts = calls + .map(c => (c[0] as { update: Record }).update) + .filter(u => u.sessionUpdate === 'agent_message_chunk') + .map(u => (u.content as { text: string }).text) + expect(texts).toContain( + 'API Error: OpenAI stream ended without returning any data', + ) + }) + + test('still filters the streamed duplicate of a normal assistant message', async () => { + const conn = makeConn() + const msgs: SDKMessage[] = [ + { + type: 'stream_event', + parent_tool_use_id: null, + event: { + type: 'content_block_delta', + delta: { type: 'text_delta', text: 'Answer' }, + }, + }, + { + type: 'assistant', + parent_tool_use_id: null, + message: { + role: 'assistant', + content: [{ type: 'text', text: 'Answer' }], + }, + } as unknown as SDKMessage, + ] + await forwardSessionUpdates( + 's1', + makeStream(msgs), + conn, + new AbortController().signal, + {}, + ) + const calls = (conn.sessionUpdate as ReturnType).mock.calls + const texts = calls + .map(c => (c[0] as { update: Record }).update) + .filter(u => u.sessionUpdate === 'agent_message_chunk') + .map(u => (u.content as { text: string }).text) + expect(texts).toEqual(['Answer']) + }) +}) diff --git a/src/services/acp/bridge/notifications.ts b/src/services/acp/bridge/notifications.ts index f4e7a9592e..cb2b3bde87 100644 --- a/src/services/acp/bridge/notifications.ts +++ b/src/services/acp/bridge/notifications.ts @@ -235,7 +235,11 @@ export function toAcpNotifications( } export function assistantMessageToAcpNotifications( - msg: { message?: unknown; parent_tool_use_id?: string | null }, + msg: { + message?: unknown + parent_tool_use_id?: string | null + isApiErrorMessage?: boolean + }, sessionId: string, toolUseCache: ToolUseCache, conn: AgentSideConnection, @@ -272,13 +276,17 @@ export function assistantMessageToAcpNotifications( // When streaming is active, text/thinking were already sent via stream_event // messages. Filter them out to avoid duplicate agent_message_chunk / - // agent_thought_chunk notifications. String content (synthetic messages) - // is unaffected — those have no corresponding stream_events. - const contentToProcess = options?.streamingActive - ? content.filter( - block => block.type !== 'text' && block.type !== 'thinking', - ) - : content + // agent_thought_chunk notifications. + // + // API error messages bypass this filter. They are synthetic — no stream_event + // ever carried their text — so filtering would drop the only copy and the + // failure would reach the client as a silent, empty turn. + const contentToProcess = + options?.streamingActive && msg.isApiErrorMessage !== true + ? content.filter( + block => block.type !== 'text' && block.type !== 'thinking', + ) + : content if (contentToProcess.length === 0) return [] diff --git a/src/services/acp/bridge/types.ts b/src/services/acp/bridge/types.ts index bdb8031f83..036c761621 100644 --- a/src/services/acp/bridge/types.ts +++ b/src/services/acp/bridge/types.ts @@ -91,6 +91,9 @@ export type BridgeAssistantMessage = { uuid?: string session_id?: string error?: unknown + // Set on synthetic messages that carry an upstream API failure. Their text + // exists only here, so the bridge must not treat it as a streamed duplicate. + isApiErrorMessage?: boolean [key: string]: unknown } diff --git a/src/utils/queryHelpers.ts b/src/utils/queryHelpers.ts index 94dbb74044..ea0a7a138b 100644 --- a/src/utils/queryHelpers.ts +++ b/src/utils/queryHelpers.ts @@ -124,6 +124,9 @@ export function* normalizeMessage(message: Message): Generator { session_id: getSessionId(), uuid: _.uuid, error: _.error, + // Carried through so the ACP bridge can tell a synthetic upstream + // failure from a real assistant turn whose text was already streamed. + ...(_.isApiErrorMessage ? { isApiErrorMessage: true } : {}), } } return From 5cd6842623ef9846951a38c38a2377fee6905ac7 Mon Sep 17 00:00:00 2001 From: jianYanZhiX7 Date: Sat, 19 Sep 2026 19:37:09 +0800 Subject: [PATCH 2/2] =?UTF-8?q?test:=20=E8=A1=A5=E5=85=85=20ACP=20?= =?UTF-8?q?=E9=94=99=E8=AF=AF=E6=B6=88=E6=81=AF=E9=80=8F=E4=BC=A0=E7=9A=84?= =?UTF-8?q?=E7=AB=AF=E5=88=B0=E7=AB=AF=E5=9B=9E=E5=BD=92=E6=B5=8B=E8=AF=95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 现有 2 项回归测试手工构造 SDKMessage 并预置 isApiErrorMessage 字段， 绕过了 normalizeMessage，无法守护 queryHelpers.ts 的字段透传改动： 撤销该透传后 98 项测试仍全绿，而生产链路上错误文本会再次被静默丢弃。 新增用例驱动真实链路：createAssistantAPIErrorMessage（查询引擎同款 工厂）→ normalizeMessage → forwardSessionUpdates，先断言字段透传， 再断言错误文本到达客户端。 反向验证：撤销 queryHelpers.ts 透传后，新用例精确失败（98 pass / 1 fail），恢复后通过。 验证： - bun test src/services/acp/__tests__/bridge.test.ts: 99 通过 / 0 失败 - bun run precheck: 类型 / lint / 测试全部通过（6025 通过 / 0 失败） --- src/services/acp/__tests__/bridge.test.ts | 47 +++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/src/services/acp/__tests__/bridge.test.ts b/src/services/acp/__tests__/bridge.test.ts index cc13d6193f..bb3a73ddb6 100644 --- a/src/services/acp/__tests__/bridge.test.ts +++ b/src/services/acp/__tests__/bridge.test.ts @@ -11,6 +11,8 @@ import { promptToQueryInput } from '../promptConversion.js' import { markdownEscape, toDisplayPath } from '../utils.js' import type { AgentSideConnection, ToolKind } from '@agentclientprotocol/sdk' import type { SDKMessage } from '../../../entrypoints/sdk/coreTypes.js' +import { createAssistantAPIErrorMessage } from '../../../utils/messages.js' +import { normalizeMessage } from '../../../utils/queryHelpers.js' // ── Helpers ──────────────────────────────────────────────────────── @@ -1958,4 +1960,49 @@ describe('forwardSessionUpdates — API error messages', () => { .map(u => (u.content as { text: string }).text) expect(texts).toEqual(['Answer']) }) + + // Unlike the two tests above, which hand-build the SDKMessage with the flag + // pre-set, this one drives the real pipeline: the same factory the query + // engine uses (createAssistantAPIErrorMessage) → normalizeMessage → bridge. + // The passthrough in normalizeMessage is what marks the error text as + // never-streamed. If that passthrough is ever removed, the bridge can no + // longer tell the difference and the dedup filter drops the text — this + // test fails while the hand-built ones stay green. + test('end-to-end: real createAssistantAPIErrorMessage survives normalizeMessage', async () => { + const conn = makeConn() + const errorMsg = createAssistantAPIErrorMessage({ + content: 'API Error: probe', + }) + const sdkMsgs = [...normalizeMessage(errorMsg)] + + // Assert the bridge-visible flag directly for a precise failure signal. + expect( + (sdkMsgs[0] as unknown as Record).isApiErrorMessage, + ).toBe(true) + + const msgs: SDKMessage[] = [ + { + type: 'stream_event', + parent_tool_use_id: null, + event: { + type: 'content_block_delta', + delta: { type: 'text_delta', text: 'partial answer' }, + }, + } as unknown as SDKMessage, + ...sdkMsgs, + ] + await forwardSessionUpdates( + 's1', + makeStream(msgs), + conn, + new AbortController().signal, + {}, + ) + const calls = (conn.sessionUpdate as ReturnType).mock.calls + const texts = calls + .map(c => (c[0] as { update: Record }).update) + .filter(u => u.sessionUpdate === 'agent_message_chunk') + .map(u => (u.content as { text: string }).text) + expect(texts).toContain('API Error: probe') + }) })