mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 08:03:28 +00:00
fix(mobile): harden native chat edge cases
This commit is contained in:
@@ -3,14 +3,22 @@ import { act, create, type ReactTestInstance, type ReactTestRenderer } from 'rea
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { MobileNativeChatView } from './MobileNativeChatView'
|
||||
|
||||
vi.mock('react-native', () => ({
|
||||
ActivityIndicator: 'ActivityIndicator',
|
||||
FlatList: 'FlatList',
|
||||
Pressable: 'Pressable',
|
||||
StyleSheet: { create: (styles: unknown) => styles, hairlineWidth: 1 },
|
||||
Text: 'Text',
|
||||
View: 'View'
|
||||
}))
|
||||
const { scrollToEnd } = vi.hoisted(() => ({ scrollToEnd: vi.fn() }))
|
||||
|
||||
vi.mock('react-native', async () => {
|
||||
const React = await import('react')
|
||||
return {
|
||||
ActivityIndicator: 'ActivityIndicator',
|
||||
FlatList: React.forwardRef((props: object, ref) => {
|
||||
React.useImperativeHandle(ref, () => ({ scrollToEnd }))
|
||||
return React.createElement('FlatList', props)
|
||||
}),
|
||||
Pressable: 'Pressable',
|
||||
StyleSheet: { create: (styles: unknown) => styles, hairlineWidth: 1 },
|
||||
Text: 'Text',
|
||||
View: 'View'
|
||||
}
|
||||
})
|
||||
|
||||
vi.mock('react-native-safe-area-context', () => ({
|
||||
useSafeAreaInsets: () => ({ top: 0, bottom: 0, left: 0, right: 0 })
|
||||
@@ -58,6 +66,7 @@ vi.mock('./MobileNativeChatComposer', async () => {
|
||||
})
|
||||
|
||||
type Overrides = {
|
||||
messages?: Parameters<typeof MobileNativeChatView>[0]['messages']
|
||||
sendErrorMessage?: string | null
|
||||
onClearSendError?: () => void
|
||||
inputLockReason?: 'disconnected' | 'waiting' | null
|
||||
@@ -84,6 +93,7 @@ describe('MobileNativeChatView', () => {
|
||||
|
||||
beforeEach(() => {
|
||||
globalThis.IS_REACT_ACT_ENVIRONMENT = true
|
||||
scrollToEnd.mockClear()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
@@ -113,6 +123,23 @@ describe('MobileNativeChatView', () => {
|
||||
}
|
||||
}
|
||||
|
||||
async function update(overrides: Overrides = {}): Promise<void> {
|
||||
await act(async () => {
|
||||
renderer?.update(
|
||||
createElement(MobileNativeChatView, {
|
||||
messages: [],
|
||||
status: 'ready',
|
||||
streamIdentity: 'test-stream',
|
||||
onSend: vi.fn().mockResolvedValue(true),
|
||||
pending: [],
|
||||
composerText: '',
|
||||
onComposerTextChange: vi.fn(),
|
||||
...overrides
|
||||
})
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
function banners(): ReactTestInstance[] {
|
||||
return renderer!.root.findAll((node) => node.props.accessibilityRole === 'alert')
|
||||
}
|
||||
@@ -190,4 +217,31 @@ describe('MobileNativeChatView', () => {
|
||||
await act(async () => list.props.ListHeaderComponent.props.onLoadEarlier())
|
||||
expect(onLoadEarlier).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('does not defeat prepend anchoring when a short list was also at bottom', async () => {
|
||||
vi.useFakeTimers()
|
||||
const current = {
|
||||
id: 'current',
|
||||
role: 'assistant' as const,
|
||||
blocks: [{ type: 'text' as const, text: 'current' }],
|
||||
timestamp: 0,
|
||||
source: 'transcript' as const
|
||||
}
|
||||
const older = { ...current, id: 'older', blocks: [{ type: 'text' as const, text: 'older' }] }
|
||||
try {
|
||||
await render({ messages: [current], hasMore: true })
|
||||
await act(async () => vi.runAllTimersAsync())
|
||||
scrollToEnd.mockClear()
|
||||
|
||||
await update({ messages: [current], hasMore: true, loadingEarlier: true })
|
||||
await update({ messages: [older, current], hasMore: false, loadingEarlier: false })
|
||||
const list = renderer!.root.find((node) => node.type === 'FlatList')
|
||||
act(() => list.props.onContentSizeChange())
|
||||
await act(async () => vi.runAllTimersAsync())
|
||||
|
||||
expect(scrollToEnd).not.toHaveBeenCalled()
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
@@ -161,6 +161,11 @@ export function MobileNativeChatView({
|
||||
const bottomPad = keyboardInset > 0 ? keyboardInset + insets.bottom : insets.bottom
|
||||
const [atBottom, setAtBottom] = useState(true)
|
||||
const sendScrollTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null)
|
||||
const keepHistoryPositionRef = useRef(Boolean(loadingEarlier))
|
||||
if (loadingEarlier) {
|
||||
keepHistoryPositionRef.current = true
|
||||
}
|
||||
const keepHistoryPosition = keepHistoryPositionRef.current
|
||||
const { fontScale, pinchGesture } = useMobileNativeChatPinchGesture()
|
||||
useEffect(
|
||||
() => () => {
|
||||
@@ -191,12 +196,17 @@ export function MobileNativeChatView({
|
||||
// we don't yank the user away while they read history. (Also fires on keyboard
|
||||
// close, which is harmless while atBottom.)
|
||||
useEffect(() => {
|
||||
if (data.length === 0 || !atBottom) {
|
||||
if (data.length === 0 || !atBottom || keepHistoryPositionRef.current) {
|
||||
return
|
||||
}
|
||||
const t = setTimeout(() => listRef.current?.scrollToEnd({ animated: true }), 60)
|
||||
return () => clearTimeout(t)
|
||||
}, [data.length, atBottom, keyboardInset])
|
||||
useEffect(() => {
|
||||
if (!loadingEarlier) {
|
||||
keepHistoryPositionRef.current = false
|
||||
}
|
||||
}, [loadingEarlier])
|
||||
|
||||
const handleSend = useCallback(
|
||||
async (text: string): Promise<boolean> => {
|
||||
@@ -297,7 +307,7 @@ export function MobileNativeChatView({
|
||||
// prepend grows the list upward instead of jumping the reader.
|
||||
maintainVisibleContentPosition={{ minIndexForVisible: 0 }}
|
||||
onContentSizeChange={() => {
|
||||
if (data.length > 0 && atBottom) {
|
||||
if (data.length > 0 && atBottom && !keepHistoryPosition) {
|
||||
listRef.current?.scrollToEnd({ animated: false })
|
||||
}
|
||||
}}
|
||||
|
||||
@@ -423,9 +423,10 @@ describe('useMobileNativeChatAnswerSend', () => {
|
||||
await expect(answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [1] }])).resolves.toBe(true)
|
||||
})
|
||||
|
||||
it('lets a superseding answer inherit the cancelled chain’s lock and releases it last', async () => {
|
||||
it('fences a superseding answer after the cancelled chain moved the selector', async () => {
|
||||
const onSendError = vi.fn()
|
||||
const sendRequest = vi.fn().mockResolvedValue(acceptedResponse())
|
||||
await mount({ sendRequest } as unknown as RpcClient, vi.fn())
|
||||
await mount({ sendRequest } as unknown as RpcClient, onSendError)
|
||||
|
||||
const prompt: AskPrompt = {
|
||||
questions: [
|
||||
@@ -438,22 +439,24 @@ describe('useMobileNativeChatAnswerSend', () => {
|
||||
await act(async () => {
|
||||
first = answerSend?.answerAsk(prompt, [{ indices: [0] }, { indices: [0] }])
|
||||
})
|
||||
// The user changes their mind mid-pacing: the new answer must not be
|
||||
// rejected by the lock its own predecessor still holds.
|
||||
// The first digit already advanced Claude to q2. Replaying a from-q1 key
|
||||
// plan now would answer the wrong question.
|
||||
await act(async () => {
|
||||
second = answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [1] }])
|
||||
})
|
||||
await act(async () => vi.runAllTimersAsync())
|
||||
|
||||
await expect(first).resolves.toBe(false)
|
||||
await expect(second).resolves.toBe(true)
|
||||
await expect(second).resolves.toBe(false)
|
||||
expect(sendRequest).toHaveBeenCalledTimes(1)
|
||||
expect(onSendError).toHaveBeenCalledWith('Answer partly sent — check chat before retrying')
|
||||
// Both chains unwound: the terminal is free for the next composed write.
|
||||
expect(acquireMobileNativeChatTerminalWrite('terminal')).toBe(true)
|
||||
releaseMobileNativeChatTerminalWrite('terminal')
|
||||
})
|
||||
|
||||
it('queues a superseding answer behind an in-flight terminal write', async () => {
|
||||
let resolveFirst: (response: ReturnType<typeof acceptedResponse>) => void = () => undefined
|
||||
it('does not write a successor after the prior in-flight key is accepted', async () => {
|
||||
let resolveFirst: (response: unknown) => void = () => undefined
|
||||
const sendRequest = vi
|
||||
.fn()
|
||||
.mockImplementationOnce(
|
||||
@@ -487,6 +490,42 @@ describe('useMobileNativeChatAnswerSend', () => {
|
||||
resolveFirst(acceptedResponse())
|
||||
await Promise.resolve()
|
||||
})
|
||||
await expect(first).resolves.toBe(false)
|
||||
await expect(second).resolves.toBe(false)
|
||||
expect(sendRequest).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('lets a queued successor continue after the prior key is definitely rejected', async () => {
|
||||
let resolveFirst: (response: unknown) => void = () => undefined
|
||||
const sendRequest = vi
|
||||
.fn()
|
||||
.mockImplementationOnce(
|
||||
() =>
|
||||
new Promise((resolve) => {
|
||||
resolveFirst = resolve
|
||||
})
|
||||
)
|
||||
.mockResolvedValue(acceptedResponse())
|
||||
await mount({ sendRequest } as unknown as RpcClient, vi.fn())
|
||||
|
||||
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 () => {
|
||||
resolveFirst({
|
||||
...acceptedResponse(),
|
||||
result: { send: { accepted: false } }
|
||||
})
|
||||
await Promise.resolve()
|
||||
})
|
||||
|
||||
await expect(first).resolves.toBe(false)
|
||||
await expect(second).resolves.toBe(true)
|
||||
expect(sendRequest).toHaveBeenCalledTimes(2)
|
||||
|
||||
@@ -263,7 +263,9 @@ export function useMobileNativeChatAnswerSend(args: {
|
||||
}
|
||||
return groups.length > 0
|
||||
} finally {
|
||||
finishTurn(predecessorSafe && !sawUnknownOutcome)
|
||||
// Any accepted key changed the live selector, so a queued replacement
|
||||
// cannot safely apply its from-scratch key plan to that new position.
|
||||
finishTurn(predecessorSafe && !sawUnknownOutcome && !sawAcceptedGroup)
|
||||
if (writeTurnsRef.current.get(handle) === turn) {
|
||||
writeTurnsRef.current.delete(handle)
|
||||
}
|
||||
|
||||
@@ -281,6 +281,35 @@ describe('useMobileNativeChatSession', () => {
|
||||
expect(state?.hasMore).toBe(false)
|
||||
})
|
||||
|
||||
it('clears a stale cursor when a replacement omits paging metadata', async () => {
|
||||
const sendRequest = vi.fn()
|
||||
const subscribe: RpcClient['subscribe'] = vi.fn((_method, _params, onData) => {
|
||||
emit = onData
|
||||
onData({
|
||||
type: 'snapshot',
|
||||
messages: Array.from({ length: 40 }, (_unused, index) => message(`win-${index}`)),
|
||||
hasMore: true,
|
||||
beforeOffset: 100
|
||||
})
|
||||
return () => {}
|
||||
})
|
||||
await mount({ sendRequest, subscribe } as unknown as RpcClient)
|
||||
|
||||
await act(async () =>
|
||||
emit({
|
||||
type: 'replacement',
|
||||
messages: Array.from({ length: 40 }, (_unused, index) => message(`new-${index}`))
|
||||
})
|
||||
)
|
||||
act(() => state?.loadEarlier())
|
||||
|
||||
expect(sendRequest).toHaveBeenCalledWith('nativeChat.readSession', {
|
||||
agent: 'claude',
|
||||
sessionId: 'session',
|
||||
limit: 100
|
||||
})
|
||||
})
|
||||
|
||||
it('surfaces a failed older-history page and clears it on a retry', async () => {
|
||||
const sendRequest = vi
|
||||
.fn()
|
||||
|
||||
@@ -187,12 +187,14 @@ export function useMobileNativeChatSession(args: {
|
||||
// Only a genuinely fresh window resets the grown read window — an
|
||||
// overlapping reconnect replay keeps the paged-in history and limit.
|
||||
limitRef.current = INITIAL_LIMIT
|
||||
beforeOffsetRef.current = applied.beforeOffset ?? null
|
||||
setHasMore(applied.hasMore ?? applied.messages.length >= INITIAL_LIMIT)
|
||||
}
|
||||
setMessages(applied.messages)
|
||||
if (applied.hasMore != null) {
|
||||
if (!applied.windowReplaced && applied.hasMore != null) {
|
||||
setHasMore(applied.hasMore)
|
||||
}
|
||||
if (applied.beforeOffset != null) {
|
||||
if (!applied.windowReplaced && applied.beforeOffset != null) {
|
||||
beforeOffsetRef.current = applied.beforeOffset
|
||||
}
|
||||
if (applied.cursorInvalidated) {
|
||||
|
||||
@@ -3,8 +3,10 @@ import type { NativeChatBlock } from './native-chat-types'
|
||||
import {
|
||||
briefToolArg,
|
||||
describeToolInput,
|
||||
formatToolInput,
|
||||
summarizeToolInput,
|
||||
summarizeToolRun
|
||||
summarizeToolRun,
|
||||
toolFilePath
|
||||
} from './native-chat-tool-summary'
|
||||
|
||||
describe('describeToolInput', () => {
|
||||
@@ -20,6 +22,16 @@ describe('describeToolInput', () => {
|
||||
expect(describeToolInput({ pattern: 'foo.*bar', glob: '*.ts' })).toBe('foo.*bar')
|
||||
})
|
||||
|
||||
it('normalizes Codex JSON-string arguments for labels, details, and file links', () => {
|
||||
expect(describeToolInput('{"cmd":"git status --short"}')).toBe('git status --short')
|
||||
expect(formatToolInput('{"cmd":"git status --short"}')).toBe(
|
||||
'{\n "cmd": "git status --short"\n}'
|
||||
)
|
||||
expect(toolFilePath('{"file_path":"src/index.ts"}')).toBe('src/index.ts')
|
||||
expect(briefToolArg('{"cmd":"git status --short"}')).toBe('git status --short')
|
||||
expect(describeToolInput('{"command":["bash","-lc","make"]}')).toBe('bash -lc make')
|
||||
})
|
||||
|
||||
it('falls back to the bounded JSON preview for other shapes', () => {
|
||||
expect(describeToolInput({ todos: [{ id: 1 }] })).toBe(
|
||||
summarizeToolInput({ todos: [{ id: 1 }] })
|
||||
|
||||
@@ -20,64 +20,95 @@ function truncatePreview(collapsed: string): string {
|
||||
* argument (command/query/…), else the bounded JSON preview. Keeps raw
|
||||
* `{"file_path":…}` JSON out of the tappable row label. */
|
||||
export function describeToolInput(input: unknown): string {
|
||||
const path = toolFilePath(input)
|
||||
const normalized = normalizeToolInput(input)
|
||||
const path = toolFilePath(normalized)
|
||||
if (path) {
|
||||
return truncatePreview(path)
|
||||
}
|
||||
if (input && typeof input === 'object') {
|
||||
const value = input as Record<string, unknown>
|
||||
if (normalized && typeof normalized === 'object') {
|
||||
const value = normalized as Record<string, unknown>
|
||||
const primary =
|
||||
value.command ?? value.cmd ?? value.query ?? value.pattern ?? value.description ?? value.url
|
||||
if (typeof primary === 'string' && primary.trim()) {
|
||||
return summarizeToolInput(primary)
|
||||
const primarySummary = summarizePrimaryToolArg(primary)
|
||||
if (primarySummary) {
|
||||
return primarySummary
|
||||
}
|
||||
}
|
||||
return summarizeToolInput(input)
|
||||
return summarizeToolInput(normalized)
|
||||
}
|
||||
|
||||
/** Full, pretty-printed tool-call input for the expanded detail view. Strings
|
||||
* pass through as-is; objects/arrays print as indented JSON so a diff-less call
|
||||
* (e.g. a question payload) reads cleanly instead of one long minified line. */
|
||||
/** Full, pretty-printed tool-call input for the expanded detail view. Structured
|
||||
* JSON strings and objects print as indented JSON; other strings pass through. */
|
||||
export function formatToolInput(input: unknown): string {
|
||||
if (input === null || input === undefined) {
|
||||
const normalized = normalizeToolInput(input)
|
||||
if (normalized === null || normalized === undefined) {
|
||||
return ''
|
||||
}
|
||||
if (typeof input === 'string') {
|
||||
return input
|
||||
if (typeof normalized === 'string') {
|
||||
return normalized
|
||||
}
|
||||
if (typeof input === 'number' || typeof input === 'boolean') {
|
||||
return String(input)
|
||||
if (typeof normalized === 'number' || typeof normalized === 'boolean') {
|
||||
return String(normalized)
|
||||
}
|
||||
try {
|
||||
return JSON.stringify(input, null, 2) ?? ''
|
||||
return JSON.stringify(normalized, null, 2) ?? ''
|
||||
} catch {
|
||||
return ''
|
||||
}
|
||||
}
|
||||
|
||||
export function toolFilePath(input: unknown): string | null {
|
||||
if (!input || typeof input !== 'object') {
|
||||
const normalized = normalizeToolInput(input)
|
||||
if (!normalized || typeof normalized !== 'object') {
|
||||
return null
|
||||
}
|
||||
const value = input as Record<string, unknown>
|
||||
const value = normalized as Record<string, unknown>
|
||||
const path = value.file_path ?? value.filePath ?? value.path ?? value.notebook_path
|
||||
return typeof path === 'string' && path.length > 0 ? path : null
|
||||
}
|
||||
|
||||
export function briefToolArg(input: unknown): string {
|
||||
if (input && typeof input === 'object') {
|
||||
const value = input as Record<string, unknown>
|
||||
const normalized = normalizeToolInput(input)
|
||||
if (normalized && typeof normalized === 'object') {
|
||||
const value = normalized as Record<string, unknown>
|
||||
const path = value.file_path ?? value.filePath ?? value.path ?? value.notebook_path
|
||||
if (typeof path === 'string' && path.length > 0) {
|
||||
const parts = path.split(/[\\/]/).filter(Boolean)
|
||||
return parts.at(-1) ?? path
|
||||
}
|
||||
const command = value.command ?? value.cmd ?? value.query ?? value.pattern
|
||||
if (typeof command === 'string') {
|
||||
return summarizeToolInput(command).slice(0, 28)
|
||||
const commandSummary = summarizePrimaryToolArg(command)
|
||||
if (commandSummary) {
|
||||
return commandSummary.slice(0, 28)
|
||||
}
|
||||
}
|
||||
return summarizeToolInput(input).slice(0, 28)
|
||||
return summarizeToolInput(normalized).slice(0, 28)
|
||||
}
|
||||
|
||||
function normalizeToolInput(input: unknown): unknown {
|
||||
if (typeof input !== 'string') {
|
||||
return input
|
||||
}
|
||||
const first = input.trimStart()[0]
|
||||
if (first !== '{' && first !== '[') {
|
||||
return input
|
||||
}
|
||||
try {
|
||||
const parsed: unknown = JSON.parse(input)
|
||||
return parsed !== null && typeof parsed === 'object' ? parsed : input
|
||||
} catch {
|
||||
return input
|
||||
}
|
||||
}
|
||||
|
||||
function summarizePrimaryToolArg(input: unknown): string | null {
|
||||
if (typeof input === 'string' && input.trim()) {
|
||||
return summarizeToolInput(input)
|
||||
}
|
||||
if (Array.isArray(input) && input.length > 0 && input.every((part) => typeof part === 'string')) {
|
||||
return summarizeToolInput(input.join(' '))
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
export function summarizeToolRun(blocks: readonly NativeChatBlock[]): string {
|
||||
|
||||
Reference in New Issue
Block a user