diff --git a/cli/src/agent/backends/acp/AcpMessageHandler.test.ts b/cli/src/agent/backends/acp/AcpMessageHandler.test.ts index cc78a6a2..66eb8390 100644 --- a/cli/src/agent/backends/acp/AcpMessageHandler.test.ts +++ b/cli/src/agent/backends/acp/AcpMessageHandler.test.ts @@ -364,4 +364,147 @@ describe('AcpMessageHandler', () => { expect(calls[0].name).toBe('Tool'); expect(calls[1].name).toBe('search'); }); + + it('drops leaked session metadata envelope from text buffer', () => { + const messages: AgentMessage[] = []; + const handler = new AcpMessageHandler((message) => messages.push(message)); + + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.agentMessageChunk, + content: { type: 'text', text: 'real answer' } + }); + + // Leaked metadata envelope with parentUuid string + const metadataJson = JSON.stringify({ + type: 'output', + data: { + parentUuid: 'abc-123', + isSidechain: false, + userType: 'external', + sessionId: 'session-456', + version: '0.0.0', + }, + }); + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.agentMessageChunk, + content: { type: 'text', text: metadataJson } + }); + + handler.flushText(); + + expect(messages).toEqual([{ type: 'text', text: 'real answer' }]); + }); + + it('drops leaked root metadata envelope with parentUuid: null', () => { + const messages: AgentMessage[] = []; + const handler = new AcpMessageHandler((message) => messages.push(message)); + + const metadataJson = JSON.stringify({ + type: 'output', + data: { + parentUuid: null, + sessionId: 'session-789', + userType: 'external', + }, + }); + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.agentMessageChunk, + content: { type: 'text', text: metadataJson } + }); + + handler.flushText(); + + expect(messages).toEqual([]); + }); + + it('clears buffered prefix when cumulative metadata chunk arrives', () => { + const messages: AgentMessage[] = []; + const handler = new AcpMessageHandler((message) => messages.push(message)); + + // First chunk: incomplete JSON prefix + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.agentMessageChunk, + content: { type: 'text', text: '{"type":"ou' } + }); + + // Second chunk: full cumulative metadata JSON (starts with buffered prefix) + const metadataJson = JSON.stringify({ + type: 'output', + data: { + parentUuid: 'abc-123', + sessionId: 'session-456', + userType: 'external', + }, + }); + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.agentMessageChunk, + content: { type: 'text', text: metadataJson } + }); + + handler.flushText(); + + // Both the prefix and the full chunk should be gone + expect(messages).toEqual([]); + }); + + it('clears buffered prefix when cumulative rate_limit_event chunk arrives', () => { + const messages: AgentMessage[] = []; + const handler = new AcpMessageHandler((message) => messages.push(message)); + + // First chunk: incomplete JSON prefix + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.agentMessageChunk, + content: { type: 'text', text: '{"type":"rate' } + }); + + // Second chunk: full cumulative rate_limit_event (allowed — should be suppressed) + const rateLimitJson = JSON.stringify({ + type: 'rate_limit_event', + rate_limit_info: { + status: 'allowed', + resetsAt: 1774278000, + }, + }); + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.agentMessageChunk, + content: { type: 'text', text: rateLimitJson } + }); + + handler.flushText(); + + // Both the prefix and the full chunk should be gone + expect(messages).toEqual([]); + }); + + it('clears buffered prefix when cumulative displayable rate_limit_event arrives', () => { + const messages: AgentMessage[] = []; + const handler = new AcpMessageHandler((message) => messages.push(message)); + + // First chunk: incomplete prefix + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.agentMessageChunk, + content: { type: 'text', text: '{"type":"rate' } + }); + + // Second chunk: full rate_limit_event with displayable status + const rateLimitJson = JSON.stringify({ + type: 'rate_limit_event', + rate_limit_info: { + status: 'allowed_warning', + resetsAt: 1774278000, + utilization: 0.9, + rateLimitType: 'five_hour', + }, + }); + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.agentMessageChunk, + content: { type: 'text', text: rateLimitJson } + }); + + handler.flushText(); + + // Should only have the converted warning, no raw JSON prefix + expect(messages).toHaveLength(1); + expect((messages[0] as { text: string }).text).toMatch(/^Claude AI usage limit warning\|/); + }); }); diff --git a/cli/src/agent/backends/acp/AcpMessageHandler.ts b/cli/src/agent/backends/acp/AcpMessageHandler.ts index c99604d1..c732f7ce 100644 --- a/cli/src/agent/backends/acp/AcpMessageHandler.ts +++ b/cli/src/agent/backends/acp/AcpMessageHandler.ts @@ -2,6 +2,7 @@ import type { AgentMessage, PlanItem } from '@/agent/types'; import { asString, isObject } from '@hapi/protocol'; import { deriveToolNameWithSource, isPlaceholderToolName } from '@/agent/utils'; import { parseRateLimitText } from '@/agent/rateLimitParser'; +import { isInternalEventJson } from '@/agent/internalEventFilter'; import { ACP_SESSION_UPDATE_TYPES } from './constants'; function normalizeStatus(status: unknown): 'pending' | 'in_progress' | 'completed' | 'failed' { @@ -158,8 +159,17 @@ export class AcpMessageHandler { const content = update.content; const text = extractTextContent(content); if (text) { + // Check once whether the buffered text is a prefix of this + // chunk (cumulative streaming). Used below by both the + // rate-limit and internal-event filters to clear stale + // prefixes that would otherwise leak on flushText(). + const hadBufferedPrefix = this.bufferedText !== '' && text.startsWith(this.bufferedText); + const rateLimit = parseRateLimitText(text); if (rateLimit) { + if (hadBufferedPrefix) { + this.bufferedText = ''; + } if (rateLimit.suppress) { return; } @@ -167,6 +177,14 @@ export class AcpMessageHandler { this.onMessage(rateLimit.message); return; } + // Drop internal event JSON (e.g. { type: "output", data: { ... } }) + // that should never appear as visible text. + if (isInternalEventJson(text)) { + if (hadBufferedPrefix) { + this.bufferedText = ''; + } + return; + } this.appendTextChunk(text); } return; diff --git a/cli/src/agent/internalEventFilter.test.ts b/cli/src/agent/internalEventFilter.test.ts new file mode 100644 index 00000000..9167df92 --- /dev/null +++ b/cli/src/agent/internalEventFilter.test.ts @@ -0,0 +1,86 @@ +import { describe, expect, it } from 'vitest'; +import { isInternalEventJson } from './internalEventFilter'; + +describe('isInternalEventJson', () => { + it('returns false for non-JSON text', () => { + expect(isInternalEventJson('Hello world')).toBe(false); + }); + + it('returns false for JSON without a type field', () => { + expect(isInternalEventJson('{"foo":"bar"}')).toBe(false); + }); + + it('returns true for the leaked session metadata envelope', () => { + const json = JSON.stringify({ + type: 'output', + data: { + parentUuid: 'abc-123', + isSidechain: false, + userType: 'external', + cwd: '/home/user/project', + sessionId: 'session-456', + version: '0.0.0', + uuid: 'def-789', + timestamp: '2026-04-05T00:00:00Z', + }, + }); + expect(isInternalEventJson(json)).toBe(true); + }); + + it('returns true for minimal metadata envelope shape', () => { + const json = JSON.stringify({ + type: 'output', + data: { + parentUuid: 'abc', + sessionId: '123', + userType: 'external', + }, + }); + expect(isInternalEventJson(json)).toBe(true); + }); + + it('returns true for root metadata envelope with parentUuid: null', () => { + const json = JSON.stringify({ + type: 'output', + data: { + parentUuid: null, + sessionId: '123', + userType: 'external', + }, + }); + expect(isInternalEventJson(json)).toBe(true); + }); + + it('returns false for output with non-metadata data', () => { + // Legitimate output that happens to have type "output" but different data shape + const json = JSON.stringify({ + type: 'output', + data: { text: 'some result' }, + }); + expect(isInternalEventJson(json)).toBe(false); + }); + + it('returns false for { type: "event" } — not the leaked shape', () => { + const json = JSON.stringify({ type: 'event', data: { type: 'ready' } }); + expect(isInternalEventJson(json)).toBe(false); + }); + + it('returns false for { type: "queue-operation" } — not the leaked shape', () => { + const json = JSON.stringify({ type: 'queue-operation', op: 'enqueue' }); + expect(isInternalEventJson(json)).toBe(false); + }); + + it('returns false for other JSON types (assistant, user)', () => { + expect(isInternalEventJson('{"type":"assistant"}')).toBe(false); + expect(isInternalEventJson('{"type":"user"}')).toBe(false); + }); + + it('returns false for invalid JSON starting with {', () => { + expect(isInternalEventJson('{not valid json')).toBe(false); + }); + + it('returns false when output data is not an object', () => { + const json = JSON.stringify({ type: 'output', data: 'string-data' }); + expect(isInternalEventJson(json)).toBe(false); + }); +}); diff --git a/cli/src/agent/internalEventFilter.ts b/cli/src/agent/internalEventFilter.ts new file mode 100644 index 00000000..6722227b --- /dev/null +++ b/cli/src/agent/internalEventFilter.ts @@ -0,0 +1,38 @@ +/** + * Detect internal session-metadata JSON that leaks into agent text output. + * + * Claude's SDK occasionally emits internal control messages as text chunks. + * The known leaked shape is the session metadata envelope: + * { type: "output", data: { parentUuid, sessionId, userType, ... } } + * + * We match on the specific structure rather than a broad type allowlist to + * avoid accidentally suppressing legitimate assistant JSON. + * + * Only called for text that starts with '{', so the fast-path for normal + * prose has zero overhead. + */ +export function isInternalEventJson(text: string): boolean { + if (text[0] !== '{') return false; + + let parsed: unknown; + try { + parsed = JSON.parse(text); + } catch { + return false; + } + if (typeof parsed !== 'object' || parsed === null) return false; + + const record = parsed as Record; + + // Match the known leaked metadata envelope: + // { type: "output", data: { parentUuid, sessionId, userType, ... } } + if (record.type === 'output' && typeof record.data === 'object' && record.data !== null) { + const data = record.data as Record; + const hasParentUuid = typeof data.parentUuid === 'string' || data.parentUuid === null; + return hasParentUuid + && typeof data.sessionId === 'string' + && typeof data.userType === 'string'; + } + + return false; +} diff --git a/cli/src/agent/rateLimitParser.test.ts b/cli/src/agent/rateLimitParser.test.ts index cae410bc..81618afd 100644 --- a/cli/src/agent/rateLimitParser.test.ts +++ b/cli/src/agent/rateLimitParser.test.ts @@ -106,7 +106,7 @@ describe('parseRateLimitText', () => { expect(result).toEqual({ suppress: true }); }); - it('passes through unknown statuses (returns null)', () => { + it('suppresses unknown statuses to prevent raw JSON leaking', () => { const result = parseRateLimitText(JSON.stringify({ type: 'rate_limit_event', rate_limit_info: { @@ -115,7 +115,7 @@ describe('parseRateLimitText', () => { }, })); - expect(result).toBeNull(); + expect(result).toEqual({ suppress: true }); }); it('handles wrapped { type: "output", data: { ... } } format', () => { @@ -140,7 +140,7 @@ describe('parseRateLimitText', () => { }); }); - it('returns null when resetsAt is missing', () => { + it('suppresses when resetsAt is missing to prevent raw JSON leak', () => { const result = parseRateLimitText(JSON.stringify({ type: 'rate_limit_event', rate_limit_info: { @@ -148,6 +148,6 @@ describe('parseRateLimitText', () => { }, })); - expect(result).toBeNull(); + expect(result).toEqual({ suppress: true }); }); }); diff --git a/cli/src/agent/rateLimitParser.ts b/cli/src/agent/rateLimitParser.ts index 156c7dc4..b811a523 100644 --- a/cli/src/agent/rateLimitParser.ts +++ b/cli/src/agent/rateLimitParser.ts @@ -42,7 +42,21 @@ export function parseRateLimitText(text: string): RateLimitResult { if (typeof info !== 'object' || info === null) return null; const { status, resetsAt, utilization, rateLimitType } = info as Record; - if (typeof resetsAt !== 'number') return null; + + // Suppress early for statuses that never need display, + // before checking resetsAt — malformed payloads should not leak. + if (status === 'allowed') { + return { suppress: true }; + } + + if (typeof resetsAt !== 'number') { + // Malformed rate_limit_event (missing resetsAt) — suppress to prevent + // raw JSON from leaking into chat. + return { suppress: true }; + } + + // Ensure integer for the pipe-delimited format (web regex uses \d+) + const resetsAtInt = Math.round(resetsAt); if (status === 'allowed_warning') { const pct = typeof utilization === 'number' ? Math.round(utilization * 100) : 0; @@ -51,7 +65,7 @@ export function parseRateLimitText(text: string): RateLimitResult { suppress: false, message: { type: 'text', - text: `Claude AI usage limit warning|${resetsAt}|${pct}|${limitType}`, + text: `Claude AI usage limit warning|${resetsAtInt}|${pct}|${limitType}`, }, }; } @@ -62,16 +76,12 @@ export function parseRateLimitText(text: string): RateLimitResult { suppress: false, message: { type: 'text', - text: `Claude AI usage limit reached|${resetsAt}|${limitType}`, + text: `Claude AI usage limit reached|${resetsAtInt}|${limitType}`, }, }; } - if (status === 'allowed') { - return { suppress: true }; - } - - // Unknown status — return null so the original text passes through. - // Suppressing unknown statuses risks hiding important new events. - return null; + // Unknown status — suppress to prevent raw JSON from leaking into chat. + // If a new status needs to be displayed, add an explicit branch above. + return { suppress: true }; }