diff --git a/src/main/computer/macos-native-provider-client.test.ts b/src/main/computer/macos-native-provider-client.test.ts index 18af7264a..7aad097a0 100644 --- a/src/main/computer/macos-native-provider-client.test.ts +++ b/src/main/computer/macos-native-provider-client.test.ts @@ -61,6 +61,21 @@ class FakeSocket extends EventEmitter { } } +class FakeProvider extends EventEmitter { + kill = vi.fn() + unref = vi.fn() +} + +function pendingConnectThatRejectsOnAbort(signal?: AbortSignal): Promise { + return new Promise((_resolve, reject) => { + signal?.addEventListener( + 'abort', + () => reject(new Error('native macOS helper app startup was cancelled')), + { once: true } + ) + }) +} + async function loadClientModule() { vi.resetModules() return await import('./macos-native-provider-client') @@ -68,15 +83,21 @@ async function loadClientModule() { describe('MacOSNativeProviderClient', () => { const sockets: FakeSocket[] = [] + const providers: FakeProvider[] = [] beforeEach(() => { vi.useFakeTimers() sockets.length = 0 + providers.length = 0 mkdtempSyncMock.mockImplementation((prefix: string) => `${prefix}${sockets.length}`) resolveMacOSComputerUseExecutablePathMock.mockReturnValue( '/Applications/Orca Computer Use.app/Contents/MacOS/orca-computer-use-macos' ) - spawnMock.mockReturnValue({ unref: vi.fn(), kill: vi.fn() }) + spawnMock.mockImplementation(() => { + const provider = new FakeProvider() + providers.push(provider) + return provider + }) connectMacOSProviderSocketMock.mockImplementation(async () => { const socket = new FakeSocket() sockets.push(socket) @@ -264,4 +285,44 @@ describe('MacOSNativeProviderClient', () => { force: true }) }) + + it('rejects helper spawn errors before socket connection and removes temp state', async () => { + const { MacOSNativeProviderClient } = await loadClientModule() + const client = new MacOSNativeProviderClient() + connectMacOSProviderSocketMock.mockImplementation((_path, _timeout, signal?: AbortSignal) => + pendingConnectThatRejectsOnAbort(signal) + ) + + const call = client.capabilities() + await vi.waitFor(() => expect(providers).toHaveLength(1)) + const socketDirectory = mkdtempSyncMock.mock.results[0]?.value as string + const connectSignal = connectMacOSProviderSocketMock.mock.calls[0]?.[2] as AbortSignal + expect(connectSignal.aborted).toBe(false) + providers[0]!.emit('error', new Error('helper missing')) + + await expect(call).rejects.toThrow('native macOS helper app failed to start: helper missing') + expect(connectSignal.aborted).toBe(true) + expect(providers[0]!.kill).toHaveBeenCalledWith('SIGTERM') + expect(rmSyncMock).toHaveBeenCalledWith(socketDirectory, { + recursive: true, + force: true + }) + }) + + it('rejects helper exits before socket connection and aborts the pending connect', async () => { + const { MacOSNativeProviderClient } = await loadClientModule() + const client = new MacOSNativeProviderClient() + connectMacOSProviderSocketMock.mockImplementation((_path, _timeout, signal?: AbortSignal) => + pendingConnectThatRejectsOnAbort(signal) + ) + + const call = client.capabilities() + await vi.waitFor(() => expect(providers).toHaveLength(1)) + const connectSignal = connectMacOSProviderSocketMock.mock.calls[0]?.[2] as AbortSignal + providers[0]!.emit('exit', 13, null) + + await expect(call).rejects.toThrow('native macOS helper app exited before connecting: code 13') + expect(connectSignal.aborted).toBe(true) + expect(providers[0]!.kill).toHaveBeenCalledWith('SIGTERM') + }) }) diff --git a/src/main/computer/macos-native-provider-client.ts b/src/main/computer/macos-native-provider-client.ts index 169cf1abd..68de47311 100644 --- a/src/main/computer/macos-native-provider-client.ts +++ b/src/main/computer/macos-native-provider-client.ts @@ -1,5 +1,5 @@ /* eslint-disable max-lines -- Why: the macOS provider transport owns one lifecycle across stdio fallback and helper-app socket mode. */ -import { spawn } from 'child_process' +import { spawn, type ChildProcess } from 'child_process' import { chmodSync, mkdtempSync, rmSync, writeFileSync } from 'fs' import type net from 'net' import { release, tmpdir } from 'os' @@ -200,14 +200,15 @@ export class MacOSNativeProviderClient { writeFileSync(socketTokenPath, socketToken, { encoding: 'utf8', mode: 0o600 }) // Why: launching the nested helper via LaunchServices can make TCC evaluate // Orca.app as responsible; the signed helper executable owns this grant. - const provider = spawn( - helperExecutablePath, - ['--agent', socketPath, '--token-file', socketTokenPath], - { detached: true, stdio: 'ignore' } - ) - provider.unref() + const provider = spawnProvider(helperExecutablePath, socketPath, socketTokenPath) + const providerFailure = waitForProviderLaunchFailure(provider) + const connectAbort = new AbortController() try { - const socket = await connectMacOSProviderSocket(socketPath, HELPER_CONNECT_TIMEOUT_MS) + const socket = await Promise.race([ + connectMacOSProviderSocket(socketPath, HELPER_CONNECT_TIMEOUT_MS, connectAbort.signal), + providerFailure.promise + ]) + providerFailure.cleanup() rmSync(socketTokenPath, { force: true }) // Why: shutdown/retry can supersede an in-flight connect; old starts // must not adopt replacement state or clean up the replacement helper. @@ -236,6 +237,8 @@ export class MacOSNativeProviderClient { this.socket = socket return socket } catch (error) { + connectAbort.abort() + providerFailure.cleanup() // Why: connect failures happen after spawn; terminate the detached // helper so repeated startup attempts do not leave orphan providers. provider.kill('SIGTERM') @@ -347,3 +350,51 @@ function isMacOS14OrNewer(): boolean { const darwinMajor = Number.parseInt(release().split('.')[0] ?? '', 10) return Number.isFinite(darwinMajor) && darwinMajor >= 23 } + +function spawnProvider( + helperExecutablePath: string, + socketPath: string, + socketTokenPath: string +): ChildProcess { + const provider = spawn( + helperExecutablePath, + ['--agent', socketPath, '--token-file', socketTokenPath], + { detached: true, stdio: 'ignore' } + ) + provider.unref() + return provider +} + +function waitForProviderLaunchFailure(provider: ChildProcess): { + promise: Promise + cleanup: () => void +} { + let cleanup = (): void => {} + const promise = new Promise((_resolve, reject) => { + const fail = (error: Error) => { + reject( + new RuntimeClientError( + 'accessibility_error', + `native macOS helper app failed to start: ${error.message}` + ) + ) + } + const exit = (code: number | null, signal: NodeJS.Signals | null) => { + reject( + new RuntimeClientError( + 'accessibility_error', + `native macOS helper app exited before connecting: ${ + typeof code === 'number' ? `code ${code}` : `signal ${signal ?? 'unknown'}` + }` + ) + ) + } + provider.once('error', fail) + provider.once('exit', exit) + cleanup = () => { + provider.off('error', fail) + provider.off('exit', exit) + } + }) + return { promise, cleanup } +} diff --git a/src/main/computer/macos-native-provider-socket.test.ts b/src/main/computer/macos-native-provider-socket.test.ts index ef9f9ba47..27a3790b7 100644 --- a/src/main/computer/macos-native-provider-socket.test.ts +++ b/src/main/computer/macos-native-provider-socket.test.ts @@ -44,4 +44,19 @@ describe('connectMacOSProviderSocket', () => { vi.useRealTimers() } }) + + it('destroys the pending socket and removes listeners when aborted', async () => { + const socket = new FakeSocket() + createConnectionMock.mockReturnValueOnce(socket) + const abort = new AbortController() + + const promise = connectMacOSProviderSocket('/tmp/orca-computer.sock', 5_000, abort.signal) + await Promise.resolve() + abort.abort() + + await expect(promise).rejects.toThrow('startup was cancelled') + expect(socket.destroy).toHaveBeenCalledTimes(1) + expect(socket.listenerCount('error')).toBe(0) + expect(socket.listenerCount('connect')).toBe(0) + }) }) diff --git a/src/main/computer/macos-native-provider-socket.ts b/src/main/computer/macos-native-provider-socket.ts index 13732d178..d63be3f05 100644 --- a/src/main/computer/macos-native-provider-socket.ts +++ b/src/main/computer/macos-native-provider-socket.ts @@ -3,30 +3,50 @@ import { RuntimeClientError } from './runtime-client-error' export async function connectMacOSProviderSocket( socketPath: string, - timeoutMs: number + timeoutMs: number, + signal?: AbortSignal ): Promise { const deadline = Date.now() + timeoutMs let lastError: Error | null = null - while (Date.now() < deadline) { + while (Date.now() < deadline && !signal?.aborted) { try { - return await connectSocket(socketPath) + return await connectSocket(socketPath, signal) } catch (error) { lastError = error instanceof Error ? error : new Error(String(error)) - await new Promise((resolve) => setTimeout(resolve, 100)) + if (signal?.aborted) { + break + } + await sleep(100, signal) } } + if (signal?.aborted) { + throw new RuntimeClientError( + 'accessibility_error', + 'native macOS helper app startup was cancelled' + ) + } throw new RuntimeClientError( 'action_timeout', `native macOS helper app did not open its socket: ${lastError?.message ?? 'timed out'}` ) } -function connectSocket(socketPath: string): Promise { +function connectSocket(socketPath: string, signal?: AbortSignal): Promise { return new Promise((resolve, reject) => { + if (signal?.aborted) { + reject( + new RuntimeClientError( + 'accessibility_error', + 'native macOS helper app startup was cancelled' + ) + ) + return + } const socket = net.createConnection(socketPath) const cleanup = (): void => { socket.off('error', onError) socket.off('connect', onConnect) + signal?.removeEventListener('abort', onAbort) } const onError = (error: Error): void => { cleanup() @@ -37,7 +57,31 @@ function connectSocket(socketPath: string): Promise { cleanup() resolve(socket) } + const onAbort = (): void => { + cleanup() + socket.destroy() + reject( + new RuntimeClientError( + 'accessibility_error', + 'native macOS helper app startup was cancelled' + ) + ) + } socket.once('error', onError) socket.once('connect', onConnect) + signal?.addEventListener('abort', onAbort, { once: true }) + }) +} + +function sleep(ms: number, signal?: AbortSignal): Promise { + return new Promise((resolve) => { + const timer = setTimeout(finish, ms) + const onAbort = (): void => finish() + function finish(): void { + clearTimeout(timer) + signal?.removeEventListener('abort', onAbort) + resolve() + } + signal?.addEventListener('abort', onAbort, { once: true }) }) }