diff --git a/src/main/browser/browser-identity-mode-record.ts b/src/main/browser/browser-identity-mode-record.ts index 00ca30e2c16..4d86e40e600 100644 --- a/src/main/browser/browser-identity-mode-record.ts +++ b/src/main/browser/browser-identity-mode-record.ts @@ -22,6 +22,12 @@ import type { BrowserUserAgentMode } from '../../shared/browser-user-agent-mode' export const BROWSER_IDENTITY_MODE_FILE = 'browser-identity-mode.json' export const BROWSER_IDENTITY_MODE_VERSION = 1 +let browserIdentityPersistenceFailure: string | null = null + +export function getBrowserIdentityPersistenceFailure(): string | null { + return browserIdentityPersistenceFailure +} + export type BrowserIdentityModeRecord = { version: typeof BROWSER_IDENTITY_MODE_VERSION mode: BrowserUserAgentMode @@ -74,30 +80,53 @@ export function writeBrowserIdentityModeRecord( ) } +function persistBrowserIdentityOperation( + userDataPath: string, + record: BrowserIdentityModeRecord, + operation: string +): boolean { + try { + writeBrowserIdentityModeRecord(userDataPath, record) + browserIdentityPersistenceFailure = null + return true + } catch (error) { + browserIdentityPersistenceFailure = error instanceof Error ? error.message : String(error) + console.error( + `[browser-identity] Could not persist ${operation}:`, + browserIdentityPersistenceFailure + ) + return false + } +} + export function updateBrowserIdentityMode(userDataPath: string, mode: BrowserUserAgentMode): void { const current = readBrowserIdentityModeRecord(userDataPath) if (current.mode === mode) { return } - writeBrowserIdentityModeRecord(userDataPath, { ...current, mode }) + persistBrowserIdentityOperation(userDataPath, { ...current, mode }, 'process identity mode') } export function recordRetiredNativeBrowserProfiles( userDataPath: string, profileIds: readonly string[] -): void { +): boolean { if (profileIds.length === 0) { - return + return true } const current = readBrowserIdentityModeRecord(userDataPath) const migratedNativeProfileIds = [ ...new Set([...(current.migratedNativeProfileIds ?? []), ...profileIds]) ] - writeBrowserIdentityModeRecord(userDataPath, { - ...current, - migratedNativeProfileIds, - migrationNoticePending: true - }) + return persistBrowserIdentityOperation( + userDataPath, + { + ...current, + migratedNativeProfileIds, + migrationNoticePending: true + }, + 'retired profile notice' + ) } export function readPendingBrowserIdentityMigrationNotice(userDataPath: string): string[] | null { diff --git a/src/main/browser/browser-session-persisted-profile-validation.ts b/src/main/browser/browser-session-persisted-profile-validation.ts index 3b5c3bcd888..e42344e40b5 100644 --- a/src/main/browser/browser-session-persisted-profile-validation.ts +++ b/src/main/browser/browser-session-persisted-profile-validation.ts @@ -40,14 +40,20 @@ export function migrateRetiredBrowserSessionProfileUserAgentModes( profiles: BrowserSessionProfile[], activeOrcaProfileId: string ): { profiles: BrowserSessionProfile[]; nativeProfileIds: string[]; changed: boolean } { - const nativeProfileIds = profiles + // Why: JSON arrays may contain scalars or null even though the persisted type says profiles. + const inspectableProfiles = profiles.filter( + (profile) => profile !== null && typeof profile === 'object' + ) + const nativeProfileIds = inspectableProfiles .filter((profile) => isValidPersistedBrowserSessionProfile(profile, activeOrcaProfileId)) .filter((profile) => Reflect.get(profile, 'userAgentMode') === 'native') .map((profile) => profile.id) return { - profiles: profiles.map(withoutRetiredUserAgentMode), + profiles: inspectableProfiles.map(withoutRetiredUserAgentMode), nativeProfileIds, - changed: profiles.some(hasRetiredUserAgentMode) + changed: + inspectableProfiles.length !== profiles.length || + inspectableProfiles.some(hasRetiredUserAgentMode) } } diff --git a/src/main/browser/browser-session-registry-import-boundary.test.ts b/src/main/browser/browser-session-registry-import-boundary.test.ts new file mode 100644 index 00000000000..96950797c4a --- /dev/null +++ b/src/main/browser/browser-session-registry-import-boundary.test.ts @@ -0,0 +1,12 @@ +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import { expect, it } from 'vitest' + +it('keeps the cookie fixture registry outside the persistence barrel graph', () => { + const source = readFileSync(join(__dirname, 'browser-session-registry.ts'), 'utf8') + + expect(source).toMatch( + /import\s*\{\s*getCanonicalUserDataPath\s*\}\s*from\s*['"]\.\.\/persistence\/loading-store\/user-data-path['"]/ + ) + expect(source).not.toMatch(/from\s*['"]\.\.\/persistence['"]/) +}) diff --git a/src/main/browser/browser-session-registry.persistence.test.ts b/src/main/browser/browser-session-registry.persistence.test.ts index 967d4d7aff2..a963b924d12 100644 --- a/src/main/browser/browser-session-registry.persistence.test.ts +++ b/src/main/browser/browser-session-registry.persistence.test.ts @@ -10,13 +10,9 @@ type FsState = { present: Set } -function fsKey(pathValue: string): string { - return pathValue.replaceAll('\\', '/') -} +const fsKey = (pathValue: string): string => pathValue.replaceAll('\\', '/') -function createFsState(): FsState { - return { files: new Map(), present: new Set() } -} +const createFsState = (): FsState => ({ files: new Map(), present: new Set() }) function seedMeta(fsState: FsState, meta: unknown): void { const raw = JSON.stringify(meta) @@ -26,15 +22,10 @@ function seedMeta(fsState: FsState, meta: unknown): void { function installModuleMocks( fsState: FsState, - copyFailures = new Set() -): { - sessionFromPartitionMock: ReturnType - installBrowserSessionUserAgentPolicyMock: ReturnType - browserManagerHandleGuestWillDownloadMock: ReturnType - browserManagerNotifyPermissionDeniedMock: ReturnType - requestSystemMediaAccessMock: ReturnType -} { - const sessionFromPartitionMock = vi.fn((partition: string) => ({ + copyFailures = new Set(), + failIdentityWrite = false +) { + const sessionFromPartitionMock: ReturnType = vi.fn((partition: string) => ({ partition, setUserAgent: vi.fn(), getUserAgent: vi.fn(() => CLEAN_USER_AGENT), @@ -47,7 +38,7 @@ function installModuleMocks( clearStorageData: vi.fn().mockResolvedValue(undefined), clearCache: vi.fn().mockResolvedValue(undefined) })) - const installBrowserSessionUserAgentPolicyMock = vi.fn(() => vi.fn()) + const installBrowserSessionUserAgentPolicyMock: ReturnType = vi.fn(() => vi.fn()) const browserManagerHandleGuestWillDownloadMock = vi.fn() const browserManagerNotifyPermissionDeniedMock = vi.fn() const requestSystemMediaAccessMock = vi.fn().mockResolvedValue(true) @@ -132,6 +123,9 @@ function installModuleMocks( vi.doMock('../persistence', () => ({ getCanonicalUserDataPath: () => USER_DATA })) + vi.doMock('../persistence/loading-store/user-data-path', () => ({ + getCanonicalUserDataPath: () => USER_DATA + })) // This suite models replay with an in-memory filesystem. The real file-backed SQLite merge has // dedicated coverage; these fixtures are legacy unmarked images and keep the copy path. vi.doMock('./browser-cookie-staged-import', () => ({ @@ -156,6 +150,9 @@ function installModuleMocks( } }), writeFileAtomically: vi.fn((pathValue: string, data: string) => { + if (failIdentityWrite && pathValue.endsWith('browser-identity-mode.json')) { + throw new Error('read-only userData') + } const key = fsKey(pathValue) fsState.files.set(key, data) fsState.present.add(key) @@ -477,6 +474,44 @@ describe('BrowserSessionRegistry persistence', () => { expect(readPendingBrowserIdentityMigrationNotice(USER_DATA)).toBeNull() }) + it.each([ + { scenario: 'malformed members', malformed: [null, 42, 'broken'], failWrite: false }, + { scenario: 'a read-only notice', malformed: [], failWrite: true } + ])('continues migration despite $scenario', async ({ malformed, failWrite }) => { + const profileId = '11111111-1111-4111-8111-111111111111' + const partition = `persist:orca-browser-session-${profileId}` + const fsState = createFsState() + seedMeta(fsState, { + defaultSource: null, + profiles: [ + ...malformed, + { + id: profileId, + scope: 'isolated', + partition, + label: 'Existing', + source: null, + userAgentMode: 'native' + } + ] + }) + installModuleMocks(fsState, new Set(), failWrite) + const { browserSessionRegistry } = await import('./browser-session-registry') + + expect(() => browserSessionRegistry.initializeBrowserSessionsFromPersistedState()).not.toThrow() + expect(browserSessionRegistry.getProfile(profileId)?.partition).toBe(partition) + const written = JSON.parse(fsState.files.get(META_PATH) ?? '{}') + expect(written.profiles).toHaveLength(1) + if (failWrite) { + expect(written.profiles[0].userAgentMode).toBe('native') + const { getBrowserIdentityPersistenceFailure } = + await import('./browser-identity-mode-record') + expect(getBrowserIdentityPersistenceFailure()).toContain('read-only userData') + } else { + expect(written.profiles[0]).not.toHaveProperty('userAgentMode') + } + }) + it('hydrates a retired native profile under the process identity', async () => { const importedPartition = 'persist:orca-browser-session-12121212-1212-4121-8121-121212121212' const fsState = createFsState() diff --git a/src/main/browser/browser-session-registry.ts b/src/main/browser/browser-session-registry.ts index c951d2a25af..ca81f7a0d97 100644 --- a/src/main/browser/browser-session-registry.ts +++ b/src/main/browser/browser-session-registry.ts @@ -38,7 +38,7 @@ import { retireProxySessionApplication } from '../network/proxy-settings' import { invalidateBrowserSessionProxyApplication } from './browser-session-proxy' import { retireFailedBrowserSessionProfile } from './browser-session-profile-retirement' import { cancelBrowserWebAuthnAccountRequestsForSession } from './browser-webauthn-account-picker' -import { getCanonicalUserDataPath } from '../persistence' +import { getCanonicalUserDataPath } from '../persistence/loading-store/user-data-path' import { recordRetiredNativeBrowserProfiles } from './browser-identity-mode-record' export type BrowserSessionRegistryProfileOptions = { @@ -115,8 +115,11 @@ class BrowserSessionRegistry { this.activeOrcaProfileId ) // Record the notice first so a metadata-write failure cannot erase the user's retired choice. - recordRetiredNativeBrowserProfiles(getCanonicalUserDataPath(), migration.nativeProfileIds) - if (migration.changed) { + const noticePersisted = recordRetiredNativeBrowserProfiles( + getCanonicalUserDataPath(), + migration.nativeProfileIds + ) + if (migration.changed && noticePersisted) { this.persistMeta({ profiles: migration.profiles }) } if (meta.defaultSource) { diff --git a/src/main/startup/main-process-ready-identity-write.test.ts b/src/main/startup/main-process-ready-identity-write.test.ts new file mode 100644 index 00000000000..3b671f88d7c --- /dev/null +++ b/src/main/startup/main-process-ready-identity-write.test.ts @@ -0,0 +1,186 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + state: { + devInstanceIdentity: { appUserModelId: 'app.id', appName: 'Orca' }, + isServeMode: false, + mainProcessI18nReady: Promise.resolve(), + managedWslCliReconciliationStatus: 'settled', + initialProxyApplicationReady: Promise.resolve(), + hangDetection: null, + store: null + }, + openMainWindow: vi.fn(), + runtimeRpcStart: vi.fn(async () => {}), + writeFileAtomically: vi.fn(() => { + throw new Error('read-only userData') + }) +})) + +vi.mock('electron', () => ({ + app: { + on: vi.fn(), + setName: vi.fn(), + getPath: vi.fn(() => '/test-userdata'), + getVersion: vi.fn(() => '1.0.0'), + isPackaged: false + }, + session: { defaultSession: {} } +})) +vi.mock('@electron-toolkit/utils', () => ({ + electronApp: { setAppUserModelId: vi.fn() }, + is: { dev: false } +})) +vi.mock('./main-process-state', () => ({ mainProcessState: mocks.state })) +vi.mock('../persistence', () => ({ + Store: class { + getSettings() { + return { browserUserAgentMode: 'native' } + } + onSettingsChanged() {} + getClaudeLivePtySessionIds() { + return [] + } + getSshTargets() { + return [] + } + }, + getCanonicalUserDataPath: () => '/test-userdata' +})) +vi.mock('../codex-accounts/fs-utils', () => ({ + writeFileAtomically: mocks.writeFileAtomically +})) +vi.mock('../window/foreground-activation-policy', () => ({ + applyBackgroundActivationPolicy: vi.fn() +})) +vi.mock('../network/proxy-settings', () => ({ + applyElectronProxySettings: vi.fn(async () => ({ source: 'direct' })) +})) +vi.mock('../network/electron-proxy-request-guard', () => ({ + installElectronProxyRequestGuard: vi.fn() +})) +vi.mock('../network/electron-proxy-credentials', () => ({ handleElectronProxyLogin: vi.fn() })) +vi.mock('../hang-watchdog/main-thread-hang-watchdog', () => ({ + installMainThreadHangWatchdog: vi.fn() +})) +vi.mock('../hang-watchdog/hang-detection-marker', () => ({ + consumeHangDetectionMarker: vi.fn(() => null), + hangDetectionMarkerPath: vi.fn(() => '/test-marker') +})) +vi.mock('../browser/browser-manager', () => ({ browserCertificateTrustController: {} })) +vi.mock('../orca-profiles/profile-index-store', () => ({ + ensureActiveOrcaProfile: () => ({ + profile: { id: 'local-default' }, + profileDirectory: '/test-profile', + dataFile: '/test-profile/data.json' + }) +})) +vi.mock('../browser/browser-client-host-id', () => ({ initializeBrowserClientHostId: vi.fn() })) +vi.mock('../host/deferred-secret-protection-report', () => ({ + scheduleSecretProtectionGapReport: vi.fn() +})) +vi.mock('../ssh/ssh-host-key-store', () => ({ initSshHostKeyStoreFile: vi.fn() })) +vi.mock('../pty/legacy-terminal-shim-dir', () => ({ neutralizeLegacyTerminalShimDir: vi.fn() })) +vi.mock('./windows-shell-path-hydration', () => ({ + createWindowsShellPathHydration: () => ({ whenReady: Promise.resolve() }) +})) +vi.mock('../git/runner', () => ({ + configureWindowsHostGitEnvironmentReadiness: vi.fn(), + setDefaultWslDistroOverride: vi.fn() +})) +vi.mock('../agent-hooks/wsl-hook-relay-manager', () => ({ + wslHookRelayManager: { setManagedHookSettingsResolver: vi.fn() } +})) +vi.mock('../claude-accounts/live-pty-gate', () => ({ + attachClaudeLivePtyPersistence: vi.fn(), + onLiveClaudePtysDrained: vi.fn(), + seedLiveClaudePtysFromPersistence: vi.fn() +})) +vi.mock('../app-icon', () => ({ applyAppIcon: vi.fn() })) +vi.mock('./dev-education-suppression', () => ({ + shouldSuppressDevEducation: () => false, + suppressDevEducationForStore: vi.fn() +})) +vi.mock('../browser/browser-session-proxy', () => ({ + applyBrowserSessionProxies: vi.fn(async () => {}), + setBrowserNetworkProxySettingsResolver: vi.fn() +})) +vi.mock('../browser/doc-preview-protocol', () => ({ installDocPreviewProtocolHandler: vi.fn() })) +vi.mock('../ipc/doc-preview-grant-ipc', () => ({ registerDocPreviewGrantHandlers: vi.fn() })) +vi.mock('../browser/browser-session-startup', () => ({ initializeBrowserSessionsForApp: vi.fn() })) +vi.mock('../browser/browser-session-registry', () => ({ + browserSessionRegistry: { listProfiles: () => [] } +})) +vi.mock('./startup-diagnostics', () => ({ logStartupMilestone: vi.fn() })) +vi.mock('./http1-compatibility-marker', () => ({ writeHttp1CompatibilityMarker: vi.fn() })) +vi.mock('../crash-reporting/durable-crash-breadcrumb', () => ({ + recordDurableCrashBreadcrumb: vi.fn() +})) +vi.mock('./main-window-actions', () => ({ syncMacMenuBarIcon: vi.fn() })) +vi.mock('./gpu-lifecycle', () => ({ updateGpuAccelerationAboutPanel: vi.fn() })) +vi.mock('../cli/wsl-cli-registration-reconciliation', () => ({ + reconcileManagedWslCliRegistrations: vi.fn(async () => []) +})) +vi.mock('./wsl-cli-reconciliation-startup-barrier', () => ({ + createWslCliReconciliationStartupBarrier: () => Promise.resolve() +})) +vi.mock('../agent-hooks/managed-agent-hook-controls', () => ({ + isAgentStatusHooksEnabled: vi.fn() +})) +vi.mock('./main-process-ready-runtime', () => ({ + initializeReadyRuntimeServices: vi.fn(async () => {}) +})) +vi.mock('./main-process-i18n-menu', () => ({ + initializeMainProcessI18nAndMenu: vi.fn(async () => {}) +})) +vi.mock('./main-process-runtime-launch', () => ({ + initializeMainProcessRuntimeLaunch: vi.fn(async (options: { openMainWindow: () => void }) => { + if (mocks.state.isServeMode) { + await mocks.runtimeRpcStart() + } else { + options.openMainWindow() + } + }) +})) + +import { initializeMainProcessReady } from './main-process-ready' +import { getBrowserIdentityPersistenceFailure } from '../browser/browser-identity-mode-record' + +describe('ready-phase identity write failure', () => { + beforeEach(() => { + mocks.openMainWindow.mockClear() + mocks.runtimeRpcStart.mockClear() + mocks.state.isServeMode = false + }) + + it('still reaches the desktop window when the identity sidecar write fails', async () => { + await expect( + initializeMainProcessReady({ + openMainWindow: mocks.openMainWindow, + handleMacAppActivation: vi.fn() + }) + ).resolves.toBeUndefined() + expect(mocks.openMainWindow).toHaveBeenCalledTimes(1) + expect(getBrowserIdentityPersistenceFailure()).toContain('read-only userData') + }) + + it('still reaches serve RPC startup and reports the write failure on stderr', async () => { + mocks.state.isServeMode = true + const stderr = vi.spyOn(console, 'error').mockImplementation(() => {}) + try { + await expect( + initializeMainProcessReady({ + openMainWindow: mocks.openMainWindow, + handleMacAppActivation: vi.fn() + }) + ).resolves.toBeUndefined() + expect(mocks.runtimeRpcStart).toHaveBeenCalledTimes(1) + expect(stderr).toHaveBeenCalledWith( + expect.stringContaining('[browser-identity]'), + expect.stringContaining('read-only userData') + ) + } finally { + stderr.mockRestore() + } + }) +})