mirror of
https://github.com/stablyai/orca.git
synced 2026-10-08 08:02:32 +00:00
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.
This commit is contained in:
+70
@@ -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<StructuredAgentSessionWriteOutcome<unknown>>()
|
||||
const writeAs = vi.fn<StopWrite>(() => answer.promise)
|
||||
const { view, stopOutbox, recordStopAnswer } = harness(writeAs)
|
||||
let pressed: Promise<void> = 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<unknown>,
|
||||
stampsSend: boolean
|
||||
): Promise<void> {
|
||||
const answer = Promise.withResolvers<StructuredAgentSessionWriteOutcome<unknown>>()
|
||||
const writeAs = vi.fn<StopWrite>(() => answer.promise)
|
||||
const { view, stopOutbox } = harness(writeAs)
|
||||
let pressed: Promise<void> = 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.')
|
||||
})
|
||||
})
|
||||
|
||||
+46
-13
@@ -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<void> {
|
||||
const { attached, outbox, recordStopAnswer, stopOutbox, submissions, writeAs } = args
|
||||
const inFlight = useRef(new Set<string>())
|
||||
// The same set for rendering, so the effects below see a request end.
|
||||
const [inFlightIds, setInFlightIds] = useState<readonly string[]>([])
|
||||
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<void> => {
|
||||
const stopOperationId = structuredSessionOperationId()
|
||||
|
||||
Reference in New Issue
Block a user