From e1de2fa987a2a7b195f22c35e342a5febf360a1a Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Tue, 14 Jul 2026 20:20:42 -0400 Subject: [PATCH] Fix mobile cutover activation and usage refresh loops Co-authored-by: Orca --- .../app/h/[hostId]/session/[worktreeId].tsx | 55 +++++++------- .../mobile-session-startup-source.test.ts | 4 +- .../mobile-session-tab-activation.test.ts | 75 +++++++++++++++++++ .../session/mobile-session-tab-activation.ts | 45 +++++++++++ src/main/rate-limits/service.test.ts | 51 +++++++++++++ src/main/rate-limits/service.ts | 11 +++ src/main/runtime/orca-runtime.ts | 11 +++ ...ime-request-connection.integration.test.ts | 5 ++ src/main/runtime/rpc/methods/accounts.test.ts | 58 ++++++++++++++ src/main/runtime/rpc/methods/accounts.ts | 8 +- 10 files changed, 288 insertions(+), 35 deletions(-) create mode 100644 mobile/src/session/mobile-session-tab-activation.test.ts create mode 100644 mobile/src/session/mobile-session-tab-activation.ts create mode 100644 src/main/runtime/rpc/methods/accounts.test.ts diff --git a/mobile/app/h/[hostId]/session/[worktreeId].tsx b/mobile/app/h/[hostId]/session/[worktreeId].tsx index b4e7207d6a5..96e56a95998 100644 --- a/mobile/app/h/[hostId]/session/[worktreeId].tsx +++ b/mobile/app/h/[hostId]/session/[worktreeId].tsx @@ -197,6 +197,10 @@ import { isDictationSetupRequiredError } from '../../../../src/dictation/mobile-dictation-setup' import { TerminalPaneView } from '../../../../src/session/TerminalPaneView' +import { + activateMobileSessionTab, + focusMobileTerminal +} from '../../../../src/session/mobile-session-tab-activation' import { getRepoIdFromMobileWorktreeId, isFileExistsErrorMessage, @@ -2931,15 +2935,13 @@ export default function SessionScreen() { } subscribeToTerminal(handle) if (client) { - void client.sendRequest('terminal.focus', { terminal: handle }).catch(() => {}) + void focusMobileTerminal(client, handle).catch(() => {}) if (matchingTab) { - void client - .sendRequest('session.tabs.activate', { - worktree: `id:${worktreeId}`, - tabId: matchingTab.id, - notifyClients: false - }) - .catch(() => {}) + void activateMobileSessionTab(client, { + worktree: `id:${worktreeId}`, + tabId: matchingTab.id, + notifyClients: false + }).catch(() => {}) } } }, @@ -2973,13 +2975,11 @@ export default function SessionScreen() { activeHandleRef.current = null setActiveHandle(null) if (client) { - void client - .sendRequest('session.tabs.activate', { - worktree: `id:${worktreeId}`, - tabId: tab.id, - notifyClients: false - }) - .catch(() => {}) + void activateMobileSessionTab(client, { + worktree: `id:${worktreeId}`, + tabId: tab.id, + notifyClients: false + }).catch(() => {}) } return } @@ -2997,13 +2997,11 @@ export default function SessionScreen() { activeHandleRef.current = null setActiveHandle(null) if (client) { - void client - .sendRequest('session.tabs.activate', { - worktree: `id:${worktreeId}`, - tabId: tab.id, - notifyClients: false - }) - .catch(() => {}) + void activateMobileSessionTab(client, { + worktree: `id:${worktreeId}`, + tabId: tab.id, + notifyClients: false + }).catch(() => {}) } if (tab.type === 'browser') { return @@ -4302,13 +4300,12 @@ export default function SessionScreen() { // Why: a hydrated headless/server-owned tab can already be active but still // pending; activation is the RPC that materializes or focuses its PTY handle. pendingTerminalActivationAttemptRef.current = activationKey - void client - .sendRequest('session.tabs.activate', { - worktree: `id:${worktreeId}`, - tabId: activePendingTerminalTab.id, - leafId: activePendingTerminalTab.leafId, - notifyClients: false - }) + void activateMobileSessionTab(client, { + worktree: `id:${worktreeId}`, + tabId: activePendingTerminalTab.id, + leafId: activePendingTerminalTab.leafId, + notifyClients: false + }) .then((response) => { if (!response.ok) { if (pendingTerminalActivationAttemptRef.current === activationKey) { diff --git a/mobile/src/session/mobile-session-startup-source.test.ts b/mobile/src/session/mobile-session-startup-source.test.ts index aca17cfb509..d84b556a531 100644 --- a/mobile/src/session/mobile-session-startup-source.test.ts +++ b/mobile/src/session/mobile-session-startup-source.test.ts @@ -58,7 +58,7 @@ describe('mobile session startup', () => { expect(pendingActivationEffect).toContain( 'pendingTerminalActivationAttemptRef.current === activationKey' ) - expect(pendingActivationEffect).toContain("sendRequest('session.tabs.activate'") + expect(pendingActivationEffect).toContain('activateMobileSessionTab(client,') expect(pendingActivationEffect).toContain('tabId: activePendingTerminalTab.id') expect(pendingActivationEffect).toContain('leafId: activePendingTerminalTab.leafId') expect(pendingActivationEffect).toContain('notifyClients: false') @@ -69,7 +69,7 @@ describe('mobile session startup', () => { }) it('keeps mobile session tab activation local to the phone', () => { - const activationRequests = source.split("sendRequest('session.tabs.activate'").slice(1) + const activationRequests = source.split('activateMobileSessionTab(client,').slice(1) expect(activationRequests).toHaveLength(4) for (const request of activationRequests) { diff --git a/mobile/src/session/mobile-session-tab-activation.test.ts b/mobile/src/session/mobile-session-tab-activation.test.ts new file mode 100644 index 00000000000..d448d7e2f53 --- /dev/null +++ b/mobile/src/session/mobile-session-tab-activation.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, it, vi } from 'vitest' +import { LogicalClientCutoverError } from '../transport/stable-logical-rpc-client' +import type { RpcClient } from '../transport/rpc-client' +import type { RpcResponse } from '../transport/types' +import { activateMobileSessionTab, focusMobileTerminal } from './mobile-session-tab-activation' + +function success(): RpcResponse { + return { id: 'rpc-1', ok: true, result: {}, _meta: { runtimeId: 'runtime-1' } } +} + +function clientWith(sendRequest: RpcClient['sendRequest']): Pick { + return { sendRequest } +} + +describe('mobile session tab activation', () => { + it('retries terminal focus once on the authenticated replacement after cutover', async () => { + const sendRequest = vi + .fn() + .mockRejectedValueOnce(new LogicalClientCutoverError()) + .mockResolvedValueOnce(success()) + + await expect(focusMobileTerminal(clientWith(sendRequest), 'terminal-1')).resolves.toMatchObject( + { + ok: true + } + ) + expect(sendRequest).toHaveBeenCalledTimes(2) + expect(sendRequest).toHaveBeenNthCalledWith(1, 'terminal.focus', { terminal: 'terminal-1' }) + expect(sendRequest).toHaveBeenNthCalledWith(2, 'terminal.focus', { terminal: 'terminal-1' }) + }) + + it('retries session-tab activation with the same target after cutover', async () => { + const sendRequest = vi + .fn() + .mockRejectedValueOnce(new LogicalClientCutoverError()) + .mockResolvedValueOnce(success()) + const params = { + worktree: 'id:worktree-1', + tabId: 'tab-1', + leafId: 'leaf-1', + notifyClients: false as const + } + + await expect(activateMobileSessionTab(clientWith(sendRequest), params)).resolves.toMatchObject({ + ok: true + }) + expect(sendRequest).toHaveBeenCalledTimes(2) + expect(sendRequest).toHaveBeenNthCalledWith(1, 'session.tabs.activate', params) + expect(sendRequest).toHaveBeenNthCalledWith(2, 'session.tabs.activate', params) + }) + + it('does not retry unrelated transport failures', async () => { + const sendRequest = vi.fn().mockRejectedValue(new Error('offline')) + + await expect(focusMobileTerminal(clientWith(sendRequest), 'terminal-1')).rejects.toThrow( + 'offline' + ) + expect(sendRequest).toHaveBeenCalledOnce() + }) + + it('retries at most once when consecutive cutovers interrupt activation', async () => { + const sendRequest = vi + .fn() + .mockRejectedValue(new LogicalClientCutoverError()) + + await expect( + activateMobileSessionTab(clientWith(sendRequest), { + worktree: 'id:worktree-1', + tabId: 'tab-1', + notifyClients: false + }) + ).rejects.toBeInstanceOf(LogicalClientCutoverError) + expect(sendRequest).toHaveBeenCalledTimes(2) + }) +}) diff --git a/mobile/src/session/mobile-session-tab-activation.ts b/mobile/src/session/mobile-session-tab-activation.ts new file mode 100644 index 00000000000..80e9beebd2b --- /dev/null +++ b/mobile/src/session/mobile-session-tab-activation.ts @@ -0,0 +1,45 @@ +import type { RpcClient } from '../transport/rpc-client' +import { LogicalClientCutoverError } from '../transport/stable-logical-rpc-client' +import type { RpcResponse } from '../transport/types' + +type ActivationClient = Pick + +type MobileSessionTabActivationParams = { + worktree: string + tabId: string + leafId?: string + notifyClients: false +} + +async function retryIdempotentActivationAfterCutover( + request: () => Promise +): Promise { + try { + return await request() + } catch (error) { + if (!(error instanceof LogicalClientCutoverError)) { + throw error + } + // Why: cutover rejects ambiguous in-flight work after the replacement is + // active; these state-setting requests are idempotent and safe to repeat once. + return request() + } +} + +export function focusMobileTerminal( + client: ActivationClient, + terminal: string +): Promise { + return retryIdempotentActivationAfterCutover(() => + client.sendRequest('terminal.focus', { terminal }) + ) +} + +export function activateMobileSessionTab( + client: ActivationClient, + params: MobileSessionTabActivationParams +): Promise { + return retryIdempotentActivationAfterCutover(() => + client.sendRequest('session.tabs.activate', params) + ) +} diff --git a/src/main/rate-limits/service.test.ts b/src/main/rate-limits/service.test.ts index 9bd66cb806a..78a374269f5 100644 --- a/src/main/rate-limits/service.test.ts +++ b/src/main/rate-limits/service.test.ts @@ -764,6 +764,39 @@ describe('RateLimitService', () => { expect(fetchCodexRateLimits).toHaveBeenCalledTimes(2) }) + it('does not refetch fresh provider data for replayed mobile subscriptions', async () => { + const service = new RateLimitService() + vi.mocked(fetchClaudeRateLimits).mockResolvedValue(okProvider('claude', 10)) + vi.mocked(fetchCodexRateLimits).mockResolvedValue(okProvider('codex', 20)) + + await service.refreshIfStale() + await service.refreshIfStale() + await service.refreshIfStale() + + expect(fetchClaudeRateLimits).toHaveBeenCalledOnce() + expect(fetchCodexRateLimits).toHaveBeenCalledOnce() + }) + + it('does not queue a follow-up fetch when a mobile subscription replays mid-fetch', async () => { + const service = new RateLimitService() + const claude = deferred() + const codex = deferred() + vi.mocked(fetchClaudeRateLimits).mockReturnValue(claude.promise) + vi.mocked(fetchCodexRateLimits).mockReturnValue(codex.promise) + + const firstRefresh = service.refreshIfStale() + await Promise.resolve() + const replayedRefresh = service.refreshIfStale() + + claude.resolve(okProvider('claude', 10)) + codex.resolve(okProvider('codex', 20)) + await firstRefresh + await replayedRefresh + + expect(fetchClaudeRateLimits).toHaveBeenCalledOnce() + expect(fetchCodexRateLimits).toHaveBeenCalledOnce() + }) + it('waits for a queued explicit refresh when another fetch is already in flight', async () => { const service = new RateLimitService() const firstClaude = deferred() @@ -1285,6 +1318,24 @@ describe('RateLimitService', () => { ) }) + it('does not start overlapping inactive Claude preview fetches', async () => { + const service = new RateLimitService() + const accountFetch = deferred() + service.setInactiveClaudeAccountsResolver(() => [ + { id: 'account-1', managedAuthPath: '/tmp/account-1/auth' } + ]) + vi.mocked(fetchManagedAccountUsage).mockReturnValueOnce(accountFetch.promise) + + const firstFetch = service.fetchInactiveClaudeAccountsOnOpen() + await Promise.resolve() + await service.fetchInactiveClaudeAccountsOnOpen() + + expect(fetchManagedAccountUsage).toHaveBeenCalledTimes(1) + + accountFetch.resolve(okProvider('claude', 50, Date.now())) + await firstFetch + }) + it('does not start overlapping inactive Codex preview fetches', async () => { const service = new RateLimitService() const accountFetch = deferred() diff --git a/src/main/rate-limits/service.ts b/src/main/rate-limits/service.ts index 84fb016fec8..db7c9ca5218 100644 --- a/src/main/rate-limits/service.ts +++ b/src/main/rate-limits/service.ts @@ -347,6 +347,14 @@ export class RateLimitService { return this.getState() } + async refreshIfStale(): Promise { + // Why: reconnecting mobile subscribers need fresh backgrounded-desktop data, + // but replaying a subscription must not queue another forced provider fetch. + const plan = this.getActiveWindowRefreshPlan(Date.now()) + await this.runActiveWindowRefreshPlan(plan) + return this.getState() + } + async refreshGrok(): Promise { await this.fetchGrokOnly({ force: true }) return this.getState() @@ -477,6 +485,9 @@ export class RateLimitService { return } this.pruneInactiveClaudeState() + if (this.inactiveClaudeFetching.size > 0) { + return + } const accounts = this.inactiveClaudeAccountsResolver?.() ?? [] if (accounts.length === 0) { return diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 6d8c041e019..43bb9a6fe6e 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -8061,6 +8061,17 @@ export class OrcaRuntimeService { ]) } + // Why: connection migration replays subscriptions; use the stale-aware lane + // so a reconnect cannot turn one mobile viewer into continuous forced fetches. + async refreshAccountsForMobileSubscriber(): Promise { + const { rateLimits } = this.requireAccountServices() + await Promise.allSettled([ + rateLimits.refreshIfStale(), + rateLimits.fetchInactiveClaudeAccountsOnOpen(), + rateLimits.fetchInactiveCodexAccountsOnOpen() + ]) + } + selectClaudeAccount(accountId: string | null): Promise { return this.requireAccountServices().claudeAccounts.selectAccount(accountId) } diff --git a/src/main/runtime/remote-runtime-request-connection.integration.test.ts b/src/main/runtime/remote-runtime-request-connection.integration.test.ts index e81b299fafd..2715883a426 100644 --- a/src/main/runtime/remote-runtime-request-connection.integration.test.ts +++ b/src/main/runtime/remote-runtime-request-connection.integration.test.ts @@ -337,6 +337,11 @@ describe('remote runtime request connection integration', () => { listener({ claude: null, codex: null }) } }, + refreshAccountsForMobileSubscriber: async () => { + for (const listener of accountsListeners) { + listener({ claude: null, codex: null }) + } + }, onAccountsChanged: (listener: (snapshot: unknown) => void) => { accountsListeners.add(listener) return () => accountsListeners.delete(listener) diff --git a/src/main/runtime/rpc/methods/accounts.test.ts b/src/main/runtime/rpc/methods/accounts.test.ts new file mode 100644 index 00000000000..ea4c80ceda0 --- /dev/null +++ b/src/main/runtime/rpc/methods/accounts.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it, vi } from 'vitest' +import type { OrcaRuntimeService } from '../../orca-runtime' +import { isStreamingMethod } from '../core' +import { ACCOUNT_METHODS } from './accounts' + +function method(name: string) { + const found = ACCOUNT_METHODS.find((candidate) => candidate.name === name) + if (!found) { + throw new Error(`Missing method ${name}`) + } + return found +} + +describe('account RPC methods', () => { + it('keeps explicit account-list refreshes on the forced refresh lane', async () => { + const snapshot = { claude: null, codex: null } + const runtime = { + refreshAccountsForMobile: vi.fn().mockResolvedValue(undefined), + getAccountsSnapshot: vi.fn(() => snapshot) + } as unknown as OrcaRuntimeService + const list = method('accounts.list') + if (isStreamingMethod(list)) { + throw new Error('accounts.list must be a request method') + } + + await expect(list.handler(undefined, { runtime })).resolves.toBe(snapshot) + expect(runtime.refreshAccountsForMobile).toHaveBeenCalledOnce() + }) + + it('uses a stale-aware refresh when a connection replays the subscription', async () => { + const snapshot = { claude: null, codex: null } + let cleanup: (() => void) | undefined + const runtime = { + getAccountsSnapshot: vi.fn(() => snapshot), + onAccountsChanged: vi.fn(() => vi.fn()), + registerSubscriptionCleanup: vi.fn((_id: string, nextCleanup: () => void) => { + cleanup = nextCleanup + }), + refreshAccountsForMobile: vi.fn().mockResolvedValue(undefined), + refreshAccountsForMobileSubscriber: vi.fn().mockResolvedValue(undefined) + } as unknown as OrcaRuntimeService + const subscribe = method('accounts.subscribe') + if (!isStreamingMethod(subscribe)) { + throw new Error('accounts.subscribe must be a streaming method') + } + const emit = vi.fn() + + const running = subscribe.handler(undefined, { runtime, connectionId: 'connection-1' }, emit) + await vi.waitFor(() => { + expect(runtime.refreshAccountsForMobileSubscriber).toHaveBeenCalledOnce() + }) + + expect(runtime.refreshAccountsForMobile).not.toHaveBeenCalled() + expect(emit).toHaveBeenCalledWith(expect.objectContaining({ type: 'ready', snapshot })) + cleanup?.() + await running + }) +}) diff --git a/src/main/runtime/rpc/methods/accounts.ts b/src/main/runtime/rpc/methods/accounts.ts index 21612a707f0..b1ac5426975 100644 --- a/src/main/runtime/rpc/methods/accounts.ts +++ b/src/main/runtime/rpc/methods/accounts.ts @@ -92,11 +92,11 @@ export const ACCOUNT_METHODS: readonly RpcAnyMethod[] = [ ) // Why: emit the current snapshot synchronously so the phone has - // something to render immediately, then kick a forced refresh that - // will broadcast a fresh snapshot through the listener once each - // provider fetch completes. + // something to render immediately, then refresh only stale data. + // Connection cutovers replay this subscription and must not turn the + // manual-force lane into an unbounded provider-fetch loop. emit({ type: 'ready', subscriptionId, snapshot: runtime.getAccountsSnapshot() }) - void runtime.refreshAccountsForMobile() + void runtime.refreshAccountsForMobileSubscriber() }) } }),