From 6d120ea9e28f835aabd9b8f6ae1b656a3eb01f03 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 4 Oct 2026 03:20:01 -0700 Subject: [PATCH] fix(native-chat): a Stop gives up on its outrun sends only once its request ends - A newer message used to make Orca give up on a Stop's outrun sends even while that Stop's own request was still out, so a send could come back as "couldn't confirm" before the Stop's real answer arrived. It now waits for the request to end. - The first press no longer shows "The agent wasn't stopped." for a lost answer that Orca is about to resend; it still does when nothing will resend it, and always for a refusal. - This app's own sends are compared with the press by when they were queued, on this machine's clock; the comment now says another client's are compared by the time in their id. --- ...d-agent-session-conversation-stop.test.tsx | 70 +++++++++++++++++++ ...uctured-agent-session-conversation-stop.ts | 59 ++++++++++++---- 2 files changed, 116 insertions(+), 13 deletions(-) diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-conversation-stop.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session-conversation-stop.test.tsx index 756e18b6262..3fa3b89396d 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session-conversation-stop.test.tsx +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-conversation-stop.test.tsx @@ -13,6 +13,7 @@ import { import { createStructuredAgentSessionOperationId } from '../../../../shared/structured-agent-session-mutation' import type { AgentJournalSubmission } from '../../../../shared/agent-session-journal-types' import { createBrowserUuid } from '@/lib/browser-uuid' +import { toast } from 'sonner' import { useStructuredAgentSessionConversationStop } from './use-structured-agent-session-conversation-stop' import type { StructuredAgentSessionWriteAs, @@ -26,6 +27,7 @@ afterEach(cleanup) let uuid = 0 beforeEach(() => { uuid = 0 + vi.mocked(toast.error).mockClear() vi.spyOn(globalThis.crypto, 'randomUUID').mockImplementation(() => { uuid += 1 return `11111111-1111-4111-8111-${uuid.toString(16).padStart(12, '0')}` @@ -212,4 +214,72 @@ describe('a conversation Stop', () => { expect(writeAs).toHaveBeenCalledOnce() expect(recordStopAnswer).toHaveBeenCalledWith(stopId, { kind: 'unanswerable' }) }) + it('gives the stamp up for a newer message only once its own request has ended', async () => { + const answer = Promise.withResolvers>() + const writeAs = vi.fn(() => answer.promise) + const { view, stopOutbox, recordStopAnswer } = harness(writeAs) + let pressed: Promise = Promise.resolve() + act(() => { + pressed = view.result.current() + }) + const stopId: string = stopOutbox.mock.calls[0]?.[0] + const newer = createStructuredAgentSessionOutboxEntry({ + clientMessageId: createStructuredAgentSessionOperationId( + createBrowserUuid, + Date.now() + 1_000 + ), + sessionId: 'session-1', + text: 'newer', + attachments: [], + queuedAt: Date.now() + 1_000 + }) + view.rerender({ outbox: [stamped(stopId), newer] }) + // Still out: its answer may yet come, and settle the stamp for real. + expect(recordStopAnswer).not.toHaveBeenCalled() + await act(async () => { + answer.resolve(DONE) + await pressed + }) + expect(recordStopAnswer.mock.calls[0]).toEqual([ + stopId, + { kind: 'answered', cursor: { epoch: 'e', sequence: 9 } } + ]) + }) +}) + +describe("a conversation Stop's first press", () => { + async function pressWithStamp( + outcome: StructuredAgentSessionWriteOutcome, + stampsSend: boolean + ): Promise { + const answer = Promise.withResolvers>() + const writeAs = vi.fn(() => answer.promise) + const { view, stopOutbox } = harness(writeAs) + let pressed: Promise = Promise.resolve() + act(() => { + pressed = view.result.current() + }) + if (stampsSend) { + view.rerender({ outbox: [stamped(stopOutbox.mock.calls[0]?.[0])] }) + } + await act(async () => { + answer.resolve(outcome) + await pressed + }) + } + + it('says nothing about a lost answer when the Stop will be sent again', async () => { + await pressWithStamp({ kind: 'not-done', notice: 'Lost.', answered: false }, true) + expect(toast.error).not.toHaveBeenCalled() + }) + + it('says a lost answer when nothing will send the Stop again', async () => { + await pressWithStamp({ kind: 'not-done', notice: 'Lost.', answered: false }, false) + expect(toast.error).toHaveBeenCalledExactlyOnceWith('Lost.') + }) + + it('always says a refusal', async () => { + await pressWithStamp({ kind: 'not-done', notice: 'No.', answered: true }, true) + expect(toast.error).toHaveBeenCalledExactlyOnceWith('No.') + }) }) diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-conversation-stop.ts b/src/renderer/src/components/native-chat/use-structured-agent-session-conversation-stop.ts index e209494f779..689436a13e7 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session-conversation-stop.ts +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-conversation-stop.ts @@ -29,19 +29,37 @@ function unansweredStop(outbox: readonly StructuredAgentSessionOutboxEntry[]): s return null } -/** Whether a message was sent after this Stop was pressed, from here or another client: by the - * time its id was made, so no host clock is compared. */ +/** Whether a message was sent after this Stop was pressed. This app's own sends compare the time + * they were queued with the press, both on this machine's clock. Another client's are known only + * by the time in their id, made on that client's clock, so a skewed clock can misjudge them. */ function newerSendExists( outbox: readonly StructuredAgentSessionOutboxEntry[], submissions: readonly AgentJournalSubmission[], stopOperationId: string ): boolean { - const pressedAt = parseAgentSessionOperationTimestamp(stopOperationId) - const madeAfter = (id: string): boolean => - (parseAgentSessionOperationTimestamp(id) ?? -Infinity) > (pressedAt ?? Infinity) + const pressedAt = parseAgentSessionOperationTimestamp(stopOperationId) ?? Infinity return ( - outbox.some((entry) => madeAfter(entry.clientMessageId)) || - submissions.some((submission) => madeAfter(submission.clientMessageId)) + outbox.some((entry) => entry.queuedAt > pressedAt) || + submissions.some( + (submission) => + (parseAgentSessionOperationTimestamp(submission.clientMessageId) ?? -Infinity) > pressedAt + ) + ) +} + +/** Whether a Stop whose answer was lost goes again from the resend effect below. */ +function stopWillBeResent( + outbox: readonly StructuredAgentSessionOutboxEntry[], + submissions: readonly AgentJournalSubmission[], + stopOperationId: string +): boolean { + return ( + outbox.some( + (entry) => + entry.stoppedBy?.operationId === stopOperationId && + !entry.stoppedBy.cursor && + entry.stoppedBy.unanswerable !== true + ) && !newerSendExists(outbox, submissions, stopOperationId) ) } @@ -55,6 +73,12 @@ export function useStructuredAgentSessionConversationStop(args: { }): () => Promise { const { attached, outbox, recordStopAnswer, stopOutbox, submissions, writeAs } = args const inFlight = useRef(new Set()) + // The same set for rendering, so the effects below see a request end. + const [inFlightIds, setInFlightIds] = useState([]) + const latest = useRef({ outbox, submissions }) + useLayoutEffect(() => { + latest.current = { outbox, submissions } + }, [outbox, submissions]) const [resends, setResends] = useState<{ id: string | null; attempts: number }>({ id: null, attempts: 0 @@ -66,6 +90,7 @@ export function useStructuredAgentSessionConversationStop(args: { return } inFlight.current.add(stopOperationId) + setInFlightIds((ids) => [...ids, stopOperationId]) try { const outcome = await writeAs( stopOperationId, @@ -77,7 +102,13 @@ export function useStructuredAgentSessionConversationStop(args: { recordStopAnswer(stopOperationId, { kind: 'answered', cursor: outcome.cursor }) return } - if (outcome.kind === 'not-done' && firstPress) { + // A refusal is said; a lost answer only when nothing will send the Stop again. + if ( + outcome.kind === 'not-done' && + firstPress && + (outcome.answered || + !stopWillBeResent(latest.current.outbox, latest.current.submissions, stopOperationId)) + ) { toast.error(outcome.notice) } // A refusal is the host's answer; a lost answer goes again from the effect below. @@ -86,7 +117,7 @@ export function useStructuredAgentSessionConversationStop(args: { } } finally { inFlight.current.delete(stopOperationId) - setResends((current) => ({ ...current })) + setInFlightIds((ids) => ids.filter((id) => id !== stopOperationId)) } }, [recordStopAnswer, writeAs] @@ -94,14 +125,16 @@ export function useStructuredAgentSessionConversationStop(args: { const owed = unansweredStop(outbox) const newer = owed !== null && newerSendExists(outbox, submissions, owed) + // Its request still out may yet be answered, so the stamp is only given up once it ends. + const owedInFlight = owed !== null && inFlightIds.includes(owed) useLayoutEffect(() => { - if (owed !== null && newer) { + if (owed !== null && newer && !owedInFlight) { recordStopAnswer(owed, { kind: 'unanswerable' }) } - }, [newer, owed, recordStopAnswer]) + }, [newer, owed, owedInFlight, recordStopAnswer]) useEffect(() => { - if (owed === null || newer || !attached || inFlight.current.has(owed)) { + if (owed === null || newer || !attached || owedInFlight) { return } const attempts = resends.id === owed ? resends.attempts : 0 @@ -113,7 +146,7 @@ export function useStructuredAgentSessionConversationStop(args: { Math.min(STOP_RESEND_BASE_DELAY_MS * 2 ** attempts, STOP_RESEND_MAX_DELAY_MS) ) return () => clearTimeout(timer) - }, [attached, newer, owed, resends, sendStop]) + }, [attached, newer, owed, owedInFlight, resends, sendStop]) return useCallback(async (): Promise => { const stopOperationId = structuredSessionOperationId()