From 8a8138b7e89007ac02539ee1597c679288d8c4f6 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 17:16:48 -0700 Subject: [PATCH] fix: avoid starting browser helpers just to reset absent sessions --- .../browser/agent-browser-bridge-execution.ts | 11 +++++ ...t-browser-bridge-session-lifecycle.test.ts | 49 ++++++++++++++++--- .../agent-browser-session-reset.test.ts | 49 +++++++++++++++++++ .../browser/agent-browser-session-reset.ts | 28 +++++++++++ 4 files changed, 129 insertions(+), 8 deletions(-) create mode 100644 src/main/browser/agent-browser-session-reset.test.ts create mode 100644 src/main/browser/agent-browser-session-reset.ts diff --git a/src/main/browser/agent-browser-bridge-execution.ts b/src/main/browser/agent-browser-bridge-execution.ts index 65f64b6fb08..a2f11fce47a 100644 --- a/src/main/browser/agent-browser-bridge-execution.ts +++ b/src/main/browser/agent-browser-bridge-execution.ts @@ -12,6 +12,7 @@ import { import { translateResult } from './agent-browser-bridge-result' import { AgentBrowserBridgeTabs } from './agent-browser-bridge-tabs' import { ORCA_TAB_SESSION_PREFIX } from './agent-browser-orphan-sweep' +import { canSkipAgentBrowserSessionReset } from './agent-browser-session-reset' import { STALE_SESSION_CLOSE_TIMEOUT_MS, type AgentBrowserExecOptions, @@ -173,6 +174,16 @@ export abstract class AgentBrowserBridgeExecution extends AgentBrowserBridgeTabs } protected closeStaleAgentBrowserSession(sessionName: string): Promise { + if ( + canSkipAgentBrowserSessionReset({ + platform: process.platform, + ownsSocketDirectory: this.ownsAgentBrowserSocketDirectory, + socketDirectory: this.agentBrowserEnv.AGENT_BROWSER_SOCKET_DIR, + sessionName + }) + ) { + return Promise.resolve() + } return new Promise((resolve, reject) => { let child: ReturnType | null = null let settled = false diff --git a/src/main/browser/agent-browser-bridge-session-lifecycle.test.ts b/src/main/browser/agent-browser-bridge-session-lifecycle.test.ts index 5eab202541c..0b028538c8a 100644 --- a/src/main/browser/agent-browser-bridge-session-lifecycle.test.ts +++ b/src/main/browser/agent-browser-bridge-session-lifecycle.test.ts @@ -1,18 +1,26 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' -const { execFileMock, webContentsFromIdMock, existsSyncMock, readFileSyncMock, stdinWrites } = - vi.hoisted(() => ({ - execFileMock: vi.fn(), - webContentsFromIdMock: vi.fn(), - existsSyncMock: vi.fn(() => false), - readFileSyncMock: vi.fn(() => Buffer.from('')), - stdinWrites: [] as string[] - })) +const { + execFileMock, + webContentsFromIdMock, + existsSyncMock, + readFileSyncMock, + lstatSyncMock, + stdinWrites +} = vi.hoisted(() => ({ + execFileMock: vi.fn(), + webContentsFromIdMock: vi.fn(), + existsSyncMock: vi.fn(() => false), + readFileSyncMock: vi.fn(() => Buffer.from('')), + lstatSyncMock: vi.fn(), + stdinWrites: [] as string[] +})) vi.mock('child_process', () => ({ execFile: execFileMock })) vi.mock('fs', () => ({ existsSync: existsSyncMock, readFileSync: readFileSyncMock, + lstatSync: lstatSyncMock, accessSync: vi.fn(), chmodSync: vi.fn(), constants: { X_OK: 1 } @@ -85,7 +93,32 @@ describe('AgentBrowserBridge', () => { bridge.setActiveTab(100) }) + it('snapshots a fresh owned session without launching a helper just to close it', async () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') + Object.assign(bridge, { + ownsAgentBrowserSocketDirectory: true, + agentBrowserEnv: { AGENT_BROWSER_SOCKET_DIR: '/tmp/orca-ab-test' } + }) + lstatSyncMock.mockImplementation(() => { + throw Object.assign(new Error('No socket'), { code: 'ENOENT' }) + }) + webContentsFromIdMock.mockReturnValue(mockWebContents(100)) + succeedWith({ snapshot: 'ready' }) + + try { + expect(await bridge.snapshot()).toMatchObject({ snapshot: 'ready' }) + expect(closeCallCount()).toBe(0) + } finally { + vi.restoreAllMocks() + } + }) + it('fails closed when stale agent-browser session ownership cannot be reset', async () => { + Object.assign(bridge, { + ownsAgentBrowserSocketDirectory: true, + agentBrowserEnv: { AGENT_BROWSER_SOCKET_DIR: '/tmp/orca-ab-test' } + }) + lstatSyncMock.mockReturnValue({}) vi.useFakeTimers() try { const closeKill = vi.fn() diff --git a/src/main/browser/agent-browser-session-reset.test.ts b/src/main/browser/agent-browser-session-reset.test.ts new file mode 100644 index 00000000000..974fa348cc9 --- /dev/null +++ b/src/main/browser/agent-browser-session-reset.test.ts @@ -0,0 +1,49 @@ +import { beforeEach, expect, it, vi } from 'vitest' +import { join } from 'node:path' + +const { lstatSync } = vi.hoisted(() => ({ lstatSync: vi.fn() })) +vi.mock('node:fs', () => ({ lstatSync })) +import { canSkipAgentBrowserSessionReset } from './agent-browser-session-reset' + +const owned = { + platform: 'darwin' as const, + ownsSocketDirectory: true, + socketDirectory: '/tmp/orca-ab-profile', + sessionName: 'orca-tab-page' +} + +beforeEach(() => { + lstatSync.mockReset() +}) + +it.each(['darwin', 'linux'] as const)('skips an absent owned socket on %s', (platform) => { + lstatSync.mockImplementation(() => { + throw Object.assign(new Error('No socket'), { code: 'ENOENT' }) + }) + expect(canSkipAgentBrowserSessionReset({ ...owned, platform })).toBe(true) + expect(lstatSync).toHaveBeenCalledWith(join(owned.socketDirectory, 'orca-tab-page.sock')) +}) + +it('requires reset when a socket or symlink exists', () => { + lstatSync.mockReturnValue({}) + expect(canSkipAgentBrowserSessionReset(owned)).toBe(false) +}) + +it.each(['EACCES', 'EIO', 'ENOTDIR'])('requires reset for %s', (code) => { + lstatSync.mockImplementation(() => { + throw Object.assign(new Error('Socket inspection failed'), { code }) + }) + expect(canSkipAgentBrowserSessionReset(owned)).toBe(false) +}) + +it.each([ + { platform: 'win32' as const }, + { ownsSocketDirectory: false }, + { socketDirectory: undefined }, + { socketDirectory: 'relative' }, + { sessionName: '../other' }, + { sessionName: '' } +])('requires reset without an owned Unix socket address: %j', (override) => { + expect(canSkipAgentBrowserSessionReset({ ...owned, ...override })).toBe(false) + expect(lstatSync).not.toHaveBeenCalled() +}) diff --git a/src/main/browser/agent-browser-session-reset.ts b/src/main/browser/agent-browser-session-reset.ts new file mode 100644 index 00000000000..a6240c03612 --- /dev/null +++ b/src/main/browser/agent-browser-session-reset.ts @@ -0,0 +1,28 @@ +import { lstatSync } from 'node:fs' +import { basename, isAbsolute, join } from 'node:path' + +export function canSkipAgentBrowserSessionReset(options: { + platform: NodeJS.Platform + ownsSocketDirectory: boolean + socketDirectory: string | undefined + sessionName: string +}): boolean { + const { socketDirectory, sessionName } = options + if ( + options.platform === 'win32' || + !options.ownsSocketDirectory || + !socketDirectory || + !isAbsolute(socketDirectory) || + !sessionName || + basename(sessionName) !== sessionName + ) { + return false + } + try { + lstatSync(join(socketDirectory, `${sessionName}.sock`)) + return false + } catch (error) { + // An absent owned socket cannot be reused; permission and other failures prove nothing. + return (error as NodeJS.ErrnoException).code === 'ENOENT' + } +}