From afd1509672f78ebe0847752613c8a25ee7015eef Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 30 May 2026 09:44:03 -0700 Subject: [PATCH] fix: clean up permission status helper listeners (#3774) --- .../macos-computer-use-permissions.test.ts | 39 ++++++++++++-- .../macos-computer-use-permissions.ts | 51 +++++++++++++++---- 2 files changed, 75 insertions(+), 15 deletions(-) diff --git a/src/main/computer/macos-computer-use-permissions.test.ts b/src/main/computer/macos-computer-use-permissions.test.ts index d2c38856025..92ca3b6016d 100644 --- a/src/main/computer/macos-computer-use-permissions.test.ts +++ b/src/main/computer/macos-computer-use-permissions.test.ts @@ -1,3 +1,5 @@ +/* oxlint-disable max-lines -- Why: these macOS permission flows share one +mocked child_process/fs platform harness. */ import { execFileSync, spawn, spawnSync } from 'child_process' import { mkdtemp, readFile, rm, stat } from 'fs/promises' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' @@ -13,14 +15,15 @@ vi.mock('child_process', () => ({ execFileSync: vi.fn(), spawn: vi.fn(() => { const child = { - stdout: { on: vi.fn(), setEncoding: vi.fn() }, - stderr: { on: vi.fn(), setEncoding: vi.fn() }, + stdout: { off: vi.fn(), on: vi.fn(), setEncoding: vi.fn() }, + stderr: { off: vi.fn(), on: vi.fn(), setEncoding: vi.fn() }, on: vi.fn((event: string, callback: (status: number) => void) => { if (event === 'close') { queueMicrotask(() => callback(0)) } return child }), + off: vi.fn(() => child), unref: vi.fn() } return child @@ -206,14 +209,15 @@ describe('openComputerUsePermissions', () => { const { getComputerUsePermissionStatus } = await import('./macos-computer-use-permissions') resolveHelperAppPathMock.mockReturnValue('/Applications/Orca Computer Use.app') const child = { - stdout: { on: vi.fn(), setEncoding: vi.fn() }, - stderr: { on: vi.fn(), setEncoding: vi.fn() }, + stdout: { off: vi.fn(), on: vi.fn(), setEncoding: vi.fn() }, + stderr: { off: vi.fn(), on: vi.fn(), setEncoding: vi.fn() }, on: vi.fn((event: string, callback: (error: Error) => void) => { if (event === 'error') { queueMicrotask(() => callback(new Error('spawn ENOENT /private/path'))) } return child }), + off: vi.fn(() => child), unref: vi.fn() } vi.mocked(spawn).mockImplementationOnce(() => child as unknown as ReturnType) @@ -229,6 +233,33 @@ describe('openComputerUsePermissions', () => { }) }) + it('removes permission status helper listeners after close', async () => { + const { getComputerUsePermissionStatus } = await import('./macos-computer-use-permissions') + resolveHelperAppPathMock.mockReturnValue('/Applications/Orca Computer Use.app') + const child = { + stdout: { off: vi.fn(), on: vi.fn(), setEncoding: vi.fn() }, + stderr: { off: vi.fn(), on: vi.fn(), setEncoding: vi.fn() }, + on: vi.fn((event: string, callback: (status: number) => void) => { + if (event === 'close') { + queueMicrotask(() => callback(0)) + } + return child + }), + off: vi.fn(() => child), + unref: vi.fn() + } + vi.mocked(spawn).mockImplementationOnce(() => child as unknown as ReturnType) + + await expect(getComputerUsePermissionStatus()).resolves.toMatchObject({ + helperUnavailableReason: null + }) + + expect(child.stdout.off).toHaveBeenCalledWith('data', expect.any(Function)) + expect(child.stderr.off).toHaveBeenCalledWith('data', expect.any(Function)) + expect(child.off).toHaveBeenCalledWith('error', expect.any(Function)) + expect(child.off).toHaveBeenCalledWith('close', expect.any(Function)) + }) + it('reads permission status through the helper app identity', async () => { const { getComputerUsePermissionStatus } = await import('./macos-computer-use-permissions') resolveHelperAppPathMock.mockReturnValue('/Applications/Orca Computer Use.app') diff --git a/src/main/computer/macos-computer-use-permissions.ts b/src/main/computer/macos-computer-use-permissions.ts index c41fec98350..0b31c7d3090 100644 --- a/src/main/computer/macos-computer-use-permissions.ts +++ b/src/main/computer/macos-computer-use-permissions.ts @@ -1,3 +1,5 @@ +/* oxlint-disable max-lines -- Why: permission setup, status probes, and TCC +reset share the helper-app identity contract and platform guards. */ import { execFileSync, spawn, spawnSync } from 'child_process' import { mkdtemp, readFile, rm, stat } from 'fs/promises' import { tmpdir } from 'os' @@ -233,30 +235,57 @@ function launchPermissionStatusHelper(helperAppPath: string, statusPath: string) launch.stdout?.setEncoding('utf8') launch.stderr?.setEncoding('utf8') - launch.stdout?.on('data', (chunk) => { + const onStdoutData = (chunk: string): void => { stdout += chunk - }) - launch.stderr?.on('data', (chunk) => { + } + const onStderrData = (chunk: string): void => { stderr += chunk - }) - launch.on('error', () => { - reject( + } + let settled = false + const removeListeners = (): void => { + launch.stdout?.off('data', onStdoutData) + launch.stderr?.off('data', onStderrData) + launch.off('error', onError) + launch.off('close', onClose) + } + const settleResolve = (): void => { + if (settled) { + return + } + settled = true + removeListeners() + resolve() + } + const settleReject = (error: Error): void => { + if (settled) { + return + } + settled = true + removeListeners() + reject(error) + } + const onError = (): void => { + settleReject( new RuntimeClientError( 'accessibility_error', 'Could not check permissions: failed to launch helper' ) ) - }) - launch.on('close', (status) => { + } + const onClose = (status: number | null): void => { if (status === 0) { - resolve() + settleResolve() return } const detail = stderr.trim() || stdout.trim() || `exit ${status ?? 'unknown'}` - reject( + settleReject( new RuntimeClientError('accessibility_error', `Could not check permissions: ${detail}`) ) - }) + } + launch.stdout?.on('data', onStdoutData) + launch.stderr?.on('data', onStderrData) + launch.on('error', onError) + launch.on('close', onClose) }) }