mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
fix(computer-use): reap detached macOS helpers through owner exit (#20926)
Reclaim detached macOS computer-use helpers on abandoned requests and transports. Keep ownership from spawn, escalate SIGTERM to SIGKILL, force pending reaps when the sidecar exits, and clean each failed startup's private socket directory. Based on #14494 by @JuuuuHong. Preserve the original helper ownership/reaping design and regression tests while retaining the upstream line-buffer optimization and adding real-process teardown and resource-bound tests. Fixes #9141. Co-authored-by: JuuuuHong <juhang720@gmail.com>
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
import { EventEmitter } from 'node:events'
|
||||
import { join } from 'node:path'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { PROVIDER_SIGKILL_GRACE_MS } from './macos-native-provider-process-reaping'
|
||||
|
||||
const {
|
||||
chmodSyncMock,
|
||||
@@ -63,8 +64,15 @@ class FakeSocket extends EventEmitter {
|
||||
}
|
||||
|
||||
class FakeProvider extends EventEmitter {
|
||||
exitCode: number | null = null
|
||||
signalCode: string | null = null
|
||||
kill = vi.fn()
|
||||
unref = vi.fn()
|
||||
|
||||
exit(code = 0): void {
|
||||
this.exitCode = code
|
||||
this.emit('exit', code, null)
|
||||
}
|
||||
}
|
||||
|
||||
function pendingConnectThatRejectsOnAbort(signal?: AbortSignal): Promise<never> {
|
||||
@@ -107,6 +115,11 @@ describe('MacOSNativeProviderClient', () => {
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
for (const result of spawnMock.mock.results) {
|
||||
if (result.value instanceof FakeProvider) {
|
||||
result.value.exit()
|
||||
}
|
||||
}
|
||||
chmodSyncMock.mockReset()
|
||||
connectMacOSProviderSocketMock.mockReset()
|
||||
mkdtempSyncMock.mockReset()
|
||||
@@ -497,15 +510,15 @@ describe('MacOSNativeProviderClient', () => {
|
||||
})
|
||||
|
||||
it('terminates the helper process when socket startup fails', async () => {
|
||||
const providerKill = vi.fn()
|
||||
spawnMock.mockReturnValueOnce({ unref: vi.fn(), kill: providerKill })
|
||||
const provider = new FakeProvider()
|
||||
spawnMock.mockReturnValueOnce(provider)
|
||||
connectMacOSProviderSocketMock.mockRejectedValueOnce(new Error('socket did not open'))
|
||||
const { MacOSNativeProviderClient } = await loadClientModule()
|
||||
const client = new MacOSNativeProviderClient()
|
||||
|
||||
await expect(client.capabilities()).rejects.toThrow('socket did not open')
|
||||
|
||||
expect(providerKill).toHaveBeenCalledWith('SIGTERM')
|
||||
expect(provider.kill).toHaveBeenCalledWith('SIGTERM')
|
||||
expect(rmSyncMock).toHaveBeenCalledWith(expect.stringContaining('orca-computer-use-'), {
|
||||
recursive: true,
|
||||
force: true
|
||||
@@ -551,6 +564,162 @@ describe('MacOSNativeProviderClient', () => {
|
||||
expect(connectSignal.aborted).toBe(true)
|
||||
expect(providers[0]!.kill).toHaveBeenCalledWith('SIGTERM')
|
||||
})
|
||||
|
||||
it('escalates to SIGKILL when a helper ignores terminate and SIGTERM', async () => {
|
||||
const { MacOSNativeProviderClient } = await loadClientModule()
|
||||
const client = new MacOSNativeProviderClient()
|
||||
|
||||
const call = client.capabilities()
|
||||
const rejection = expect(call).rejects.toThrow('native macOS provider handshake timed out')
|
||||
await vi.waitFor(() => expect(sockets).toHaveLength(1))
|
||||
const socket = sockets[0]!
|
||||
await vi.waitFor(() => expect(socket.writes).toHaveLength(1))
|
||||
|
||||
await vi.advanceTimersByTimeAsync(60_000)
|
||||
await rejection
|
||||
|
||||
const provider = providers[0]!
|
||||
// Why: a wedged helper never reads `terminate`, so the socket write alone
|
||||
// is what used to leak the process on every request timeout.
|
||||
expect(socket.writes.at(-1)).toContain('"method":"terminate"')
|
||||
expect(provider.kill).toHaveBeenCalledWith('SIGTERM')
|
||||
expect(provider.kill).not.toHaveBeenCalledWith('SIGKILL')
|
||||
|
||||
await vi.advanceTimersByTimeAsync(PROVIDER_SIGKILL_GRACE_MS)
|
||||
expect(provider.kill).toHaveBeenCalledWith('SIGKILL')
|
||||
})
|
||||
|
||||
it('does not escalate to SIGKILL when the helper exits after SIGTERM', async () => {
|
||||
const { MacOSNativeProviderClient } = await loadClientModule()
|
||||
const client = new MacOSNativeProviderClient()
|
||||
|
||||
const call = client.capabilities()
|
||||
const rejection = expect(call).rejects.toThrow('native macOS provider handshake timed out')
|
||||
await vi.waitFor(() => expect(sockets).toHaveLength(1))
|
||||
await vi.waitFor(() => expect(sockets[0]!.writes).toHaveLength(1))
|
||||
|
||||
await vi.advanceTimersByTimeAsync(60_000)
|
||||
await rejection
|
||||
|
||||
const provider = providers[0]!
|
||||
expect(provider.kill).toHaveBeenCalledWith('SIGTERM')
|
||||
provider.exit(0)
|
||||
|
||||
await vi.advanceTimersByTimeAsync(PROVIDER_SIGKILL_GRACE_MS * 2)
|
||||
expect(provider.kill).not.toHaveBeenCalledWith('SIGKILL')
|
||||
})
|
||||
|
||||
it('reaps the previous helper process before a replacement is started', async () => {
|
||||
const { MacOSNativeProviderClient } = await loadClientModule()
|
||||
const client = new MacOSNativeProviderClient()
|
||||
|
||||
const firstCall = client.capabilities()
|
||||
const firstRejection = expect(firstCall).rejects.toThrow('active helper failed')
|
||||
await vi.waitFor(() => expect(sockets).toHaveLength(1))
|
||||
await vi.waitFor(() => expect(sockets[0]!.writes).toHaveLength(1))
|
||||
sockets[0]!.emit('error', new Error('active helper failed'))
|
||||
await firstRejection
|
||||
|
||||
expect(providers[0]!.kill).toHaveBeenCalledWith('SIGTERM')
|
||||
|
||||
const secondCall = client.capabilities()
|
||||
await vi.waitFor(() => expect(providers).toHaveLength(2))
|
||||
// Why: the replacement must not inherit the previous generation's teardown.
|
||||
expect(providers[1]!.kill).not.toHaveBeenCalled()
|
||||
|
||||
const secondSocket = sockets[1]!
|
||||
await vi.waitFor(() => expect(secondSocket.writes).toHaveLength(1))
|
||||
const secondRequest = JSON.parse(secondSocket.writes[0]!) as { id: number }
|
||||
secondSocket.emit(
|
||||
'data',
|
||||
`${JSON.stringify({
|
||||
id: secondRequest.id,
|
||||
ok: true,
|
||||
result: { protocolVersion: 1, supports: {} }
|
||||
})}\n`
|
||||
)
|
||||
await expect(secondCall).resolves.toMatchObject({ protocolVersion: 1 })
|
||||
})
|
||||
|
||||
it('reaps the helper process when the active socket closes on its own', async () => {
|
||||
const { MacOSNativeProviderClient } = await loadClientModule()
|
||||
const client = new MacOSNativeProviderClient()
|
||||
|
||||
const call = client.capabilities()
|
||||
const rejection = expect(call).rejects.toThrow('native macOS helper app connection closed')
|
||||
await vi.waitFor(() => expect(sockets).toHaveLength(1))
|
||||
await vi.waitFor(() => expect(sockets[0]!.writes).toHaveLength(1))
|
||||
|
||||
// Why: a helper that dies takes its socket down with a bare 'close', with no
|
||||
// preceding 'error' — the teardown path most likely to run in the wild.
|
||||
sockets[0]!.emit('close')
|
||||
await rejection
|
||||
|
||||
expect(providers[0]!.kill).toHaveBeenCalledWith('SIGTERM')
|
||||
})
|
||||
|
||||
it('does not signal a helper that already exited before teardown', async () => {
|
||||
const { MacOSNativeProviderClient } = await loadClientModule()
|
||||
const client = new MacOSNativeProviderClient()
|
||||
|
||||
const call = client.capabilities()
|
||||
const rejection = expect(call).rejects.toThrow('native macOS helper app connection closed')
|
||||
await vi.waitFor(() => expect(sockets).toHaveLength(1))
|
||||
await vi.waitFor(() => expect(sockets[0]!.writes).toHaveLength(1))
|
||||
|
||||
const provider = providers[0]!
|
||||
provider.exitCode = 0
|
||||
sockets[0]!.emit('close')
|
||||
await rejection
|
||||
|
||||
// Why: signalling a reaped pid is how a recycled pid gets hit.
|
||||
expect(provider.kill).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('reaps the helper process of a superseded startup', async () => {
|
||||
const pendingConnects: {
|
||||
resolve: (socket: FakeSocket) => void
|
||||
}[] = []
|
||||
connectMacOSProviderSocketMock.mockImplementation(
|
||||
async () =>
|
||||
await new Promise<FakeSocket>((resolve) => {
|
||||
pendingConnects.push({ resolve })
|
||||
})
|
||||
)
|
||||
const { MacOSNativeProviderClient } = await loadClientModule()
|
||||
const client = new MacOSNativeProviderClient()
|
||||
|
||||
const firstCall = client.capabilities()
|
||||
await vi.waitFor(() => expect(pendingConnects).toHaveLength(1))
|
||||
|
||||
client.shutdown()
|
||||
|
||||
const secondCall = client.capabilities()
|
||||
await vi.waitFor(() => expect(pendingConnects).toHaveLength(2))
|
||||
const secondSocket = new FakeSocket()
|
||||
pendingConnects[1]!.resolve(secondSocket)
|
||||
await vi.waitFor(() => expect(secondSocket.writes).toHaveLength(1))
|
||||
const secondRequest = JSON.parse(secondSocket.writes[0]!) as { id: number }
|
||||
|
||||
pendingConnects[0]!.resolve(new FakeSocket())
|
||||
await expect(firstCall).rejects.toThrow('native macOS provider startup was superseded')
|
||||
|
||||
// Why: the superseded throw is caught by this function's own catch, so the
|
||||
// helper must be reaped exactly once, not once per handler.
|
||||
expect(providers[0]!.kill).toHaveBeenCalledTimes(1)
|
||||
expect(providers[0]!.kill).toHaveBeenCalledWith('SIGTERM')
|
||||
expect(providers[1]!.kill).not.toHaveBeenCalled()
|
||||
|
||||
secondSocket.emit(
|
||||
'data',
|
||||
`${JSON.stringify({
|
||||
id: secondRequest.id,
|
||||
ok: true,
|
||||
result: { protocolVersion: 1, supports: {} }
|
||||
})}\n`
|
||||
)
|
||||
await expect(secondCall).resolves.toMatchObject({ protocolVersion: 1 })
|
||||
})
|
||||
})
|
||||
|
||||
function macOSProviderCapabilities(actions: Partial<Record<string, boolean>> = {}) {
|
||||
|
||||
@@ -18,6 +18,7 @@ import {
|
||||
writeNativeProviderLine
|
||||
} from './macos-native-provider-contract'
|
||||
import { resolveMacOSComputerUseExecutablePath } from './macos-native-provider-paths'
|
||||
import { MacOSProviderProcessOwner } from './macos-native-provider-process-reaping'
|
||||
import {
|
||||
attachMacOSNativeProviderSocketListeners,
|
||||
NativeProviderLineBuffer,
|
||||
@@ -31,6 +32,7 @@ const REQUEST_TIMEOUT_MS = 60_000
|
||||
|
||||
export class MacOSNativeProviderClient {
|
||||
private socket: net.Socket | null = null
|
||||
private readonly providerProcess = new MacOSProviderProcessOwner()
|
||||
private socketStartPromise: Promise<net.Socket> | null = null
|
||||
private socketPath: string | null = null
|
||||
private socketDirectory: string | null = null
|
||||
@@ -84,7 +86,7 @@ export class MacOSNativeProviderClient {
|
||||
)
|
||||
this.pending.delete(id)
|
||||
}
|
||||
this.cleanupSocketDirectory()
|
||||
this.releaseHelperGeneration()
|
||||
}
|
||||
private async call(method: NativeMethod, params: unknown): Promise<unknown> {
|
||||
if (method !== 'handshake') {
|
||||
@@ -124,7 +126,7 @@ export class MacOSNativeProviderClient {
|
||||
clearTimeout(pending.timer)
|
||||
this.pending.delete(id)
|
||||
}
|
||||
this.invalidateActiveSocketAfterWriteFailure(transport, wrapped)
|
||||
this.invalidateActiveSocket(transport, wrapped)
|
||||
throw wrapped
|
||||
}
|
||||
return await result
|
||||
@@ -192,7 +194,8 @@ export class MacOSNativeProviderClient {
|
||||
helperExecutablePath,
|
||||
isCurrent: (socketPath) =>
|
||||
this.socketStartGeneration === startGeneration &&
|
||||
(this.socketPath === null || this.socketPath === socketPath)
|
||||
(this.socketPath === null || this.socketPath === socketPath),
|
||||
providerProcess: this.providerProcess
|
||||
})
|
||||
this.socketDirectory = started.socketDirectory
|
||||
this.socketPath = started.socketPath
|
||||
@@ -243,43 +246,37 @@ export class MacOSNativeProviderClient {
|
||||
this.cleanupActiveSocketListeners()
|
||||
this.socket = null
|
||||
this.socketBuffer.clear()
|
||||
this.cleanupSocketDirectory()
|
||||
this.releaseHelperGeneration()
|
||||
this.rejectPending(
|
||||
new RuntimeClientError('accessibility_error', 'native macOS helper app connection closed')
|
||||
)
|
||||
}
|
||||
private handleTransportError(socket: net.Socket, error: Error): void {
|
||||
// Why: stale socket errors can arrive after shutdown/restart.
|
||||
if (this.socket !== socket) {
|
||||
return
|
||||
}
|
||||
this.cleanupActiveSocketListeners()
|
||||
// Why: an active transport error makes the helper socket unreliable for the next request.
|
||||
this.socket = null
|
||||
this.socketBuffer.clear()
|
||||
if (!socket.destroyed) {
|
||||
socket.destroy()
|
||||
}
|
||||
this.cleanupSocketDirectory()
|
||||
this.rejectPending(new RuntimeClientError('accessibility_error', error.message))
|
||||
this.invalidateActiveSocket(
|
||||
socket,
|
||||
new RuntimeClientError('accessibility_error', error.message)
|
||||
)
|
||||
}
|
||||
private invalidateActiveSocketAfterWriteFailure(
|
||||
socket: net.Socket,
|
||||
error: RuntimeClientError
|
||||
): void {
|
||||
private invalidateActiveSocket(socket: net.Socket, error: RuntimeClientError): void {
|
||||
// Why: stale socket errors and late write failures can arrive after
|
||||
// shutdown/restart; only the active socket may tear down this generation.
|
||||
if (this.socket !== socket) {
|
||||
return
|
||||
}
|
||||
this.cleanupActiveSocketListeners()
|
||||
// Why: a failed transport makes the helper socket unreliable for the next request.
|
||||
this.socket = null
|
||||
this.socketBuffer.clear()
|
||||
if (!socket.destroyed) {
|
||||
socket.destroy()
|
||||
}
|
||||
this.cleanupSocketDirectory()
|
||||
this.releaseHelperGeneration()
|
||||
this.rejectPending(error)
|
||||
}
|
||||
private cleanupSocketDirectory(): void {
|
||||
private releaseHelperGeneration(): void {
|
||||
// Why: `terminate` only lands if the helper is still reading its socket, and
|
||||
// the wedged helpers this reaps are exactly the ones that are not.
|
||||
this.providerProcess.reap()
|
||||
if (!this.socketDirectory) {
|
||||
return
|
||||
}
|
||||
|
||||
@@ -0,0 +1,48 @@
|
||||
import { ChildProcess } from 'node:child_process'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
PROVIDER_SIGKILL_GRACE_MS,
|
||||
reapMacOSProviderProcess
|
||||
} from './macos-native-provider-process-reaping'
|
||||
|
||||
describe('macOS provider reaping resource bounds', () => {
|
||||
const providers: ChildProcess[] = []
|
||||
|
||||
afterEach(() => {
|
||||
for (const provider of providers.splice(0)) {
|
||||
provider.emit('exit', 0, null)
|
||||
}
|
||||
vi.useRealTimers()
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
|
||||
it.each(['exit', 'escalation'] as const)(
|
||||
'shares one exit hook across 200 helpers and releases it on %s',
|
||||
(mode) => {
|
||||
vi.useFakeTimers()
|
||||
const baseline = process.listenerCount('exit')
|
||||
for (let index = 0; index < 200; index++) {
|
||||
const provider = new ChildProcess()
|
||||
vi.spyOn(provider, 'kill').mockReturnValue(true)
|
||||
providers.push(provider)
|
||||
reapMacOSProviderProcess(provider)
|
||||
reapMacOSProviderProcess(provider)
|
||||
expect(provider.kill).toHaveBeenCalledTimes(1)
|
||||
}
|
||||
expect(process.listenerCount('exit')).toBe(baseline + 1)
|
||||
if (mode === 'exit') {
|
||||
for (const provider of providers) {
|
||||
provider.emit('exit', 0, null)
|
||||
}
|
||||
}
|
||||
vi.advanceTimersByTime(PROVIDER_SIGKILL_GRACE_MS)
|
||||
|
||||
for (const provider of providers) {
|
||||
expect(provider.kill).toHaveBeenCalledTimes(mode === 'exit' ? 1 : 2)
|
||||
expect(provider.listenerCount('exit')).toBe(0)
|
||||
}
|
||||
expect(vi.getTimerCount()).toBe(0)
|
||||
expect(process.listenerCount('exit')).toBe(baseline)
|
||||
}
|
||||
)
|
||||
})
|
||||
@@ -0,0 +1,72 @@
|
||||
import type { ChildProcessHandle as ChildProcess } from '../../shared/child-process/run-process'
|
||||
|
||||
export const PROVIDER_SIGKILL_GRACE_MS = 2_000
|
||||
|
||||
const reaped = new WeakSet<ChildProcess>()
|
||||
const pendingReaps = new Set<() => void>()
|
||||
|
||||
function forcePendingReaps(): void {
|
||||
for (const forceReap of pendingReaps) {
|
||||
forceReap()
|
||||
}
|
||||
}
|
||||
|
||||
// Why: signal the child handle, not a raw pid. Node no-ops once the child has
|
||||
// exited, so a recycled pid can never be signalled.
|
||||
export function reapMacOSProviderProcess(provider: ChildProcess): void {
|
||||
if (reaped.has(provider) || hasProviderExited(provider)) {
|
||||
return
|
||||
}
|
||||
reaped.add(provider)
|
||||
const cleanup = (): void => {
|
||||
clearTimeout(escalation)
|
||||
provider.off('exit', cleanup)
|
||||
pendingReaps.delete(forceReap)
|
||||
if (pendingReaps.size === 0) {
|
||||
process.off('exit', forcePendingReaps)
|
||||
}
|
||||
}
|
||||
const forceReap = (): void => {
|
||||
try {
|
||||
if (!hasProviderExited(provider)) {
|
||||
provider.kill('SIGKILL')
|
||||
}
|
||||
} finally {
|
||||
cleanup()
|
||||
}
|
||||
}
|
||||
const escalation = setTimeout(forceReap, PROVIDER_SIGKILL_GRACE_MS)
|
||||
escalation.unref()
|
||||
if (pendingReaps.size === 0) {
|
||||
// Sidecar shutdown calls process.exit(), so timer escalation alone can strand a helper.
|
||||
process.once('exit', forcePendingReaps)
|
||||
}
|
||||
pendingReaps.add(forceReap)
|
||||
provider.once('exit', cleanup)
|
||||
provider.kill('SIGTERM')
|
||||
}
|
||||
|
||||
export class MacOSProviderProcessOwner {
|
||||
private provider: ChildProcess | null = null
|
||||
|
||||
// Why: adopting a new generation must never strand the previous one, whatever
|
||||
// teardown did or did not run first.
|
||||
adopt(provider: ChildProcess): void {
|
||||
this.reap()
|
||||
this.provider = provider
|
||||
}
|
||||
|
||||
reap(): void {
|
||||
const provider = this.provider
|
||||
this.provider = null
|
||||
if (provider) {
|
||||
reapMacOSProviderProcess(provider)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Why: typeof, not `!== null` — test doubles leave these undefined, which
|
||||
// `!== null` would read as "already exited" and silently skip the reap.
|
||||
function hasProviderExited(provider: ChildProcess): boolean {
|
||||
return typeof provider.exitCode === 'number' || typeof provider.signalCode === 'string'
|
||||
}
|
||||
@@ -0,0 +1,51 @@
|
||||
import { once } from 'node:events'
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import { spawnProcess, type ChildProcessHandle } from '../../shared/child-process/run-process'
|
||||
import { reapMacOSProviderProcess } from './macos-native-provider-process-reaping'
|
||||
|
||||
describe.skipIf(process.platform === 'win32')('real macOS provider process reaping', () => {
|
||||
const children: ChildProcessHandle[] = []
|
||||
|
||||
afterEach(async () => {
|
||||
await Promise.all(
|
||||
children.splice(0).map(async (child) => {
|
||||
if (child.exitCode !== null || child.signalCode !== null) {
|
||||
return
|
||||
}
|
||||
const exit = once(child, 'exit')
|
||||
child.kill('SIGKILL')
|
||||
await exit
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
it.each(['healthy', 'ignores SIGTERM', 'stopped'])('reaps a %s detached child', async (mode) => {
|
||||
const child = spawnProcess({
|
||||
program: process.execPath,
|
||||
args: [
|
||||
'-e',
|
||||
`
|
||||
${mode === 'ignores SIGTERM' ? "process.on('SIGTERM', () => {});" : ''}
|
||||
setInterval(() => {}, 1000);
|
||||
process.stdout.write('ready');
|
||||
`
|
||||
],
|
||||
detached: true
|
||||
})
|
||||
children.push(child)
|
||||
const exited = once(child, 'exit')
|
||||
await once(child.stdout, 'data')
|
||||
if (mode === 'stopped') {
|
||||
child.kill('SIGSTOP')
|
||||
}
|
||||
const exitListeners = process.listenerCount('exit')
|
||||
|
||||
reapMacOSProviderProcess(child)
|
||||
reapMacOSProviderProcess(child)
|
||||
|
||||
const [code, signal] = await exited
|
||||
expect(code).toBeNull()
|
||||
expect(signal).toBe(mode === 'healthy' ? 'SIGTERM' : 'SIGKILL')
|
||||
expect(process.listenerCount('exit')).toBe(exitListeners)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,87 @@
|
||||
import { EventEmitter } from 'node:events'
|
||||
import { existsSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { dirname, join } from 'node:path'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { MacOSProviderProcessOwner } from './macos-native-provider-process-reaping'
|
||||
import { startMacOSNativeProviderSocket } from './macos-native-provider-transport'
|
||||
|
||||
const { connectMock, spawnMock } = vi.hoisted(() => ({
|
||||
connectMock: vi.fn(),
|
||||
spawnMock: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('node:child_process', () => ({ spawn: spawnMock }))
|
||||
vi.mock('./macos-native-provider-socket', () => ({
|
||||
connectMacOSProviderSocket: connectMock
|
||||
}))
|
||||
|
||||
class Provider extends EventEmitter {
|
||||
exitCode: number | null = null
|
||||
signalCode: string | null = null
|
||||
kill = vi.fn()
|
||||
unref(): void {}
|
||||
}
|
||||
|
||||
describe('superseded macOS provider startup cleanup', () => {
|
||||
const directories: string[] = []
|
||||
|
||||
afterEach(() => {
|
||||
for (const result of spawnMock.mock.results) {
|
||||
if (result.value instanceof Provider) {
|
||||
result.value.emit('exit', 0, null)
|
||||
}
|
||||
}
|
||||
vi.useRealTimers()
|
||||
vi.resetAllMocks()
|
||||
for (const directory of directories.splice(0)) {
|
||||
rmSync(directory, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
it.each(['socket rejection', 'provider exit'])(
|
||||
'removes only its own directory on %s',
|
||||
async (failure) => {
|
||||
vi.useFakeTimers()
|
||||
const provider = new Provider()
|
||||
spawnMock.mockReturnValue(provider)
|
||||
let rejectConnection = (_error: Error): void => {}
|
||||
connectMock.mockImplementation(
|
||||
(socketPath: string, _timeout: number, signal: AbortSignal) => {
|
||||
directories.push(dirname(socketPath))
|
||||
return new Promise((_resolve, reject) => {
|
||||
rejectConnection = reject
|
||||
signal.addEventListener('abort', () => reject(new Error('cancelled')), { once: true })
|
||||
})
|
||||
}
|
||||
)
|
||||
let current = true
|
||||
const owner = new MacOSProviderProcessOwner()
|
||||
const startup = startMacOSNativeProviderSocket({
|
||||
helperExecutablePath: 'fixture-provider',
|
||||
isCurrent: () => current,
|
||||
providerProcess: owner
|
||||
})
|
||||
const rejection = expect(startup).rejects.toThrow()
|
||||
const ownDirectory = directories[0]!
|
||||
expect(existsSync(join(ownDirectory, 'provider.token'))).toBe(true)
|
||||
const replacementDirectory = mkdtempSync(join(tmpdir(), 'orca-computer-use-replacement-'))
|
||||
directories.push(replacementDirectory)
|
||||
const replacementToken = join(replacementDirectory, 'provider.token')
|
||||
writeFileSync(replacementToken, 'replacement-token')
|
||||
|
||||
current = false
|
||||
owner.reap()
|
||||
if (failure === 'provider exit') {
|
||||
provider.exitCode = 0
|
||||
provider.emit('exit', 0, null)
|
||||
} else {
|
||||
rejectConnection(new Error('socket did not open'))
|
||||
}
|
||||
|
||||
await rejection
|
||||
expect(existsSync(ownDirectory)).toBe(false)
|
||||
expect(existsSync(replacementToken)).toBe(true)
|
||||
}
|
||||
)
|
||||
})
|
||||
@@ -5,6 +5,10 @@ import { release, tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { randomUUID } from 'node:crypto'
|
||||
import { connectMacOSProviderSocket } from './macos-native-provider-socket'
|
||||
import {
|
||||
reapMacOSProviderProcess,
|
||||
type MacOSProviderProcessOwner
|
||||
} from './macos-native-provider-process-reaping'
|
||||
import { RuntimeClientError } from './runtime-client-error'
|
||||
|
||||
const HELPER_CONNECT_TIMEOUT_MS = 10_000
|
||||
@@ -86,10 +90,12 @@ export function consumeNativeProviderLines(
|
||||
|
||||
export async function startMacOSNativeProviderSocket({
|
||||
helperExecutablePath,
|
||||
isCurrent
|
||||
isCurrent,
|
||||
providerProcess
|
||||
}: {
|
||||
helperExecutablePath: string
|
||||
isCurrent: (socketPath: string) => boolean
|
||||
providerProcess: MacOSProviderProcessOwner
|
||||
}): Promise<StartedMacOSProviderSocket> {
|
||||
const socketDirectory = mkdtempSync(join(tmpdir(), 'orca-computer-use-'))
|
||||
chmodSync(socketDirectory, 0o700)
|
||||
@@ -100,6 +106,9 @@ export async function startMacOSNativeProviderSocket({
|
||||
// Why: launching the nested helper via LaunchServices can make TCC evaluate
|
||||
// Orca.app as responsible; the signed helper executable owns this grant.
|
||||
const provider = spawnProvider(helperExecutablePath, socketPath, socketTokenPath)
|
||||
// Why: own the helper from birth. Adopting only after connect leaves a window
|
||||
// where a quit during startup strands it with nobody holding the handle.
|
||||
providerProcess.adopt(provider)
|
||||
const providerFailure = waitForProviderLaunchFailure(provider)
|
||||
const connectAbort = new AbortController()
|
||||
try {
|
||||
@@ -111,7 +120,6 @@ export async function startMacOSNativeProviderSocket({
|
||||
rmSync(socketTokenPath, { force: true })
|
||||
if (!isCurrent(socketPath)) {
|
||||
socket.destroy()
|
||||
cleanupSocketDirectory(socketDirectory)
|
||||
throw new RuntimeClientError(
|
||||
'accessibility_error',
|
||||
'native macOS provider startup was superseded'
|
||||
@@ -121,12 +129,11 @@ export async function startMacOSNativeProviderSocket({
|
||||
} 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')
|
||||
if (isCurrent(socketPath)) {
|
||||
cleanupSocketDirectory(socketDirectory)
|
||||
}
|
||||
// Why: connect failures and superseded startups both happen after spawn;
|
||||
// escalate so a helper that ignores SIGTERM cannot outlive the attempt.
|
||||
reapMacOSProviderProcess(provider)
|
||||
// Each attempt owns a unique directory, even after its generation is superseded.
|
||||
cleanupSocketDirectory(socketDirectory)
|
||||
throw error
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,121 @@
|
||||
import { once } from 'node:events'
|
||||
import { mkdtemp, readFile, rm } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { build } from 'esbuild'
|
||||
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'
|
||||
import { runProcess, spawnProcess } from '../../shared/child-process/run-process'
|
||||
|
||||
describe.skipIf(process.platform === 'win32')('real sidecar exit reaping', () => {
|
||||
let directory = ''
|
||||
let entry = ''
|
||||
|
||||
beforeAll(async () => {
|
||||
directory = await mkdtemp(join(tmpdir(), 'orca-sidecar-reaping-'))
|
||||
entry = join(directory, 'sidecar.cjs')
|
||||
await build({
|
||||
entryPoints: [join(__dirname, 'sidecar-entry.ts')],
|
||||
outfile: entry,
|
||||
bundle: true,
|
||||
platform: 'node',
|
||||
format: 'cjs',
|
||||
logLevel: 'silent',
|
||||
plugins: [
|
||||
{
|
||||
name: 'fault-injected-provider',
|
||||
setup(builder) {
|
||||
builder.onLoad({ filter: /computer-provider-lifecycle\.ts$/ }, () => ({
|
||||
resolveDir: __dirname,
|
||||
loader: 'ts',
|
||||
contents: `
|
||||
import { once } from 'node:events';
|
||||
import { writeFileSync } from 'node:fs';
|
||||
import { spawnProcess } from '../../shared/child-process/run-process';
|
||||
import { reapMacOSProviderProcess } from './macos-native-provider-process-reaping';
|
||||
let child;
|
||||
export function currentComputerProvider() {
|
||||
return { capabilities: async () => {
|
||||
child = spawnProcess({
|
||||
program: process.execPath,
|
||||
args: ['-e', "process.on('SIGTERM', () => {}); process.on('SIGHUP', () => {}); setInterval(() => {}, 1000); process.stdout.write('ready');"],
|
||||
detached: true,
|
||||
stdio: ['ignore', 'pipe', 'ignore']
|
||||
});
|
||||
writeFileSync(process.env.ORCA_TEST_PROVIDER_PID_FILE, String(child.pid));
|
||||
child.unref();
|
||||
await once(child.stdout, 'data');
|
||||
child.stdout.destroy();
|
||||
child.kill('SIGSTOP');
|
||||
return { ready: true };
|
||||
}};
|
||||
}
|
||||
export function shutdownComputerProviders() {
|
||||
if (child) reapMacOSProviderProcess(child);
|
||||
}
|
||||
`
|
||||
}))
|
||||
}
|
||||
}
|
||||
]
|
||||
})
|
||||
})
|
||||
|
||||
afterAll(async () => {
|
||||
if (directory) {
|
||||
await rm(directory, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
async function isRunning(pid: number): Promise<boolean> {
|
||||
const result = await runProcess({
|
||||
program: '/bin/ps',
|
||||
args: ['-o', 'stat=', '-p', String(pid)],
|
||||
timeoutMs: 5_000
|
||||
})
|
||||
// Linux containers may retain exited grandchildren as zombies until PID 1 reaps them.
|
||||
return result.code === 0 && !result.stdout.trim().startsWith('Z')
|
||||
}
|
||||
|
||||
it.each(['SIGTERM', 'SIGINT', 'disconnect'] as const)(
|
||||
'does not leave a stopped, SIGTERM-resistant helper after %s',
|
||||
async (mode) => {
|
||||
const pidFile = join(directory, `${mode}.pid`)
|
||||
const sidecar = spawnProcess({
|
||||
program: process.execPath,
|
||||
args: [entry],
|
||||
env: { ...process.env, ORCA_BACKGROUND_LAUNCH: '1', ORCA_TEST_PROVIDER_PID_FILE: pidFile },
|
||||
stdio: ['ignore', 'pipe', 'pipe', 'ipc']
|
||||
})
|
||||
const sidecarExit = once(sidecar, 'exit')
|
||||
try {
|
||||
const response = once(sidecar, 'message')
|
||||
sidecar.send({ id: 1, method: 'capabilities' })
|
||||
expect((await response)[0]).toMatchObject({ id: 1, ok: true, result: { ready: true } })
|
||||
const pid = Number(await readFile(pidFile, 'utf8'))
|
||||
expect(Number.isInteger(pid) && pid > 0).toBe(true)
|
||||
expect(await isRunning(pid)).toBe(true)
|
||||
|
||||
if (mode === 'disconnect') {
|
||||
sidecar.disconnect()
|
||||
} else {
|
||||
sidecar.kill(mode)
|
||||
}
|
||||
await sidecarExit
|
||||
|
||||
await vi.waitFor(async () => expect(await isRunning(pid)).toBe(false), {
|
||||
timeout: 5_000,
|
||||
interval: 100
|
||||
})
|
||||
} finally {
|
||||
if (sidecar.exitCode === null && sidecar.signalCode === null) {
|
||||
sidecar.kill('SIGKILL')
|
||||
await sidecarExit
|
||||
}
|
||||
const pid = Number(await readFile(pidFile, 'utf8').catch(() => '0'))
|
||||
if (Number.isInteger(pid) && pid > 0 && (await isRunning(pid))) {
|
||||
process.kill(pid, 'SIGKILL')
|
||||
}
|
||||
}
|
||||
}
|
||||
)
|
||||
})
|
||||
Reference in New Issue
Block a user