diff --git a/src/cli/handlers/computer.ts b/src/cli/handlers/computer.ts index 1c23f7f6bb8..8f7df829357 100644 --- a/src/cli/handlers/computer.ts +++ b/src/cli/handlers/computer.ts @@ -50,6 +50,7 @@ export const COMPUTER_HANDLERS: Record = { helperAppPath: string | null openedSettings: boolean launchedHelper: boolean + launchQueued?: boolean permissions?: { id: string; status: string }[] nextStep?: string | null }>('computer.permissions', id ? { id } : {}) @@ -57,9 +58,7 @@ export const COMPUTER_HANDLERS: Record = { if (value.platform !== 'darwin') { return 'Computer-use permission setup is only required on macOS.' } - const firstLine = value.launchedHelper - ? 'Opened Orca Computer Use permission setup.' - : 'Computer Use permissions checked.' + const firstLine = permissionSetupHeadline(value) return [ firstLine, `Helper app: ${value.helperAppPath}`, @@ -251,3 +250,15 @@ function formatComputerCapabilities(value: ComputerProviderCapabilities): string ...(value.supports.input ? [` Input: background=${value.supports.input.background}`] : []) ].join('\n') } + +function permissionSetupHeadline(result: { + launchedHelper: boolean + launchQueued?: boolean +}): string { + if (result.launchQueued) { + return 'Queued Orca Computer Use permission setup; a newer request or a reset may replace it.' + } + return result.launchedHelper + ? 'Opened Orca Computer Use permission setup.' + : 'Computer Use permissions checked.' +} diff --git a/src/main/computer/macos-computer-use-permission-status-gate.test.ts b/src/main/computer/macos-computer-use-permission-status-gate.test.ts index 9bb57c9ad55..94f27c337c3 100644 --- a/src/main/computer/macos-computer-use-permission-status-gate.test.ts +++ b/src/main/computer/macos-computer-use-permission-status-gate.test.ts @@ -1,13 +1,19 @@ -import { describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { ComputerUsePermissionStatus, ComputerUsePermissionStatusResult } from '../../shared/computer-use-permissions-types' import { - bindPermissionStatusGateToAppActivity, - createPermissionStatusProbeGate + computerUsePermissionStatusGate, + createPermissionStatusGate } from './macos-computer-use-permission-status-gate' +const getStatusMock = vi.hoisted(() => vi.fn()) + +vi.mock('./macos-computer-use-permissions', () => ({ + getComputerUsePermissionStatus: getStatusMock +})) + function statusWith(accessibility: ComputerUsePermissionStatus): ComputerUsePermissionStatusResult { return { platform: 'darwin', @@ -22,148 +28,240 @@ function statusWith(accessibility: ComputerUsePermissionStatus): ComputerUsePerm const denied = statusWith('not-granted') const granted = statusWith('granted') -const background = { background: true } -function deferred(): { promise: Promise; resolve: (value: T) => void } { - let resolve: (value: T) => void = () => {} - const promise = new Promise((settle) => { - resolve = settle - }) - return { promise, resolve } +// Most spawns inside any half-open window [t, t + windowMs). +function maxSpawnsInAnyWindow(spawnedAt: number[], windowMs: number): number { + return Math.max( + 0, + ...spawnedAt.map( + (start) => spawnedAt.filter((at) => at >= start && at < start + windowMs).length + ) + ) } -describe('createPermissionStatusProbeGate', () => { - it('stops relaunching the helper for passive readers while Accessibility stays denied (#34390)', async () => { - let clock = 0 - const probe = vi.fn(async () => denied) - const gate = createPermissionStatusProbeGate({ probe, now: () => clock }) - - // Three always-mounted setup-guide hooks read on every window focus. - await Promise.all([gate.read(background), gate.read(background), gate.read(background)]) - expect(probe).toHaveBeenCalledTimes(1) - - for (let elapsed = 0; elapsed < 600_000; elapsed += 500) { - clock += 500 - await gate.read(background) - } - // Backoff 10s, 20s, 40s, 80s, then 120s cap: 8 launches in 10 minutes instead of 1,200. - expect(probe).toHaveBeenCalledTimes(8) - - gate.noteAppReturn() - await gate.read(background) - expect(probe).toHaveBeenCalledTimes(8) +describe('createPermissionStatusGate', () => { + beforeEach(() => { + vi.useFakeTimers({ now: 0 }) }) - it('re-probes once per return to the app so a grant or revocation shows immediately', async () => { - const probe = vi.fn().mockResolvedValueOnce(denied).mockResolvedValueOnce(granted) - const gate = createPermissionStatusProbeGate({ probe, now: () => 0 }) + afterEach(() => { + vi.useRealTimers() + }) - await expect(gate.read(background)).resolves.toBe(denied) - gate.noteAppDeparture() - gate.noteAppReturn() - const reads = await Promise.all([gate.read(background), gate.read(background)]) - expect(reads).toEqual([granted, granted]) - expect(probe).toHaveBeenCalledTimes(2) + it('shares one launch between concurrent readers', async () => { + const probe = vi.fn(async () => denied) + const gate = createPermissionStatusGate(probe) - probe.mockResolvedValueOnce(denied) - gate.noteAppDeparture() - gate.noteAppReturn() - await expect(gate.read(background)).resolves.toBe(denied) + await Promise.all([gate.read(), gate.read(), gate.read()]) + expect(probe).toHaveBeenCalledTimes(1) + }) + + it('allows at most 12 real spawns in any 60s window, even with a slow pre-spawn step', async () => { + const spawnedAt: number[] = [] + let preSpawnDelayMs = 100 + const probe = vi.fn(async (markSpawned: () => void) => { + // ast-grep-ignore: test-no-real-time-wait -- fake timers drive this simulated mkdtemp delay + await new Promise((resolve) => setTimeout(resolve, preSpawnDelayMs)) + preSpawnDelayMs = 0 + markSpawned() + spawnedAt.push(performance.now()) + return denied + }) + const gate = createPermissionStatusGate(probe) + + // Fire-and-forget focus reads every 500ms, then a caller that re-reads as each answer lands. + for (let elapsed = 0; elapsed < 120_000; elapsed += 500) { + void gate.read() + await vi.advanceTimersByTimeAsync(500) + } + const loopEnd = performance.now() + 120_000 + while (performance.now() < loopEnd) { + let answered = false + void gate.read().then(() => (answered = true)) + while (!answered) { + await vi.advanceTimersByTimeAsync(100) + } + } + expect(maxSpawnsInAnyWindow(spawnedAt, 60_000)).toBeLessThanOrEqual(12) + }) + + it('never serves a denial or error from cache, so a grant shows within one floor', async () => { + const failure = new Error('Timed out checking permissions') + const probe = vi + .fn() + .mockResolvedValueOnce(denied) + .mockRejectedValueOnce(failure) + .mockResolvedValueOnce(granted) + const gate = createPermissionStatusGate(probe) + + await expect(gate.read()).resolves.toBe(denied) + await vi.advanceTimersByTimeAsync(1_000) + const afterDenial = expect(gate.read()).rejects.toBe(failure) + await vi.advanceTimersByTimeAsync(4_000) + await afterDenial + + await vi.advanceTimersByTimeAsync(1_000) + const afterError = gate.read() + await vi.advanceTimersByTimeAsync(4_000) + await expect(afterError).resolves.toBe(granted) expect(probe).toHaveBeenCalledTimes(3) }) - it('serves a fresh read when the renderer focus read beats did-become-active', async () => { - let active = false - const probe = vi.fn().mockResolvedValueOnce(denied).mockResolvedValueOnce(granted) - const gate = createPermissionStatusProbeGate({ probe, now: () => 0 }) - gate.setAppActiveProbe(() => active) - - await gate.read(background) - gate.noteAppDeparture() - await expect(gate.read(background)).resolves.toBe(denied) - expect(probe).toHaveBeenCalledTimes(1) - - active = true - await expect(gate.read(background)).resolves.toBe(granted) - gate.noteAppReturn() - await expect(gate.read(background)).resolves.toBe(granted) - expect(probe).toHaveBeenCalledTimes(2) - }) - - it('counts the first app return after a startup spent inactive', async () => { - const probe = vi.fn().mockResolvedValueOnce(denied).mockResolvedValueOnce(granted) - const gate = createPermissionStatusProbeGate({ probe, now: () => 0 }) - let returnToApp = (): void => {} - let active = false - bindPermissionStatusGateToAppActivity(gate, { - onDeparture: () => {}, - onReturn: (listener) => { - returnToApp = listener - }, - isAppActive: () => active - }) - - await gate.read(background) - active = true - returnToApp() - await expect(gate.read(background)).resolves.toBe(granted) - }) - - it('never serves a finished denial to foreground readers, even on a quick return', async () => { - let clock = 0 - const probe = vi.fn().mockResolvedValueOnce(denied).mockResolvedValueOnce(granted) - const gate = createPermissionStatusProbeGate({ probe, now: () => clock }) + it('serves a grant until the floor expires, then re-checks immediately', async () => { + const probe = vi.fn(async () => granted) + const gate = createPermissionStatusGate(probe) await gate.read() - // The user grants at 250ms and the browser returns at 500ms. - clock += 500 - await expect(gate.read(background)).resolves.toBe(denied) - const reads = await Promise.all([gate.read(), gate.read(), gate.read()]) - expect(reads).toEqual([granted, granted, granted]) - expect(probe).toHaveBeenCalledTimes(2) - - clock += 2_000 - 1 - await expect(gate.read()).resolves.toBe(granted) + await vi.advanceTimersByTimeAsync(4_999) + await gate.read() + expect(probe).toHaveBeenCalledTimes(1) + await vi.advanceTimersByTimeAsync(1) + await gate.read() expect(probe).toHaveBeenCalledTimes(2) }) - it('discards an invalidated in-flight probe without overlapping helpers', async () => { - let clock = 0 - const first = deferred() - const probe = vi.fn().mockReturnValueOnce(first.promise).mockResolvedValue(denied) - const gate = createPermissionStatusProbeGate({ probe, now: () => clock }) + it('does not revive an expired grant when a new probe starts', async () => { + let finishSecond: (result: ComputerUsePermissionStatusResult) => void = () => {} + const probe = vi + .fn() + .mockResolvedValueOnce(granted) + .mockReturnValueOnce( + new Promise((resolve) => { + finishSecond = resolve + }) + ) + const gate = createPermissionStatusGate(probe) - const obsolete = gate.read(background) + await gate.read() + await vi.advanceTimersByTimeAsync(5_000) + void gate.read() + const joined = gate.read() + finishSecond(denied) + await expect(joined).resolves.toBe(denied) + }) + + it('launches no trailing probe without a waiter', async () => { + const probe = vi.fn(async () => denied) + const gate = createPermissionStatusGate(probe) + + await gate.read() + await vi.advanceTimersByTimeAsync(60_000) + expect(probe).toHaveBeenCalledTimes(1) + }) + + it('keeps readers off a probe that started before an invalidation, without overlapping helpers', async () => { + let finishFirst: (result: ComputerUsePermissionStatusResult) => void = () => {} + const probe = vi + .fn() + .mockReturnValueOnce( + new Promise((resolve) => { + finishFirst = resolve + }) + ) + .mockResolvedValue(denied) + const gate = createPermissionStatusGate(probe) + + void gate.read() gate.invalidate() - const current = gate.read(background) + const afterReset = gate.read() + await vi.advanceTimersByTimeAsync(10_000) expect(probe).toHaveBeenCalledTimes(1) - first.resolve(denied) - await expect(obsolete).resolves.toBe(denied) - await expect(current).resolves.toBe(denied) + finishFirst(granted) + await vi.advanceTimersByTimeAsync(0) + await expect(afterReset).resolves.toBe(denied) expect(probe).toHaveBeenCalledTimes(2) + }) - // Only the live probe counts toward the backoff: first step is 10s, not 20s. - clock += 10_000 - await gate.read(background) + it('never lets a stale trailing timer launch inside the floor', async () => { + const probe = vi.fn(async () => denied) + const gate = createPermissionStatusGate(probe) + + await gate.read() + await vi.advanceTimersByTimeAsync(1_000) + void gate.read() + // The trailing timer runs late: a reader that sees the floor expired launches first. + const lateClock = vi.spyOn(performance, 'now').mockReturnValue(5_001) + await gate.read() + await vi.advanceTimersByTimeAsync(0) + lateClock.mockRestore() + void gate.read() + expect(probe).toHaveBeenCalledTimes(2) + await vi.advanceTimersByTimeAsync(8_999) + expect(probe).toHaveBeenCalledTimes(2) + await vi.advanceTimersByTimeAsync(100) expect(probe).toHaveBeenCalledTimes(3) }) - it('shares a failed check briefly instead of backing it off like a denial', async () => { - let clock = 0 - const failure = new Error('Timed out checking permissions') - const probe = vi.fn().mockRejectedValueOnce(failure).mockResolvedValueOnce(granted) - const gate = createPermissionStatusProbeGate({ probe, now: () => clock }) + it('stops reusing a denial for setup once the floor expires', async () => { + const probe = vi.fn().mockResolvedValueOnce(denied).mockResolvedValueOnce(granted) + const gate = createPermissionStatusGate(probe) - const results = await Promise.allSettled([gate.read(background), gate.read(background)]) - expect(results).toEqual([ - { status: 'rejected', reason: failure }, - { status: 'rejected', reason: failure } - ]) - expect(probe).toHaveBeenCalledTimes(1) - - clock += 2_000 - await expect(gate.read(background)).resolves.toBe(granted) + await gate.read() + await expect(gate.peek()).resolves.toBe(denied) + await vi.advanceTimersByTimeAsync(5_000) + await expect(gate.peek()).resolves.toBe(granted) expect(probe).toHaveBeenCalledTimes(2) }) + + it('sends a late joiner that gets a denial to the next probe', async () => { + let finishFirst: (result: ComputerUsePermissionStatusResult) => void = () => {} + const probe = vi + .fn() + .mockReturnValueOnce( + new Promise((resolve) => { + finishFirst = resolve + }) + ) + .mockResolvedValueOnce(granted) + const gate = createPermissionStatusGate(probe) + + // A web read starts a probe; the user grants and returns to Orca a second later. + void gate.read() + await vi.advanceTimersByTimeAsync(1_000) + const onReturn = gate.read() + finishFirst(denied) + await vi.advanceTimersByTimeAsync(4_000) + await expect(onReturn).resolves.toBe(granted) + expect(probe).toHaveBeenCalledTimes(2) + }) + + it('does not cache a grant from a probe that started before an invalidation', async () => { + let finishFirst: (result: ComputerUsePermissionStatusResult) => void = () => {} + const probe = vi + .fn() + .mockReturnValueOnce( + new Promise((resolve) => { + finishFirst = resolve + }) + ) + .mockResolvedValue(denied) + const gate = createPermissionStatusGate(probe) + + void gate.read() + gate.invalidate() + finishFirst(granted) + await vi.advanceTimersByTimeAsync(1_000) + const afterReset = gate.read() + await vi.advanceTimersByTimeAsync(4_000) + await expect(afterReset).resolves.toBe(denied) + }) +}) + +describe('computerUsePermissionStatusGate', () => { + const originalPlatform = process.platform + + afterEach(() => { + Object.defineProperty(process, 'platform', { value: originalPlatform }) + }) + + it('never waits off macOS, where the probe launches no helper', async () => { + Object.defineProperty(process, 'platform', { value: 'win32' }) + getStatusMock.mockResolvedValue(statusWith('unsupported')) + + await computerUsePermissionStatusGate.read() + await computerUsePermissionStatusGate.read() + await computerUsePermissionStatusGate.read() + expect(getStatusMock).toHaveBeenCalledTimes(3) + }) }) diff --git a/src/main/computer/macos-computer-use-permission-status-gate.ts b/src/main/computer/macos-computer-use-permission-status-gate.ts index 2ff62be0363..bdf929bd7fa 100644 --- a/src/main/computer/macos-computer-use-permission-status-gate.ts +++ b/src/main/computer/macos-computer-use-permission-status-gate.ts @@ -1,186 +1,166 @@ import type { ComputerUsePermissionStatusResult } from '../../shared/computer-use-permissions-types' -// Each macOS probe launches a helper that polls TCC for ~3.5s; denied answers only change on user action. -const PERMISSION_STATUS_RESULT_REUSE_MS = 2_000 -const PERMISSION_STATUS_DENIED_MIN_BACKOFF_MS = 10_000 -const PERMISSION_STATUS_DENIED_MAX_BACKOFF_MS = 120_000 +// Each macOS probe launches a helper that settles TCC for ~2s; 5s between real spawns allows 12 per 60s window. +// performance.now is monotonic, so a wall-clock step can't stretch or skip the floor. +const PERMISSION_STATUS_LAUNCH_FLOOR_MS = 5_000 +// A probe older than this may predate a grant, so a denial from it sends the reader to the next probe. +const PERMISSION_STATUS_FRESH_JOIN_MS = 300 -type ProbeOutcome = - | { ok: true; result: ComputerUsePermissionStatusResult } - | { ok: false; error: unknown } +type StatusResult = ComputerUsePermissionStatusResult -type InFlightProbe = { - epoch: number - promise: Promise - followUp: Promise | null -} - -export type PermissionStatusReadOptions = { - // Local passive badges opt into the denied backoff; foreground reads never get a cached denial. - background?: boolean -} - -export type PermissionStatusProbeGate = { - read: (options?: PermissionStatusReadOptions) => Promise - noteAppDeparture: () => void - noteAppReturn: () => void - setAppActiveProbe: (isAppActive: () => boolean) => void +export type PermissionStatusGate = { + read: () => Promise + // Setup's cheapest answer: in-flight, recent grant or last denial, before launching anything new. + peek: () => Promise invalidate: () => void } -export type AppActivitySource = { - onDeparture: (listener: () => void) => void - onReturn: (listener: () => void) => void - isAppActive: () => boolean -} +// probe calls markSpawned when its helper actually starts; the floor counts from the later of claim and spawn. +export function createPermissionStatusGate( + probe: (markSpawned: () => void) => Promise +): PermissionStatusGate { + let inFlight: { promise: Promise; joinable: boolean } | null = null + let lastLaunchAt = Number.NEGATIVE_INFINITY + // Only grants are cached; a denial or error is never served, so a fresh grant shows within one floor. + let granted: StatusResult | null = null + // Reuse ends one floor after the probe that produced the answer, not after whichever probe ran last. + let cachedUntil = Number.NEGATIVE_INFINITY + // Setup may reuse a denial until the floor expires, so it never acts on one older than a floor. + let lastDenied: StatusResult | null = null + let waiters: { + promise: Promise + resolve: (next: Promise) => void + } | null = null + let trailingTimer: ReturnType | null = null -export function createPermissionStatusProbeGate(options: { - probe: () => Promise - now?: () => number -}): PermissionStatusProbeGate { - const now = options.now ?? Date.now - // A new epoch means the answer may have changed; results from older epochs are never served. - let epoch = 0 - let departed = false - let isAppActive = (): boolean => false - let inFlight: InFlightProbe | null = null - let cached: { epoch: number; outcome: ProbeOutcome; finishedAt: number } | null = null - let reuseUntil = 0 - let deniedStreak = 0 + const floorExpiresAt = (): number => lastLaunchAt + PERMISSION_STATUS_LAUNCH_FLOOR_MS - const noteAppReturn = (): void => { - if (departed) { - departed = false - epoch += 1 - } - } - - const recordOutcome = (probeEpoch: number, outcome: ProbeOutcome): void => { - if (probeEpoch !== epoch) { + const armTrailing = (): void => { + if (!waiters || inFlight || trailingTimer) { return } - const finishedAt = now() - cached = { epoch: probeEpoch, outcome, finishedAt } - if (!outcome.ok || !isMissingPermission(outcome.result)) { - if (outcome.ok) { - deniedStreak = 0 - } - reuseUntil = finishedAt + PERMISSION_STATUS_RESULT_REUSE_MS - return - } - deniedStreak += 1 - const backoff = PERMISSION_STATUS_DENIED_MIN_BACKOFF_MS * 2 ** (deniedStreak - 1) - reuseUntil = finishedAt + Math.min(backoff, PERMISSION_STATUS_DENIED_MAX_BACKOFF_MS) - } - - const startProbe = (): Promise => { - const probeEpoch = epoch - const entry: InFlightProbe = { - epoch: probeEpoch, - promise: options.probe().then( - (result) => { - recordOutcome(probeEpoch, { ok: true, result }) - return result - }, - (error: unknown) => { - recordOutcome(probeEpoch, { ok: false, error }) - throw error + trailingTimer = setTimeout( + () => { + trailingTimer = null + if (waiters && !inFlight && performance.now() >= floorExpiresAt()) { + void launch().catch(() => {}) + } else { + armTrailing() } - ), - followUp: null + }, + Math.max(0, Math.ceil(floorExpiresAt() - performance.now())) + ) + trailingTimer.unref?.() + } + + const launch = (): Promise => { + lastLaunchAt = performance.now() + if (trailingTimer) { + clearTimeout(trailingTimer) + trailingTimer = null + } + const entry = { + promise: probe(() => { + lastLaunchAt = Math.max(lastLaunchAt, performance.now()) + }), + joinable: true } inFlight = entry - void entry.promise + entry.promise + .then( + (result) => { + if (entry.joinable) { + granted = isCacheable(result) ? result : null + lastDenied = granted ? null : result + cachedUntil = floorExpiresAt() + } + }, + () => { + granted = null + lastDenied = null + } + ) .finally(() => { if (inFlight === entry) { inFlight = null } + armTrailing() }) - .catch(() => {}) + // Readers waiting out the floor take whichever probe starts next, whatever triggered it. + const pending = waiters + waiters = null + pending?.resolve(entry.promise) return entry.promise } - const reuseLimit = ( - entry: { outcome: ProbeOutcome; finishedAt: number }, - readOptions: PermissionStatusReadOptions - ): number => { - if (readOptions.background) { - return reuseUntil + const waitForNextProbe = (): Promise => { + if (!waiters) { + let resolve: (next: Promise) => void = () => {} + const promise = new Promise((settle) => { + resolve = settle + }) + waiters = { promise, resolve } } - // Foreground readers may have just granted; only passive badges accept a finished denial. - if (entry.outcome.ok && isMissingPermission(entry.outcome.result)) { - return 0 - } - return entry.finishedAt + PERMISSION_STATUS_RESULT_REUSE_MS + armTrailing() + return waiters.promise } - const read = ( - readOptions: PermissionStatusReadOptions = {} - ): Promise => { - // The renderer's focus read can reach main before did-become-active does. - if (departed && isAppActive()) { - noteAppReturn() + const read = (): Promise => { + if (granted && performance.now() < cachedUntil) { + return Promise.resolve(granted) } - const current = inFlight - if (current) { - if (current.epoch === epoch) { - return current.promise + if (inFlight?.joinable) { + const joined = inFlight.promise + if (performance.now() - lastLaunchAt <= PERMISSION_STATUS_FRESH_JOIN_MS) { + return joined } - // Never run two helpers at once; re-probe once the stale one settles. - current.followUp ??= current.promise.then(noop, noop).then(() => read(readOptions)) - return current.followUp + return joined.then((result) => (isCacheable(result) ? result : waitForNextProbe())) } - if (cached && cached.epoch === epoch) { - if (now() < reuseLimit(cached, readOptions)) { - const { outcome } = cached - return outcome.ok ? Promise.resolve(outcome.result) : Promise.reject(outcome.error) - } + if (!inFlight && performance.now() >= floorExpiresAt()) { + return launch() } - return startProbe() + return waitForNextProbe() } return { read, - noteAppDeparture: () => { - departed = true - }, - noteAppReturn, - setAppActiveProbe: (probe) => { - isAppActive = probe + peek: () => { + if (inFlight?.joinable) { + return inFlight.promise + } + if (granted && performance.now() < cachedUntil) { + return Promise.resolve(granted) + } + return lastDenied && performance.now() < cachedUntil ? Promise.resolve(lastDenied) : read() }, invalidate: () => { - epoch += 1 - deniedStreak = 0 + granted = null + lastDenied = null + if (inFlight) { + inFlight.joinable = false + } } } } -// App-level activation only: moving between Orca's own windows can't change a TCC answer. -export function bindPermissionStatusGateToAppActivity( - gate: PermissionStatusProbeGate, - activity: AppActivitySource -): void { - gate.setAppActiveProbe(activity.isAppActive) - // Listeners attach after startup; a grant made while Orca was inactive must count on first return. - if (!activity.isAppActive()) { - gate.noteAppDeparture() - } - activity.onDeparture(() => gate.noteAppDeparture()) - activity.onReturn(() => gate.noteAppReturn()) -} - -function noop(): void {} - -function isMissingPermission(result: ComputerUsePermissionStatusResult): boolean { +// A missing helper answers without launching anything, so it is as safe to reuse as a grant. +function isCacheable(result: StatusResult): boolean { return ( - result.helperUnavailableReason === null && - result.permissions.some((permission) => permission.status === 'not-granted') + result.helperUnavailableReason !== null || + result.permissions.every((permission) => permission.status !== 'not-granted') ) } -export const computerUsePermissionStatusGate = createPermissionStatusProbeGate({ - probe: async () => { - const { getComputerUsePermissionStatus } = await import('./macos-computer-use-permissions') - return getComputerUsePermissionStatus() - } -}) +async function probeStatus(markSpawned: () => void = () => {}): Promise { + const { getComputerUsePermissionStatus } = await import('./macos-computer-use-permissions') + return getComputerUsePermissionStatus(markSpawned) +} + +const macosGate = createPermissionStatusGate(probeStatus) + +// Off macOS the probe answers 'unsupported' instantly without a helper, so it skips the gate. +export const computerUsePermissionStatusGate: PermissionStatusGate = { + read: () => (process.platform === 'darwin' ? macosGate.read() : probeStatus()), + peek: () => (process.platform === 'darwin' ? macosGate.peek() : probeStatus()), + invalidate: () => macosGate.invalidate() +} diff --git a/src/main/computer/macos-computer-use-permission-status.ts b/src/main/computer/macos-computer-use-permission-status.ts index 711c38bb9ef..3d3eb8190bd 100644 --- a/src/main/computer/macos-computer-use-permission-status.ts +++ b/src/main/computer/macos-computer-use-permission-status.ts @@ -17,11 +17,16 @@ import { stopMacOSPermissionStatusHelper } from './macos-computer-use-permission const PERMISSION_STATUS_HELPER_LAUNCH_TIMEOUT_MS = 5_000 -export function getComputerUsePermissionStatus(): Promise { - return getComputerUsePermissionStatusAsync() +// onHelperSpawn fires when `open` actually starts, so launch budgets measure real spawns. +export function getComputerUsePermissionStatus( + onHelperSpawn: () => void = () => {} +): Promise { + return getComputerUsePermissionStatusAsync(onHelperSpawn) } -async function getComputerUsePermissionStatusAsync(): Promise { +async function getComputerUsePermissionStatusAsync( + onHelperSpawn: () => void +): Promise { if (process.platform !== 'darwin') { return { platform: process.platform, @@ -35,19 +40,15 @@ async function getComputerUsePermissionStatusAsync(): Promise void ): Promise>> { const tempDir = await mkdtemp(join(tmpdir(), 'orca-computer-use-permissions-')) const statusPath = join(tempDir, 'status.json') try { // Launch the helper app so TCC checks its identity instead of inheriting the parent's grants. - await launchPermissionStatusHelper(helperAppPath, statusPath) + await launchPermissionStatusHelper(helperAppPath, statusPath, onHelperSpawn) for (let attempt = 0; attempt < 50; attempt++) { if (await fileExists(statusPath)) { @@ -100,7 +114,11 @@ async function readPermissionStatusFromHelperApp( } } -function launchPermissionStatusHelper(helperAppPath: string, statusPath: string): Promise { +function launchPermissionStatusHelper( + helperAppPath: string, + statusPath: string, + onHelperSpawn: () => void +): Promise { return new Promise((resolve, reject) => { const launch = spawn( '/usr/bin/open', @@ -109,6 +127,7 @@ function launchPermissionStatusHelper(helperAppPath: string, statusPath: string) stdio: ['ignore', 'pipe', 'pipe'] } ) + onHelperSpawn() let stdout = '' let stderr = '' diff --git a/src/main/computer/macos-computer-use-permissions-shutdown.test.ts b/src/main/computer/macos-computer-use-permissions-shutdown.test.ts new file mode 100644 index 00000000000..71993398f89 --- /dev/null +++ b/src/main/computer/macos-computer-use-permissions-shutdown.test.ts @@ -0,0 +1,159 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { ComputerUsePermissionStatusResult } from '../../shared/computer-use-permissions-types' +import type * as PermissionsModule from './macos-computer-use-permissions' + +const mocks = vi.hoisted(() => ({ + spawn: vi.fn(), + spawnSync: vi.fn(), + peek: vi.fn(), + readBundleId: vi.fn() +})) + +vi.mock('node:child_process', () => ({ spawn: mocks.spawn, spawnSync: mocks.spawnSync })) +vi.mock('./macos-native-provider-paths', () => ({ + resolveMacOSComputerUseAppPath: () => '/Applications/Orca Computer Use.app' +})) +vi.mock('./macos-computer-use-permission-status', () => ({ + getComputerUsePermissionStatus: vi.fn(), + resolveComputerUseHelperUnavailableReason: () => null +})) +vi.mock('./macos-computer-use-permission-status-gate', () => ({ + computerUsePermissionStatusGate: { peek: mocks.peek, read: mocks.peek, invalidate: () => {} } +})) +vi.mock('../macos-tcc-reset', () => ({ + readMacosBundleId: mocks.readBundleId, + resetMacosTccPermission: async () => ({ ok: true }) +})) + +const denied: ComputerUsePermissionStatusResult = { + platform: 'darwin', + helperAppPath: '/Applications/Orca Computer Use.app', + helperUnavailableReason: null, + permissions: [ + { id: 'accessibility', status: 'not-granted' }, + { id: 'screenshots', status: 'not-granted' } + ] +} + +function setupLaunches(): unknown[] { + return mocks.spawn.mock.calls + .filter(([program]) => program === '/usr/bin/open') + .map(([, args]) => (Array.isArray(args) ? args.slice(3) : [])) +} + +function deferredStatus(): { + promise: Promise + resolve: () => void +} { + let resolve = (): void => {} + const promise = new Promise((settle) => { + resolve = () => settle(denied) + }) + return { promise, resolve } +} + +// Fresh modules per test: the shutdown flag and setup window are process-wide. +async function loadModules(): Promise<{ + permissions: typeof PermissionsModule + markHostShuttingDown: () => void +}> { + const permissions = await import('./macos-computer-use-permissions') + const { markHostShuttingDown } = await import('../host-shutdown-state') + return { permissions, markHostShuttingDown } +} + +describe('computer-use setup cancellation', () => { + const originalPlatform = process.platform + + beforeEach(() => { + vi.resetModules() + vi.useFakeTimers() + mocks.spawn.mockReset().mockReturnValue({ unref: () => {} }) + mocks.spawnSync.mockReset() + mocks.peek.mockReset().mockResolvedValue(denied) + mocks.readBundleId.mockReset().mockResolvedValue(null) + Object.defineProperty(process, 'platform', { value: 'darwin' }) + }) + + afterEach(() => { + vi.useRealTimers() + Object.defineProperty(process, 'platform', { value: originalPlatform }) + }) + + it('never spawns a queued helper once the host is shutting down', async () => { + const { permissions, markHostShuttingDown } = await loadModules() + await permissions.openComputerUsePermissions('accessibility') + await permissions.openComputerUsePermissions('screenshots') + markHostShuttingDown() + await vi.advanceTimersByTimeAsync(5_000) + expect(setupLaunches()).toEqual([['--permission', 'accessibility']]) + }) + + it('cancels an Open whose status resolves during shutdown, as an error', async () => { + const status = deferredStatus() + mocks.peek.mockReturnValue(status.promise) + const { permissions, markHostShuttingDown } = await loadModules() + const open = permissions.openComputerUsePermissions('accessibility') + markHostShuttingDown() + status.resolve() + await expect(open).rejects.toThrow('cancelled') + expect(setupLaunches()).toEqual([]) + }) + + it('cancels an Open that predates a reset, and the next Open still launches', async () => { + const status = deferredStatus() + mocks.peek.mockReturnValueOnce(status.promise) + const { permissions } = await loadModules() + const stale = permissions.openComputerUsePermissions('accessibility') + await permissions.resetComputerUsePermissions() + const fresh = permissions.openComputerUsePermissions('accessibility') + status.resolve() + await expect(stale).rejects.toThrow('cancelled') + await expect(fresh).resolves.toMatchObject({ launchedHelper: true, launchQueued: false }) + expect(setupLaunches()).toEqual([['--permission', 'accessibility']]) + }) + + it('lets the newest request win when an older one resumes after it launched', async () => { + const status = deferredStatus() + mocks.peek.mockReturnValueOnce(status.promise).mockReturnValueOnce(status.promise) + const { permissions } = await loadModules() + // A1 and B wait on a slow probe; A2 joins an answer that is already in. + const a1 = permissions.openComputerUsePermissions('accessibility') + const b = permissions.openComputerUsePermissions('screenshots') + await permissions.openComputerUsePermissions('accessibility') + status.resolve() + await Promise.all([a1, b]) + await vi.advanceTimersByTimeAsync(10_000) + expect(setupLaunches()).toEqual([['--permission', 'accessibility']]) + }) + + it('cancels an untargeted Open during shutdown even when everything is granted', async () => { + mocks.peek.mockResolvedValue({ + ...denied, + permissions: [ + { id: 'accessibility', status: 'granted' }, + { id: 'screenshots', status: 'granted' } + ] + }) + const { permissions, markHostShuttingDown } = await loadModules() + markHostShuttingDown() + await expect(permissions.openComputerUsePermissions()).rejects.toThrow('cancelled') + }) + + it('cancels a queued launch before reset waits on the bundle id', async () => { + const { permissions } = await loadModules() + await permissions.openComputerUsePermissions('accessibility') + await permissions.openComputerUsePermissions('screenshots') + let finishBundleRead = (): void => {} + mocks.readBundleId.mockReturnValue( + new Promise((resolve) => { + finishBundleRead = () => resolve(null) + }) + ) + const reset = permissions.resetComputerUsePermissions() + await vi.advanceTimersByTimeAsync(6_000) + finishBundleRead() + await reset + expect(setupLaunches()).toEqual([['--permission', 'accessibility']]) + }) +}) diff --git a/src/main/computer/macos-computer-use-permissions.test.ts b/src/main/computer/macos-computer-use-permissions.test.ts index 5c63885198b..c4aa4b68b71 100644 --- a/src/main/computer/macos-computer-use-permissions.test.ts +++ b/src/main/computer/macos-computer-use-permissions.test.ts @@ -7,6 +7,12 @@ import { openComputerUsePermissions, resetComputerUsePermissions } from './macos-computer-use-permissions' +import { getComputerUsePermissionStatus } from './macos-computer-use-permission-status' +import type * as permissionStatusGateModule from './macos-computer-use-permission-status-gate' +import { + createPermissionStatusGate, + type PermissionStatusGate +} from './macos-computer-use-permission-status-gate' const resolveHelperAppPathMock = vi.hoisted(() => vi.fn()) const resolveHelperExecutablePathMock = vi.hoisted(() => vi.fn()) @@ -15,6 +21,18 @@ const helperAppPath = '/Applications/Orca Computer Use.app' const helperInfoPlistPath = join(helperAppPath, 'Contents', 'Info.plist') const plistBuddyStdout = vi.hoisted(() => ({ value: 'com.example.orca.computer-use\n' })) +let monotonicOffset = 0 +const gate = vi.hoisted((): { current: PermissionStatusGate | null } => ({ current: null })) + +// A fresh gate per test so one test's floor or cached grant never leaks into the next. +vi.mock('./macos-computer-use-permission-status-gate', async (importOriginal) => ({ + ...(await importOriginal()), + computerUsePermissionStatusGate: { + read: () => gate.current!.read(), + peek: () => gate.current!.peek(), + invalidate: () => gate.current!.invalidate() + } +})) vi.mock('child_process', () => ({ execFileSync: vi.fn(), @@ -60,6 +78,10 @@ describe('openComputerUsePermissions', () => { vi.mocked(stat).mockResolvedValue({} as Awaited>) mockPermissionStatus('{"accessibility":"granted","screenshots":"granted"}') setPlatform('darwin') + // Fake timers restart the monotonic clock; step past any earlier test's setup window. + vi.useFakeTimers() + vi.advanceTimersByTime((monotonicOffset += 600_000)) + gate.current = createPermissionStatusGate(getComputerUsePermissionStatus) }) afterEach(() => { @@ -99,6 +121,7 @@ describe('openComputerUsePermissions', () => { permissionId: undefined, openedSettings: false, launchedHelper: true, + launchQueued: false, permissions: [ { id: 'accessibility', status: 'granted' }, { id: 'screenshots', status: 'not-granted' } @@ -132,6 +155,7 @@ describe('openComputerUsePermissions', () => { permissionId: 'accessibility', openedSettings: true, launchedHelper: true, + launchQueued: false, permissions: [ { id: 'accessibility', status: 'not-granted' }, { id: 'screenshots', status: 'not-granted' } @@ -155,6 +179,7 @@ describe('openComputerUsePermissions', () => { permissionId: 'accessibility', openedSettings: true, launchedHelper: true, + launchQueued: false, permissions: [ { id: 'accessibility', status: 'granted' }, { id: 'screenshots', status: 'not-granted' } @@ -205,9 +230,9 @@ describe('openComputerUsePermissions', () => { it('resets stale macOS TCC grants for the helper bundle id', async () => { resolveHelperAppPathMock.mockReturnValue('/Applications/Orca Computer Use.app') - vi.mocked(readFile) - .mockResolvedValueOnce('{"accessibility":"granted","screenshots":"granted"}') - .mockResolvedValueOnce('{"accessibility":"not-granted","screenshots":"not-granted"}') + vi.mocked(readFile).mockResolvedValueOnce( + '{"accessibility":"not-granted","screenshots":"not-granted"}' + ) vi.mocked(spawnSync).mockReturnValue({ status: 0 } as ReturnType) await expect(resetComputerUsePermissions()).resolves.toEqual({ @@ -245,6 +270,189 @@ describe('openComputerUsePermissions', () => { }) }) +describe('setup helper launch window', () => { + const originalPlatform = process.platform + + beforeEach(() => { + vi.mocked(spawn) + .mockReset() + .mockImplementation(fakeSpawnDispatch(() => ({}))) + vi.mocked(spawnSync).mockClear() + vi.mocked(mkdtemp).mockResolvedValue(permissionStatusTempDir) + vi.mocked(stat).mockResolvedValue({} as Awaited>) + resolveHelperAppPathMock.mockReturnValue(helperAppPath) + resolveHelperExecutablePathMock.mockReturnValue( + `${helperAppPath}/Contents/MacOS/orca-computer-use-macos` + ) + mockPermissionStatus('{"accessibility":"not-granted","screenshots":"not-granted"}') + setPlatform('darwin') + vi.useFakeTimers() + vi.advanceTimersByTime((monotonicOffset += 600_000)) + gate.current = createPermissionStatusGate(getComputerUsePermissionStatus) + }) + + afterEach(async () => { + // Drain any queued launch so it cannot carry into the next test. + await vi.advanceTimersByTimeAsync(5_000) + vi.useRealTimers() + setPlatform(originalPlatform) + }) + + const setupLaunches = (): string[][] => + vi + .mocked(spawn) + .mock.calls.filter( + ([program, args]) => + program === '/usr/bin/open' && + Array.isArray(args) && + !args.includes('--permission-status-file') + ) + .map(([, args]) => (Array.isArray(args) ? args.slice(3) : [])) + + const setupKills = (): number => + vi.mocked(spawnSync).mock.calls.filter(([program]) => program === '/usr/bin/pkill').length + + it('launches once per window across permission keys and queues the latest request', async () => { + const results = await Promise.all([ + openComputerUsePermissions('accessibility'), + openComputerUsePermissions('screenshots'), + openComputerUsePermissions() + ]) + expect(setupLaunches()).toEqual([['--permission', 'accessibility']]) + expect(results.map((result) => result.launchQueued)).toEqual([false, true, true]) + expect(results[1]).toMatchObject({ openedSettings: false, launchedHelper: true }) + + await vi.advanceTimersByTimeAsync(4_999) + expect(setupLaunches()).toHaveLength(1) + await vi.advanceTimersByTimeAsync(1) + expect(setupLaunches()).toEqual([['--permission', 'accessibility'], ['--permissions']]) + }) + + it('re-shows the helper on a second Open by killing and relaunching it after the window', async () => { + await openComputerUsePermissions('accessibility') + const killsAfterFirst = setupKills() + await expect(openComputerUsePermissions('accessibility')).resolves.toMatchObject({ + launchQueued: true + }) + await vi.advanceTimersByTimeAsync(5_000) + expect(setupLaunches()).toEqual([ + ['--permission', 'accessibility'], + ['--permission', 'accessibility'] + ]) + expect(setupKills()).toBe(killsAfterFirst * 2) + }) + + it('reuses the status it already has instead of probing again for setup', async () => { + await gate.current!.read() + await openComputerUsePermissions('accessibility') + const statusLaunches = vi + .mocked(spawn) + .mock.calls.filter( + ([, args]) => Array.isArray(args) && args.includes('--permission-status-file') + ) + expect(statusLaunches).toHaveLength(1) + }) + + it('keeps latest-wins when a later request coalesces onto an earlier one (A, B, A)', async () => { + await openComputerUsePermissions() + await Promise.all([ + openComputerUsePermissions('accessibility'), + openComputerUsePermissions('screenshots'), + openComputerUsePermissions('accessibility') + ]) + await vi.advanceTimersByTimeAsync(5_000) + expect(setupLaunches()).toEqual([['--permissions'], ['--permission', 'accessibility']]) + }) + + it('lets an A that arrives after A launches queue and win over B', async () => { + await openComputerUsePermissions() + await vi.advanceTimersByTimeAsync(5_000) + const first = openComputerUsePermissions('accessibility') + const second = openComputerUsePermissions('screenshots') + while (setupLaunches().length < 2) { + await Promise.resolve() + } + const third = openComputerUsePermissions('accessibility') + await Promise.all([first, second, third]) + await vi.advanceTimersByTimeAsync(5_000) + expect(setupLaunches().slice(1)).toEqual([ + ['--permission', 'accessibility'], + ['--permission', 'accessibility'] + ]) + }) + + it('spawns concurrent same-permission Opens once per window', async () => { + await Promise.all(Array.from({ length: 5 }, () => openComputerUsePermissions('accessibility'))) + await vi.advanceTimersByTimeAsync(10_000) + expect(setupLaunches()).toEqual([['--permission', 'accessibility']]) + }) + + it('never fires a queued setup spawn early when the timer delay is fractional', async () => { + const spawnedAt: number[] = [] + vi.mocked(spawn).mockImplementation((program, args) => { + if (program === '/usr/bin/open') { + spawnedAt.push(performance.now()) + } + return fakeSpawnDispatch(() => ({}))(program, Array.isArray(args) ? args : []) + }) + await openComputerUsePermissions('accessibility') + // Timers truncate fractional delays, so this one would fire half a millisecond early. + vi.advanceTimersByTime(0.5) + await openComputerUsePermissions('screenshots') + await vi.advanceTimersByTimeAsync(6_000) + const setupSpawns = spawnedAt.slice(-2) + expect(setupSpawns[1]! - setupSpawns[0]!).toBeGreaterThanOrEqual(5_000) + }) + + it('spaces real setup spawns 5s apart even when pkill is slow', async () => { + const spawnedAt: number[] = [] + let pkillDelayMs = 100 + const pkillResult: ReturnType = { + pid: 0, + output: [], + stdout: Buffer.alloc(0), + stderr: Buffer.alloc(0), + status: 0, + signal: null + } + vi.mocked(spawnSync).mockImplementation(() => { + vi.advanceTimersByTime(pkillDelayMs) + return pkillResult + }) + vi.mocked(spawn).mockImplementation((program, args) => { + if (program === '/usr/bin/open') { + spawnedAt.push(performance.now()) + } + return fakeSpawnDispatch(() => ({}))(program, Array.isArray(args) ? args : []) + }) + await openComputerUsePermissions('accessibility') + pkillDelayMs = 0 + await openComputerUsePermissions('screenshots') + await vi.advanceTimersByTimeAsync(6_000) + const setupSpawns = spawnedAt.slice(-2) + expect(setupSpawns).toHaveLength(2) + expect(setupSpawns[1]! - setupSpawns[0]!).toBeGreaterThanOrEqual(5_000) + }) + + it('never serves a seeded grant after a partial reset failure', async () => { + mockPermissionStatus('{"accessibility":"granted","screenshots":"granted"}') + await gate.current!.read() + vi.mocked(spawn).mockImplementation( + fakeSpawnDispatch((program, args) => + program === '/usr/bin/tccutil' && args[1] === 'ScreenCapture' ? { code: 1 } : {} + ) + ) + await expect(resetComputerUsePermissions()).rejects.toThrow('Could not reset ScreenCapture') + + mockPermissionStatus('{"accessibility":"not-granted","screenshots":"granted"}') + const afterReset = gate.current!.read() + await vi.advanceTimersByTimeAsync(5_000) + await expect(afterReset).resolves.toMatchObject({ + permissions: expect.arrayContaining([{ id: 'accessibility', status: 'not-granted' }]) + }) + }) +}) + function mockPermissionStatus(json: string): void { vi.mocked(spawnSync).mockReturnValue({ status: 0 } as ReturnType) vi.mocked(readFile).mockResolvedValue(json) diff --git a/src/main/computer/macos-computer-use-permissions.ts b/src/main/computer/macos-computer-use-permissions.ts index fda6da19201..0137c88ceed 100644 --- a/src/main/computer/macos-computer-use-permissions.ts +++ b/src/main/computer/macos-computer-use-permissions.ts @@ -1,8 +1,10 @@ import { spawn, spawnSync } from 'node:child_process' import { RuntimeClientError } from './runtime-client-error' +import { isHostShuttingDown } from '../host-shutdown-state' import { readMacosBundleId, resetMacosTccPermission } from '../macos-tcc-reset' import { resolveMacOSComputerUseAppPath } from './macos-native-provider-paths' -import { getComputerUsePermissionStatus } from './macos-computer-use-permission-status' +import { resolveComputerUseHelperUnavailableReason } from './macos-computer-use-permission-status' +import { computerUsePermissionStatusGate } from './macos-computer-use-permission-status-gate' import type { ComputerUsePermissionId, ComputerUsePermissionResetResult, @@ -14,14 +16,115 @@ const DEFAULT_COMPUTER_USE_BUNDLE_ID = 'com.stablyai.orca.computer-use' export { getComputerUsePermissionStatus } from './macos-computer-use-permission-status' +// One setup-helper spawn per 5s across every permission, on the monotonic clock; claimed synchronously. +const SETUP_HELPER_LAUNCH_WINDOW_MS = 5_000 +// Requests are numbered on arrival; a queued request loses to any later one (latest wins). +type SetupLaunchRequest = { seq: number; helperAppPath: string; helperArgs: string[] } +let setupRequestSeq = 0 +let lastLaunch: { seq: number; coversSeq: number; argsKey: string; at: number } | null = null +let queuedSetupLaunch: SetupLaunchRequest | null = null +let queuedSetupTimer: ReturnType | null = null +// Bumped by reset so Opens still waiting on status never launch afterwards. +let setupGeneration = 0 +// Opens that arrive mid-reset wait for it, so reset's helper cleanup can't kill what they open. +let resetInProgress: Promise | null = null + export function openComputerUsePermissions( permissionId?: ComputerUsePermissionId ): Promise { - return openComputerUsePermissionsAsync(permissionId) + return openComputerUsePermissionsAsync(permissionId, ++setupRequestSeq, setupGeneration) +} + +function cancelPendingSetupLaunches(): void { + setupGeneration += 1 + if (queuedSetupTimer) { + clearTimeout(queuedSetupTimer) + queuedSetupTimer = null + } + queuedSetupLaunch = null +} + +const setupWindowOpensAt = (): number => + (lastLaunch?.at ?? Number.NEGATIVE_INFINITY) + SETUP_HELPER_LAUNCH_WINDOW_MS + +// Launches now, or queues for when the window reopens; a later request supersedes a queued one. +function requestSetupHelperLaunch( + request: SetupLaunchRequest +): 'launched' | 'queued' | 'superseded' { + // A newer request already spawned; showing this older one now would replace it. + if (lastLaunch && request.seq <= lastLaunch.seq) { + return 'superseded' + } + if (performance.now() >= setupWindowOpensAt() && !queuedSetupLaunch) { + launchSetupHelper(request) + return 'launched' + } + if (!queuedSetupLaunch || request.seq > queuedSetupLaunch.seq) { + queuedSetupLaunch = request + } + armQueuedSetupTimer() + return 'queued' +} + +function armQueuedSetupTimer(): void { + if (queuedSetupTimer) { + return + } + queuedSetupTimer = setTimeout( + () => { + queuedSetupTimer = null + // Timers can fire a fraction early; never spawn inside the window. + if (performance.now() < setupWindowOpensAt()) { + armQueuedSetupTimer() + return + } + const queued = queuedSetupLaunch + queuedSetupLaunch = null + // Same-args requests that arrived before the last spawn were already shown by it. + const covered = + queued && + lastLaunch && + (queued.seq <= lastLaunch.seq || + (lastLaunch.argsKey === queued.helperArgs.join(' ') && + queued.seq <= lastLaunch.coversSeq)) + if (queued && !covered) { + try { + launchSetupHelper(queued) + } catch (error) { + console.error('[computer-use] queued setup helper launch failed:', error) + } + } + }, + Math.max(0, Math.ceil(setupWindowOpensAt() - performance.now())) + ) + queuedSetupTimer.unref?.() +} + +function launchSetupHelper(request: SetupLaunchRequest): void { + if (isHostShuttingDown()) { + return + } + // Kill and relaunch so a repeated click always brings the helper back to the front. + closeExistingPermissionHelpers() + const helper = spawn( + '/usr/bin/open', + ['-n', request.helperAppPath, '--args', ...request.helperArgs], + { detached: true, stdio: 'ignore' } + ) + // Measured after the spawn so a slow pkill can't pull the next spawn inside the window. + lastLaunch = { + seq: request.seq, + coversSeq: setupRequestSeq, + argsKey: request.helperArgs.join(' '), + at: performance.now() + } + helper.unref() } async function openComputerUsePermissionsAsync( - permissionId?: ComputerUsePermissionId + permissionId: ComputerUsePermissionId | undefined, + seq: number, + generation: number ): Promise { if (process.platform !== 'darwin') { return { @@ -42,11 +145,22 @@ async function openComputerUsePermissionsAsync( if (!helperAppPath) { throw new RuntimeClientError('accessibility_error', 'Orca Computer Use.app was not found') } - const status = await getComputerUsePermissionStatus() + // Wait out a reset before reading status, so this Open never sees pre-reset grants. + while (resetInProgress) { + await resetInProgress.catch(() => {}) + } + const status = await computerUsePermissionStatusGate.peek() if (status.helperUnavailableReason) { throw new RuntimeClientError('accessibility_error', status.helperUnavailableReason) } const nextStep = nextPermissionStep(status.permissions) + if (generation !== setupGeneration || isHostShuttingDown()) { + // An error rather than launchedHelper: false, which clients render as "setup complete". + throw new RuntimeClientError( + 'accessibility_error', + 'Computer Use setup was cancelled because permissions were reset or Orca is quitting' + ) + } if (!permissionId && !nextStep) { return { @@ -59,31 +173,35 @@ async function openComputerUsePermissionsAsync( nextStep } } - - closeExistingPermissionHelpers() const helperArgs = permissionId ? ['--permission', permissionId] : ['--permissions'] - const helper = spawn('/usr/bin/open', ['-n', helperAppPath, '--args', ...helperArgs], { - detached: true, - stdio: 'ignore' - }) - helper.unref() + const launch = requestSetupHelperLaunch({ seq, helperAppPath, helperArgs }) return { platform: process.platform, helperAppPath, permissionId, - openedSettings: permissionId !== undefined, + openedSettings: launch === 'launched' && permissionId !== undefined, + // Also true when superseded: a newer request's helper is already showing. launchedHelper: true, + launchQueued: launch === 'queued', permissions: status.permissions, nextStep } } export function resetComputerUsePermissions(): Promise { - return resetComputerUsePermissionsAsync() + const reset = resetComputerUsePermissionsAsync().finally(() => { + if (resetInProgress === reset) { + resetInProgress = null + } + }) + resetInProgress = reset + return reset } async function resetComputerUsePermissionsAsync(): Promise { + // Before any await, so a queued or preflighting Open can't slip a helper in during the reset. + cancelPendingSetupLaunches() if (process.platform !== 'darwin') { return { platform: process.platform, @@ -102,18 +220,23 @@ async function resetComputerUsePermissionsAsync(): Promise unknown> - focusedWindow: { current: object | null } - probe: ReturnType -} - -const mocks = vi.hoisted((): HoistedMocks => ({ - app: null, - handlers: new Map(), - focusedWindow: { current: null }, - probe: vi.fn() -})) - -vi.mock('electron', async () => { - const { EventEmitter: Emitter } = await import('node:events') - mocks.app = new Emitter() - return { - app: mocks.app, - BrowserWindow: { getFocusedWindow: () => mocks.focusedWindow.current }, - ipcMain: { - handle: (name: string, handler: (...args: unknown[]) => unknown) => - mocks.handlers.set(name, handler) - } - } -}) - -vi.mock('../computer/macos-computer-use-permissions', () => ({ - getComputerUsePermissionStatus: mocks.probe -})) - -describe('registerComputerUsePermissionHandlers', () => { - const originalPlatform = process.platform - - afterEach(() => { - Object.defineProperty(process, 'platform', { value: originalPlatform }) - }) - - it('re-probes passive readers only after the app itself returns', async () => { - Object.defineProperty(process, 'platform', { value: 'darwin' }) - mocks.probe.mockResolvedValue({ - platform: 'darwin', - helperAppPath: '/Applications/Orca Computer Use.app', - helperUnavailableReason: null, - permissions: [ - { id: 'accessibility', status: 'not-granted' }, - { id: 'screenshots', status: 'granted' } - ] - }) - const { registerComputerUsePermissionHandlers } = await import('./computer-use-permissions') - registerComputerUsePermissionHandlers() - const app = mocks.app! - const readPassively = (): unknown => - mocks.handlers.get('computerUsePermissions:getStatus')!({}, { background: true }) - - mocks.focusedWindow.current = {} - await readPassively() - await readPassively() - expect(mocks.probe).toHaveBeenCalledTimes(1) - - app.emit('did-resign-active') - app.emit('did-become-active') - await readPassively() - expect(mocks.probe).toHaveBeenCalledTimes(2) - }) -}) diff --git a/src/main/ipc/computer-use-permissions.ts b/src/main/ipc/computer-use-permissions.ts index cd97f865bb9..8919acc42c7 100644 --- a/src/main/ipc/computer-use-permissions.ts +++ b/src/main/ipc/computer-use-permissions.ts @@ -1,24 +1,13 @@ -import { app, BrowserWindow, ipcMain } from 'electron' +import { ipcMain } from 'electron' import type { ComputerUsePermissionId, ComputerUsePermissionResetResult, ComputerUsePermissionSetupResult, ComputerUsePermissionStatusResult } from '../../shared/computer-use-permissions-types' -import { - bindPermissionStatusGateToAppActivity, - computerUsePermissionStatusGate, - type PermissionStatusReadOptions -} from '../computer/macos-computer-use-permission-status-gate' +import { computerUsePermissionStatusGate } from '../computer/macos-computer-use-permission-status-gate' export function registerComputerUsePermissionHandlers(): void { - if (process.platform === 'darwin') { - bindPermissionStatusGateToAppActivity(computerUsePermissionStatusGate, { - onDeparture: (listener) => app.on('did-resign-active', listener), - onReturn: (listener) => app.on('did-become-active', listener), - isAppActive: () => BrowserWindow.getFocusedWindow() !== null - }) - } ipcMain.handle( 'computerUsePermissions:openSetup', async ( @@ -27,27 +16,19 @@ export function registerComputerUsePermissionHandlers(): void { ): Promise => { const { openComputerUsePermissions } = await import('../computer/macos-computer-use-permissions') - computerUsePermissionStatusGate.invalidate() return openComputerUsePermissions(args?.id) } ) ipcMain.handle( 'computerUsePermissions:getStatus', - (_event, args?: PermissionStatusReadOptions): Promise => - computerUsePermissionStatusGate.read({ background: args?.background === true }) + (): Promise => computerUsePermissionStatusGate.read() ) ipcMain.handle( 'computerUsePermissions:reset', async (): Promise => { const { resetComputerUsePermissions } = await import('../computer/macos-computer-use-permissions') - computerUsePermissionStatusGate.invalidate() - try { - return await resetComputerUsePermissions() - } finally { - // Reads during the reset may have cached the pre-reset grants. - computerUsePermissionStatusGate.invalidate() - } + return resetComputerUsePermissions() } ) } diff --git a/src/main/orcad/orcad-lifecycle.ts b/src/main/orcad/orcad-lifecycle.ts index 2f8ca2f3922..a67307b9fae 100644 --- a/src/main/orcad/orcad-lifecycle.ts +++ b/src/main/orcad/orcad-lifecycle.ts @@ -1,4 +1,5 @@ import { setRuntimeBrowserCommandsFactory } from '../runtime/runtime-browser-commands-factory' +import { markHostShuttingDown } from '../host-shutdown-state' import { setDesktopBrowserBridgeHostEnabled } from '../runtime/desktop-browser-bridge/desktop-browser-bridge-registry' import { ORCAD_MANAGED_ACTIVATION_ROOT_ENV } from '../../shared/orcad-managed-launch-env' import { startOrcadBrowserProvider } from './orcad-browser-startup' @@ -59,6 +60,7 @@ export function installOrcadShutdownSignals( return false } stopping = true + markHostShuttingDown() holdOrcadTerminalAdmissionUntilExit() exitCode = stoppedExitCode retractClean = onFailed @@ -105,6 +107,8 @@ export async function startOrcadWithLifecycle( const cleanup = createIdempotentOrcadCleanup(async () => { // Stamp quit before awaiting teardown so dying chat agents read as restart rather than crash. recordAgentSessionRuntimeEnd('quit') + // Startup failures reach cleanup without shutdown admission while RPC may already be serving. + markHostShuttingDown() let runtimeCleanupSucceeded = false try { await runtime.stop() diff --git a/src/main/runtime/rpc/methods/computer.test.ts b/src/main/runtime/rpc/methods/computer.test.ts index 69420d2fa2b..64cfc4f62cc 100644 --- a/src/main/runtime/rpc/methods/computer.test.ts +++ b/src/main/runtime/rpc/methods/computer.test.ts @@ -89,7 +89,7 @@ describe('computer RPC methods', () => { computerMocks.getComputerUsePermissionStatus.mockReturnValue(result) await expect(call('computer.permissionsStatus', {})).resolves.toBe(result) - expect(computerMocks.getComputerUsePermissionStatus).toHaveBeenCalledWith() + expect(computerMocks.getComputerUsePermissionStatus).toHaveBeenCalledWith(expect.any(Function)) }) it('lists windows through the sidecar', async () => { diff --git a/src/main/runtime/rpc/methods/computer.ts b/src/main/runtime/rpc/methods/computer.ts index 12219d8995c..b30985e5557 100644 --- a/src/main/runtime/rpc/methods/computer.ts +++ b/src/main/runtime/rpc/methods/computer.ts @@ -55,9 +55,6 @@ export const COMPUTER_METHODS = [ handler: async (params) => { const { openComputerUsePermissions } = await import('../../../computer/macos-computer-use-permissions') - const { computerUsePermissionStatusGate } = - await import('../../../computer/macos-computer-use-permission-status-gate') - computerUsePermissionStatusGate.invalidate() return openComputerUsePermissions(params.id) } }), diff --git a/src/main/startup/main-process-quit.ts b/src/main/startup/main-process-quit.ts index 3df4f2a65f2..57433a75e59 100644 --- a/src/main/startup/main-process-quit.ts +++ b/src/main/startup/main-process-quit.ts @@ -1,4 +1,5 @@ import { killWslChannelsInCloseGrace } from '../wsl/wsl-command-channel' +import { markHostShuttingDown } from '../host-shutdown-state' import { app, type Event } from 'electron' import { closeAllWatchers } from '../ipc/filesystem/watchers/filesystem-watcher-shutdown' import { disposeWorktreeBaseDirectoryWatchers } from '../ipc/worktree-base-directory-watcher' @@ -111,6 +112,7 @@ function installWillQuitHandler(): void { // Include pane grids still inside their write burst in final save. const paneGridFlush = state.runtime?.flushPaneGrids() ?? null // A renderer can veto before-quit; push must survive until quit is committed. + markHostShuttingDown() state.desktopPushService?.stop() state.unsubscribeSystemResumeBroadcast?.() state.unsubscribeSystemResumeBroadcast = null diff --git a/src/preload/api/computer-use-permissions-bridge.ts b/src/preload/api/computer-use-permissions-bridge.ts index 586b66a5b16..bd36efbdcc3 100644 --- a/src/preload/api/computer-use-permissions-bridge.ts +++ b/src/preload/api/computer-use-permissions-bridge.ts @@ -2,8 +2,7 @@ import { ipcRenderer } from 'electron' import type { PreloadApi } from '../api-types' export const computerUsePermissionsApi = { - getStatus: (args?: { background?: boolean }) => - ipcRenderer.invoke('computerUsePermissions:getStatus', args), + getStatus: () => ipcRenderer.invoke('computerUsePermissions:getStatus'), openSetup: (args?: { id?: string }) => ipcRenderer.invoke('computerUsePermissions:openSetup', args), reset: () => ipcRenderer.invoke('computerUsePermissions:reset') diff --git a/src/preload/api/os-permission-api.ts b/src/preload/api/os-permission-api.ts index 9b3a3cbaea6..ddf44beac23 100644 --- a/src/preload/api/os-permission-api.ts +++ b/src/preload/api/os-permission-api.ts @@ -61,8 +61,7 @@ export type DeveloperPermissionsApi = { } export type ComputerUsePermissionsApi = { - // Passive badges pass background to accept a backed-off denial instead of relaunching the helper. - getStatus: (args?: { background?: boolean }) => Promise + getStatus: () => Promise openSetup: (args?: { id?: ComputerUsePermissionId }) => Promise reset: () => Promise } diff --git a/src/renderer/src/components/feature-wall/agent-capability-setup-status.ts b/src/renderer/src/components/feature-wall/agent-capability-setup-status.ts index 6c521293fd5..8de29e771b3 100644 --- a/src/renderer/src/components/feature-wall/agent-capability-setup-status.ts +++ b/src/renderer/src/components/feature-wall/agent-capability-setup-status.ts @@ -275,7 +275,7 @@ function useComputerUsePermissionStatus(enabled: boolean): { const refresh = (): void => { setStatus((current) => ({ ...current, checking: true })) window.api.computerUsePermissions - .getStatus({ background: true }) + .getStatus() .then((next) => { if (stale) { return diff --git a/src/renderer/src/components/settings/ComputerUsePane.tsx b/src/renderer/src/components/settings/ComputerUsePane.tsx index b65d1b66fab..321a9963562 100644 --- a/src/renderer/src/components/settings/ComputerUsePane.tsx +++ b/src/renderer/src/components/settings/ComputerUsePane.tsx @@ -197,7 +197,14 @@ export function ComputerUsePane(): React.JSX.Element { if (!mountedRef.current) { return } - if (result.launchedHelper) { + if (result.launchQueued) { + toast.message( + translate( + 'auto.components.settings.ComputerUsePane.794807aa21', + 'Queued macOS Privacy & Security; a newer request or a reset may replace it' + ) + ) + } else if (result.launchedHelper) { toast.message( translate( 'auto.components.settings.ComputerUsePane.697005758f', diff --git a/src/renderer/src/components/setup-guide/use-setup-guide-progress.ts b/src/renderer/src/components/setup-guide/use-setup-guide-progress.ts index 8c5590b6689..5d82f43ab90 100644 --- a/src/renderer/src/components/setup-guide/use-setup-guide-progress.ts +++ b/src/renderer/src/components/setup-guide/use-setup-guide-progress.ts @@ -191,9 +191,7 @@ export function useSetupGuideProgress( }, [orderedGitRepos, settings, setupScriptProbeSignature, shouldRefreshCoreState]) const readComputerUsePermissions = useCallback(async (isStale: () => boolean): Promise => { - const status = await window.api.computerUsePermissions - .getStatus({ background: true }) - .catch(() => null) + const status = await window.api.computerUsePermissions.getStatus().catch(() => null) if (isStale()) { return } diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 574f78d6b66..5baad1c2f84 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -7853,6 +7853,7 @@ "7801ac08ec": "Computer Use permissions are only required on macOS", "740766c291": "Computer Use setup is already complete", "697005758f": "Opened macOS Privacy & Security", + "794807aa21": "Queued macOS Privacy & Security; a newer request or a reset may replace it", "2168fa5ab0": "Could not load Computer Use permissions", "0c9a33f468": "Capture app windows so agents can inspect visual state.", "07bbe4c4cb": "Screenshots", diff --git a/src/renderer/src/web/preload-api/web-host-capability-api.ts b/src/renderer/src/web/preload-api/web-host-capability-api.ts index 0d36535fa48..189704a310a 100644 --- a/src/renderer/src/web/preload-api/web-host-capability-api.ts +++ b/src/renderer/src/web/preload-api/web-host-capability-api.ts @@ -114,7 +114,6 @@ export function createComputerUsePermissionsApi(): NonNullable< Partial['computerUsePermissions'] > { return { - // Browser focus can't activate the host app, so remote reads never opt into the denied backoff. getStatus: () => callRuntimeResult( 'computer.permissionsStatus', @@ -126,13 +125,19 @@ export function createComputerUsePermissionsApi(): NonNullable< 'computer.permissions', args ?? {}, 15_000 - ).catch(() => ({ - platform: getBrowserPlatform(), - helperAppPath: null, - openedSettings: false, - launchedHelper: false, - nextStep: 'Computer-use permissions are managed on the Orca server.' - })), + ).catch((error: unknown) => { + // Only a host without this method falls back; real failures (e.g. a cancelled Open) must surface. + if (!(error instanceof Error && 'code' in error && error.code === 'method_not_found')) { + throw error + } + return { + platform: getBrowserPlatform(), + helperAppPath: null, + openedSettings: false, + launchedHelper: false, + nextStep: 'Computer-use permissions are managed on the Orca server.' + } + }), reset: () => Promise.resolve({ platform: getBrowserPlatform(), diff --git a/src/renderer/src/web/web-preload-api-ui.test.ts b/src/renderer/src/web/web-preload-api-ui.test.ts index da86ee94300..ab48147e1fa 100644 --- a/src/renderer/src/web/web-preload-api-ui.test.ts +++ b/src/renderer/src/web/web-preload-api-ui.test.ts @@ -718,10 +718,7 @@ describe('web UI preload API', () => { skills: [{ name: 'computer-use', installed: true }], scannedAt: 123 }) - // Passive local callers pass background; the browser can't activate the host, so it must not reach RPC. - const permissionsStatus = await globals.window.api.computerUsePermissions.getStatus({ - background: true - }) + const permissionsStatus = await globals.window.api.computerUsePermissions.getStatus() expect(permissionsStatus.helperUnavailableReason).toBeNull() expect(permissionsStatus.permissions).toContainEqual({ id: 'accessibility', status: 'granted' }) await expect( @@ -831,4 +828,38 @@ describe('web UI preload API', () => { 'runtime disconnected' ) }) + + it('surfaces a cancelled paired web setup instead of reporting it as complete', async () => { + vi.doMock('./web-runtime-client', () => ({ + WebRuntimeClient: class { + call(method: string): Promise> { + if (method === 'computer.permissions') { + return Promise.resolve({ + id: method, + ok: false, + error: { code: 'accessibility_error', message: 'Computer Use setup was cancelled' }, + _meta: { runtimeId: 'runtime-1' } + }) + } + return Promise.resolve({ + id: method, + ok: true, + result: {}, + _meta: { runtimeId: 'runtime-1' } + }) + } + + close(): void {} + } + })) + + const globals = installBrowserGlobals('Linux') + writeStoredRuntimeEnvironment(globals.storage) + const { installWebPreloadApi } = await import('./web-preload-api') + installWebPreloadApi() + + await expect( + globals.window.api.computerUsePermissions.openSetup({ id: 'accessibility' }) + ).rejects.toThrow('Computer Use setup was cancelled') + }) }) diff --git a/src/shared/computer-use-permissions-types.ts b/src/shared/computer-use-permissions-types.ts index 92fbcb3dc35..e8f57c131c0 100644 --- a/src/shared/computer-use-permissions-types.ts +++ b/src/shared/computer-use-permissions-types.ts @@ -19,7 +19,10 @@ export type ComputerUsePermissionSetupResult = { helperAppPath: string | null permissionId?: ComputerUsePermissionId openedSettings: boolean + // True when a helper launched, is showing for a newer request, or is queued; older clients only read this. launchedHelper: boolean + // Queued for when the 5s setup window reopens; a newer request or a reset may replace it. + launchQueued?: boolean permissions?: ComputerUsePermissionState[] nextStep?: string | null }