From 96d766d7f102a6217ffae044a1caf719739398a3 Mon Sep 17 00:00:00 2001 From: Junmo Kim Date: Thu, 23 Apr 2026 21:45:20 +0900 Subject: [PATCH] feat(acp): forward agent_thought_chunk as reasoning message (#520) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor(agent): extend AgentMessage and CodexMessage unions with reasoning variant Add a reasoning variant to the shared AgentMessage union that flows out of the ACP backend, and pass it through to CodexMessage so the existing web reducer (which already renders { type: 'reasoning' } parts as collapsible blocks) can consume ACP-sourced thoughts identically to Codex. No behavior change yet: the ACP handler still drops thought chunks, and the remote launchers receive the new variant as a no-op. The behavior is wired up in the following commit. * feat(acp): forward agent_thought_chunk as reasoning message Route ACP thought chunks to the session as reasoning AgentMessages so OpenCode and Gemini thinking output reaches the web UI's Reasoning block, matching the existing Codex behavior. Thought chunks are emitted inline without flushing the pending text buffer — text and thought live on independent interleave lanes, so splitting a live text segment on every thought arrival would be wrong. The inline-emit ordering is documented alongside the test that depends on it. extractTextContent is not reused for thought content: its assistant-audience filter is correct for regular message chunks but would silently drop thoughts annotated with a non-assistant audience, which have no meaningful audience to filter against. A direct text block shape check handles the narrower need. In the remote launchers, reasoning is surfaced to the local terminal buffer as a truncated system-role hint prefixed with [Thinking], matching how the Codex flavor already displays reasoning chunks in-terminal without mixing them into the assistant reply stream. --- .../backends/acp/AcpMessageHandler.test.ts | 122 ++++++++++++++++++ .../agent/backends/acp/AcpMessageHandler.ts | 21 ++- cli/src/agent/messageConverter.ts | 7 + cli/src/agent/types.ts | 1 + cli/src/gemini/geminiRemoteLauncher.ts | 3 + cli/src/opencode/opencodeRemoteLauncher.ts | 3 + 6 files changed, 154 insertions(+), 3 deletions(-) diff --git a/cli/src/agent/backends/acp/AcpMessageHandler.test.ts b/cli/src/agent/backends/acp/AcpMessageHandler.test.ts index 2418748e..095dca85 100644 --- a/cli/src/agent/backends/acp/AcpMessageHandler.test.ts +++ b/cli/src/agent/backends/acp/AcpMessageHandler.test.ts @@ -657,4 +657,126 @@ describe('AcpMessageHandler', () => { expect(messages).toHaveLength(1); expect((messages[0] as { text: string }).text).toMatch(/^Claude AI usage limit warning\|/); }); + + it('forwards agent_thought_chunk as a reasoning message', () => { + const messages: AgentMessage[] = []; + const handler = new AcpMessageHandler((message) => messages.push(message)); + + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.agentThoughtChunk, + content: { type: 'text', text: 'thinking about the problem' } + }); + + expect(messages).toHaveLength(1); + expect(messages[0]).toEqual({ type: 'reasoning', text: 'thinking about the problem' }); + }); + + it('silently drops agent_thought_chunk when content is not a text block', () => { + const messages: AgentMessage[] = []; + const handler = new AcpMessageHandler((message) => messages.push(message)); + + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.agentThoughtChunk, + content: { type: 'image', url: 'https://example.com/img.png' } + }); + + expect(messages).toHaveLength(0); + }); + + it('does not flush the text buffer when a thought chunk arrives mid-stream', () => { + const messages: AgentMessage[] = []; + const handler = new AcpMessageHandler((message) => messages.push(message)); + + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.agentMessageChunk, + content: { type: 'text', text: 'partial answer' } + }); + + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.agentThoughtChunk, + content: { type: 'text', text: 'mid-stream thought' } + }); + + handler.flushText(); + + // Both messages are delivered intact with no loss. Reasoning is + // emitted inline (see AcpMessageHandler) so it precedes the + // flushed text segment — this is an intentional contract to let + // thoughts and text interleave without splitting a live segment. + expect(messages).toHaveLength(2); + expect(messages).toContainEqual({ type: 'reasoning', text: 'mid-stream thought' }); + expect(messages).toContainEqual({ type: 'text', text: 'partial answer' }); + expect(messages[0]).toEqual({ type: 'reasoning', text: 'mid-stream thought' }); + }); + + it('does not drop thought chunks annotated with a non-assistant audience', () => { + const messages: AgentMessage[] = []; + const handler = new AcpMessageHandler((message) => messages.push(message)); + + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.agentThoughtChunk, + content: { + type: 'text', + text: 'private reasoning', + annotations: { audience: ['user'] } + } + }); + + expect(messages).toHaveLength(1); + expect(messages[0]).toEqual({ type: 'reasoning', text: 'private reasoning' }); + }); + + it('forwards sequential thought chunks in arrival order as separate reasoning messages', () => { + const messages: AgentMessage[] = []; + const handler = new AcpMessageHandler((message) => messages.push(message)); + + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.agentThoughtChunk, + content: { type: 'text', text: 'first thought' } + }); + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.agentThoughtChunk, + content: { type: 'text', text: 'second thought' } + }); + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.agentThoughtChunk, + content: { type: 'text', text: 'third thought' } + }); + + expect(messages).toEqual([ + { type: 'reasoning', text: 'first thought' }, + { type: 'reasoning', text: 'second thought' }, + { type: 'reasoning', text: 'third thought' } + ]); + }); + + it('silently drops agent_thought_chunk with empty text', () => { + const messages: AgentMessage[] = []; + const handler = new AcpMessageHandler((message) => messages.push(message)); + + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.agentThoughtChunk, + content: { type: 'text', text: '' } + }); + + expect(messages).toHaveLength(0); + }); + + it.each([ + ['null', null], + ['undefined', undefined], + ['number', 42], + ['string', 'not a block'], + ['array', ['text']] + ])('silently drops agent_thought_chunk when content is %s', (_label, content) => { + const messages: AgentMessage[] = []; + const handler = new AcpMessageHandler((message) => messages.push(message)); + + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.agentThoughtChunk, + content + }); + + expect(messages).toHaveLength(0); + }); }); diff --git a/cli/src/agent/backends/acp/AcpMessageHandler.ts b/cli/src/agent/backends/acp/AcpMessageHandler.ts index 0224b666..c3f934c3 100644 --- a/cli/src/agent/backends/acp/AcpMessageHandler.ts +++ b/cli/src/agent/backends/acp/AcpMessageHandler.ts @@ -197,9 +197,24 @@ export class AcpMessageHandler { } if (updateType === ACP_SESSION_UPDATE_TYPES.agentThoughtChunk) { - // Thought chunks are not forwarded as messages, so they do not - // participate in intra-turn ordering and must not flush the - // text buffer (that would split a live text segment). + // Thought chunks do not participate in intra-turn ordering and + // must not flush the text buffer (that would split a live text + // segment). Forward as a reasoning message so the web UI can + // render the model's thinking in a collapsible block. + // + // Reasoning messages are emitted inline (never buffered), so they + // arrive before any still-pending text segment is flushed. Tests + // in this file rely on that contract. + // + // We deliberately do not reuse `extractTextContent` here: that + // helper applies an assistant-audience filter which only makes + // sense for regular message chunks. Thought content has no + // meaningful audience — a non-assistant audience annotation + // should not cause the reasoning to be silently dropped. + const content = update.content; + if (isObject(content) && content.type === 'text' && typeof content.text === 'string' && content.text.length > 0) { + this.onMessage({ type: 'reasoning', text: content.text }); + } return; } diff --git a/cli/src/agent/messageConverter.ts b/cli/src/agent/messageConverter.ts index 9bf8d6a1..c51d982f 100644 --- a/cli/src/agent/messageConverter.ts +++ b/cli/src/agent/messageConverter.ts @@ -1,7 +1,9 @@ +import { randomUUID } from 'node:crypto'; import type { AgentMessage, PlanItem } from './types'; export type CodexMessage = | { type: 'message'; message: string } + | { type: 'reasoning'; message: string; id: string } | { type: 'tool-call'; name: string; @@ -22,6 +24,11 @@ export function convertAgentMessage(message: AgentMessage): CodexMessage | null switch (message.type) { case 'text': return { type: 'message', message: message.text }; + case 'reasoning': + // AgentMessage uses `text` (consistent with the `text` variant); + // the wire-level CodexMessage uses `message` to match the + // existing reasoning format emitted by the Codex path. + return { type: 'reasoning', message: message.text, id: randomUUID() }; case 'tool_call': return { type: 'tool-call', diff --git a/cli/src/agent/types.ts b/cli/src/agent/types.ts index 1441e4c1..ad8c12e1 100644 --- a/cli/src/agent/types.ts +++ b/cli/src/agent/types.ts @@ -28,6 +28,7 @@ export type PlanItem = { export type AgentMessage = | { type: 'text'; text: string } + | { type: 'reasoning'; text: string } | { type: 'tool_call'; id: string; name: string; input: unknown; status: 'pending' | 'in_progress' | 'completed' | 'failed' } | { type: 'tool_result'; id: string; output: unknown; status: 'completed' | 'failed' } | { type: 'plan'; items: PlanItem[] } diff --git a/cli/src/gemini/geminiRemoteLauncher.ts b/cli/src/gemini/geminiRemoteLauncher.ts index 40b8ffa6..aad2145d 100644 --- a/cli/src/gemini/geminiRemoteLauncher.ts +++ b/cli/src/gemini/geminiRemoteLauncher.ts @@ -183,6 +183,9 @@ class GeminiRemoteLauncher extends RemoteLauncherBase { case 'text': this.messageBuffer.addMessage(message.text, 'assistant'); break; + case 'reasoning': + this.messageBuffer.addMessage(`[Thinking] ${message.text.substring(0, 100)}...`, 'system'); + break; case 'tool_call': this.messageBuffer.addMessage(`Tool call: ${message.name}`, 'tool'); break; diff --git a/cli/src/opencode/opencodeRemoteLauncher.ts b/cli/src/opencode/opencodeRemoteLauncher.ts index 1e5b1fe1..0be617ef 100644 --- a/cli/src/opencode/opencodeRemoteLauncher.ts +++ b/cli/src/opencode/opencodeRemoteLauncher.ts @@ -178,6 +178,9 @@ class OpencodeRemoteLauncher extends RemoteLauncherBase { case 'text': this.messageBuffer.addMessage(message.text, 'assistant'); break; + case 'reasoning': + this.messageBuffer.addMessage(`[Thinking] ${message.text.substring(0, 100)}...`, 'system'); + break; case 'tool_call': this.messageBuffer.addMessage(`Tool call: ${message.name}`, 'tool'); break;