mirror of
https://github.com/agentclientprotocol/claude-agent-acp.git
synced 2026-10-27 04:30:59 +00:00
fix: don't emit empty agent_message_chunk (#841)
## Why this is needed `toAcpNotifications` can emit an `agent_message_chunk` whose content is an empty string. This happens in two situations: 1. **Streamed text deltas** — the `text` / `text_delta` case builds the update unconditionally from `chunk.text`, and the Claude Agent SDK can stream zero-length text deltas (for example, an empty trailing assistant block at the end of a turn). 2. **Full-string content** — the string-content path emits a chunk even when the string is empty. An empty message chunk carries no information, so at best it is noise. At worst it breaks clients: the ACP spec does not forbid empty content, so clients handle it inconsistently and some throw. In JetBrains Air the frontend event consumer throws on an empty-content `agent_message_chunk` (`Failed to handle agent event`); once that happens the transcript is **frozen for the rest of the turn** — no further updates render and queued prompts are never dispatched. In practice this reproduced repeatedly around image reads and end-of-turn, and the only recovery was restarting the IDE. Because the adapter is client-agnostic, this affects any ACP client that is strict about chunk content, not a single editor — so the correct place to fix it is here, in the adapter, rather than working around it in each client. ## Fix Do not emit empty content. Both call sites are guarded, mirroring the empty-`thinking` guard that already exists a few lines below in the same function, so behavior is consistent across content types: - `text` / `text_delta`: only build the update when `chunk.text.length > 0`; - string content: return `[]` when the string is empty. The change is intentionally minimal and conservative — no information is lost, and non-empty chunks are entirely unaffected. ## Tests Three unit tests added on `toAcpNotifications`: - empty string content → no notifications emitted; - non-empty string content → still emitted unchanged; - a stream containing an empty text block followed by a non-empty one → only the non-empty block is emitted. Full suite green (476 passing); eslint and prettier clean.
This commit is contained in:
@@ -4857,6 +4857,9 @@ export function toAcpNotifications(
|
||||
const registerHooks = options?.registerHooks !== false;
|
||||
const supportsTerminalOutput = options?.clientCapabilities?._meta?.["terminal_output"] === true;
|
||||
if (typeof content === "string") {
|
||||
if (content.length === 0) {
|
||||
return [];
|
||||
}
|
||||
const update: SessionNotification["update"] = {
|
||||
sessionUpdate: role === "assistant" ? "agent_message_chunk" : "user_message_chunk",
|
||||
content: {
|
||||
@@ -4886,13 +4889,15 @@ export function toAcpNotifications(
|
||||
switch (chunk.type) {
|
||||
case "text":
|
||||
case "text_delta":
|
||||
update = {
|
||||
sessionUpdate: role === "assistant" ? "agent_message_chunk" : "user_message_chunk",
|
||||
content: {
|
||||
type: "text",
|
||||
text: chunk.text,
|
||||
},
|
||||
};
|
||||
if (chunk.text.length > 0) {
|
||||
update = {
|
||||
sessionUpdate: role === "assistant" ? "agent_message_chunk" : "user_message_chunk",
|
||||
content: {
|
||||
type: "text",
|
||||
text: chunk.text,
|
||||
},
|
||||
};
|
||||
}
|
||||
break;
|
||||
case "image":
|
||||
update = {
|
||||
|
||||
@@ -2153,3 +2153,55 @@ describe("createTaskHook", () => {
|
||||
expect(changes).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe("empty message content is not emitted", () => {
|
||||
const mockClient = {} as AcpClient;
|
||||
const mockLogger: Logger = { log: () => {}, error: () => {} };
|
||||
|
||||
it("drops an empty string content instead of emitting an empty agent_message_chunk", () => {
|
||||
const notifications = toAcpNotifications(
|
||||
"",
|
||||
"assistant",
|
||||
"test-session",
|
||||
{},
|
||||
mockClient,
|
||||
mockLogger,
|
||||
);
|
||||
expect(notifications).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("still emits non-empty string content", () => {
|
||||
const notifications = toAcpNotifications(
|
||||
"hello",
|
||||
"assistant",
|
||||
"test-session",
|
||||
{},
|
||||
mockClient,
|
||||
mockLogger,
|
||||
);
|
||||
expect(notifications).toHaveLength(1);
|
||||
expect(notifications[0].update).toMatchObject({
|
||||
sessionUpdate: "agent_message_chunk",
|
||||
content: { type: "text", text: "hello" },
|
||||
});
|
||||
});
|
||||
|
||||
it("drops an empty streamed text block but keeps a non-empty one", () => {
|
||||
const notifications = toAcpNotifications(
|
||||
[
|
||||
{ type: "text", text: "" },
|
||||
{ type: "text", text: "real" },
|
||||
] as any,
|
||||
"assistant",
|
||||
"test-session",
|
||||
{},
|
||||
mockClient,
|
||||
mockLogger,
|
||||
);
|
||||
expect(notifications).toHaveLength(1);
|
||||
expect(notifications[0].update).toMatchObject({
|
||||
sessionUpdate: "agent_message_chunk",
|
||||
content: { type: "text", text: "real" },
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user