From 8972d744846192a27a28fd6e8e7a299a7499d1b5 Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 16:37:44 -0700 Subject: [PATCH] fix(relay): stop an unreadable session file reporting as a sign-out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The coordinator's own comment states the contract: readContext "throws on transient failures and returns null solely when the cloud session is gone (absent, or cleared by a 401)". The implementation did not honour it. readFreshOrcaCloudSession collapsed readOrcaCloudSession's `unreadable` status into `reconnect-required`, so readRelayAuthContext returned null and the coordinator published RELAY_HOST_CLOSE_REASON.SIGNED_OUT, closed the broker with that wire reason, and armed no retry because a terminal cause arms none. `unreadable` fires on EPERM/EACCES/EBUSY/EMFILE/ENFILE/EIO — descriptor exhaustion on a busy machine, a Windows AV file lock. The phone latches setHostSignedOut and shows "Desktop signed out — sign in to Orca on your desktop to reconnect" for a failure the next read would have cleared. The contrast is the argument: `unreadable` is the one status the session store's own doc says "licenses nothing", and clearCloudSessionIfUnchanged already refuses to delete a session because of it — "a session we were denied is not a session we may delete". The relay taxonomy was the single place treating it as evidence the user signed out. Give it its own arm on FreshCloudSessionResult and throw for it in readRelayAuthContext, which lands it on the retryable auth_unavailable path the coordinator already has. Every other caller tests `status !== 'found'`, so their behaviour is unchanged. Correct the coordinator comment to say what it actually depends on: null-means-gone is a contract readRelayAuthContext owes it, not something that branch can verify. Measured before the fix: offlineReason "signed-out", mint code relay_signed_out. After: auth_unavailable. Not fixed here, and worth a separate look: `decrypt-failed` when safeStorage.isEncryptionAvailable() is false (a Linux keyring still locked at login) takes the same route to SIGNED_OUT, but reclassifying it changes sign-in semantics well beyond the relay. --- .../profile-cloud-session-refresh.ts | 9 +++ ...ay-auth-context-unreadable-session.test.ts | 65 +++++++++++++++++++ src/main/runtime/relay/relay-auth-context.ts | 6 ++ .../runtime/relay/relay-auth-coordinator.ts | 9 ++- 4 files changed, 86 insertions(+), 3 deletions(-) create mode 100644 src/main/runtime/relay/relay-auth-context-unreadable-session.test.ts diff --git a/src/main/orca-profiles/profile-cloud-session-refresh.ts b/src/main/orca-profiles/profile-cloud-session-refresh.ts index 273f71f016e..c5cea26e964 100644 --- a/src/main/orca-profiles/profile-cloud-session-refresh.ts +++ b/src/main/orca-profiles/profile-cloud-session-refresh.ts @@ -32,6 +32,12 @@ const CLOUD_SESSION_REFRESH_SKEW_MS = 60_000 export type FreshCloudSessionResult = | { status: 'found'; session: OrcaCloudSession } | { status: 'reconnect-required' } + /** + * The session file is there and this process could not read it (EACCES/EBUSY/EMFILE/…). Distinct + * from `reconnect-required`, which means the session is genuinely gone: the store refuses to + * delete an unreadable session for the same reason a caller must not report one as a sign-out. + */ + | { status: 'unreadable' } export type CloudSessionOperationResult = | { status: 'ok'; value: T } @@ -226,6 +232,9 @@ export async function readFreshOrcaCloudSession( userDataPath: string ): Promise { const session = readOrcaCloudSession(active.profile.id, userDataPath) + if (session.status === 'unreadable') { + return { status: 'unreadable' } + } if (session.status !== 'found') { return { status: 'reconnect-required' } } diff --git a/src/main/runtime/relay/relay-auth-context-unreadable-session.test.ts b/src/main/runtime/relay/relay-auth-context-unreadable-session.test.ts new file mode 100644 index 00000000000..6b2f514f816 --- /dev/null +++ b/src/main/runtime/relay/relay-auth-context-unreadable-session.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, it, vi } from 'vitest' +import { RELAY_HOST_CLOSE_REASON } from '../../../shared/relay-host-close-reason' + +const fakes = vi.hoisted(() => ({ + ensureActiveOrcaProfile: vi.fn(), + readFreshOrcaCloudSession: vi.fn() +})) + +vi.mock('../../orca-profiles/profile-index-store', () => ({ + ensureActiveOrcaProfile: fakes.ensureActiveOrcaProfile +})) +vi.mock('../../orca-profiles/profile-cloud-session-refresh', () => ({ + readFreshOrcaCloudSession: fakes.readFreshOrcaCloudSession +})) + +import { readRelayAuthContext } from './relay-auth-context' +import { RelayAuthCoordinator } from './relay-auth-coordinator' + +const profile = { + profile: { + id: 'profile-1', + cloud: { userId: 'u1', cloudProfileId: 'cp1', activeOrgId: 'org-1' } + } +} + +const authConfig = {} as never + +describe('readRelayAuthContext session-read taxonomy', () => { + it('refuses to call an unreadable session file a sign-out', async () => { + // EACCES/EBUSY/EMFILE on the session file means "present, could not read it" — the store + // itself declines to delete one for that reason. Reporting it as signed-out tells every + // paired phone to sign in on the desktop for a failure a retry would have cleared. + fakes.ensureActiveOrcaProfile.mockReturnValue(profile) + fakes.readFreshOrcaCloudSession.mockResolvedValue({ status: 'unreadable' }) + + await expect(readRelayAuthContext(authConfig, '/tmp/x')).rejects.toThrow( + 'orca_cloud_session_unreadable' + ) + }) + + it('still reports a genuinely absent session as gone', async () => { + fakes.ensureActiveOrcaProfile.mockReturnValue(profile) + fakes.readFreshOrcaCloudSession.mockResolvedValue({ status: 'reconnect-required' }) + + await expect(readRelayAuthContext(authConfig, '/tmp/x')).resolves.toBeNull() + }) + + it('classifies an unreadable session as auth_unavailable, never signed_out', async () => { + fakes.ensureActiveOrcaProfile.mockReturnValue(profile) + fakes.readFreshOrcaCloudSession.mockResolvedValue({ status: 'unreadable' }) + const broker = { closeNow: vi.fn() } + const coordinator = new RelayAuthCoordinator({ + readContext: () => readRelayAuthContext(authConfig, '/tmp/x'), + openBroker: async () => broker, + onStatus: vi.fn() + }) + coordinator.reconcile() + + const result = await coordinator.waitForLiveBrokerResult(0) + expect(result).toEqual({ broker: null, offlineReason: 'auth_unavailable' }) + expect(result.broker).toBeNull() + // The wire close reason is what the phone latches on; it must not be spent here. + expect(broker.closeNow).not.toHaveBeenCalledWith(RELAY_HOST_CLOSE_REASON.SIGNED_OUT) + }) +}) diff --git a/src/main/runtime/relay/relay-auth-context.ts b/src/main/runtime/relay/relay-auth-context.ts index 117f3f78057..53f156e8915 100644 --- a/src/main/runtime/relay/relay-auth-context.ts +++ b/src/main/runtime/relay/relay-auth-context.ts @@ -12,6 +12,12 @@ export async function readRelayAuthContext( return null } const session = await readFreshOrcaCloudSession(authConfig, active, userDataPath) + // Why throw rather than return null: the coordinator reads null as "the cloud session is gone" + // and closes every paired phone's relay with signed-out, arming no retry. A session file this + // process could not read is intact, so the retryable auth_unavailable path is the honest word. + if (session.status === 'unreadable') { + throw new Error('orca_cloud_session_unreadable') + } if (session.status !== 'found') { return null } diff --git a/src/main/runtime/relay/relay-auth-coordinator.ts b/src/main/runtime/relay/relay-auth-coordinator.ts index a284618ac72..80cff8f878e 100644 --- a/src/main/runtime/relay/relay-auth-coordinator.ts +++ b/src/main/runtime/relay/relay-auth-coordinator.ts @@ -189,9 +189,12 @@ export class RelayAuthCoordinator { if (!context || !context.relayEntitled) { this.cancelLinger() this.retry.reset() - // Why only the null case: readContext throws on transient failures and - // returns null solely when the cloud session is gone (absent, or cleared - // by a 401). A present-but-unentitled context is still a signed-in + // Why only the null case: null must mean the cloud session is gone (absent, or cleared + // by a 401), and every other read outcome must throw so it lands in auth_unavailable. + // That is a contract readRelayAuthContext owes this branch, not something this branch + // can verify — it held for a refresh failure but not for a session file the process + // could not read, which spent SIGNED_OUT on transient I/O until relay-auth-context.ts + // started throwing for it. A present-but-unentitled context is still a signed-in // desktop, and "sign in to reconnect" would be wrong advice for it. this.invalidateOwnership(context ? undefined : RELAY_HOST_CLOSE_REASON.SIGNED_OUT) this.publish('offline', context ? 'not_entitled' : RELAY_HOST_CLOSE_REASON.SIGNED_OUT)