mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 08:03:28 +00:00
fix(mobile): harden native chat replay boundaries
This commit is contained in:
@@ -2,6 +2,7 @@ import { createElement } from 'react'
|
||||
import { act, create, type ReactTestRenderer } from 'react-test-renderer'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { NativeChatMessage } from '../../../src/shared/native-chat-types'
|
||||
import { MAX_TOOL_RESULT_CHARS } from './mobile-native-chat-message-styles'
|
||||
|
||||
vi.mock('react-native', async () => {
|
||||
const React = await import('react')
|
||||
@@ -30,7 +31,7 @@ function userMessage(blocks: NativeChatMessage['blocks']): NativeChatMessage {
|
||||
return { id: 'u1', role: 'user', blocks, timestamp: null, source: 'transcript' }
|
||||
}
|
||||
|
||||
describe('MobileNativeChatMessage image-ref rendering', () => {
|
||||
describe('MobileNativeChatMessage', () => {
|
||||
let renderer: ReactTestRenderer | null = null
|
||||
|
||||
beforeEach(() => {
|
||||
@@ -41,7 +42,10 @@ describe('MobileNativeChatMessage image-ref rendering', () => {
|
||||
renderer = null
|
||||
})
|
||||
|
||||
function render(message: NativeChatMessage): ReactTestRenderer {
|
||||
function render(
|
||||
message: NativeChatMessage,
|
||||
props: { toolsExpanded?: boolean } = {}
|
||||
): ReactTestRenderer {
|
||||
const original = console.error
|
||||
const spy = vi.spyOn(console, 'error').mockImplementation((...a) => {
|
||||
if (typeof a[0] === 'string' && a[0].includes('react-test-renderer is deprecated')) {
|
||||
@@ -51,7 +55,7 @@ describe('MobileNativeChatMessage image-ref rendering', () => {
|
||||
})
|
||||
try {
|
||||
act(() => {
|
||||
renderer = create(createElement(MobileNativeChatMessage, { message }))
|
||||
renderer = create(createElement(MobileNativeChatMessage, { message, ...props }))
|
||||
})
|
||||
} finally {
|
||||
spy.mockRestore()
|
||||
@@ -84,4 +88,26 @@ describe('MobileNativeChatMessage image-ref rendering', () => {
|
||||
.map((node) => String(node.children.join('')))
|
||||
expect(texts.some((text) => text.includes('/tmp/host.png'))).toBe(true)
|
||||
})
|
||||
|
||||
it('bounds expanded diff-less tool input before native text layout', () => {
|
||||
const tree = render(
|
||||
{
|
||||
id: 'a1',
|
||||
role: 'assistant',
|
||||
blocks: [
|
||||
{ type: 'tool-call', name: 'CustomTool', input: { payload: 'x'.repeat(100_000) } }
|
||||
],
|
||||
timestamp: null,
|
||||
source: 'transcript'
|
||||
},
|
||||
{ toolsExpanded: true }
|
||||
)
|
||||
|
||||
const detail = tree.root
|
||||
.findAllByType('Text' as never)
|
||||
.map((node) => String(node.children.join('')))
|
||||
.find((text) => text.startsWith('{\n'))
|
||||
expect(detail).toHaveLength(MAX_TOOL_RESULT_CHARS + 1)
|
||||
expect(detail?.endsWith('…')).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -73,6 +73,13 @@ function ResultBody({
|
||||
)
|
||||
}
|
||||
|
||||
function formatBoundedToolInput(input: unknown): string {
|
||||
const detail = formatToolInput(input)
|
||||
return detail.length > MAX_TOOL_RESULT_CHARS
|
||||
? `${detail.slice(0, MAX_TOOL_RESULT_CHARS)}…`
|
||||
: detail
|
||||
}
|
||||
|
||||
/** One request: a tool call and its result rendered together as a single
|
||||
* expandable line. `defaultExpanded` lets the group toggle open every line. */
|
||||
function ToolLine({
|
||||
@@ -135,9 +142,9 @@ function ToolLine({
|
||||
<View style={styles.toolDetail}>
|
||||
{callDiff ? <DiffView lines={callDiff} /> : null}
|
||||
{/* Diff-less calls show the full formatted input (pretty JSON), not
|
||||
the one-line label repeated (desktop parity). */}
|
||||
the one-line label repeated, bounded like desktop tool detail. */}
|
||||
{!callDiff && call ? (
|
||||
<Text style={styles.mono}>{formatToolInput(call.input)}</Text>
|
||||
<Text style={styles.mono}>{formatBoundedToolInput(call.input)}</Text>
|
||||
) : null}
|
||||
{result ? (
|
||||
<ResultBody output={result.output} isError={result.isError} diff={resultDiff} />
|
||||
|
||||
@@ -43,7 +43,12 @@ describe('applyMobileNativeChatStreamFrame', () => {
|
||||
|
||||
const result = applyMobileNativeChatStreamFrame({
|
||||
merger,
|
||||
frame: { type: 'snapshot', messages: [message('b'), message('c'), message('d')] },
|
||||
frame: {
|
||||
type: 'snapshot',
|
||||
messages: [message('b'), message('c'), message('d')],
|
||||
hasMore: true,
|
||||
beforeOffset: 20
|
||||
},
|
||||
limit: 3,
|
||||
replaceSnapshot: false
|
||||
})
|
||||
@@ -51,7 +56,32 @@ describe('applyMobileNativeChatStreamFrame', () => {
|
||||
expect(result).toMatchObject({
|
||||
kind: 'messages',
|
||||
messages: [message('b'), message('c'), message('d')],
|
||||
cursorInvalidated: true
|
||||
cursorInvalidated: true,
|
||||
hasMore: true
|
||||
})
|
||||
})
|
||||
|
||||
it('refreshes paging metadata when a replay still starts at the retained oldest row', () => {
|
||||
const merger = createNativeChatMerger()
|
||||
replaceList(merger, [message('a'), message('b')])
|
||||
|
||||
expect(
|
||||
applyMobileNativeChatStreamFrame({
|
||||
merger,
|
||||
frame: {
|
||||
type: 'snapshot',
|
||||
messages: [message('a'), message('b')],
|
||||
hasMore: false,
|
||||
beforeOffset: 0
|
||||
},
|
||||
limit: 40,
|
||||
replaceSnapshot: false
|
||||
})
|
||||
).toEqual({
|
||||
kind: 'messages',
|
||||
messages: [message('a'), message('b')],
|
||||
hasMore: false,
|
||||
beforeOffset: 0
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -25,19 +25,19 @@ export type AppliedMobileNativeChatFrame =
|
||||
windowReplaced?: boolean
|
||||
}
|
||||
|
||||
function replayExtendsRetainedTail(
|
||||
function replayRetainedTailStart(
|
||||
merger: NativeChatMerger,
|
||||
messages: readonly NativeChatMessage[],
|
||||
hasMore: boolean | undefined
|
||||
): boolean {
|
||||
): number | null {
|
||||
const firstIndex = messages[0] ? merger.indexById.get(messages[0].id) : undefined
|
||||
if (firstIndex === undefined) {
|
||||
return false
|
||||
return null
|
||||
}
|
||||
// `hasMore: false` is authoritative: retained rows before the replay window
|
||||
// were removed while disconnected, even when the newest IDs still match.
|
||||
if (hasMore === false && firstIndex > 0) {
|
||||
return false
|
||||
return null
|
||||
}
|
||||
let expectedIndex = firstIndex
|
||||
let sawNewMessage = false
|
||||
@@ -46,12 +46,12 @@ function replayExtendsRetainedTail(
|
||||
if (existingIndex === undefined) {
|
||||
sawNewMessage = true
|
||||
} else if (sawNewMessage || existingIndex !== expectedIndex) {
|
||||
return false
|
||||
return null
|
||||
} else {
|
||||
expectedIndex += 1
|
||||
}
|
||||
}
|
||||
return expectedIndex === merger.list.length
|
||||
return expectedIndex === merger.list.length ? firstIndex : null
|
||||
}
|
||||
|
||||
/** Applies runtime stream frames while preserving the initial-snapshot versus
|
||||
@@ -79,12 +79,11 @@ export function applyMobileNativeChatStreamFrame(args: {
|
||||
if (!Array.isArray(frame.messages)) {
|
||||
return { kind: 'ignored' }
|
||||
}
|
||||
const replayExtendsHistory =
|
||||
frame.type === 'snapshot' &&
|
||||
!replaceSnapshot &&
|
||||
merger.list.length > 0 &&
|
||||
replayExtendsRetainedTail(merger, frame.messages, frame.hasMore)
|
||||
if (frame.type === 'replacement' || (frame.type === 'snapshot' && !replayExtendsHistory)) {
|
||||
const replayStartIndex =
|
||||
frame.type === 'snapshot' && !replaceSnapshot && merger.list.length > 0
|
||||
? replayRetainedTailStart(merger, frame.messages, frame.hasMore)
|
||||
: null
|
||||
if (frame.type === 'replacement' || (frame.type === 'snapshot' && replayStartIndex === null)) {
|
||||
replaceList(merger, frame.messages)
|
||||
return {
|
||||
kind: 'messages',
|
||||
@@ -96,11 +95,23 @@ export function applyMobileNativeChatStreamFrame(args: {
|
||||
}
|
||||
const previousFirstId = merger.list[0]?.id
|
||||
const messages = applyAppend(merger, frame.messages, limit)
|
||||
const cursorInvalidated = Boolean(previousFirstId && messages[0]?.id !== previousFirstId)
|
||||
const replayStillStartsAtOldest = frame.type === 'snapshot' && replayStartIndex === 0
|
||||
return {
|
||||
kind: 'messages',
|
||||
messages,
|
||||
// Why: once the bounded live window drops its oldest row, the snapshot's
|
||||
// byte cursor no longer describes the oldest retained message.
|
||||
...(previousFirstId && messages[0]?.id !== previousFirstId ? { cursorInvalidated: true } : {})
|
||||
...(cursorInvalidated ? { cursorInvalidated: true } : {}),
|
||||
// A trimmed replay creates page-able history even if the prior window had
|
||||
// none; otherwise only a replay sharing our oldest row owns its metadata.
|
||||
...(frame.type === 'snapshot' && cursorInvalidated
|
||||
? { hasMore: true }
|
||||
: replayStillStartsAtOldest
|
||||
? {
|
||||
...(frame.hasMore == null ? {} : { hasMore: frame.hasMore }),
|
||||
...(frame.beforeOffset == null ? {} : { beforeOffset: frame.beforeOffset })
|
||||
}
|
||||
: {})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -491,4 +491,39 @@ describe('useMobileNativeChatAnswerSend', () => {
|
||||
await expect(second).resolves.toBe(true)
|
||||
expect(sendRequest).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('does not write a successor after the prior delivery becomes ambiguous', async () => {
|
||||
let rejectFirst: (error: Error) => void = () => undefined
|
||||
const onSendError = vi.fn()
|
||||
const sendRequest = vi
|
||||
.fn()
|
||||
.mockImplementationOnce(
|
||||
() =>
|
||||
new Promise((_resolve, reject) => {
|
||||
rejectFirst = reject
|
||||
})
|
||||
)
|
||||
.mockResolvedValue(acceptedResponse())
|
||||
await mount({ sendRequest } as unknown as RpcClient, onSendError)
|
||||
|
||||
let first: Promise<boolean> | undefined
|
||||
let second: Promise<boolean> | undefined
|
||||
await act(async () => {
|
||||
first = answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [0] }])
|
||||
await Promise.resolve()
|
||||
})
|
||||
await act(async () => {
|
||||
second = answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [1] }])
|
||||
await Promise.resolve()
|
||||
})
|
||||
await act(async () => {
|
||||
rejectFirst(markRpcDeliveryUnknown(new Error('Connection closed')))
|
||||
await Promise.resolve()
|
||||
})
|
||||
|
||||
await expect(first).resolves.toBe(false)
|
||||
await expect(second).resolves.toBe(false)
|
||||
expect(sendRequest).toHaveBeenCalledTimes(1)
|
||||
expect(onSendError).toHaveBeenCalledWith('Answer unconfirmed — check chat before retrying')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -80,8 +80,8 @@ export function useMobileNativeChatAnswerSend(args: {
|
||||
// superseding answer inherits the cancelled chain's hold (it re-enters before
|
||||
// the old chain unwinds), and only the last chain out releases the lock.
|
||||
const writeHoldsRef = useRef(new Map<string, number>())
|
||||
// Successors wait for the prior chain's active RPC before sharing its hold.
|
||||
const writeTurnsRef = useRef(new Map<string, Promise<void>>())
|
||||
// Successors wait for the prior RPC and inherit any delivery ambiguity.
|
||||
const writeTurnsRef = useRef(new Map<string, Promise<boolean>>())
|
||||
const delaysRef = useRef<
|
||||
Set<{ timer: ReturnType<typeof setTimeout>; resolve: (completed: boolean) => void }>
|
||||
>(new Set())
|
||||
@@ -124,9 +124,9 @@ export function useMobileNativeChatAnswerSend(args: {
|
||||
return false
|
||||
}
|
||||
holds.set(handle, heldCount + 1)
|
||||
const previousTurn = writeTurnsRef.current.get(handle) ?? Promise.resolve()
|
||||
let finishTurn: () => void = () => undefined
|
||||
const turn = new Promise<void>((resolve) => {
|
||||
const previousTurn = writeTurnsRef.current.get(handle) ?? Promise.resolve(true)
|
||||
let finishTurn: (safeToContinue: boolean) => void = () => undefined
|
||||
const turn = new Promise<boolean>((resolve) => {
|
||||
finishTurn = resolve
|
||||
})
|
||||
writeTurnsRef.current.set(handle, turn)
|
||||
@@ -135,9 +135,10 @@ export function useMobileNativeChatAnswerSend(args: {
|
||||
const generation = generationRef.current
|
||||
let sawUnknownOutcome = false
|
||||
let sawAcceptedGroup = false
|
||||
let predecessorSafe = true
|
||||
try {
|
||||
await previousTurn
|
||||
if (generationRef.current !== generation) {
|
||||
predecessorSafe = await previousTurn
|
||||
if (!predecessorSafe || generationRef.current !== generation) {
|
||||
return false
|
||||
}
|
||||
// One budget for the whole answer instead of a fresh timeout per keystroke
|
||||
@@ -262,7 +263,7 @@ export function useMobileNativeChatAnswerSend(args: {
|
||||
}
|
||||
return groups.length > 0
|
||||
} finally {
|
||||
finishTurn()
|
||||
finishTurn(predecessorSafe && !sawUnknownOutcome)
|
||||
if (writeTurnsRef.current.get(handle) === turn) {
|
||||
writeTurnsRef.current.delete(handle)
|
||||
}
|
||||
|
||||
@@ -222,6 +222,35 @@ describe('useMobileNativeChatSession', () => {
|
||||
expect(state?.messages.at(-1)?.id).toBe('live-1')
|
||||
})
|
||||
|
||||
it('enables paging when a replay trims a window that previously had no earlier rows', async () => {
|
||||
const sendRequest = vi.fn()
|
||||
const window = Array.from({ length: 40 }, (_unused, index) => message(`win-${index}`))
|
||||
const subscribe: RpcClient['subscribe'] = vi.fn((_method, _params, onData) => {
|
||||
emit = onData
|
||||
onData({ type: 'snapshot', messages: window, hasMore: false, beforeOffset: 0 })
|
||||
return () => {}
|
||||
})
|
||||
await mount({ sendRequest, subscribe } as unknown as RpcClient)
|
||||
|
||||
await act(async () =>
|
||||
emit({
|
||||
type: 'snapshot',
|
||||
messages: [...window.slice(1), message('live-1')],
|
||||
hasMore: true,
|
||||
beforeOffset: 10
|
||||
})
|
||||
)
|
||||
|
||||
expect(state?.messages[0]?.id).toBe('win-1')
|
||||
expect(state?.hasMore).toBe(true)
|
||||
act(() => state?.loadEarlier())
|
||||
expect(sendRequest).toHaveBeenCalledWith('nativeChat.readSession', {
|
||||
agent: 'claude',
|
||||
sessionId: 'session',
|
||||
limit: 100
|
||||
})
|
||||
})
|
||||
|
||||
it('drops paged rows that an authoritative replay says were removed', async () => {
|
||||
const sendRequest = vi.fn().mockResolvedValue({
|
||||
ok: true,
|
||||
|
||||
Reference in New Issue
Block a user