starred/orca
1
0
Fork 0
mirror of https://github.com/stablyai/orca.git synced 2026-10-31 00:57:45 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(mobile): keep newer quick-command edits when an earlier save finishes (#31704)

Browse Source 
* fix(mobile): retain quick command editor ownership during saves

A deferred Save discards newer edits or closes a replacement editor, including after host/repository changes. Bind each save to its editor and scope; preserve newer text and the persisted command id.

* fix(mobile): preserve quick command saves through suspended renders

* test(mobile): preserve quick-command edits during connection migration
This commit is contained in:
Neil
2026-10-21 21:20:51 -07:00
committed by GitHub
parent aec02a76ee
commit d8e997c529
2 changed files with 257 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files
mobile/src/session/QuickCommandsSheet.test.ts
+208 -1
View File
@@ -1,10 +1,12 @@
import { createElement, type ReactNode } from 'react'
import { createElement, Fragment, startTransition, Suspense, type ReactNode } from 'react'
import { act, create, type ReactTestRenderer } from 'react-test-renderer'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import type { TerminalQuickCommand } from '../../../src/shared/terminal-quick-command-types'
import type { RpcClient } from '../transport/rpc-client'
import { MAX_QUICK_COMMANDS } from '../terminal/quick-commands'
import { QuickCommandsSheet } from './QuickCommandsSheet'
import { createStableLogicalRpcClient } from '../transport/stable-logical-rpc-client'
import { FakeSession } from '../transport/mobile-endpoint-supervisor-test-fakes'
const mocks = vi.hoisted(() => ({
alert: vi.fn(),
@@ -13,6 +15,15 @@ const mocks = vi.hoisted(() => ({
}))
const quickCommandEditorForm = 'QuickCommandEditorForm'
const quickCommandsList = 'QuickCommandsList'
const suspended = new Promise<void>(() => {})
const suspendedRender = vi.fn()
function Suspend({ active }: { active: boolean }) {
if (active) {
suspendedRender()
throw suspended
}
return null
}
const command: TerminalQuickCommand = {
id: 'command',
label: 'Test',
@@ -135,6 +146,202 @@ describe('QuickCommandsSheet', () => {
})
})
it('keeps save ownership when a repository transition suspends before commit', async () => {
const save = deferred<boolean>()
mocks.persist.mockReturnValue(save.promise)
const onClose = vi.fn()
const onLaunch = () => true
const render = (suspend = false) =>
createElement(
Suspense,
{ fallback: null },
createElement(
Fragment,
null,
createElement(QuickCommandsSheet, {
visible: true,
client: null,
repoId: suspend ? 'repo-2' : 'repo-1',
repoName: 'Repo',
agentPromptMaxLength: null,
onClose,
onLaunch
}),
createElement(Suspend, { active: suspend })
)
)
act(() => {
renderer = create(render())
})
act(() => renderer!.root.findByType(quickCommandsList).props.onAdd())
act(() =>
renderer!.root.findByType(quickCommandEditorForm).props.onChange({
label: 'Test',
command: 'pnpm test'
})
)
act(() => renderer!.root.findByType(quickCommandEditorForm).props.onSave())
suspendedRender.mockClear()
act(() => {
startTransition(() => renderer?.update(render(true)))
})
expect(suspendedRender).toHaveBeenCalled()
await act(async () => {
save.resolve(true)
await save.promise
})
expect(renderer!.root.findAllByType(quickCommandEditorForm)).toHaveLength(0)
expect(renderer!.root.findAllByType(quickCommandsList)).toHaveLength(1)
})
it.each(['new editor', 'newer edits'])(
'preserves %s when an older save completes',
async (change) => {
const save = deferred<boolean>()
mocks.persist.mockReturnValue(save.promise)
await act(async () => {
renderer = create(
createElement(QuickCommandsSheet, {
visible: true,
onClose: vi.fn(),
client: null,
repoId: 'repo-1',
repoName: 'Repo',
agentPromptMaxLength: null,
onLaunch: () => true
})
)
})
act(() => renderer!.root.findByType(quickCommandsList).props.onAdd())
act(() =>
renderer!.root
.findByType(quickCommandEditorForm)
.props.onChange({ label: 'First', command: 'first' })
)
act(() => renderer!.root.findByType(quickCommandEditorForm).props.onSave())
if (change === 'new editor') {
act(() => renderer!.root.findByType(quickCommandEditorForm).props.onCancel())
act(() => renderer!.root.findByType(quickCommandsList).props.onAdd())
}
act(() =>
renderer!.root
.findByType(quickCommandEditorForm)
.props.onChange({ label: 'Second', command: 'second' })
)
await act(async () => {
save.resolve(true)
await save.promise
})
expect(renderer!.root.findAllByType(quickCommandEditorForm)).toHaveLength(1)
expect(renderer!.root.findByType(quickCommandEditorForm).props.draft.label).toBe('Second')
if (change === 'newer edits') {
act(() => renderer!.root.findByType(quickCommandEditorForm).props.onSave())
expect(mocks.persist.mock.calls[1]?.[0].command).toMatchObject({
id: mocks.persist.mock.calls[0]?.[0].command.id,
label: 'Second',
command: 'second'
})
}
}
)
it.each(['client', 'repo'])('isolates pending saves across a changed %s', async (context) => {
const first = deferred<boolean>()
const second = deferred<boolean>()
mocks.persist.mockReturnValueOnce(first.promise).mockReturnValueOnce(second.promise)
const props = {
visible: true,
onClose: vi.fn(),
client: null,
repoId: 'repo-1',
repoName: 'Repo',
agentPromptMaxLength: null,
onLaunch: () => true
}
act(() => {
renderer = create(createElement(QuickCommandsSheet, props))
})
act(() => renderer!.root.findByType(quickCommandsList).props.onAdd())
act(() =>
renderer!.root
.findByType(quickCommandEditorForm)
.props.onChange({ label: 'First', command: 'first' })
)
act(() => renderer!.root.findByType(quickCommandEditorForm).props.onSave())
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The settings hook is mocked; this client is used only as an identity by the sheet.
const nextClient = {} as RpcClient
act(() =>
renderer!.update(
createElement(QuickCommandsSheet, {
...props,
client: context === 'client' ? nextClient : null,
repoId: context === 'repo' ? 'repo-2' : 'repo-1'
})
)
)
act(() => renderer!.root.findByType(quickCommandsList).props.onAdd())
act(() =>
renderer!.root
.findByType(quickCommandEditorForm)
.props.onChange({ label: 'Second', command: 'second' })
)
act(() => renderer!.root.findByType(quickCommandEditorForm).props.onSave())
expect(mocks.persist).toHaveBeenCalledTimes(2)
await act(async () => {
first.resolve(true)
await first.promise
})
const editor = renderer!.root.findByType(quickCommandEditorForm)
expect(editor.props.draft.label).toBe('Second')
expect(editor.props.saving).toBe(true)
act(() => editor.props.onSave())
expect(mocks.persist).toHaveBeenCalledTimes(2)
await act(async () => {
second.resolve(true)
await second.promise
})
expect(renderer!.root.findAllByType(quickCommandEditorForm)).toHaveLength(0)
})
it('keeps newer edits and the pending save when the same computer changes connection paths', async () => {
const saved = deferred<boolean>()
mocks.persist.mockReturnValue(saved.promise)
const client = createStableLogicalRpcClient(new FakeSession('connected'), 'relay')
const props = {
visible: true,
onClose: vi.fn(),
client,
repoId: 'repo-1',
repoName: 'Repo',
agentPromptMaxLength: null,
onLaunch: () => true
}
act(() => {
renderer = create(createElement(QuickCommandsSheet, props))
})
act(() => renderer!.root.findByType(quickCommandsList).props.onAdd())
const editor = () => renderer!.root.findByType(quickCommandEditorForm)
act(() => editor().props.onChange({ label: 'First', command: 'first' }))
act(() => editor().props.onSave())
act(() => editor().props.onChange({ label: 'Newer', command: 'newer' }))
await act(async () => {
await client.migrateTo(new FakeSession('connected'), 'lan')
renderer!.update(createElement(QuickCommandsSheet, props))
})
expect(editor().props.draft.label).toBe('Newer')
expect(editor().props.saving).toBe(true)
await act(async () => {
saved.resolve(true)
await saved.promise
})
expect(editor().props.draft).toMatchObject({
label: 'Newer',
id: mocks.persist.mock.calls[0]?.[0].command.id
})
expect(editor().props.saving).toBe(false)
client.close()
})
it('keeps creation closed when the host command limit is reached', async () => {
mocks.commands = Array.from({ length: MAX_QUICK_COMMANDS }, (_, index) => ({
...command,
mobile/src/session/QuickCommandsSheet.tsx
+49 -19
View File
@@ -1,4 +1,4 @@
import { useMemo, useRef, useState } from 'react'
import { useLayoutEffect, useMemo, useRef, useState } from 'react'
import { Alert, View, Text, Pressable, StyleSheet } from 'react-native'
import { ChevronLeft } from 'lucide-react-native'
import { colors, spacing } from '../theme/mobile-theme'
@@ -50,17 +50,29 @@ export function QuickCommandsSheet({
const [query, setQuery] = useState('')
const [draft, setDraft] = useState<QuickCommandDraft | null>(null)
const [saving, setSaving] = useState(false)
const savingRef = useRef(false)
const saveOwnerRef = useRef<symbol | null>(null)
const draftRevisionRef = useRef(0)
const editorGenerationRef = useRef(0)
const [wasVisible, setWasVisible] = useState(visible)
if (visible !== wasVisible) {
setWasVisible(visible)
if (visible) {
setView('list')
setQuery('')
setDraft(null)
const previousSurface = useRef({ visible, client, repoId })
useLayoutEffect(() => {
const contextChanged =
previousSurface.current.client !== client || previousSurface.current.repoId !== repoId
if (visible !== previousSurface.current.visible || contextChanged) {
draftRevisionRef.current += 1
editorGenerationRef.current += 1
previousSurface.current = { visible, client, repoId }
if (contextChanged) {
saveOwnerRef.current = null
setSaving(false)
}
if (visible || contextChanged) {
setView('list')
setQuery('')
setDraft(null)
}
}
}
}, [visible, client, repoId])
// Why: prompt bodies can total megabytes. Lowercase them once per settings
// update instead of allocating the same search text on every keystroke.
@@ -89,6 +101,8 @@ export function QuickCommandsSheet({
if (!command && commands.length >= MAX_QUICK_COMMANDS) {
return
}
draftRevisionRef.current += 1
editorGenerationRef.current += 1
setDraft(
command
? quickCommandToDraft(command)
@@ -123,7 +137,7 @@ export function QuickCommandsSheet({
}
const handleSave = async () => {
if (!draft || savingRef.current) {
if (!draft || saveOwnerRef.current !== null) {
return
}
const built = draftToQuickCommand(draft, agentPromptMaxLength)
@@ -131,17 +145,24 @@ export function QuickCommandsSheet({
return
}
// Why: state cannot lock out a second tap until React commits the disabled UI.
savingRef.current = true
const saveOwner = Symbol()
saveOwnerRef.current = saveOwner
setSaving(true)
const savedRevision = draftRevisionRef.current
const savedEditor = editorGenerationRef.current
try {
const ok = await persist({ type: 'upsert', command: built })
if (ok) {
if (ok && draftRevisionRef.current === savedRevision) {
setView('list')
setDraft(null)
} else if (ok && editorGenerationRef.current === savedEditor) {
setDraft((current) => (current ? { ...current, id: built.id } : current))
}
} finally {
savingRef.current = false
setSaving(false)
if (saveOwnerRef.current === saveOwner) {
saveOwnerRef.current = null
setSaving(false)
}
}
}
@@ -162,7 +183,10 @@ export function QuickCommandsSheet({
) : (
<Pressable
style={({ pressed }) => [styles.backButton, pressed && styles.pressed]}
onPress={() => setView(view === 'agent' ? 'editor' : 'list')}
onPress={() => {
draftRevisionRef.current += 1
setView(view === 'agent' ? 'editor' : 'list')
}}
accessibilityRole="button"
accessibilityLabel="Back"
>
@@ -208,11 +232,16 @@ export function QuickCommandsSheet({
repoId={repoId}
repoName={repoName}
agentPromptMaxLength={agentPromptMaxLength}
onChange={(patch) =>
onChange={(patch) => {
draftRevisionRef.current += 1
setDraft((current) => (current ? { ...current, ...patch } : current))
}
onOpenAgentPicker={() => setView('agent')}
}}
onOpenAgentPicker={() => {
draftRevisionRef.current += 1
setView('agent')
}}
onCancel={() => {
draftRevisionRef.current += 1
setView('list')
setDraft(null)
}}
@@ -224,6 +253,7 @@ export function QuickCommandsSheet({
<QuickCommandAgentPicker
selected={draft.agent}
onSelect={(agent) => {
draftRevisionRef.current += 1
setDraft((current) => (current ? { ...current, agent } : current))
setView('editor')
}}