diff --git a/src/main/computer/sidecar-client.test.ts b/src/main/computer/sidecar-client.test.ts new file mode 100644 index 00000000000..c7041ca877e --- /dev/null +++ b/src/main/computer/sidecar-client.test.ts @@ -0,0 +1,82 @@ +import { EventEmitter } from 'events' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { callComputerSidecarCapabilities, resetComputerSidecarForTest } from './sidecar-client' + +const { forkMock } = vi.hoisted(() => ({ + forkMock: vi.fn() +})) + +vi.mock('child_process', () => ({ + fork: forkMock +})) + +type SentRequest = { + id: number + method: string + params: unknown +} + +class FakeChildProcess extends EventEmitter { + killed = false + sent: SentRequest[] = [] + + send(message: SentRequest, callback?: (error: Error | null) => void): boolean { + this.sent.push(message) + callback?.(null) + return true + } + + kill(): boolean { + this.killed = true + return true + } +} + +describe('computer sidecar client', () => { + const children: FakeChildProcess[] = [] + + beforeEach(() => { + vi.useFakeTimers() + children.length = 0 + forkMock.mockImplementation(() => { + const child = new FakeChildProcess() + children.push(child) + return child + }) + }) + + afterEach(() => { + resetComputerSidecarForTest() + forkMock.mockReset() + vi.useRealTimers() + }) + + it('ignores stale child exit and error after a replacement sidecar starts', async () => { + const firstCall = callComputerSidecarCapabilities() + const firstRejection = expect(firstCall).rejects.toThrow( + 'computer sidecar capabilities timed out' + ) + const firstChild = children[0]! + + await vi.advanceTimersByTimeAsync(60_000) + await firstRejection + expect(firstChild.killed).toBe(true) + + const secondCall = callComputerSidecarCapabilities() + const secondChild = children[1]! + const secondRequest = secondChild.sent[0]! + + // Why: OS process events from a timed-out child can arrive after restart. + // They must not clear/reject the replacement child's active request. + firstChild.emit('error', new Error('old sidecar failed late')) + firstChild.emit('exit', 1, null) + + secondChild.emit('message', { + id: secondRequest.id, + ok: true, + result: { supports: { screenshots: true } } + }) + + await expect(secondCall).resolves.toEqual({ supports: { screenshots: true } }) + }) +}) diff --git a/src/main/computer/sidecar-client.ts b/src/main/computer/sidecar-client.ts index 8981a0c6568..d0b5e99bc5e 100644 --- a/src/main/computer/sidecar-client.ts +++ b/src/main/computer/sidecar-client.ts @@ -171,8 +171,8 @@ class ComputerSidecarProcess { }) child.on('message', (message) => this.handleMessage(message)) - child.on('exit', (code, signal) => this.handleExit(code, signal)) - child.on('error', (error) => this.handleError(error)) + child.on('exit', (code, signal) => this.handleExit(child, code, signal)) + child.on('error', (error) => this.handleError(child, error)) this.child = child return child } @@ -194,7 +194,16 @@ class ComputerSidecarProcess { pending.reject(new RuntimeClientError(message.error.code, message.error.message)) } - private handleExit(code: number | null, signal: NodeJS.Signals | null): void { + private handleExit( + child: ChildProcess, + code: number | null, + signal: NodeJS.Signals | null + ): void { + // Why: a timed-out child can exit after a replacement has started; stale + // exits must not clear the live child or reject its in-flight requests. + if (this.child !== child) { + return + } this.child = null const detail = signal ? `signal ${signal}` : `code ${code ?? 'unknown'}` const error = new RuntimeClientError( @@ -208,7 +217,11 @@ class ComputerSidecarProcess { } } - private handleError(error: Error): void { + private handleError(child: ChildProcess, error: Error): void { + // Why: late errors from a prior child should not poison the current sidecar. + if (this.child !== child) { + return + } const wrapped = new RuntimeClientError('accessibility_error', error.message) for (const [id, pending] of this.pending) { clearTimeout(pending.timer)