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

fix(mobile): gate the Step 2 mint on this flow visit's address lookup

Browse Source 
The first attempt gated on a single boolean ref meaning "an address lookup
is running". That cannot describe a re-entrant operation: entering the
flow, leaving, and re-entering runs two overlapping lookups, and the first
to land clears the flag while the second is still out — so the mint went
out against the superseded lookup's address and the second lookup then
reminted with rotate: true. The same double mint the change exists to
remove, one path over.

Gate on positive evidence instead. Each flow entry bumps a visit counter;
the lookup records the visit it answered (max, so an abandoned visit
landing last cannot walk the marker backwards); the mint waits for
addressedFlowVisit === pairingFlowVisit, which is false at t=0 by
construction and makes exactly one false-to-true transition per visit. The
ref is gone and the effect's dependencies now name what it depends on.

A superseded lookup's response is also discarded outright, so it cannot
move the picker onto an address a newer lookup already replaced — that
reselection is itself a remint trigger.

The derived busy flag collapses to one clause and is renamed
awaitingPairingAddress: it was being passed down as pairLoading while
local readers used the real one. It stays separate from pairLoading
because that feeds shouldRegenerate in the invalidation hook, where
merging them would let a mode switch mint before the address settles.
This commit is contained in:
Jinwoo-H
2026-09-17 14:00:25 -04:00
parent 1e8d37174f
commit 9875a5f91f
2 changed files with 230 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/renderer/src/components/mobile/MobilePage.test.tsx
+199 -6
View File
@@ -596,13 +596,10 @@ describe('MobilePage pairing connection mode', () => {
await waitFor(() =>
expect(screen.getByTestId('selected-address')).toHaveTextContent('10.0.0.9')
)
await waitFor(() => expect(getPairingQR).toHaveBeenCalledTimes(1))
await new Promise((resolve) => setTimeout(resolve, 20))
// One Continue is one offer. Minting against the stale address and then
// rotating to the settled one runs two overlapping mints through main, whose
// rotate deletes the pending credential the first mint already returned.
expect(getPairingQR).toHaveBeenCalledTimes(1)
await waitFor(() => expect(getPairingQR).toHaveBeenCalledTimes(1))
expect(getPairingQR).toHaveBeenCalledWith({
address: '10.0.0.9',
connectionMode: 'automatic'
@@ -624,13 +621,209 @@ describe('MobilePage pairing connection mode', () => {
// lookup, so the auto-mint always runs before any address is known.
await user.click(screen.getByRole('button', { name: 'Pair another device' }))
await waitFor(() =>
expect(screen.getByTestId('selected-address')).toHaveTextContent('10.0.0.5')
)
await waitFor(() => expect(getPairingQR).toHaveBeenCalledTimes(1))
await new Promise((resolve) => setTimeout(resolve, 20))
expect(getPairingQR).toHaveBeenCalledTimes(1)
expect(getPairingQR).toHaveBeenCalledWith({
address: '10.0.0.5',
connectionMode: 'automatic'
})
// Returning to the paired list and pairing again is a fresh visit: it must
// wait for its own lookup, not inherit the previous visit's answer.
await user.click(screen.getByRole('button', { name: 'Back' }))
await user.click(screen.getByRole('button', { name: 'Back' }))
await waitFor(() => expect(screen.getByTestId('stage')).toHaveTextContent('paired'))
getPairingQR.mockClear()
let resolveSecondLookup: ((value: Record<string, unknown>) => void) | undefined
listNetworkInterfaces.mockImplementationOnce(
() =>
new Promise((resolve) => {
resolveSecondLookup = resolve
})
)
await user.click(screen.getByRole('button', { name: 'Pair another device' }))
expect(getPairingQR).not.toHaveBeenCalled()
resolveSecondLookup?.({ interfaces: [{ name: 'Wi-Fi', address: '10.0.0.9' }] })
await waitFor(() =>
expect(screen.getByTestId('selected-address')).toHaveTextContent('10.0.0.9')
)
await waitFor(() => expect(getPairingQR).toHaveBeenCalledTimes(1))
expect(getPairingQR).toHaveBeenCalledWith({
address: '10.0.0.9',
connectionMode: 'automatic'
})
})
it('waits for the newest lookup when the flow is re-entered mid-refresh', async () => {
const user = userEvent.setup()
let resolveFirstLookup: ((value: Record<string, unknown>) => void) | undefined
let resolveSecondLookup: ((value: Record<string, unknown>) => void) | undefined
listNetworkInterfaces
.mockImplementationOnce(
() =>
new Promise((resolve) => {
resolveFirstLookup = resolve
})
)
.mockImplementationOnce(
() =>
new Promise((resolve) => {
resolveSecondLookup = resolve
})
)
render(<MobilePage />)
await waitFor(() => expect(screen.getByTestId('stage')).toHaveTextContent('intro'))
// Enter, leave, and re-enter while the first lookup is still unanswered, so
// two lookups overlap and the older one is the first to settle.
await user.click(screen.getByRole('button', { name: 'Enter flow' }))
await user.click(screen.getByRole('button', { name: 'Back' }))
await waitFor(() => expect(screen.getByTestId('stage')).toHaveTextContent('intro'))
await user.click(screen.getByRole('button', { name: 'Enter flow' }))
await user.click(screen.getByRole('button', { name: 'Continue' }))
await waitFor(() => expect(listNetworkInterfaces).toHaveBeenCalledTimes(2))
// The superseded lookup answers first. It must neither move the picker nor
// release the mint — this visit's lookup has not answered yet.
resolveFirstLookup?.({ interfaces: [{ name: 'Wi-Fi', address: '10.0.0.5' }] })
await waitFor(() => expect(screen.getByTestId('pair-loading')).toHaveTextContent('true'))
expect(getPairingQR).not.toHaveBeenCalled()
expect(screen.getByTestId('selected-address')).toHaveTextContent('none')
resolveSecondLookup?.({ interfaces: [{ name: 'Wi-Fi', address: '10.0.0.9' }] })
await waitFor(() =>
expect(screen.getByTestId('selected-address')).toHaveTextContent('10.0.0.9')
)
await waitFor(() => expect(getPairingQR).toHaveBeenCalledTimes(1))
expect(getPairingQR).toHaveBeenCalledWith({
address: '10.0.0.9',
connectionMode: 'automatic'
})
})
it('does not re-block Step 2 when an abandoned visit’s lookup settles last', async () => {
const user = userEvent.setup()
let resolveAbandonedLookup: ((value: Record<string, unknown>) => void) | undefined
let resolveCurrentLookup: ((value: Record<string, unknown>) => void) | undefined
listNetworkInterfaces
.mockImplementationOnce(
() =>
new Promise((resolve) => {
resolveAbandonedLookup = resolve
})
)
.mockImplementationOnce(
() =>
new Promise((resolve) => {
resolveCurrentLookup = resolve
})
)
render(<MobilePage />)
await waitFor(() => expect(screen.getByTestId('stage')).toHaveTextContent('intro'))
await user.click(screen.getByRole('button', { name: 'Enter flow' }))
await user.click(screen.getByRole('button', { name: 'Back' }))
await waitFor(() => expect(screen.getByTestId('stage')).toHaveTextContent('intro'))
await user.click(screen.getByRole('button', { name: 'Enter flow' }))
await user.click(screen.getByRole('button', { name: 'Continue' }))
await waitFor(() => expect(listNetworkInterfaces).toHaveBeenCalledTimes(2))
resolveCurrentLookup?.({ interfaces: [{ name: 'Wi-Fi', address: '10.0.0.9' }] })
await waitFor(() => expect(getPairingQR).toHaveBeenCalledTimes(1))
await waitFor(() => expect(screen.getByTestId('pair-loading')).toHaveTextContent('false'))
// The abandoned visit answers last. Recording it as the settled visit would
// walk the marker backwards and leave Step 2 waiting on a lookup that is
// never coming, with its Generate action disabled.
resolveAbandonedLookup?.({ interfaces: [{ name: 'Wi-Fi', address: '10.0.0.5' }] })
await waitFor(() =>
expect(screen.getByTestId('refreshing-addresses')).toHaveTextContent('false')
)
expect(screen.getByTestId('pair-loading')).toHaveTextContent('false')
expect(screen.getByTestId('selected-address')).toHaveTextContent('10.0.0.9')
expect(getPairingQR).toHaveBeenCalledTimes(1)
})
it('ignores an interface lookup that settles after a newer one', async () => {
listNetworkInterfaces.mockResolvedValue({
interfaces: [{ name: 'Wi-Fi', address: '10.0.0.5' }]
})
const user = userEvent.setup()
await openPairingStep()
await waitFor(() => expect(getPairingQR).toHaveBeenCalledTimes(1))
// Two manual refreshes overlap; the older one answers last with a stale list.
let resolveStaleRefresh: ((value: Record<string, unknown>) => void) | undefined
listNetworkInterfaces.mockImplementationOnce(
() =>
new Promise((resolve) => {
resolveStaleRefresh = resolve
})
)
await user.click(screen.getByRole('button', { name: 'Refresh addresses' }))
listNetworkInterfaces.mockResolvedValueOnce({
interfaces: [{ name: 'Wi-Fi', address: '10.0.0.7' }]
})
await user.click(screen.getByRole('button', { name: 'Refresh addresses' }))
await waitFor(() =>
expect(screen.getByTestId('selected-address')).toHaveTextContent('10.0.0.7')
)
await waitFor(() => expect(getPairingQR).toHaveBeenCalledTimes(2))
expect(getPairingQR).toHaveBeenLastCalledWith({
address: '10.0.0.7',
connectionMode: 'automatic',
rotate: true
})
resolveStaleRefresh?.({ interfaces: [{ name: 'Wi-Fi', address: '10.0.0.1' }] })
await waitFor(() =>
expect(screen.getByTestId('refreshing-addresses')).toHaveTextContent('false')
)
// The stale list must not reselect an address and rotate the live offer away.
expect(screen.getByTestId('selected-address')).toHaveTextContent('10.0.0.7')
expect(getPairingQR).toHaveBeenCalledTimes(2)
})
it('does not report the pairing step as busy during a manual address refresh', async () => {
listNetworkInterfaces.mockResolvedValue({
interfaces: [{ name: 'Wi-Fi', address: '10.0.0.5' }]
})
// Leave Step 2 with no QR on screen: that is the state where a refresh could
// be mistaken for a mint in progress.
getPairingQR.mockResolvedValueOnce({
available: false,
reason: 'websocket_unavailable',
guidance: 'WebSocket transport is not running'
})
const user = userEvent.setup()
await openPairingStep()
await waitFor(() => expect(getPairingQR).toHaveBeenCalledTimes(1))
await waitFor(() => expect(screen.getByTestId('pair-loading')).toHaveTextContent('false'))
expect(screen.getByTestId('pairing-qr')).toHaveTextContent('none')
expect(screen.getByTestId('relay-failure')).toHaveTextContent('none')
let resolveRefresh: ((value: Record<string, unknown>) => void) | undefined
listNetworkInterfaces.mockImplementationOnce(
() =>
new Promise((resolve) => {
resolveRefresh = resolve
})
)
await user.click(screen.getByRole('button', { name: 'Refresh addresses' }))
await waitFor(() =>
expect(screen.getByTestId('refreshing-addresses')).toHaveTextContent('true')
)
// Nothing is minting, so Step 2 must not claim it is — that would disable the
// Generate action while the user is only re-reading the interface list.
expect(screen.getByTestId('pair-loading')).toHaveTextContent('false')
resolveRefresh?.({ interfaces: [{ name: 'Wi-Fi', address: '10.0.0.5' }] })
await waitFor(() =>
expect(screen.getByTestId('refreshing-addresses')).toHaveTextContent('false')
)
})
it('keeps custom intent when the saved address is also discovered', async () => {
src/renderer/src/components/mobile/MobilePage.tsx
+31 -22
View File
@@ -62,9 +62,13 @@ export default function MobilePage(): React.JSX.Element {
const [refreshingNetworkInterfaces, setRefreshingNetworkInterfaces] = useState(false)
const hasGeneratedRef = useRef(false)
const pairingRequestIdRef = useRef(0)
// Why: written synchronously so the Step 2 auto-mint sees a lookup that started
// in the same commit, before `refreshingNetworkInterfaces` has been committed.
const networkInterfacesPendingRef = useRef(false)
// Why: each flow entry starts its own address lookup. Gating the Step 2 mint on
// "has this visit's lookup settled" is false until it answers, where "is a lookup
// running" cannot tell overlapping lookups apart and clears on the first to land.
const [pairingFlowVisit, setPairingFlowVisit] = useState(0)
const [addressedFlowVisit, setAddressedFlowVisit] = useState(-1)
const pairingAddressSettled = addressedFlowVisit === pairingFlowVisit
const networkInterfacesRequestIdRef = useRef(0)
const mountedRef = useMountedRef()
const closeMobilePage = useAppStore((s) => s.closeMobilePage)
const showMobileButton = useAppStore((s) => s.settings?.showMobileButton !== false)
@@ -191,25 +195,32 @@ export default function MobilePage(): React.JSX.Element {
})
const loadNetworkInterfaces = useCallback(async () => {
networkInterfacesPendingRef.current = true
const requestId = ++networkInterfacesRequestIdRef.current
const visit = pairingFlowVisit
if (mountedRef.current) {
setRefreshingNetworkInterfaces(true)
}
try {
const result = await window.api.mobile.listNetworkInterfaces()
if (mountedRef.current) {
// Why: a superseded lookup must not move the selection a newer one already
// resolved — that address change remints over the offer just advertised.
if (mountedRef.current && requestId === networkInterfacesRequestIdRef.current) {
setNetworkInterfaces(result.interfaces)
selectAddressAfterRefresh(result.interfaces)
}
} catch {
// Network list is non-critical; the QR will still mint with default routing.
} finally {
networkInterfacesPendingRef.current = false
if (mountedRef.current) {
setRefreshingNetworkInterfaces(false)
// Why: max, not assignment — an older visit's lookup landing last must not
// un-settle the visit a newer one already answered.
setAddressedFlowVisit((settled) => Math.max(settled, visit))
if (requestId === networkInterfacesRequestIdRef.current) {
setRefreshingNetworkInterfaces(false)
}
}
}
}, [mountedRef, selectAddressAfterRefresh])
}, [mountedRef, pairingFlowVisit, selectAddressAfterRefresh])
useEffect(() => {
if (stage !== 'flow') {
@@ -268,19 +279,20 @@ export default function MobilePage(): React.JSX.Element {
if (!canGenerate) {
return
}
// Why: entering Step 2 also starts the address lookup, and minting before it
// settles advertises an address the lookup is about to replace — the
// Why: entering Step 2 also starts this visit's address lookup, and minting
// before it settles advertises an address the lookup is about to replace — the
// replacement then rotates away the credential this mint just created, so one
// Continue runs two overlapping offers through main for one pending token.
if (refreshingNetworkInterfaces || networkInterfacesPendingRef.current) {
if (!pairingAddressSettled) {
return
}
void generatePairing(false)
}, [stage, stepIdx, canGenerate, generatePairing, refreshingNetworkInterfaces])
}, [stage, stepIdx, canGenerate, generatePairing, pairingAddressSettled])
// Why: entering the flow must mint a fresh pairing token — clear stale QR
// state so we never flash an expired code from a previous session.
const enterFlow = (): void => {
setPairingFlowVisit((visit) => visit + 1)
hasGeneratedRef.current = false
setPairQrDataUrl(null)
setPairQrSize(null)
@@ -293,6 +305,7 @@ export default function MobilePage(): React.JSX.Element {
// Why: from the paired summary, "Pair another device" jumps straight to
// Step 2 since the app is presumably already installed on the user's phone.
const pairAnotherDevice = (): void => {
setPairingFlowVisit((visit) => visit + 1)
hasGeneratedRef.current = false
setPairQrDataUrl(null)
setPairQrSize(null)
@@ -325,15 +338,11 @@ export default function MobilePage(): React.JSX.Element {
// Why: while the deferred first mint waits on the address, Step 2 would
// otherwise read "Generate a pairing code to continue" — a prompt for work it
// is already about to do on the user's behalf.
const pairPreparing =
pairLoading ||
(stage === 'flow' &&
stepIdx === 1 &&
canGenerate &&
refreshingNetworkInterfaces &&
pairQrDataUrl === null &&
relayMintFailure === null)
// is already about to do on the user's behalf. Kept separate from pairLoading:
// that one feeds the invalidation hook's shouldRegenerate, so folding this into
// it would let a mode switch mint before the address settles.
const awaitingPairingAddress =
stage === 'flow' && stepIdx === 1 && canGenerate && !pairingAddressSettled
return (
<MobilePageContent
@@ -360,7 +369,7 @@ export default function MobilePage(): React.JSX.Element {
openAndroidInstallGuide={openAndroidInstallGuide}
openInstallUrl={openInstallUrl}
pairAnotherDevice={pairAnotherDevice}
pairLoading={pairPreparing}
pairLoading={pairLoading || awaitingPairingAddress}
connectionMode={connectionMode}
handleConnectionModeChange={handleConnectionModeChange}
pairQrDataUrl={pairQrDataUrl}