mirror of
https://github.com/stablyai/orca.git
synced 2026-09-29 08:03:20 +00:00
fix(mobile): stop hybrid-only failures from blocking unpair, sleep, diagnostics
Three native-path regressions plus a doc correction: - Unpair awaited removeMobileWebHostCache before removeHost. The native store throws on an empty identity or a failed tree delete, and that cache need not exist at all on a native build, so a hybrid-only failure stranded a paired host. Both cache cleanups are best-effort now. - Activation diagnostics dropped the target and the RPC failure code, leaving concurrent activations indistinguishable and failures unexplained. Restore the redacting helpers from main; a new test pins that only the 8-char suffix reaches the log. - The Sleep action lost its `.catch`, so a rejected fire-and-forget sleep surfaced as an unhandled rejection. - The README claimed an unset architecture keeps native. It does for release builds, but a development build defaults to hybrid; document the real rule and how to opt a dev build back into native. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
This commit is contained in:
+12
-3
@@ -136,9 +136,18 @@ EXPO_PUBLIC_ORCA_MOBILE_ARCHITECTURE=hybrid pnpm exec expo run:ios
|
||||
EXPO_PUBLIC_ORCA_MOBILE_ARCHITECTURE=hybrid pnpm exec expo run:android
|
||||
```
|
||||
|
||||
Unset or `native` keeps the current native workspace experience. The
|
||||
development-only `EXPO_PUBLIC_ORCA_E2E_MOBILE_NATIVE_BASELINE=1` override still
|
||||
selects native routes for parity captures.
|
||||
`EXPO_PUBLIC_ORCA_MOBILE_ARCHITECTURE=native` always selects the native routes.
|
||||
Leaving it unset is **not** the same everywhere: a release build defaults to
|
||||
native, but a development build defaults to hybrid so the hosted journeys and
|
||||
the native baselines share one Metro bundle. Set it to `native` explicitly to
|
||||
exercise the native app from a dev build:
|
||||
|
||||
```bash
|
||||
EXPO_PUBLIC_ORCA_MOBILE_ARCHITECTURE=native pnpm exec expo run:ios
|
||||
```
|
||||
|
||||
The development-only `EXPO_PUBLIC_ORCA_E2E_MOBILE_NATIVE_BASELINE=1` override
|
||||
also selects native routes, for parity captures.
|
||||
|
||||
## Package Build
|
||||
|
||||
|
||||
@@ -183,11 +183,14 @@ export function HostScreenOverlays({ controller }: { controller: HostScreenContr
|
||||
state.setSleptIds((prev) =>
|
||||
new Set(prev).add(getWorktreeRowIdentity(actionTarget))
|
||||
)
|
||||
void actions.sleepWorktree(actionTarget.worktreeId)
|
||||
// Why: an unhandled rejection here would surface as a redbox on a fire-and-forget sleep.
|
||||
void actions.sleepWorktree(actionTarget.worktreeId).catch(() => null)
|
||||
} else if (client) {
|
||||
void client.sendRequest('worktree.sleep', {
|
||||
worktree: `id:${actionTarget.worktreeId}`
|
||||
})
|
||||
void client
|
||||
.sendRequest('worktree.sleep', {
|
||||
worktree: `id:${actionTarget.worktreeId}`
|
||||
})
|
||||
.catch(() => null)
|
||||
}
|
||||
state.setActionTarget(null)
|
||||
}
|
||||
|
||||
@@ -66,6 +66,50 @@ describe('mobile session tab activation', () => {
|
||||
expect(sendRequest).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('reports which target failed and why, without logging the full identity', async () => {
|
||||
const log = vi.spyOn(console, 'log').mockImplementation(() => {})
|
||||
const tabId = 'tab-secret-0123abcd'
|
||||
const sendRequest = vi.fn<RpcClient['sendRequest']>().mockResolvedValue({
|
||||
id: 'rpc-1',
|
||||
ok: false,
|
||||
error: { code: 'worktree_not_found', message: 'gone' },
|
||||
_meta: { runtimeId: 'runtime-1' }
|
||||
} as RpcResponse)
|
||||
|
||||
await activateMobileSessionTab(clientWith(sendRequest), {
|
||||
worktree: 'id:worktree-1',
|
||||
tabId,
|
||||
notifyClients: false,
|
||||
navigation: 'caller',
|
||||
intent: 'user'
|
||||
})
|
||||
|
||||
expect(log).toHaveBeenCalledWith('[terminal-diagnostic]', 'activation-result', {
|
||||
terminal: false,
|
||||
target: '0123abcd',
|
||||
ok: false,
|
||||
rpcCode: 'worktree_not_found'
|
||||
})
|
||||
expect(JSON.stringify(log.mock.calls)).not.toContain(tabId)
|
||||
log.mockRestore()
|
||||
})
|
||||
|
||||
it('names the failure class when activation throws outside a cutover', async () => {
|
||||
const log = vi.spyOn(console, 'log').mockImplementation(() => {})
|
||||
const sendRequest = vi.fn<RpcClient['sendRequest']>().mockRejectedValue(new TypeError('nope'))
|
||||
|
||||
await expect(
|
||||
focusMobileTerminal(clientWith(sendRequest), 'terminal-secret-89abcdef')
|
||||
).rejects.toThrow('nope')
|
||||
|
||||
expect(log).toHaveBeenCalledWith('[terminal-diagnostic]', 'activation-error', {
|
||||
terminal: true,
|
||||
target: '89abcdef',
|
||||
errorName: 'TypeError'
|
||||
})
|
||||
log.mockRestore()
|
||||
})
|
||||
|
||||
it('retries at most once when consecutive cutovers interrupt activation', async () => {
|
||||
const sendRequest = vi
|
||||
.fn<RpcClient['sendRequest']>()
|
||||
|
||||
@@ -2,7 +2,11 @@ import type { TabActivationIntent } from '../../../src/shared/tab-activation-int
|
||||
import type { RpcClient } from '../transport/rpc-client'
|
||||
import { LogicalClientCutoverError } from '../transport/stable-logical-rpc-client'
|
||||
import type { RpcResponse } from '../transport/types'
|
||||
import { logMobileTerminalDiagnostic } from './mobile-terminal-diagnostics'
|
||||
import {
|
||||
getMobileTerminalDiagnosticErrorName,
|
||||
logMobileTerminalDiagnostic,
|
||||
shortenMobileTerminalDiagnosticId
|
||||
} from './mobile-terminal-diagnostics'
|
||||
|
||||
type ActivationClient = Pick<RpcClient, 'sendRequest'>
|
||||
|
||||
@@ -18,41 +22,48 @@ type MobileSessionTabActivationParams = {
|
||||
|
||||
async function retryIdempotentActivationAfterCutover(
|
||||
request: () => Promise<RpcResponse>,
|
||||
operation: 'terminal.focus' | 'session.tabs.activate'
|
||||
operation: 'terminal.focus' | 'session.tabs.activate',
|
||||
target: string
|
||||
): Promise<RpcResponse> {
|
||||
const terminal = operation === 'terminal.focus'
|
||||
logMobileTerminalDiagnostic('activation-request', { terminal })
|
||||
try {
|
||||
const response = await request()
|
||||
const diagnosticTarget = shortenMobileTerminalDiagnosticId(target)
|
||||
logMobileTerminalDiagnostic('activation-request', { terminal, target: diagnosticTarget })
|
||||
const logResult = (response: RpcResponse) =>
|
||||
logMobileTerminalDiagnostic('activation-result', {
|
||||
terminal,
|
||||
ok: response.ok
|
||||
target: diagnosticTarget,
|
||||
ok: response.ok,
|
||||
// Why: without the code a failed activation is indistinguishable from a rejected one.
|
||||
rpcCode: response.ok ? null : response.error.code
|
||||
})
|
||||
try {
|
||||
const response = await request()
|
||||
logResult(response)
|
||||
return response
|
||||
} catch (error) {
|
||||
if (!(error instanceof LogicalClientCutoverError)) {
|
||||
logMobileTerminalDiagnostic('activation-error', {
|
||||
terminal,
|
||||
isErrorObject: error instanceof Error
|
||||
target: diagnosticTarget,
|
||||
errorName: getMobileTerminalDiagnosticErrorName(error)
|
||||
})
|
||||
throw error
|
||||
}
|
||||
logMobileTerminalDiagnostic('activation-cutover-retry', {
|
||||
terminal
|
||||
terminal,
|
||||
target: diagnosticTarget
|
||||
})
|
||||
// Why: cutover rejects ambiguous in-flight work after the replacement is
|
||||
// active; these state-setting requests are idempotent and safe to repeat once.
|
||||
try {
|
||||
const response = await request()
|
||||
logMobileTerminalDiagnostic('activation-result', {
|
||||
terminal,
|
||||
ok: response.ok
|
||||
})
|
||||
logResult(response)
|
||||
return response
|
||||
} catch (retryError) {
|
||||
logMobileTerminalDiagnostic('activation-error', {
|
||||
terminal,
|
||||
isErrorObject: retryError instanceof Error
|
||||
target: diagnosticTarget,
|
||||
errorName: getMobileTerminalDiagnosticErrorName(retryError)
|
||||
})
|
||||
throw retryError
|
||||
}
|
||||
@@ -65,7 +76,8 @@ export function focusMobileTerminal(
|
||||
): Promise<RpcResponse> {
|
||||
return retryIdempotentActivationAfterCutover(
|
||||
() => client.sendRequest('terminal.focus', { terminal, navigation: 'host' }),
|
||||
'terminal.focus'
|
||||
'terminal.focus',
|
||||
terminal
|
||||
)
|
||||
}
|
||||
|
||||
@@ -75,6 +87,7 @@ export function activateMobileSessionTab(
|
||||
): Promise<RpcResponse> {
|
||||
return retryIdempotentActivationAfterCutover(
|
||||
() => client.sendRequest('session.tabs.activate', params),
|
||||
'session.tabs.activate'
|
||||
'session.tabs.activate',
|
||||
params.tabId
|
||||
)
|
||||
}
|
||||
|
||||
@@ -24,12 +24,22 @@ type MobileTerminalDiagnosticEvent =
|
||||
| 'webview-ready'
|
||||
| 'webview-ref'
|
||||
|
||||
type MobileTerminalDiagnosticValue = number | boolean | null | undefined
|
||||
type MobileTerminalDiagnosticValue = string | number | boolean | null | undefined
|
||||
|
||||
export type MobileTerminalDiagnosticDetails = Readonly<
|
||||
Record<string, MobileTerminalDiagnosticValue>
|
||||
>
|
||||
|
||||
// Why: full runtime identifiers make shared logs unnecessarily sensitive; the
|
||||
// suffix is enough to correlate lifecycle events within one reproduction.
|
||||
export function shortenMobileTerminalDiagnosticId(value: string | null | undefined): string | null {
|
||||
return value ? value.slice(-8) : null
|
||||
}
|
||||
|
||||
export function getMobileTerminalDiagnosticErrorName(error: unknown): string {
|
||||
return error instanceof Error && error.name ? error.name : typeof error
|
||||
}
|
||||
|
||||
type MobileTerminalDiagnosticRecord = {
|
||||
event: MobileTerminalDiagnosticEvent
|
||||
details: MobileTerminalDiagnosticDetails
|
||||
|
||||
@@ -115,31 +115,35 @@ describe('host removal lifecycle', () => {
|
||||
expect(asyncStorage.removeItem).toHaveBeenCalledWith('orca:mobileNotificationsWatermark:host-1')
|
||||
})
|
||||
|
||||
it('keeps pairing metadata and the live client when native cache deletion fails', async () => {
|
||||
it('unpairs even when the hybrid native cache deletion fails', async () => {
|
||||
// The Kotlin/Swift store throws on an empty identity or a failed tree delete, and that
|
||||
// cache does not exist at all on a native build — neither may strand a paired host.
|
||||
removeMobileWebHostCacheMock.mockRejectedValue(new Error('native cache unavailable'))
|
||||
removeHostMock.mockResolvedValue(undefined)
|
||||
const closeHostClient = vi.fn()
|
||||
|
||||
await expect(
|
||||
removeHostAndCloseClient('host-1', 'public-key-1', closeHostClient)
|
||||
).rejects.toThrow('native cache unavailable')
|
||||
).resolves.toBeUndefined()
|
||||
|
||||
expect(removeHostMock).not.toHaveBeenCalled()
|
||||
expect(closeHostClient).not.toHaveBeenCalled()
|
||||
expect(deleteConnectionLogMock).not.toHaveBeenCalled()
|
||||
expect(removeHostMock).toHaveBeenCalledWith('host-1')
|
||||
expect(closeHostClient).toHaveBeenCalledWith('host-1')
|
||||
expect(deleteConnectionLogMock).toHaveBeenCalledWith('host-1')
|
||||
})
|
||||
|
||||
it('keeps pairing metadata when cold-route cleanup cannot commit', async () => {
|
||||
it('unpairs even when cold-route cleanup cannot commit', async () => {
|
||||
clearMobileWebColdResumeRouteForHostMock.mockRejectedValue(
|
||||
new Error('route storage unavailable')
|
||||
)
|
||||
removeHostMock.mockResolvedValue(undefined)
|
||||
const closeHostClient = vi.fn()
|
||||
|
||||
await expect(
|
||||
removeHostAndCloseClient('host-1', 'public-key-1', closeHostClient)
|
||||
).rejects.toThrow('route storage unavailable')
|
||||
).resolves.toBeUndefined()
|
||||
|
||||
expect(removeHostMock).not.toHaveBeenCalled()
|
||||
expect(closeHostClient).not.toHaveBeenCalled()
|
||||
expect(removeHostMock).toHaveBeenCalledWith('host-1')
|
||||
expect(closeHostClient).toHaveBeenCalledWith('host-1')
|
||||
})
|
||||
|
||||
it('forgets removed-host logs even when client teardown throws', async () => {
|
||||
|
||||
@@ -12,9 +12,10 @@ export async function removeHostAndCloseClient(
|
||||
hostPublicKey: string,
|
||||
forgetHostClient: (hostId: string) => void
|
||||
): Promise<void> {
|
||||
// Why: cache deletion is recoverable by redownload, while a completed unpair must not leave host code behind.
|
||||
await removeMobileWebHostCache(hostPublicKey)
|
||||
await clearMobileWebColdResumeRouteForHost(hostId)
|
||||
// Why: cache deletion is recoverable by redownload, so a hybrid-only failure here must
|
||||
// never block the unpair itself — on a native build the cache may not even exist.
|
||||
await removeMobileWebHostCache(hostPublicKey).catch(() => null)
|
||||
await clearMobileWebColdResumeRouteForHost(hostId).catch(() => null)
|
||||
// Why: closing before the metadata commit can strand a still-paired host on
|
||||
// storage failure; closing immediately after success prevents socket leaks.
|
||||
await removeHost(hostId)
|
||||
|
||||
Reference in New Issue
Block a user