From c0ee83c8d0cdee93a7fefb3531869b3161c5f9ee Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 20 Aug 2026 17:25:31 -0700 Subject: [PATCH] fix(native-chat): trust Claude's own isReplay marker for the send identity "Not a tool result" was too weak. Claude publishes harness-injected turns - interruption notices, system reminders, local-command output - in the same top-level `user` shape with a null parent_tool_use_id, and those are the WORSE half of the class: unlike a tool result they do build a journal body, so adopting one upserts harness text over the user's own bubble and still leaves the real replay to append as a second row. `--replay-user-messages` stamps `isReplay` on the genuine echo, so trust that when present. Keep a shape test as the fallback for a CLI that omits the field, now excluding harness turns via the repo's existing `isKnownHarnessInjectedUserTurnText` rather than a second enumeration. Also accept string `content`. `readClaudeMessageEnvelope` already tolerates it, so rejecting it was a new way for a send to go unacknowledged - which times out into a false "delivery unconfirmed" and sends the message twice on retry. Found by the review loop on #15657. --- .../claude/claude-structured-dispatch.test.ts | 54 +++++++++++++++++ src/main/claude/claude-structured-dispatch.ts | 60 ++++++++++++------- 2 files changed, 93 insertions(+), 21 deletions(-) diff --git a/src/main/claude/claude-structured-dispatch.test.ts b/src/main/claude/claude-structured-dispatch.test.ts index ef4602ca6b1..b770eacc38f 100644 --- a/src/main/claude/claude-structured-dispatch.test.ts +++ b/src/main/claude/claude-structured-dispatch.test.ts @@ -128,6 +128,60 @@ describe('Claude structured dispatch image limits', () => { }) }) + // An injected turn is the worse half of this class: unlike a tool result it + // DOES build a journal body, so adopting its uuid upserts harness text over the + // user's own bubble AND leaves the real replay to append as a second row. + it('does not adopt a harness-injected user turn as the user replay', async () => { + const session = sessionFor() + const dispatched = dispatchClaudeTurn( + session, + { clientMessageId: 'client-1', body: userMessage([{ type: 'text', text: 'queued' }]) }, + 100 + ) + await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) + + resolveClaudeReplayWaiter(session, { + type: 'user', + message: { role: 'user', content: [{ type: 'text', text: '[Request interrupted by user]' }] }, + parent_tool_use_id: null, + session_id: 'provider-session', + uuid: 'injected-uuid' + }) + expect(session.dispatchWaiters).toHaveLength(1) + + resolveClaudeReplayWaiter(session, userReplayFrame('user-replay-uuid', 'queued')) + + await expect(dispatched).resolves.toMatchObject({ + state: 'accepted', + providerIdentity: { uuid: 'user-replay-uuid' } + }) + }) + + // `readClaudeMessageEnvelope` already tolerates string content, so refusing it + // here would be a new way for a send to never be acknowledged. + it('accepts a replay whose content is a plain string', async () => { + const session = sessionFor() + const dispatched = dispatchClaudeTurn( + session, + { clientMessageId: 'client-1', body: userMessage([{ type: 'text', text: 'hello' }]) }, + 100 + ) + await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) + + resolveClaudeReplayWaiter(session, { + type: 'user', + message: { role: 'user', content: 'hello' }, + parent_tool_use_id: null, + session_id: 'provider-session', + uuid: 'string-replay-uuid' + }) + + await expect(dispatched).resolves.toMatchObject({ + state: 'accepted', + providerIdentity: { uuid: 'string-replay-uuid' } + }) + }) + // Dispatch base64-encodes a local image path, and `claudeMessageBody` models // only text and URL images - so gating the waiter on "the translator would // build a body" refuses an image-only send its own replay, times out, and diff --git a/src/main/claude/claude-structured-dispatch.ts b/src/main/claude/claude-structured-dispatch.ts index 174aa5f714a..739ef8a3cbe 100644 --- a/src/main/claude/claude-structured-dispatch.ts +++ b/src/main/claude/claude-structured-dispatch.ts @@ -5,6 +5,7 @@ import type { NativeChatBlock } from '../../shared/native-chat-types' import type { AgentSessionDispatchOutcome } from '../native-chat/agent-session-wire/structured-agent-session-adapter' import type { ClaudeSession } from './claude-structured-session-state' import { readClaudeFrameString } from './claude-structured-init-proof' +import { isKnownHarnessInjectedUserTurnText } from '../../shared/harness-injected-user-turns' import { claudeRecord } from './claude-structured-item-translation' const MAX_IMAGE_BYTES = 5 * 1024 * 1024 @@ -51,37 +52,54 @@ const IMAGE_MIME_BY_EXTENSION: Record = { '.webp': 'image/webp' } +/** Text of a top-level user frame, or null when it is a tool-result turn (or + * carries nothing). `content` may be a plain string - a shape + * `readClaudeMessageEnvelope` already tolerates. An image-only echo has no text + * and correctly yields ''. */ +function claudeUserFrameText(content: unknown): string | null { + if (typeof content === 'string') { + return content + } + if (!Array.isArray(content) || content.length === 0) { + return null + } + const parts = content.map((part) => claudeRecord(part)) + if (parts.some((part) => part?.type === 'tool_result')) { + return null + } + return parts.map((part) => (typeof part?.text === 'string' ? part.text : '')).join('\n') +} + /** * The frame that carries the user's own message back. * - * Claude publishes a top-level `user` turn for tool results too, with the same - * null `parent_tool_use_id`, so "any user turn" adopts whichever arrives first. - * A send issued mid-turn is dispatched while tools are running, and adopting a - * tool-result uuid leaves the real replay to land under an identity nothing - * claims — the user's message then renders twice, once from the submission row - * and once from the replay. + * `type: 'user'` with a null `parent_tool_use_id` is not enough on its own: + * Claude publishes tool results and harness-injected turns (interruption + * notices, system reminders, local-command output) in the same shape. Adopting + * one of those leaves the real replay to land under an identity nothing claims, + * so the user's message renders twice - and an injected turn is worse than a + * tool result, because it DOES build a body and upserts over the user's own + * bubble. * - * Measured against the real CLI: a queued send's replay is published at the first - * tool boundary after it is queued, 2-3 ms AFTER that boundary's tool result. So - * rejecting tool results costs no time — whenever there was a frame to adopt, the - * right one follows immediately. + * `--replay-user-messages` stamps `isReplay` on the genuine echo, so trust that + * when it is there. The shape test is the fallback for a CLI that omits it: + * without one, a send that is never acknowledged times out into a false + * "delivery unconfirmed", and retrying it sends the message to Claude twice. * - * Reject on the tool-result shape rather than on "the journal translator would - * build a body from this": `claudeMessageBody` models only text and URL images, - * so an image-only send (dispatch base64-encodes local paths) has no modeled - * block and would be refused its own replay, timing out into a false - * "delivery unconfirmed" that invites a genuine duplicate send on retry. + * Measured against the real CLI: a queued send's replay is published at the + * first tool boundary after it is queued, 2-3 ms AFTER that boundary's tool + * result. So rejecting tool results costs no time - whenever there was a frame + * to adopt, the right one follows immediately. */ function isClaudeUserMessageReplay(message: Record): boolean { if (message.type !== 'user' || message.parent_tool_use_id !== null) { return false } - const content = claudeRecord(message.message)?.content - return ( - Array.isArray(content) && - content.length > 0 && - !content.some((part) => claudeRecord(part)?.type === 'tool_result') - ) + if (message.isReplay === true) { + return true + } + const text = claudeUserFrameText(claudeRecord(message.message)?.content) + return text !== null && !isKnownHarnessInjectedUserTurnText(text) } export function resolveClaudeReplayWaiter(