mirror of
https://github.com/stablyai/orca.git
synced 2026-10-24 00:04:01 +00:00
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.
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user