From 279db7d6bc579a4b70d7f64463289ea2948ab84e Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 20 Aug 2026 19:22:33 -0700 Subject: [PATCH] fix(native-chat): don't treat a path-leading message as a slash command `acceptsResult` lets a send settle on a bare `result` frame, which carries no correlation to the waiting dispatch. It was set for any text starting with '/', so `/Users/me/repo/src/foo.ts - explain this` opted in: an unrelated in-flight turn's result claimed the send's identity, nothing upserted into the adopted key (readClaudeMessageEnvelope returns null for `type: 'result'`), and the real echo appended a second row - this PR's own symptom through the one path it had not touched. Pre-existing, but narrowing the user-frame gate made the `result` race the only quick settle for these sends rather than one of two, so it ships here. A slash command's first token has no second slash; that is exactly what separates `/model sonnet` and `/project:foo` from a path. Also corrects the image-replay test's comment: it is NOT red against the merge base (the old predicate accepts that frame too). It pins that the gate never sniffs content, which is a different guarantee. Found by the review loop on #15657. --- .../claude/claude-structured-dispatch.test.ts | 41 +++++++++++++++++-- src/main/claude/claude-structured-dispatch.ts | 20 ++++++++- 2 files changed, 56 insertions(+), 5 deletions(-) diff --git a/src/main/claude/claude-structured-dispatch.test.ts b/src/main/claude/claude-structured-dispatch.test.ts index 0fa1429258b..6e38f456e57 100644 --- a/src/main/claude/claude-structured-dispatch.test.ts +++ b/src/main/claude/claude-structured-dispatch.test.ts @@ -161,10 +161,10 @@ describe('Claude structured dispatch image limits', () => { }) }) - // Measured: the CLI stamps an image-only echo like any other, and its content - // carries no text. A gate that sniffed content for a modeled body would refuse - // this send its own replay, time out, and report a delivered message as - // unconfirmed - which sends it twice on retry. + // NOT red against the merge base - the old predicate accepts this frame too. + // It pins that the gate never sniffs content: an image-only echo carries no + // text and builds no journal body, so any body-model gate would refuse this + // send its own replay and report a delivered message as unconfirmed. it('accepts the replay of a send that carries only a local image', async () => { const directory = await mkdtemp(join(tmpdir(), 'orca-claude-replay-')) try { @@ -201,6 +201,39 @@ describe('Claude structured dispatch image limits', () => { } }) + // `acceptsResult` settles a send on a bare `result` frame, which carries no + // correlation to the waiting dispatch - so a message that merely opens with a + // path must not opt into it, or an unrelated turn's result claims its identity. + it('does not let a path-leading message settle on an unrelated result frame', async () => { + const session = sessionFor() + const dispatched = dispatchClaudeTurn( + session, + { + clientMessageId: 'client-1', + body: userMessage([{ type: 'text', text: '/Users/me/repo/src/foo.ts - explain this' }]) + }, + ACK_BUDGET_MS + ) + await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) + + resolveClaudeReplayWaiter(session, { + type: 'result', + subtype: 'success', + session_id: 'provider-session', + uuid: 'unrelated-turn-result' + }) + expect(session.dispatchWaiters).toHaveLength(1) + + resolveClaudeReplayWaiter( + session, + userReplayFrame('real-replay', '/Users/me/repo/src/foo.ts - explain this') + ) + await expect(dispatched).resolves.toMatchObject({ + state: 'accepted', + providerIdentity: { uuid: 'real-replay' } + }) + }) + // 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. diff --git a/src/main/claude/claude-structured-dispatch.ts b/src/main/claude/claude-structured-dispatch.ts index 80585df1323..cb1aaa82324 100644 --- a/src/main/claude/claude-structured-dispatch.ts +++ b/src/main/claude/claude-structured-dispatch.ts @@ -185,6 +185,24 @@ function waitForReplay( return { waiter, replayed } } +/** + * A slash command, not a message that merely opens with a path. + * + * `acceptsResult` lets a send settle on a bare `result` frame, which carries no + * correlation to the dispatch that is waiting - so a false positive lets an + * unrelated turn's result claim the send's identity, and the real echo then + * appends a second row. `/Users/me/repo/foo.ts - explain this` is an ordinary + * message; the second slash is what gives it away. + */ +function isSlashCommandText(text: string): boolean { + const trimmed = text.trimStart() + if (!trimmed.startsWith('/')) { + return false + } + const token = trimmed.slice(1).split(/\s/, 1)[0] ?? '' + return token.length > 0 && !token.includes('/') +} + export async function dispatchClaudeTurn( session: ClaudeSession, input: { clientMessageId: string; body: AgentJournalMessageItem }, @@ -197,7 +215,7 @@ export async function dispatchClaudeTurn( return { state: 'rejected', reason: (error as Error).message } } const acceptsResult = input.body.blocks.some( - (block) => block.type === 'text' && block.text.trimStart().startsWith('/') + (block) => block.type === 'text' && isSlashCommandText(block.text) ) const { waiter, replayed } = waitForReplay(session, timeoutMs, acceptsResult) try {