From de6f1c705d6f14ad2bc2b4c0e5367b7ee52c6d77 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 20 Aug 2026 20:13:04 -0700 Subject: [PATCH] fix(native-chat): reject harness breadcrumbs that Claude stamps as replays `isReplay` is necessary but NOT sufficient, and the binary says so. Verified in claude 2.1.237: the model-switch breadcrumb is enqueued as `{type:'user', parent_tool_use_id:null, isReplay:true}` whose content is the string `Set model to ...`. Orca triggers that path itself from the model picker (setClaudeStructuredOption sends set_model, and reconnect replays it), so a send in flight during a model switch adopted the breadcrumb's uuid and the real echo appended a second row. The CLI cannot use the marker alone either - its own RemoteSessionManager filters the same frames by content prefix: isReplay"in e&&e.isReplay===!0&&typeof e.message?.content==="string" &&e.message.content.startsWith(cvl) // "Set model to " -> "Dropped own set_model breadcrumb echo" So: require the marker, then reject the harness shapes with the existing isKnownHarnessInjectedUserTurnText. This is the classifier an earlier round deleted - but as a NARROWING on top of the marker, not a fallback instead of it, and now on primary evidence rather than speculation. Also stop discarding an identity the replay already claimed: a write can reach Claude and still reject on the following flush, and reporting `unknown` there stranded a uuid nothing else could adopt. Found by the review loop on #15657. --- .../claude/claude-structured-dispatch.test.ts | 53 +++++++++++++ src/main/claude/claude-structured-dispatch.ts | 77 ++++++++++++++----- 2 files changed, 111 insertions(+), 19 deletions(-) diff --git a/src/main/claude/claude-structured-dispatch.test.ts b/src/main/claude/claude-structured-dispatch.test.ts index 2e0f9f2fb57..67f7e1d97aa 100644 --- a/src/main/claude/claude-structured-dispatch.test.ts +++ b/src/main/claude/claude-structured-dispatch.test.ts @@ -260,6 +260,39 @@ describe('Claude structured dispatch image limits', () => { }) }) + // Captured from claude 2.1.237: the model-switch breadcrumb is enqueued as + // `{type:'user', parent_tool_use_id:null, isReplay:true}` with STRING content. + // Orca triggers it itself from the model picker, and the CLI's own + // RemoteSessionManager filters the same frames by content prefix. + it('does not adopt a model-switch breadcrumb even though it is stamped', async () => { + const session = sessionFor() + const dispatched = dispatchClaudeTurn( + session, + { clientMessageId: 'client-1', body: userMessage([{ type: 'text', text: 'queued' }]) }, + ACK_BUDGET_MS + ) + await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) + + resolveClaudeReplayWaiter(session, { + type: 'user', + message: { + role: 'user', + content: 'Set model to Sonnet' + }, + parent_tool_use_id: null, + session_id: 'provider-session', + uuid: 'breadcrumb-uuid', + isReplay: true + }) + expect(session.dispatchWaiters).toHaveLength(1) + + resolveClaudeReplayWaiter(session, userReplayFrame('user-replay-uuid', 'queued')) + await expect(dispatched).resolves.toMatchObject({ + state: 'accepted', + providerIdentity: { uuid: 'user-replay-uuid' } + }) + }) + // The waiter queue is positional, so shifting the head on a send failure // resolves whichever dispatch happens to be first - reporting a delivered // message as unconfirmed while the send that actually failed keeps waiting. @@ -290,6 +323,26 @@ describe('Claude structured dispatch image limits', () => { }) }) + // A write can reach Claude and still reject on the following flush. + it('keeps an identity the replay already claimed when the send then rejects', async () => { + const session = sessionFor() + session.connection.send = vi.fn().mockImplementation(async () => { + resolveClaudeReplayWaiter(session, userReplayFrame('landed-anyway', 'hello')) + throw new Error('flush failed') + }) + + await expect( + dispatchClaudeTurn( + session, + { clientMessageId: 'client-1', body: userMessage([{ type: 'text', text: 'hello' }]) }, + ACK_BUDGET_MS + ) + ).resolves.toMatchObject({ + state: 'accepted', + providerIdentity: { uuid: 'landed-anyway' } + }) + }) + it('rejects more than twenty URL images before sending', async () => { const session = sessionFor() const body = userMessage( diff --git a/src/main/claude/claude-structured-dispatch.ts b/src/main/claude/claude-structured-dispatch.ts index 276561a93a8..2e748116d54 100644 --- a/src/main/claude/claude-structured-dispatch.ts +++ b/src/main/claude/claude-structured-dispatch.ts @@ -4,7 +4,9 @@ import type { AgentJournalMessageItem } from '../../shared/agent-session-journal import type { NativeChatBlock } from '../../shared/native-chat-types' import type { AgentSessionDispatchOutcome } from '../native-chat/agent-session-wire/structured-agent-session-adapter' import type { ClaudeDispatchWaiter, ClaudeSession } from './claude-structured-session-state' +import { isKnownHarnessInjectedUserTurnText } from '../../shared/harness-injected-user-turns' import { readClaudeFrameString } from './claude-structured-init-proof' +import { claudeRecord } from './claude-structured-item-translation' const MAX_IMAGE_BYTES = 5 * 1024 * 1024 const MAX_IMAGE_COUNT = 20 @@ -50,32 +52,53 @@ const IMAGE_MIME_BY_EXTENSION: Record = { '.webp': 'image/webp' } +/** Leading text of a top-level user frame, for harness classification. The CLI + * serializes an injected breadcrumb's content as a plain string and a typed + * echo's as a block array, so both shapes have to be read. */ +function claudeUserFrameText(message: Record): string { + const content = claudeRecord(message.message)?.content + if (typeof content === 'string') { + return content + } + if (!Array.isArray(content)) { + return '' + } + return content + .map((part) => { + const text = claudeRecord(part)?.text + return typeof text === 'string' ? text : '' + }) + .join('\n') +} + /** * The frame that carries the user's own message back. * - * `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 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. + * `type: 'user'` with a null `parent_tool_use_id` is not enough: Claude + * publishes tool results and harness-injected turns in the same shape. Adopting + * one leaves the real replay under an identity nothing claims, so the user's + * message renders twice - and an injected turn is worse than a tool result, + * because it can build a body and upsert over the user's own bubble. * - * `CLAUDE_STRUCTURED_BASE_ARGS` always passes `--replay-user-messages`, and the - * CLI stamps `isReplay` on every echo it publishes in reply - measured for text, - * image-only and text+image sends. So the marker is the whole test: no content - * sniffing, and nothing here depends on the injected-turn list that - * prompt-derived UI maintains for its own reasons. + * `isReplay` is necessary but NOT sufficient. Claude stamps it on its own + * injected breadcrumbs too: 2.1.237 enqueues the model-switch notice as + * `{type:'user', parent_tool_use_id:null, isReplay:true}` whose content is the + * string `Set model to ...`. Orca triggers that path + * itself from the model picker (`setClaudeStructuredOption` sends `set_model`, + * and reconnect replays it). The CLI's own RemoteSessionManager cannot use the + * marker alone either - it filters the same frames by content prefix, logging + * "Dropped own set_model breadcrumb echo". * - * A slash command may get no user echo at all; `acceptsResult` settles those on - * the command's result receipt instead. - * - * 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. + * So: require the marker, then reject the harness shapes. The residual is a + * genuine echo whose own text opens with a harness tag (a user pasting a + * transcript excerpt), which is rarer than a model switch and fails the safe + * way - `unknown`, not a corrupted bubble. */ function isClaudeUserMessageReplay(message: Record): boolean { - return message.type === 'user' && message.parent_tool_use_id === null && message.isReplay === true + if (message.type !== 'user' || message.parent_tool_use_id !== null || message.isReplay !== true) { + return false + } + return !isKnownHarnessInjectedUserTurnText(claudeUserFrameText(message)) } export function resolveClaudeReplayWaiter( @@ -239,6 +262,22 @@ export async function dispatchClaudeTurn( session_id: session.providerSessionId }) } catch (error) { + // A write can reach Claude and still reject on the flush that follows, so the + // replay may already have claimed this waiter. Reporting `unknown` then + // strands an identity nothing else can adopt, and the echo duplicates. + if (!session.dispatchWaiters.includes(waiter)) { + const settled = await replayed + if (settled) { + return { + state: 'accepted', + providerIdentity: { + provider: 'claude', + sessionId: session.providerSessionId, + uuid: settled + } + } + } + } dropWaiter(session, waiter) clearTimeout(waiter.timer) waiter.resolve(null)