fix: ignore stale computer sidecar exits (#3718)

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 } })
+  })
+})

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)