From cb4d67174b59708088522af4a4ff0848cc848149 Mon Sep 17 00:00:00 2001 From: cryptotavares Date: Wed, 28 Oct 2026 15:35:24 +0000 Subject: [PATCH 1/4] feat: add session-scoped IDB companion teardown --- CHANGELOG.md | 8 ++ README.md | 19 +++ SKILL.md | 2 + src/backends/idb-backend.test.ts | 175 ++++++++++++++++++------ src/backends/idb-backend.ts | 189 ++++++++++++++++++++------ src/backends/idb-connection.test.ts | 200 ++++++++++++++++++++++++++++ src/backends/idb-connection.ts | 141 ++++++++++++++++++++ src/backends/index.ts | 69 ++++++++-- src/backends/lazy-backend.test.ts | 105 ++++++++++++++- src/backends/types.ts | 3 + src/cli/device-mcp.test.ts | 68 ++++++++++ src/cli/device-mcp.ts | 36 ++++- src/tools/select-device.ts | 2 +- src/utils/stop-child-process.ts | 48 +++++++ 14 files changed, 974 insertions(+), 91 deletions(-) create mode 100644 src/backends/idb-connection.test.ts create mode 100644 src/backends/idb-connection.ts create mode 100644 src/cli/device-mcp.test.ts create mode 100644 src/utils/stop-child-process.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index f987435..673d58c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add optional backend disposal and private, session-owned IDB companions. Release owned resources on stdio close, stdin EOF, SIGINT, and SIGTERM. + +### Changed + +- `LazyDeviceBackend.selectDevice()` now returns a promise and disposes the previous backend before switching. Programmatic callers must await it. + ## [0.6.0] ### Added diff --git a/README.md b/README.md index 78c86d2..59e1580 100644 --- a/README.md +++ b/README.md @@ -63,6 +63,25 @@ The server selects a backend in this order: When multiple devices are connected and no `DEVICE_ID` is set, the server enters an "awaiting selection" state. Any tool call returns the list of available devices. Use `device_list_devices` to enumerate them and `device_select_device` to choose one. +### Connection lifecycle + +IDB backends start a private `idb_companion` child with a unique Unix socket and route their commands through `IDB_COMPANION`. They do not register, reuse, or disconnect the global UDID connection. This isolates teardown across simulators and across backend instances targeting the same simulator. + +Programmatic callers should release their backend after use: + +```typescript +const backend = await createBackend(deviceId, 'ios'); +try { + await backend.snapshot(); +} finally { + await backend.dispose?.(); +} +``` + +`dispose()` is idempotent and stops only that backend's companion and active recording, then removes its socket directory. It does not close apps or shut down simulators. Existing shared companions and other sessions remain running. Other backends can omit this optional lifecycle hook. The stdio server disposes its backend on transport close, SIGINT, and SIGTERM; an uncatchable process kill cannot run disposal. + +`await lazyBackend.selectDevice(deviceId)` releases the previous backend before switching. Device selection now returns a promise; callers must await it. Disposing an unused lazy backend does not connect to a device. + ### Device Discovery - **iOS simulators** are discovered via `xcrun simctl list devices booted --json` — no IDB needed for discovery diff --git a/SKILL.md b/SKILL.md index 1342105..d6d9763 100644 --- a/SKILL.md +++ b/SKILL.md @@ -21,6 +21,8 @@ device_select_device # Pick the one you want to use If only one device is connected, it's auto-selected. If `DEVICE_ID` is set, this step is skipped. +Each iOS backend owns a private IDB companion. Device selection releases the previous connection. Closing stdio (including stdin EOF), SIGINT, or SIGTERM releases that server's companion and recording without shutting down the simulator or disconnecting other sessions. Programmatic callers must `await backend.dispose?.()` when finished and `await lazyBackend.selectDevice(deviceId)` when switching. Use explicit device IDs for independent sessions; app state is still shared if two sessions target the same simulator. + ## Core Loop ``` diff --git a/src/backends/idb-backend.test.ts b/src/backends/idb-backend.test.ts index 93c2057..0f2be0f 100644 --- a/src/backends/idb-backend.test.ts +++ b/src/backends/idb-backend.test.ts @@ -1,3 +1,5 @@ +import { spawn } from 'node:child_process'; +import { EventEmitter } from 'node:events'; import { readFileSync } from 'node:fs'; import { readFile } from 'node:fs/promises'; import { join } from 'node:path'; @@ -9,6 +11,22 @@ import { findElement, generateLocators } from '../utils/element.js'; import * as execModule from '../utils/exec.js'; import * as platformModule from '../utils/platform.js'; +const connectionMocks = vi.hoisted(() => ({ + connect: vi.fn().mockResolvedValue(undefined), + dispose: vi.fn().mockResolvedValue(undefined), +})); +vi.mock('./idb-connection.js', () => ({ + IdbConnection: vi.fn().mockImplementation(() => ({ + ...connectionMocks, + env: { IDB_COMPANION: '/tmp/private-companion.sock' }, + })), +})); +const connectionOptions = { + env: { IDB_COMPANION: '/tmp/private-companion.sock' }, +}; + +vi.mock('node:child_process', () => ({ spawn: vi.fn(), execFile: vi.fn() })); + vi.mock('node:fs/promises', () => ({ readFile: vi.fn(), })); @@ -299,12 +317,11 @@ describe('IdbBackend simctl fallback', () => { await backend.openApp('io.metamask'); - expect(mockExecStrict).toHaveBeenCalledWith('/usr/local/bin/idb', [ - 'launch', - 'io.metamask', - '--udid', - udid, - ]); + expect(mockExecStrict).toHaveBeenCalledWith( + '/usr/local/bin/idb', + ['launch', 'io.metamask', '--udid', udid], + connectionOptions, + ); expect(mockExecStrict).toHaveBeenCalledTimes(1); }); @@ -331,12 +348,11 @@ describe('IdbBackend simctl fallback', () => { await backend.closeApp('io.metamask'); - expect(mockExecStrict).toHaveBeenCalledWith('/usr/local/bin/idb', [ - 'terminate', - 'io.metamask', - '--udid', - udid, - ]); + expect(mockExecStrict).toHaveBeenCalledWith( + '/usr/local/bin/idb', + ['terminate', 'io.metamask', '--udid', udid], + connectionOptions, + ); }); it('falls back to simctl terminate when idb fails', async () => { @@ -482,12 +498,11 @@ describe('IdbBackend.screenshot', () => { const result = await backend.screenshot('/tmp/shot.png'); - expect(mockExecStrict).toHaveBeenCalledWith('/usr/local/bin/idb', [ - 'screenshot', - '/tmp/shot.png', - '--udid', - udid, - ]); + expect(mockExecStrict).toHaveBeenCalledWith( + '/usr/local/bin/idb', + ['screenshot', '/tmp/shot.png', '--udid', udid], + connectionOptions, + ); expect(mockReadFile).toHaveBeenCalledWith('/tmp/shot.png', 'base64'); expect(result).toStrictEqual({ data: 'ZmFrZS1wbmc=', @@ -597,18 +612,22 @@ describe('IdbBackend.swipe and getWindowSize', () => { // start: (round(402/2), round(874/2)) = (201, 437); // distance: round(874 * 0.4) = 350 -> end y = 87. - expect(mockExecStrict).toHaveBeenCalledWith('/usr/local/bin/idb', [ - 'ui', - 'swipe', - '201', - '437', - '201', - '87', - '--duration', - '0.3', - '--udid', - udid, - ]); + expect(mockExecStrict).toHaveBeenCalledWith( + '/usr/local/bin/idb', + [ + 'ui', + 'swipe', + '201', + '437', + '201', + '87', + '--duration', + '0.3', + '--udid', + udid, + ], + connectionOptions, + ); }); it('keeps a computed default endpoint inside the viewport', async () => { @@ -625,6 +644,7 @@ describe('IdbBackend.swipe and getWindowSize', () => { expect(mockExecStrict).toHaveBeenCalledWith( '/usr/local/bin/idb', expect.arrayContaining(['201', '100', '201', '1']), + connectionOptions, ); }); @@ -642,6 +662,7 @@ describe('IdbBackend.swipe and getWindowSize', () => { expect(mockExecStrict).toHaveBeenCalledWith( '/usr/local/bin/idb', expect.arrayContaining(['200', '400', '200', '-100']), + connectionOptions, ); }); @@ -659,6 +680,7 @@ describe('IdbBackend.swipe and getWindowSize', () => { expect(mockExecStrict).toHaveBeenCalledWith( '/usr/local/bin/idb', expect.arrayContaining(['201', '437', '40', '437']), + connectionOptions, ); }); @@ -738,6 +760,7 @@ describe('IdbBackend.tapElement and longPress viewport handling', () => { expect(mockExecStrict).toHaveBeenCalledWith( '/usr/local/bin/idb', expect.arrayContaining(['ui', 'tap', '60', '42']), + connectionOptions, ); }); @@ -775,6 +798,7 @@ describe('IdbBackend.tapElement and longPress viewport handling', () => { expect(mockExecStrict).not.toHaveBeenCalledWith( '/usr/local/bin/idb', expect.arrayContaining(['ui', 'tap']), + connectionOptions, ); }); @@ -808,10 +832,7 @@ describe('IdbBackend.ensureConnected', () => { it('connects and enables the simulator accessibility bridge', async () => { await backend.ensureConnected(); - expect(mockExec).toHaveBeenCalledWith('/usr/local/bin/idb', [ - 'connect', - udid, - ]); + expect(connectionMocks.connect).toHaveBeenCalledWith('/usr/local/bin/idb'); expect(mockExec).toHaveBeenCalledWith('xcrun', [ 'simctl', 'spawn', @@ -845,10 +866,79 @@ describe('IdbBackend.ensureConnected', () => { }); expect(await backend.ensureConnected()).toBeUndefined(); - expect(mockExec).toHaveBeenCalledWith('/usr/local/bin/idb', [ - 'connect', - udid, - ]); + expect(connectionMocks.connect).toHaveBeenCalledWith('/usr/local/bin/idb'); + }); + + it('shares in-flight initialization and waits for it before disposal', async () => { + let ready!: () => void; + connectionMocks.connect.mockReturnValueOnce( + new Promise((resolve) => { + ready = resolve; + }), + ); + const first = backend.ensureConnected(); + const second = backend.ensureConnected(); + // Let path/capability probes finish before ending the session. + await vi.waitFor(() => + expect(connectionMocks.connect).toHaveBeenCalledTimes(1), + ); + const firstRejected = first.catch((error: unknown) => error); + const secondRejected = second.catch((error: unknown) => error); + const disposing = backend.dispose(); + expect(connectionMocks.dispose).not.toHaveBeenCalled(); + ready(); + await Promise.all([disposing, backend.dispose()]); + expect(await firstRejected).toMatchObject({ + message: 'IDB backend has been disposed', + }); + expect(await secondRejected).toMatchObject({ + message: 'IDB backend has been disposed', + }); + expect(connectionMocks.dispose).toHaveBeenCalledTimes(1); + await expect(backend.snapshot()).rejects.toThrow('disposed'); + }); + + it('releases a partially started connection after initialization fails', async () => { + connectionMocks.connect.mockRejectedValueOnce(new Error('startup failed')); + await expect(backend.ensureConnected()).rejects.toThrow('startup failed'); + expect(connectionMocks.dispose).toHaveBeenCalledTimes(1); + await backend.ensureConnected(); + expect(connectionMocks.connect).toHaveBeenCalledTimes(2); + }); + + it('waits for an owned recording to exit before disposing its companion', async () => { + const child = Object.assign(new EventEmitter(), { + pid: 123, + exitCode: null, + signalCode: null, + kill: vi.fn().mockReturnValue(true), + }); + vi.mocked(spawn).mockReturnValue( + child as unknown as ReturnType, + ); + await backend.startScreenRecording('/tmp/session-recording.mp4'); + expect(spawn).toHaveBeenCalledWith( + '/usr/local/bin/idb', + ['record-video', '/tmp/session-recording.mp4', '--udid', udid], + connectionOptions, + ); + const disposing = backend.dispose(); + await vi.waitFor(() => expect(child.kill).toHaveBeenCalledWith('SIGINT')); + expect(connectionMocks.dispose).not.toHaveBeenCalled(); + child.emit('exit', 0); + await disposing; + expect(connectionMocks.dispose).toHaveBeenCalledTimes(1); + }); + + it('retries failed disposal without allowing new device commands', async () => { + await backend.ensureConnected(); + connectionMocks.dispose.mockRejectedValueOnce(new Error('stop failed')); + await expect(backend.dispose()).rejects.toThrow( + 'IDB backend disposal failed', + ); + await expect(backend.snapshot()).rejects.toThrow('disposed'); + await backend.dispose(); + expect(connectionMocks.dispose).toHaveBeenCalledTimes(2); }); }); @@ -880,10 +970,12 @@ describe('IdbBackend.snapshot accessibility backend selection', () => { expect(mockExecStrict).toHaveBeenCalledWith( '/usr/local/bin/idb', describeArgs(), + connectionOptions, ); expect(mockExecStrict).not.toHaveBeenCalledWith( '/usr/local/bin/idb', expect.arrayContaining(['--api']), + connectionOptions, ); }); @@ -903,6 +995,7 @@ describe('IdbBackend.snapshot accessibility backend selection', () => { expect(mockExecStrict).toHaveBeenCalledWith( '/usr/local/bin/idb', describeArgs(['--api', 'axbridge']), + connectionOptions, ); }); @@ -924,10 +1017,12 @@ describe('IdbBackend.snapshot accessibility backend selection', () => { expect(mockExecStrict).toHaveBeenCalledWith( '/usr/local/bin/idb', describeArgs(['--api', 'axbridge']), + connectionOptions, ); expect(mockExecStrict).toHaveBeenCalledWith( '/usr/local/bin/idb', describeArgs(['--api', 'ax']), + connectionOptions, ); }); @@ -1005,11 +1100,13 @@ describe('IdbBackend.snapshot accessibility backend selection', () => { 1, '/usr/local/bin/idb', describeArgs(supported ? ['--nested'] : []), + connectionOptions, ); expect(mockExecStrict).toHaveBeenNthCalledWith( 2, '/usr/local/bin/idb', describeArgs(supported ? ['--nested'] : []), + connectionOptions, ); expect(mockSupportsNested).toHaveBeenCalledExactlyOnceWith( '/usr/local/bin/idb', @@ -1036,10 +1133,12 @@ describe('IdbBackend.snapshot accessibility backend selection', () => { expect(mockExecStrict).toHaveBeenCalledWith( '/usr/local/bin/idb', describeArgs(['--api', 'axbridge', '--nested']), + connectionOptions, ); expect(mockExecStrict).toHaveBeenCalledWith( '/usr/local/bin/idb', describeArgs(['--api', 'ax', '--nested']), + connectionOptions, ); }); diff --git a/src/backends/idb-backend.ts b/src/backends/idb-backend.ts index 614a1fb..a10c683 100644 --- a/src/backends/idb-backend.ts +++ b/src/backends/idb-backend.ts @@ -2,6 +2,7 @@ import { execFile as execFileCb, spawn } from 'node:child_process'; import type { ChildProcess } from 'node:child_process'; import { readFile } from 'node:fs/promises'; +import { IdbConnection } from './idb-connection.js'; import type { DeviceBackend, DeviceButton, @@ -34,6 +35,7 @@ import { idbSupportsNestedDescribe, resolveIdbPath, } from '../utils/platform.js'; +import { stopChildProcess } from '../utils/stop-child-process.js'; export class IdbBackend implements DeviceBackend { readonly kind = 'idb' as const; @@ -42,6 +44,14 @@ export class IdbBackend implements DeviceBackend { #connected = false; + #connecting: Promise | undefined; + + #disposed = false; + + #disposing: Promise | undefined; + + readonly #connection: IdbConnection; + readonly #udid: string; #idbPath = 'idb'; @@ -54,23 +64,78 @@ export class IdbBackend implements DeviceBackend { #recordingPath: string | null = null; + #stoppingRecording: Promise | undefined; + constructor(udid: string) { this.#udid = udid; + this.#connection = new IdbConnection(udid); } async ensureConnected(): Promise { + if (this.#disposed) { + throw new Error('IDB backend has been disposed'); + } if (this.#connected) { return; } - this.#idbPath = await resolveIdbPath(); - [this.#idbSupportsApi, this.#idbSupportsNested] = await Promise.all([ - idbSupportsApiSelection(this.#idbPath), - idbSupportsNestedDescribe(this.#idbPath), - ]); - await exec(this.#idbPath, ['connect', this.#udid]); - await this.#enableSimulatorAccessibility(); - this.#connected = true; + this.#connecting ??= this.#connect(); + await this.#connecting; + if (this.#disposed) { + throw new Error('IDB backend has been disposed'); + } + } + + async #connect(): Promise { + try { + this.#idbPath = await resolveIdbPath(); + [this.#idbSupportsApi, this.#idbSupportsNested] = await Promise.all([ + idbSupportsApiSelection(this.#idbPath), + idbSupportsNestedDescribe(this.#idbPath), + ]); + await this.#connection.connect(this.#idbPath); + await this.#enableSimulatorAccessibility(); + this.#connected = true; + } catch (error) { + await this.#connection.dispose(); + this.#connecting = undefined; + throw error; + } + } + + async dispose(): Promise { + this.#disposed = true; + this.#disposing ??= (async (): Promise => { + await this.#connecting?.catch(() => undefined); + const errors: unknown[] = []; + // Finalize the recording while its companion can still serve it. + if (this.#recordingProcess) { + await this.stopScreenRecording().catch((error: unknown) => + errors.push(error), + ); + } + await this.#connection + .dispose() + .catch((error: unknown) => errors.push(error)); + this.#connected = false; + if (errors.length) { + this.#disposing = undefined; + throw new AggregateError(errors, 'IDB backend disposal failed'); + } + })(); + await this.#disposing; + } + + async #execStrict(command: string, args: string[]): Promise { + return command === this.#idbPath + ? execStrict(command, args, { env: this.#connection.env }) + : execStrict(command, args); + } + + async #exec(command: string, args: string[]): ReturnType { + return command === this.#idbPath + ? exec(command, args, { env: this.#connection.env }) + : exec(command, args); } /** @@ -85,7 +150,7 @@ export class IdbBackend implements DeviceBackend { */ async #enableSimulatorAccessibility(): Promise { try { - await exec('xcrun', [ + await this.#exec('xcrun', [ 'simctl', 'spawn', this.#udid, @@ -104,7 +169,7 @@ export class IdbBackend implements DeviceBackend { async getDeviceInfo(): Promise { await this.ensureConnected(); - const raw = await execStrict(this.#idbPath, [ + const raw = await this.#execStrict(this.#idbPath, [ 'describe', '--udid', this.#udid, @@ -159,7 +224,7 @@ export class IdbBackend implements DeviceBackend { args.push('--nested'); } try { - return await execStrict(this.#idbPath, args); + return await this.#execStrict(this.#idbPath, args); } catch (error) { lastError = error; } @@ -255,7 +320,7 @@ export class IdbBackend implements DeviceBackend { async tapCoordinates(x: number, y: number): Promise { await this.ensureConnected(); - await execStrict(this.#idbPath, [ + await this.#execStrict(this.#idbPath, [ 'ui', 'tap', String(x), @@ -267,7 +332,7 @@ export class IdbBackend implements DeviceBackend { async typeText(text: string): Promise { await this.ensureConnected(); - await exec(this.#idbPath, [ + await this.#exec(this.#idbPath, [ 'ui', 'key-sequence', '--', @@ -275,8 +340,20 @@ export class IdbBackend implements DeviceBackend { '--udid', this.#udid, ]); - await exec(this.#idbPath, ['ui', 'key', 'DELETE', '--udid', this.#udid]); - await execStrict(this.#idbPath, ['ui', 'text', text, '--udid', this.#udid]); + await this.#exec(this.#idbPath, [ + 'ui', + 'key', + 'DELETE', + '--udid', + this.#udid, + ]); + await this.#execStrict(this.#idbPath, [ + 'ui', + 'text', + text, + '--udid', + this.#udid, + ]); } async swipe( @@ -322,7 +399,7 @@ export class IdbBackend implements DeviceBackend { endY = Math.min(Math.max(endY, 1), height - 2); } - await execStrict(this.#idbPath, [ + await this.#execStrict(this.#idbPath, [ 'ui', 'swipe', String(sx), @@ -358,7 +435,7 @@ export class IdbBackend implements DeviceBackend { async getAppState(bundleId: string): Promise { await this.ensureConnected(); try { - const raw = await execStrict(this.#idbPath, [ + const raw = await this.#execStrict(this.#idbPath, [ 'list-apps', '--udid', this.#udid, @@ -391,7 +468,11 @@ export class IdbBackend implements DeviceBackend { } async #getAppStateViaSimctl(bundleId: string): Promise { - const result = await exec('xcrun', ['simctl', 'listapps', this.#udid]); + const result = await this.#exec('xcrun', [ + 'simctl', + 'listapps', + this.#udid, + ]); if (result.exitCode === 0 && result.stdout.includes(bundleId)) { return { bundleId, state: 'Running' }; } @@ -419,7 +500,12 @@ export class IdbBackend implements DeviceBackend { ): Promise { await this.ensureConnected(); const path = resolveArtifactPath(outputPath, 'screenshot'); - await execStrict(this.#idbPath, ['screenshot', path, '--udid', this.#udid]); + await this.#execStrict(this.#idbPath, [ + 'screenshot', + path, + '--udid', + this.#udid, + ]); hardenArtifactFile(path); if (options?.encode === false) { return { data: undefined, format: 'png', path }; @@ -431,28 +517,38 @@ export class IdbBackend implements DeviceBackend { async openApp(bundleId: string): Promise { await this.ensureConnected(); try { - await execStrict(this.#idbPath, [ + await this.#execStrict(this.#idbPath, [ 'launch', bundleId, '--udid', this.#udid, ]); } catch { - await execStrict('xcrun', ['simctl', 'launch', this.#udid, bundleId]); + await this.#execStrict('xcrun', [ + 'simctl', + 'launch', + this.#udid, + bundleId, + ]); } } async closeApp(bundleId: string): Promise { await this.ensureConnected(); try { - await execStrict(this.#idbPath, [ + await this.#execStrict(this.#idbPath, [ 'terminate', bundleId, '--udid', this.#udid, ]); } catch { - await execStrict('xcrun', ['simctl', 'terminate', this.#udid, bundleId]); + await this.#execStrict('xcrun', [ + 'simctl', + 'terminate', + this.#udid, + bundleId, + ]); } } @@ -464,7 +560,7 @@ export class IdbBackend implements DeviceBackend { back: 'HOME', enter: 'RETURN', }; - await execStrict(this.#idbPath, [ + await this.#execStrict(this.#idbPath, [ 'ui', 'key', keyMap[button], @@ -475,7 +571,7 @@ export class IdbBackend implements DeviceBackend { async dismissKeyboard(): Promise { await this.ensureConnected(); - await execStrict(this.#idbPath, [ + await this.#execStrict(this.#idbPath, [ 'ui', 'key', 'RETURN', @@ -515,7 +611,7 @@ export class IdbBackend implements DeviceBackend { if (filter) { args.push('--predicate', `eventMessage CONTAINS "${filter}"`); } - const raw = await execStrict(this.#idbPath, args); + const raw = await this.#execStrict(this.#idbPath, args); const entries = raw .trim() .split('\n') @@ -540,7 +636,7 @@ export class IdbBackend implements DeviceBackend { } const { x: cx, y: cy } = await this.#visibleTapPoint(element); - await execStrict(this.#idbPath, [ + await this.#execStrict(this.#idbPath, [ 'ui', 'tap', String(cx), @@ -601,7 +697,7 @@ export class IdbBackend implements DeviceBackend { async getWindowSize(): Promise { await this.ensureConnected(); - const raw = await execStrict(this.#idbPath, [ + const raw = await this.#execStrict(this.#idbPath, [ 'describe', '--udid', this.#udid, @@ -652,7 +748,7 @@ export class IdbBackend implements DeviceBackend { async getClipboard(): Promise { await this.ensureConnected(); - return execStrict('xcrun', ['simctl', 'pbpaste', this.#udid]); + return this.#execStrict('xcrun', ['simctl', 'pbpaste', this.#udid]); } async setClipboard(text: string): Promise { @@ -675,29 +771,40 @@ export class IdbBackend implements DeviceBackend { } const path = resolveArtifactPath(outputPath, 'recording'); this.#recordingPath = path; - this.#recordingProcess = spawn(this.#idbPath, [ - 'record-video', - path, - '--udid', - this.#udid, - ]); + this.#recordingProcess = spawn( + this.#idbPath, + ['record-video', path, '--udid', this.#udid], + { env: this.#connection.env }, + ); this.#recordingProcess.on('error', () => { this.#recordingProcess = null; this.#recordingPath = null; }); + this.#recordingProcess.stdout?.resume(); + this.#recordingProcess.stderr?.resume(); } async stopScreenRecording(): Promise { + if (this.#stoppingRecording) { + return this.#stoppingRecording; + } if (!this.#recordingProcess || !this.#recordingPath) { throw new Error('No screen recording in progress'); } const path = this.#recordingPath; - this.#recordingProcess.kill('SIGINT'); - await new Promise((resolve) => setTimeout(resolve, 1000)); - this.#recordingProcess = null; - this.#recordingPath = null; - hardenArtifactFile(path); - return path; + const child = this.#recordingProcess; + this.#stoppingRecording = (async (): Promise => { + await stopChildProcess(child, 'SIGINT'); + this.#recordingProcess = null; + this.#recordingPath = null; + hardenArtifactFile(path); + return path; + })(); + try { + return await this.#stoppingRecording; + } finally { + this.#stoppingRecording = undefined; + } } } diff --git a/src/backends/idb-connection.test.ts b/src/backends/idb-connection.test.ts new file mode 100644 index 0000000..a3325ad --- /dev/null +++ b/src/backends/idb-connection.test.ts @@ -0,0 +1,200 @@ +import { spawn } from 'node:child_process'; +import { EventEmitter } from 'node:events'; +import { mkdtemp, rm } from 'node:fs/promises'; +import { PassThrough } from 'node:stream'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { IdbConnection } from './idb-connection.js'; +import { exec } from '../utils/exec.js'; + +vi.mock('node:child_process', () => ({ spawn: vi.fn() })); +vi.mock('node:fs/promises', () => ({ mkdtemp: vi.fn(), rm: vi.fn() })); +vi.mock('../utils/exec.js', () => ({ exec: vi.fn() })); + +function childProcess() { + const child = Object.assign(new EventEmitter(), { + pid: 123, + exitCode: null as number | null, + signalCode: null as string | null, + stdout: new PassThrough(), + stderr: new PassThrough(), + kill: vi.fn((signal: string) => { + child.signalCode = signal; + queueMicrotask(() => child.emit('exit', null, signal)); + return true; + }), + }); + return child; +} + +describe('IdbConnection', () => { + beforeEach(() => { + vi.resetAllMocks(); + vi.mocked(exec).mockResolvedValue({ stdout: '', stderr: '', exitCode: 0 }); + vi.mocked(rm).mockResolvedValue(undefined); + let counter = 0; + vi.mocked(mkdtemp).mockImplementation(async () => { + counter += 1; + return `/tmp/device-mcp-idb-${counter}`; + }); + vi.mocked(spawn).mockImplementation((_command, args) => { + const child = childProcess(); + queueMicrotask(() => + child.stdout.write(`${JSON.stringify({ grpc_path: args?.[3] })}\n`), + ); + return child as unknown as ReturnType; + }); + }); + afterEach(() => vi.useRealTimers()); + + it.each([ + ['sim-a', 'sim-b'], + ['same-sim', 'same-sim'], + ])('isolates companions for %s / %s', async (a, b) => { + const first = new IdbConnection(a); + const second = new IdbConnection(b); + await Promise.all([ + first.connect('/opt/homebrew/bin/idb'), + second.connect('/opt/homebrew/bin/idb'), + ]); + const firstChild = vi.mocked(spawn).mock.results[0].value; + const secondChild = vi.mocked(spawn).mock.results[1].value; + expect(first.env.IDB_COMPANION).not.toBe(second.env.IDB_COMPANION); + await first.dispose(); + await first.dispose(); + expect(firstChild.kill).toHaveBeenCalledExactlyOnceWith('SIGTERM'); + expect(secondChild.kill).not.toHaveBeenCalled(); + expect(exec).not.toHaveBeenCalledWith( + expect.anything(), + expect.arrayContaining(['disconnect']), + ); + expect(rm).toHaveBeenCalledExactlyOnceWith('/tmp/device-mcp-idb-1', { + recursive: true, + force: true, + }); + await second.dispose(); + }); + + it('does not start resources when disposing an unused connection', async () => { + await new IdbConnection('sim-a').dispose(); + expect(spawn).not.toHaveBeenCalled(); + expect(exec).not.toHaveBeenCalled(); + expect(rm).not.toHaveBeenCalled(); + }); + + it('cleans up a child that reports an unexpected socket', async () => { + const child = childProcess(); + vi.mocked(spawn).mockImplementation(() => { + queueMicrotask(() => + child.stdout.write('{"grpc_path":"/tmp/not-ours"}\n'), + ); + return child as unknown as ReturnType; + }); + const connection = new IdbConnection('sim-a'); + await expect(connection.connect('idb')).rejects.toThrow( + 'unexpected socket', + ); + await connection.dispose(); + expect(child.kill).toHaveBeenCalledExactlyOnceWith('SIGTERM'); + }); + + it('escalates only its own child when TERM does not stop it', async () => { + vi.useFakeTimers(); + const connection = new IdbConnection('sim-a'); + const connecting = connection.connect('idb'); + await vi.advanceTimersByTimeAsync(0); + await connecting; + const child = vi.mocked(spawn).mock.results[0].value; + vi.mocked(child.kill).mockImplementation((signal) => { + if (signal === 'SIGKILL') { + child.emit('exit', null, signal); + } + return true; + }); + const disposing = connection.dispose(); + await vi.advanceTimersByTimeAsync(5000); + await disposing; + expect(child.kill).toHaveBeenNthCalledWith(1, 'SIGTERM'); + expect(child.kill).toHaveBeenNthCalledWith(2, 'SIGKILL'); + }); + + it('fails without creating resources when no companion executable is available', async () => { + vi.mocked(exec).mockResolvedValue({ stdout: '', stderr: '', exitCode: 1 }); + await expect(new IdbConnection('sim-a').connect('idb')).rejects.toThrow( + 'idb_companion not found', + ); + expect(mkdtemp).not.toHaveBeenCalled(); + expect(spawn).not.toHaveBeenCalled(); + }); + + it.each(['spawn error', 'early exit', 'invalid JSON', 'oversized report'])( + 'releases partial startup resources after %s', + async (failure) => { + const child = childProcess(); + vi.mocked(spawn).mockImplementation(() => { + queueMicrotask(() => { + switch (failure) { + case 'spawn error': + child.exitCode = -1; + child.emit('error', new Error('spawn failed')); + break; + case 'early exit': + child.stderr.write('companion failed'); + child.exitCode = 1; + child.emit('exit', 1); + break; + case 'invalid JSON': + child.stdout.write('invalid\n'); + break; + default: + child.stdout.write('x'.repeat(65_537)); + } + }); + return child as unknown as ReturnType; + }); + const connection = new IdbConnection('sim-a'); + await expect(connection.connect('idb')).rejects.toThrow(/.+/u); + await connection.dispose(); + expect(rm).toHaveBeenCalledWith('/tmp/device-mcp-idb-1', { + recursive: true, + force: true, + }); + }, + ); + + it('times out startup and permits cleanup of the unready child', async () => { + vi.useFakeTimers(); + const child = childProcess(); + vi.mocked(spawn).mockReturnValue( + child as unknown as ReturnType, + ); + const connection = new IdbConnection('sim-a'); + const failed = connection.connect('idb').catch((error: unknown) => error); + await vi.advanceTimersByTimeAsync(10_000); + expect(await failed).toMatchObject({ + message: 'Timed out starting private IDB companion', + }); + await connection.dispose(); + expect(child.kill).toHaveBeenCalledExactlyOnceWith('SIGTERM'); + }); + + it('retains ownership for a retry when stopping the child times out', async () => { + vi.useFakeTimers(); + const connection = new IdbConnection('sim-a'); + const connecting = connection.connect('idb'); + await vi.advanceTimersByTimeAsync(0); + await connecting; + const child = vi.mocked(spawn).mock.results[0].value; + vi.mocked(child.kill).mockReturnValue(true); + const failed = connection.dispose().catch((error: unknown) => error); + await vi.advanceTimersByTimeAsync(10_000); + expect(await failed).toMatchObject({ + message: 'Timed out stopping owned child process', + }); + expect(rm).not.toHaveBeenCalled(); + const retry = connection.dispose(); + child.emit('exit', 0); + await retry; + expect(rm).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/backends/idb-connection.ts b/src/backends/idb-connection.ts new file mode 100644 index 0000000..235237d --- /dev/null +++ b/src/backends/idb-connection.ts @@ -0,0 +1,141 @@ +import { spawn } from 'node:child_process'; +import type { ChildProcess } from 'node:child_process'; +import { mkdtemp, rm } from 'node:fs/promises'; +import { dirname, join } from 'node:path'; + +import { exec } from '../utils/exec.js'; +import { stopChildProcess } from '../utils/stop-child-process.js'; + +const START_TIMEOUT_MS = 10_000; + +/** A private companion. Never registers or disconnects a shared UDID mapping. */ +export class IdbConnection { + #child: ChildProcess | undefined; + + #directory: string | undefined; + + #socket: string | undefined; + + readonly #udid: string; + + constructor(udid: string) { + this.#udid = udid; + } + + get env(): NodeJS.ProcessEnv { + if (!this.#socket) { + throw new Error('IDB companion is not connected'); + } + return { ...process.env, IDB_COMPANION: this.#socket }; + } + + async connect(idbPath: string): Promise { + const candidates = [ + join(dirname(idbPath), 'idb_companion'), + 'idb_companion', + '/opt/homebrew/bin/idb_companion', + '/usr/local/bin/idb_companion', + ]; + let executable: string | undefined; + for (const candidate of new Set(candidates)) { + const probe = await exec(candidate, ['--version']).catch(() => null); + if (probe?.exitCode === 0) { + executable = candidate; + break; + } + } + if (!executable) { + throw new Error( + 'idb_companion not found. Install it with brew install idb-companion.', + ); + } + + // macOS Unix sockets have a short path limit; the normal TMPDIR can be too long. + this.#directory = await mkdtemp('/tmp/device-mcp-idb-'); + this.#socket = join(this.#directory, 'companion.sock'); + const socket = this.#socket; + const child = spawn( + executable, + ['--udid', this.#udid, '--grpc-domain-sock', socket], + { + stdio: ['ignore', 'pipe', 'pipe'], + }, + ); + this.#child = child; + + await new Promise((resolve, reject) => { + let output = ''; + let stderr = ''; + const timer = setTimeout( + () => finish(new Error('Timed out starting private IDB companion')), + START_TIMEOUT_MS, + ); + const onError = (error: Error): void => finish(error); + const onExit = (code: number | null): void => + finish( + new Error( + `Private IDB companion exited before readiness (${code}): ${stderr}`, + ), + ); + const onStderr = (data: Buffer): void => { + stderr = (stderr + data.toString()).slice(-4096); + }; + const onData = (data: Buffer): void => { + output += data.toString(); + if (output.length > 65_536) { + finish(new Error('Invalid IDB companion readiness report')); + return; + } + const newline = output.indexOf('\n'); + if (newline < 0) { + return; + } + try { + const report = JSON.parse(output.slice(0, newline)) as Record< + string, + unknown + >; + finish( + report.grpc_path === socket + ? undefined + : new Error('IDB companion reported an unexpected socket'), + ); + } catch (error) { + finish(error as Error); + } + }; + function finish(error?: Error): void { + clearTimeout(timer); + child.off('error', onError); + child.off('exit', onExit); + child.stdout?.off('data', onData); + child.stderr?.off('data', onStderr); + if (error) { + reject(error); + } else { + resolve(); + } + } + child.once('error', onError); + child.once('exit', onExit); + child.stdout?.on('data', onData); + child.stderr?.on('data', onStderr); + }); + // Keep draining output so a chatty companion cannot block on a full pipe. + child.stdout?.resume(); + child.stderr?.resume(); + } + + async dispose(): Promise { + const child = this.#child; + if (child) { + await stopChildProcess(child); + } + this.#child = undefined; + if (this.#directory) { + await rm(this.#directory, { recursive: true, force: true }); + this.#directory = undefined; + } + this.#socket = undefined; + } +} diff --git a/src/backends/index.ts b/src/backends/index.ts index a0af1bd..402ef57 100644 --- a/src/backends/index.ts +++ b/src/backends/index.ts @@ -36,7 +36,7 @@ export { AppiumBackend } from './appium-backend.js'; export { IdbBackend } from './idb-backend.js'; export type LazyDeviceBackend = DeviceBackend & { - selectDevice(deviceId: string): void; + selectDevice(deviceId: string): Promise; listDevices(): Promise; }; @@ -74,12 +74,17 @@ export async function createBackend( explicitPlatform, ); - const backend = + const backend: DeviceBackend & { ensureConnected(): Promise } = platform === 'ios' ? new IdbBackend(deviceId) : new AdbBackend(deviceId, options.adb); - await backend.ensureConnected(); + try { + await backend.ensureConnected(); + } catch (error) { + await backend.dispose?.(); + throw error; + } return backend; } @@ -92,8 +97,15 @@ export function createLazyBackend( let connecting: Promise | null = null; let pendingDevices: DetectedDevice[] | null = null; let selectedOverride: string | null = null; + let disposed = false; + let disposing: Promise | null = null; + let selecting: Promise | null = null; async function resolve(): Promise { + await selecting; + if (disposed) { + throw new Error('Device backend has been disposed'); + } if (inner) { return inner; } @@ -118,7 +130,14 @@ export function createLazyBackend( throw error; }); } - return connecting; + const backend = await connecting; + if (disposed) { + throw new Error('Device backend has been disposed'); + } + if (selecting) { + return resolve(); + } + return backend; } function screenshot( @@ -149,10 +168,44 @@ export function createLazyBackend( return inner?.platform ?? 'ios'; }, - selectDevice(deviceId: string): void { - selectedOverride = deviceId; - inner = null; - connecting = null; + async selectDevice(deviceId: string): Promise { + if (disposed) { + throw new Error('Device backend has been disposed'); + } + const previous = selecting; + const selection = (async (): Promise => { + await previous; + const backend = inner ?? (await connecting?.catch(() => null)); + await backend?.dispose?.(); + selectedOverride = deviceId; + // resolve waits for selecting; no new connection can race these resets. + inner = null; // eslint-disable-line require-atomic-updates + connecting = null; // eslint-disable-line require-atomic-updates + })(); + selecting = selection; + try { + await selection; + } finally { + if (selecting === selection) { + selecting = null; + } + } + }, + + async dispose(): Promise { + disposed = true; + disposing ??= (async (): Promise => { + await selecting?.catch(() => undefined); + const backend = inner ?? (await connecting?.catch(() => null)); + await backend?.dispose?.(); + // disposed prevents new connections while the current one is drained. + inner = null; // eslint-disable-line require-atomic-updates + connecting = null; // eslint-disable-line require-atomic-updates + })().catch((error: unknown) => { + disposing = null; + throw error; + }); + await disposing; }, async listDevices(): Promise { diff --git a/src/backends/lazy-backend.test.ts b/src/backends/lazy-backend.test.ts index fc0252b..76eef11 100644 --- a/src/backends/lazy-backend.test.ts +++ b/src/backends/lazy-backend.test.ts @@ -22,10 +22,12 @@ vi.mock('../utils/platform.js', async () => { }); const mockEnsureConnected = vi.fn().mockResolvedValue(undefined); +const mockDispose = vi.fn().mockResolvedValue(undefined); vi.mock('./idb-backend.js', () => ({ IdbBackend: vi.fn().mockImplementation((deviceId: string) => ({ ...createMockBackendFields('ios', deviceId), ensureConnected: mockEnsureConnected, + dispose: mockDispose, })), })); @@ -110,6 +112,7 @@ function createMockBackendFields( describe('createLazyBackend', () => { beforeEach(() => { vi.clearAllMocks(); + mockDispose.mockReset().mockResolvedValue(undefined); }); async function getLazyBackend() { @@ -117,6 +120,104 @@ describe('createLazyBackend', () => { return createLazyBackend; } + describe('disposal', () => { + it('does not resolve an unused lazy backend during teardown', async () => { + const createLazyBackend = await getLazyBackend(); + const lazy = createLazyBackend(); + await lazy.dispose?.(); + expect(mockDetectPlatform).not.toHaveBeenCalled(); + expect(mockDispose).not.toHaveBeenCalled(); + await expect(lazy.snapshot()).rejects.toThrow('disposed'); + }); + + it('releases the old backend before selecting another device', async () => { + const createLazyBackend = await getLazyBackend(); + mockDetectPlatform.mockResolvedValue({ + platform: 'ios', + deviceId: 'sim-a', + }); + const lazy = createLazyBackend(); + await lazy.snapshot(); + await lazy.selectDevice('sim-b'); + expect(mockDispose).toHaveBeenCalledTimes(1); + mockDetectPlatform.mockResolvedValue({ + platform: 'ios', + deviceId: 'sim-b', + }); + await lazy.snapshot(); + expect(mockDetectPlatform).toHaveBeenLastCalledWith('sim-b', undefined); + await Promise.all([lazy.dispose?.(), lazy.dispose?.()]); + expect(mockDispose).toHaveBeenCalledTimes(2); + }); + + it('waits for an in-flight backend connection during teardown', async () => { + const createLazyBackend = await getLazyBackend(); + mockDetectPlatform.mockResolvedValue({ + platform: 'ios', + deviceId: 'sim-a', + }); + let ready!: () => void; + mockEnsureConnected.mockReturnValueOnce( + new Promise((resolve) => { + ready = resolve; + }), + ); + const lazy = createLazyBackend(); + const snapshot = lazy.snapshot(); + await vi.waitFor(() => + expect(mockEnsureConnected).toHaveBeenCalledTimes(1), + ); + const disposing = lazy.dispose?.(); + const rejected = snapshot.catch((error: unknown) => error); + expect(mockDispose).not.toHaveBeenCalled(); + ready(); + await disposing; + expect(await rejected).toMatchObject({ + message: 'Device backend has been disposed', + }); + expect(mockDispose).toHaveBeenCalledTimes(1); + }); + + it('waits for selection teardown before disposing', async () => { + const createLazyBackend = await getLazyBackend(); + mockDetectPlatform.mockResolvedValue({ + platform: 'ios', + deviceId: 'sim-a', + }); + const lazy = createLazyBackend(); + await lazy.snapshot(); + let release!: () => void; + mockDispose.mockReturnValueOnce( + new Promise((resolve) => { + release = resolve; + }), + ); + const selection = lazy.selectDevice('sim-b'); + const disposing = lazy.dispose?.(); + await vi.waitFor(() => expect(mockDispose).toHaveBeenCalledTimes(1)); + expect(mockDetectPlatform).toHaveBeenCalledTimes(1); + release(); + await selection; + await disposing; + await expect(lazy.snapshot()).rejects.toThrow('disposed'); + expect(mockDispose).toHaveBeenCalledTimes(1); + }); + + it('retains the backend when disposal fails so cleanup can retry', async () => { + const createLazyBackend = await getLazyBackend(); + mockDetectPlatform.mockResolvedValue({ + platform: 'ios', + deviceId: 'sim-a', + }); + const lazy = createLazyBackend(); + await lazy.snapshot(); + mockDispose.mockRejectedValueOnce(new Error('stop failed')); + await expect(lazy.dispose?.()).rejects.toThrow('stop failed'); + await lazy.dispose?.(); + expect(mockDispose).toHaveBeenCalledTimes(2); + }); + }); + describe('single device resolution', () => { it('defers backend creation until first tool call', async () => { const createLazyBackend = await getLazyBackend(); @@ -240,7 +341,7 @@ describe('createLazyBackend', () => { await expect(lazy.getDeviceInfo()).rejects.toThrow(MultipleDevicesError); - lazy.selectDevice('udid-1'); + await lazy.selectDevice('udid-1'); const info = await lazy.getDeviceInfo(); expect(info.deviceId).toBe('udid-1'); @@ -263,7 +364,7 @@ describe('createLazyBackend', () => { deviceId: 'emulator-5554', }); - lazy.selectDevice('emulator-5554'); + await lazy.selectDevice('emulator-5554'); await lazy.getDeviceInfo(); expect(mockDetectPlatform).toHaveBeenCalledTimes(2); diff --git a/src/backends/types.ts b/src/backends/types.ts index 6fb70ca..84ddc73 100644 --- a/src/backends/types.ts +++ b/src/backends/types.ts @@ -113,6 +113,9 @@ export type DeviceBackend = { readonly platform: Platform; + /** Release only resources owned by this backend. Idempotent; never shuts down a device. */ + dispose?(): Promise; + getDeviceInfo(): Promise; snapshot(): Promise; diff --git a/src/cli/device-mcp.test.ts b/src/cli/device-mcp.test.ts new file mode 100644 index 0000000..d00c01e --- /dev/null +++ b/src/cli/device-mcp.test.ts @@ -0,0 +1,68 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const mocks = vi.hoisted(() => ({ + dispose: vi.fn(), + connect: vi.fn(), + close: vi.fn(), +})); +vi.mock('../backends/index.js', () => ({ + createLazyBackend: () => ({ dispose: mocks.dispose }), +})); +vi.mock('../server.js', () => ({ + createMcpServer: () => ({ + server: {}, + connect: mocks.connect, + close: mocks.close, + }), +})); +vi.mock('@modelcontextprotocol/sdk/server/stdio.js', () => ({ + StdioServerTransport: vi.fn(), +})); + +describe('device-mcp CLI shutdown', () => { + const callbacks = new Map void>(); + + beforeEach(() => { + vi.resetModules(); + vi.resetAllMocks(); + callbacks.clear(); + mocks.dispose.mockResolvedValue(undefined); + mocks.connect.mockResolvedValue(undefined); + mocks.close.mockResolvedValue(undefined); + vi.spyOn(console, 'error').mockImplementation(() => undefined); + vi.spyOn(process, 'exit').mockReturnValue(undefined as never); + vi.spyOn(process, 'once').mockImplementation((event, callback) => { + callbacks.set(event, callback); + return process; + }); + vi.spyOn(process.stdin, 'once').mockImplementation((event, callback) => { + callbacks.set(event, callback); + return process.stdin; + }); + }); + afterEach(() => vi.restoreAllMocks()); + + it.each(['SIGINT', 'SIGTERM', 'end'])( + 'disposes the backend and closes stdio on %s', + async (event) => { + await import('./device-mcp.js'); + await vi.waitFor(() => expect(mocks.connect).toHaveBeenCalledTimes(1)); + callbacks.get(event)?.(); + callbacks.get(event)?.(); + await vi.waitFor(() => expect(process.exit).toHaveBeenCalledWith(0)); + expect(mocks.dispose).toHaveBeenCalledTimes(1); + expect(mocks.close).toHaveBeenCalledTimes(1); + expect(mocks.dispose.mock.invocationCallOrder[0]).toBeLessThan( + mocks.close.mock.invocationCallOrder[0], + ); + }, + ); + + it('closes the server and reports a disposal failure', async () => { + mocks.dispose.mockRejectedValue(new Error('stop failed')); + await import('./device-mcp.js'); + callbacks.get('SIGTERM')?.(); + await vi.waitFor(() => expect(process.exit).toHaveBeenCalledWith(1)); + expect(mocks.close).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/cli/device-mcp.ts b/src/cli/device-mcp.ts index cf3979f..60fbd73 100644 --- a/src/cli/device-mcp.ts +++ b/src/cli/device-mcp.ts @@ -22,7 +22,41 @@ async function main(): Promise { const backend = createLazyBackend(deviceId, devicePlatform); const server = createMcpServer(backend); const transport = new StdioServerTransport(); - await server.connect(transport); + let stopping: Promise | undefined; + const shutdown = async (): Promise => { + stopping ??= (async (): Promise => { + try { + await backend.dispose?.(); + } finally { + await server.close(); + } + })(); + return stopping; + }; + server.server.onclose = (): void => { + backend + .dispose?.() + .catch((error: unknown) => console.error('Cleanup failed:', error)); + }; + const exit = (): void => { + shutdown() + .then(() => process.exit(0)) + .catch((error: unknown) => { + console.error('Cleanup failed:', error); + process.exit(1); + }); + }; + for (const signal of ['SIGINT', 'SIGTERM'] as const) { + process.once(signal, exit); + } + // The SDK stdio transport does not turn stdin EOF into an onclose callback. + process.stdin.once('end', exit); + try { + await server.connect(transport); + } catch (error) { + await shutdown(); + throw error; + } console.error('@metamask/device-mcp ready on stdio'); } diff --git a/src/tools/select-device.ts b/src/tools/select-device.ts index 2c98539..fe4be62 100644 --- a/src/tools/select-device.ts +++ b/src/tools/select-device.ts @@ -22,7 +22,7 @@ export function registerSelectDeviceTool( }, async ({ deviceId }) => { try { - backend.selectDevice(deviceId); + await backend.selectDevice(deviceId); const info = await backend.getDeviceInfo(); return { diff --git a/src/utils/stop-child-process.ts b/src/utils/stop-child-process.ts new file mode 100644 index 0000000..018fe64 --- /dev/null +++ b/src/utils/stop-child-process.ts @@ -0,0 +1,48 @@ +import type { ChildProcess } from 'node:child_process'; + +const STOP_TIMEOUT_MS = 5_000; + +/** + * Stop an owned child and wait for exit, escalating only that child if needed. + * + * @param child - The process created by this session. + * @param signal - The initial graceful-stop signal. + */ +export async function stopChildProcess( + child: ChildProcess, + signal: NodeJS.Signals = 'SIGTERM', +): Promise { + if ( + child.exitCode !== null || + child.signalCode !== null || + child.pid === undefined + ) { + return; + } + await new Promise((resolve, reject) => { + let escalated = false; + const timer = setInterval(() => { + if (escalated) { + onError(new Error('Timed out stopping owned child process')); + return; + } + // Never look up or kill arbitrary PIDs or process groups. + child.kill('SIGKILL'); + escalated = true; + }, STOP_TIMEOUT_MS); + function onError(error: Error): void { + clearInterval(timer); + child.off('exit', onExit); + child.off('error', onError); + reject(error); + } + function onExit(): void { + clearInterval(timer); + child.off('error', onError); + resolve(); + } + child.once('exit', onExit); + child.once('error', onError); + child.kill(signal); + }); +} From 217542216964de21a73200c839c319ef5b041ffe Mon Sep 17 00:00:00 2001 From: cryptotavares Date: Wed, 28 Oct 2026 18:53:07 +0000 Subject: [PATCH 2/4] fix: identify explicit simulator IDs without idb describe detectFromExplicitId ran a plain `idb describe --udid` for non-UUID IDs. That call runs outside any session and can start and register a shared companion in /tmp/idb/state. Check simctl first; keep the idb probe for physical devices. --- src/utils/platform.test.ts | 35 +++++++++++++++++++++++++++++++++++ src/utils/platform.ts | 30 ++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/src/utils/platform.test.ts b/src/utils/platform.test.ts index 906d93b..c872eb0 100644 --- a/src/utils/platform.test.ts +++ b/src/utils/platform.test.ts @@ -531,6 +531,41 @@ describe('detectPlatform', () => { ]); }); + it('identifies a known simulator through simctl without running idb', async () => { + mockIsCommandAvailable.mockResolvedValue(true); + mockExec.mockImplementation(async (cmd) => + cmd === 'xcrun' + ? { + exitCode: 0, + stdout: JSON.stringify({ + devices: { r: [{ udid: 'odd_sim.id', state: 'Shutdown' }] }, + }), + stderr: '', + } + : { exitCode: 1, stdout: '', stderr: '' }, + ); + + expect(await detectPlatform('odd_sim.id')).toStrictEqual({ + platform: 'ios', + deviceId: 'odd_sim.id', + }); + expect(mockExec).not.toHaveBeenCalledWith('idb', expect.anything()); + }); + + it('falls back to idb when simctl output is unusable', async () => { + mockIsCommandAvailable.mockResolvedValue(true); + mockExec.mockImplementation(async (cmd) => + cmd === 'xcrun' + ? { exitCode: 0, stdout: 'not json', stderr: '' } + : { exitCode: 0, stdout: '{}', stderr: '' }, + ); + + expect(await detectPlatform('device:1')).toStrictEqual({ + platform: 'ios', + deviceId: 'device:1', + }); + }); + it('falls back to adb probe when idb fails for ambiguous device ID', async () => { mockIsCommandAvailable.mockResolvedValue(true); mockExec.mockImplementation(async (cmd) => { diff --git a/src/utils/platform.ts b/src/utils/platform.ts index b17c306..221d0ea 100644 --- a/src/utils/platform.ts +++ b/src/utils/platform.ts @@ -89,7 +89,15 @@ async function detectFromExplicitId(deviceId: string): Promise { isCommandAvailable('adb'), ]); + // Identify a simulator through simctl first: `idb describe` runs outside + // any session and can spawn and register a shared companion + // (`/tmp/idb/state`) that no session owns. + if (await isKnownIOSSimulator(deviceId)) { + return { platform: 'ios', deviceId }; + } + if (hasIdb) { + // Physical iOS devices still need idb to be identified. const result = await exec('idb', ['describe', '--udid', deviceId]); if (result.exitCode === 0) { return { platform: 'ios', deviceId }; @@ -116,6 +124,28 @@ type SimctlDevice = { isAvailable: boolean; }; +async function isKnownIOSSimulator(deviceId: string): Promise { + const result = await exec('xcrun', [ + 'simctl', + 'list', + 'devices', + '--json', + ]).catch(() => null); + if (result?.exitCode !== 0) { + return false; + } + try { + const parsed = JSON.parse(result.stdout) as { + devices: Record; + }; + return Object.values(parsed.devices).some((devices) => + devices.some((device) => device.udid === deviceId), + ); + } catch { + return false; + } +} + async function findAllBootedIOSSimulators(): Promise { const result = await exec('xcrun', [ 'simctl', From 145b0e120fe40381ad08378e35fb9daf8774af35 Mon Sep 17 00:00:00 2001 From: cryptotavares Date: Wed, 28 Oct 2026 18:53:16 +0000 Subject: [PATCH 3/4] feat: recover IDB sessions from companion loss and reap orphans - Keep a permanent exit listener on the private companion. A companion that exits after readiness marks the connection dead, and the next call starts a fresh one instead of failing on a dead socket. - Classify idb failures (dtuhidd invalidated/interrupted/did not answer, "Failed to connect to companion", StatusCode.UNAVAILABLE). Replace the private companion and send the command once more. Do not resend input commands when delivery is uncertain (UNAVAILABLE, "retry: unsafe"). If the retry fails, throw IdbUnavailableError with code IDB_HID_UNAVAILABLE or IDB_COMPANION_UNAVAILABLE and a remediation. - Add optional DeviceBackend.reconnect() (IDB implements it, the lazy backend forwards it). - Start companions with --terminate-offline true and --idle-shutdown-time 3600, so orphans exit on their own. - Record owner/companion pids in the private directory, and remove stale /tmp/device-mcp-idb-* directories once per process (dead owner and dead socket only; never shared companions). --- README.md | 17 ++ SKILL.md | 2 + src/backends/idb-backend.test.ts | 265 +++++++++++++++++++++++ src/backends/idb-backend.ts | 210 +++++++++++++++++- src/backends/idb-companion-sweep.test.ts | 232 ++++++++++++++++++++ src/backends/idb-companion-sweep.ts | 215 ++++++++++++++++++ src/backends/idb-connection.test.ts | 80 ++++++- src/backends/idb-connection.ts | 76 ++++++- src/backends/idb-errors.test.ts | 79 +++++++ src/backends/idb-errors.ts | 92 ++++++++ src/backends/index.ts | 5 + src/backends/lazy-backend.test.ts | 26 +++ src/backends/types.ts | 7 + src/index.test.ts | 6 + src/index.ts | 2 + 15 files changed, 1296 insertions(+), 18 deletions(-) create mode 100644 src/backends/idb-companion-sweep.test.ts create mode 100644 src/backends/idb-companion-sweep.ts create mode 100644 src/backends/idb-errors.test.ts create mode 100644 src/backends/idb-errors.ts diff --git a/README.md b/README.md index 59e1580..052ecb4 100644 --- a/README.md +++ b/README.md @@ -82,6 +82,23 @@ try { `await lazyBackend.selectDevice(deviceId)` releases the previous backend before switching. Device selection now returns a promise; callers must await it. Disposing an unused lazy backend does not connect to a device. +#### Recovery and lifetime + +| Condition | What the IDB backend does | +| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| The private companion exits after it started (crash, external kill, idle shutdown, simulator shutdown) | The next call starts a fresh private companion, then runs the command. | +| A command fails with a lost companion (`Failed to connect to companion`) or an invalid simulator HID connection (`dtuhidd ... invalidated / interrupted / did not answer`) | The backend replaces its private companion and sends the command **once** more. The companion discards HID events sent on an invalid connection, and an unreachable companion received nothing, so the resend cannot repeat a tap. | +| The channel fails after the request may have been sent (`StatusCode.UNAVAILABLE`), or idb prints `retry: unsafe` | Read-only commands are resent once. Input commands (`tap`, `swipe`, `text`, `key`, ...) are not resent; the error says the action may have run. | +| The retry also fails | The call rejects with `IdbUnavailableError` (exported). `err.code` is `IDB_HID_UNAVAILABLE` (HID connection still invalid; usually the simulator rebooted during the session) or `IDB_COMPANION_UNAVAILABLE` (companion cannot be reached or started). The message ends with a remediation. | + +`backend.reconnect?.()` replaces the backend's own connection on demand (iOS: its private companion). It is optional on `DeviceBackend`; the lazy backend forwards it. It never reboots or shuts down a device. + +Each private companion starts with `--terminate-offline true` (it exits when the simulator shuts down) and `--idle-shutdown-time 3600` (it exits after one hour without requests). An orphan left by a killed host process therefore exits on its own. The backend reconnects transparently when the companion is gone. + +Each private directory (`/tmp/device-mcp-idb-*`) holds an `owner.json` with the host and companion pids. Before the first companion starts, each process removes stale directories: the owner process is gone **and** the socket does not accept a connection. It stops a leftover companion only when that process's command line names the directory's socket. It never touches `/tmp/idb/state`, shared companions, or directories of a live owner. + +Resolving an explicit simulator ID uses `simctl`, not `idb describe`, so device detection does not start a shared companion. + ### Device Discovery - **iOS simulators** are discovered via `xcrun simctl list devices booted --json` — no IDB needed for discovery diff --git a/SKILL.md b/SKILL.md index d6d9763..c6e56d1 100644 --- a/SKILL.md +++ b/SKILL.md @@ -23,6 +23,8 @@ If only one device is connected, it's auto-selected. If `DEVICE_ID` is set, this Each iOS backend owns a private IDB companion. Device selection releases the previous connection. Closing stdio (including stdin EOF), SIGINT, or SIGTERM releases that server's companion and recording without shutting down the simulator or disconnecting other sessions. Programmatic callers must `await backend.dispose?.()` when finished and `await lazyBackend.selectDevice(deviceId)` when switching. Use explicit device IDs for independent sessions; app state is still shared if two sessions target the same simulator. +iOS recovers by itself from a dead private companion or an invalid simulator HID connection (for example after the simulator rebooted): it starts a fresh companion and sends the command once more. If that also fails, the tool error starts with `IDB_HID_UNAVAILABLE` or `IDB_COMPANION_UNAVAILABLE` and ends with a remediation. Do not repeat the action in a loop: check the screen with `device_snapshot`, then recreate the session (or reboot the simulator if the error says so). If the error says the action may have run, check the screen before you repeat it. + ## Core Loop ``` diff --git a/src/backends/idb-backend.test.ts b/src/backends/idb-backend.test.ts index 0f2be0f..e5d608d 100644 --- a/src/backends/idb-backend.test.ts +++ b/src/backends/idb-backend.test.ts @@ -6,6 +6,7 @@ import { join } from 'node:path'; import { describe, it, expect, vi, beforeEach } from 'vitest'; import { parseIdbHierarchy, mapIdbElement, IdbBackend } from './idb-backend.js'; +import { IdbUnavailableError } from './idb-errors.js'; import type { UIElement } from './types.js'; import { findElement, generateLocators } from '../utils/element.js'; import * as execModule from '../utils/exec.js'; @@ -15,9 +16,13 @@ const connectionMocks = vi.hoisted(() => ({ connect: vi.fn().mockResolvedValue(undefined), dispose: vi.fn().mockResolvedValue(undefined), })); +const connectionState = vi.hoisted(() => ({ alive: true })); vi.mock('./idb-connection.js', () => ({ IdbConnection: vi.fn().mockImplementation(() => ({ ...connectionMocks, + get alive(): boolean { + return connectionState.alive; + }, env: { IDB_COMPANION: '/tmp/private-companion.sock' }, })), })); @@ -1179,3 +1184,263 @@ describe('IdbBackend.snapshot accessibility backend selection', () => { ); }); }); + +describe('IdbBackend companion recovery', () => { + const udid = 'AAAA1111-BBBB-CCCC-DDDD-EEEE2222FFFF'; + const hidInvalidated = + 'Command failed: idb ui tap 10 20\nExit code: 1\nstderr: The dtuhidd connection ' + + '(com.apple.coredevice.feature.remote.hid.digitizer) has been invalidated; ' + + 'events sent on it would be discarded'; + const companionGone = + "Command failed: idb ui describe-all\nExit code: 1\nstderr: Failed to connect to companion at address DomainSocketAddress(path='/tmp/device-mcp-idb-x/companion.sock'): [Errno 61] Connection refused"; + let backend: IdbBackend; + + beforeEach(() => { + vi.clearAllMocks(); + connectionState.alive = true; + mockSupportsApi.mockResolvedValue(false); + mockSupportsNested.mockResolvedValue(false); + mockExec.mockResolvedValue({ exitCode: 0, stdout: '', stderr: '' }); + backend = new IdbBackend(udid); + }); + + const tapCalls = (): unknown[][] => + mockExecStrict.mock.calls.filter(([, args]) => args?.[1] === 'tap'); + + it('starts a fresh companion on the next call when the companion died after readiness', async () => { + mockExecStrict.mockResolvedValue(''); + await backend.tapCoordinates(1, 2); + expect(connectionMocks.connect).toHaveBeenCalledTimes(1); + + connectionState.alive = false; + connectionMocks.connect.mockImplementationOnce(async () => { + connectionState.alive = true; + }); + await backend.tapCoordinates(3, 4); + + expect(connectionMocks.dispose).toHaveBeenCalledTimes(1); + expect(connectionMocks.connect).toHaveBeenCalledTimes(2); + expect(tapCalls()).toHaveLength(2); + expect(tapCalls()[1][1]).toStrictEqual([ + 'ui', + 'tap', + '3', + '4', + '--udid', + udid, + ]); + // Capability probes are not repeated on reconnect. + expect(platformModule.resolveIdbPath).toHaveBeenCalledTimes(1); + }); + + it('reconnects once and retries once when the HID connection is invalidated', async () => { + mockExecStrict + .mockRejectedValueOnce(new Error(hidInvalidated)) + .mockResolvedValueOnce(''); + + await backend.tapCoordinates(10, 20); + + expect(connectionMocks.dispose).toHaveBeenCalledTimes(1); + expect(connectionMocks.connect).toHaveBeenCalledTimes(2); + expect(tapCalls()).toHaveLength(2); + }); + + it('throws IDB_HID_UNAVAILABLE after one failed retry, without looping', async () => { + mockExecStrict.mockRejectedValue(new Error(hidInvalidated)); + + const error = await backend.tapCoordinates(10, 20).catch((e: unknown) => e); + + expect(error).toBeInstanceOf(IdbUnavailableError); + expect(error).toMatchObject({ code: 'IDB_HID_UNAVAILABLE', udid }); + expect((error as Error).message).toContain('xcrun simctl shutdown'); + expect(tapCalls()).toHaveLength(2); + expect(connectionMocks.connect).toHaveBeenCalledTimes(2); + }); + + it('throws IDB_COMPANION_UNAVAILABLE when the companion cannot be reached after a restart', async () => { + mockExecStrict.mockRejectedValue(new Error(companionGone)); + + await expect(backend.snapshot()).rejects.toMatchObject({ + code: 'IDB_COMPANION_UNAVAILABLE', + }); + const describeCalls = mockExecStrict.mock.calls.filter( + ([, args]) => args?.[1] === 'describe-all', + ); + expect(describeCalls).toHaveLength(2); + }); + + it('does not try another accessibility API after a typed transport failure', async () => { + mockSupportsApi.mockResolvedValue(true); + mockChooseAxApi.mockReturnValue('axbridge'); + mockExecStrict.mockImplementation(async (_cmd, args) => { + if (args?.includes('--json')) { + return JSON.stringify({ os_version: '26.1' }); + } + throw new Error(companionGone); + }); + + await expect(backend.snapshot()).rejects.toBeInstanceOf( + IdbUnavailableError, + ); + const apis = mockExecStrict.mock.calls + .filter(([, args]) => args?.[1] === 'describe-all') + .map(([, args]) => args?.[args.indexOf('--api') + 1]); + expect(apis).toStrictEqual(['axbridge', 'axbridge']); + }); + + it('does not resend a mutating command when delivery is uncertain', async () => { + mockExecStrict.mockRejectedValue( + new Error('stderr: StatusCode.UNAVAILABLE: Socket closed'), + ); + + await expect(backend.tapCoordinates(1, 1)).rejects.toMatchObject({ + code: 'IDB_COMPANION_UNAVAILABLE', + message: expect.stringContaining('may have run'), + }); + expect(tapCalls()).toHaveLength(1); + // The companion was still replaced for the next command. + expect(connectionMocks.connect).toHaveBeenCalledTimes(2); + }); + + it('honours an explicit unsafe retry verdict', async () => { + mockExecStrict.mockRejectedValue( + new Error(`${hidInvalidated}\nretry: unsafe (outcome_unknown)`), + ); + + await expect(backend.tapCoordinates(1, 1)).rejects.toMatchObject({ + code: 'IDB_HID_UNAVAILABLE', + }); + expect(tapCalls()).toHaveLength(1); + }); + + it('resends a read-only command when delivery is uncertain', async () => { + mockExecStrict + .mockRejectedValueOnce(new Error('StatusCode.UNAVAILABLE')) + .mockResolvedValueOnce('[]'); + + await backend.snapshot(); + + expect(connectionMocks.connect).toHaveBeenCalledTimes(2); + }); + + it('reports a failed reconnect as IDB_COMPANION_UNAVAILABLE', async () => { + await backend.ensureConnected(); + mockExecStrict.mockRejectedValue(new Error(hidInvalidated)); + connectionMocks.connect.mockRejectedValueOnce( + new Error('Private IDB companion exited before readiness (1)'), + ); + + await expect(backend.tapCoordinates(1, 1)).rejects.toMatchObject({ + code: 'IDB_COMPANION_UNAVAILABLE', + message: expect.stringContaining('exited before readiness'), + }); + expect(tapCalls()).toHaveLength(1); + }); + + it('passes other failures through unchanged', async () => { + mockExecStrict.mockRejectedValue(new Error('No such app io.metamask')); + + await expect(backend.tapCoordinates(1, 1)).rejects.toThrow( + 'No such app io.metamask', + ); + expect(connectionMocks.connect).toHaveBeenCalledTimes(1); + }); + + it('keeps non-strict commands non-throwing and retries them once', async () => { + mockExecStrict.mockResolvedValue(''); + mockExec.mockImplementation(async (cmd, args) => { + if (cmd !== 'xcrun' && args?.[1] === 'key-sequence') { + return { exitCode: 1, stdout: '', stderr: hidInvalidated }; + } + return { exitCode: 0, stdout: '', stderr: '' }; + }); + + expect(await backend.typeText('abc')).toBeUndefined(); + const keySequence = mockExec.mock.calls.filter( + ([, args]) => args?.[1] === 'key-sequence', + ); + expect(keySequence).toHaveLength(2); + }); + + it('returns the first non-strict failure when the reconnect fails', async () => { + await backend.ensureConnected(); + mockExecStrict.mockResolvedValue(''); + mockExec.mockImplementation(async (cmd, args) => + cmd !== 'xcrun' && args?.[1] === 'key-sequence' + ? { exitCode: 1, stdout: '', stderr: hidInvalidated } + : { exitCode: 0, stdout: '', stderr: '' }, + ); + connectionMocks.connect.mockRejectedValueOnce(new Error('no companion')); + + // The failed key-sequence result is returned (not resent); the next + // command starts a fresh companion first, and the strict `ui text` succeeds. + expect(await backend.typeText('abc')).toBeUndefined(); + expect( + mockExec.mock.calls.filter(([, args]) => args?.[1] === 'key-sequence'), + ).toHaveLength(1); + expect(connectionMocks.connect).toHaveBeenCalledTimes(3); + }); + + it('returns a non-strict failure without resending when delivery is uncertain', async () => { + mockExecStrict.mockResolvedValue(''); + mockExec.mockImplementation(async (cmd, args) => + cmd !== 'xcrun' && args?.[1] === 'key' + ? { exitCode: 1, stdout: '', stderr: 'StatusCode.UNAVAILABLE' } + : { exitCode: 0, stdout: '', stderr: '' }, + ); + + await backend.typeText('abc'); + + expect( + mockExec.mock.calls.filter(([, args]) => args?.[1] === 'key'), + ).toHaveLength(1); + }); + + it('shares one reconnect between concurrent failing calls', async () => { + await backend.ensureConnected(); + let failures = 2; + mockExecStrict.mockImplementation(async () => { + if (failures > 0) { + failures -= 1; + throw new Error(hidInvalidated); + } + return ''; + }); + + await Promise.all([ + backend.tapCoordinates(1, 1), + backend.tapCoordinates(2, 2), + ]); + + expect(connectionMocks.dispose).toHaveBeenCalledTimes(1); + expect(connectionMocks.connect).toHaveBeenCalledTimes(2); + }); + + it('exposes reconnect() and refuses it after disposal', async () => { + await backend.ensureConnected(); + await backend.reconnect(); + expect(connectionMocks.connect).toHaveBeenCalledTimes(2); + await backend.dispose(); + await expect(backend.reconnect()).rejects.toThrow('disposed'); + }); + + it('does not reconnect when disposed during a reconnect', async () => { + await backend.ensureConnected(); + let release!: () => void; + connectionMocks.dispose.mockImplementationOnce( + async () => + new Promise((resolve) => { + release = resolve; + }), + ); + const reconnecting = backend.reconnect().catch((e: unknown) => e); + await vi.waitFor(() => expect(release).toBeDefined()); + const disposing = backend.dispose(); + release(); + expect(await reconnecting).toMatchObject({ + message: expect.stringContaining('disposed'), + }); + await disposing; + expect(connectionMocks.connect).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/backends/idb-backend.ts b/src/backends/idb-backend.ts index a10c683..4f3f402 100644 --- a/src/backends/idb-backend.ts +++ b/src/backends/idb-backend.ts @@ -3,6 +3,7 @@ import type { ChildProcess } from 'node:child_process'; import { readFile } from 'node:fs/promises'; import { IdbConnection } from './idb-connection.js'; +import { classifyIdbFailure, IdbUnavailableError } from './idb-errors.js'; import type { DeviceBackend, DeviceButton, @@ -25,6 +26,7 @@ import { computeSwipeEnd, } from '../utils/element.js'; import { execStrict, exec } from '../utils/exec.js'; +import type { ExecResult } from '../utils/exec.js'; import { hardenArtifactFile, resolveArtifactPath, @@ -50,6 +52,13 @@ export class IdbBackend implements DeviceBackend { #disposing: Promise | undefined; + #reconnecting: Promise | undefined; + + #probed = false; + + /** Incremented on every successful connect; detects reconnects by others. */ + #generation = 0; + readonly #connection: IdbConnection; readonly #udid: string; @@ -75,6 +84,14 @@ export class IdbBackend implements DeviceBackend { if (this.#disposed) { throw new Error('IDB backend has been disposed'); } + // Never start a second connection while a reconnect owns the companion. + await this.#reconnecting?.catch(() => undefined); + if (this.#connected && !this.#connection.alive) { + // The private companion exited after readiness (crash, simulator + // shutdown, idle shutdown, external kill). Start a fresh one. + await this.reconnect(); + return; + } if (this.#connected) { return; } @@ -86,15 +103,47 @@ export class IdbBackend implements DeviceBackend { } } + /** + * Replace this backend's private companion with a fresh one. Concurrent + * callers share one reconnect. Only the companion owned by this backend is + * stopped; shared companions and other sessions are not touched. + */ + async reconnect(): Promise { + if (this.#disposed) { + throw new Error('IDB backend has been disposed'); + } + this.#reconnecting ??= (async (): Promise => { + await this.#connecting?.catch(() => undefined); + this.#connected = false; + this.#connecting = undefined; + await this.#connection.dispose(); + if (this.#disposed) { + return; + } + this.#connecting = this.#connect(); + await this.#connecting; + })().finally(() => { + this.#reconnecting = undefined; + }); + await this.#reconnecting; + if (this.#disposed) { + throw new Error('IDB backend has been disposed'); + } + } + async #connect(): Promise { try { - this.#idbPath = await resolveIdbPath(); - [this.#idbSupportsApi, this.#idbSupportsNested] = await Promise.all([ - idbSupportsApiSelection(this.#idbPath), - idbSupportsNestedDescribe(this.#idbPath), - ]); + if (!this.#probed) { + this.#idbPath = await resolveIdbPath(); + [this.#idbSupportsApi, this.#idbSupportsNested] = await Promise.all([ + idbSupportsApiSelection(this.#idbPath), + idbSupportsNestedDescribe(this.#idbPath), + ]); + this.#probed = true; + } await this.#connection.connect(this.#idbPath); await this.#enableSimulatorAccessibility(); + this.#generation += 1; this.#connected = true; } catch (error) { await this.#connection.dispose(); @@ -106,6 +155,7 @@ export class IdbBackend implements DeviceBackend { async dispose(): Promise { this.#disposed = true; this.#disposing ??= (async (): Promise => { + await this.#reconnecting?.catch(() => undefined); await this.#connecting?.catch(() => undefined); const errors: unknown[] = []; // Finalize the recording while its companion can still serve it. @@ -127,15 +177,106 @@ export class IdbBackend implements DeviceBackend { } async #execStrict(command: string, args: string[]): Promise { - return command === this.#idbPath - ? execStrict(command, args, { env: this.#connection.env }) - : execStrict(command, args); + if (command !== this.#idbPath) { + return execStrict(command, args); + } + return this.#idb( + args, + async () => execStrict(command, args, { env: this.#connection.env }), + () => undefined, + true, + ); } - async #exec(command: string, args: string[]): ReturnType { - return command === this.#idbPath - ? exec(command, args, { env: this.#connection.env }) - : exec(command, args); + async #exec(command: string, args: string[]): Promise { + if (command !== this.#idbPath) { + return exec(command, args); + } + // Non-strict callers keep their semantics: a failure that survives the + // retry is returned, not thrown. + return this.#idb( + args, + async () => exec(command, args, { env: this.#connection.env }), + (result) => (result.exitCode === 0 ? undefined : result.stderr), + false, + ); + } + + /** + * Run one idb command through this backend's private companion and recover + * once from a lost companion or an invalidated simulator HID connection: + * classify the failure, start a fresh private companion, and send the + * command once more. HID events sent on an invalidated dtuhidd connection + * are discarded and an unreachable companion received nothing, so the + * retry cannot repeat an action. When delivery is uncertain + * (`StatusCode.UNAVAILABLE`), mutating `ui` commands are not resent. + * + * @param args - The idb arguments (used to decide whether a resend is safe). + * @param run - Sends the command once. + * @param failureOf - Failure text of a resolved run, if it failed. + * @param strict - Throw a typed error when recovery does not help. + * @returns The result of the first successful (or unrecoverable) run. + */ + async #idb( + args: string[], + run: () => Promise, + failureOf: (value: Result) => string | undefined, + strict: boolean, + ): Promise { + if (!this.#connected || !this.#connection.alive) { + // The companion died (or a reconnect failed) since this method's own + // ensureConnected(); start a fresh one before sending. + await this.ensureConnected(); + } + const generation = this.#generation; + const first = await attempt(run, failureOf); + const failure = + first.text === undefined ? undefined : classifyIdbFailure(first.text); + if (!failure) { + return first.settle(); + } + const resend = failure.notDelivered || !isMutatingIdbCommand(args); + try { + if (this.#generation === generation) { + await this.reconnect(); + } else { + // A concurrent call already replaced the companion; use the new one. + await this.ensureConnected(); + } + } catch (error) { + if (!strict) { + return first.settle(); + } + throw new IdbUnavailableError( + 'IDB_COMPANION_UNAVAILABLE', + this.#udid, + error instanceof Error ? error.message : String(error), + { cause: error, mayHaveRun: !resend }, + ); + } + if (!resend) { + if (!strict) { + return first.settle(); + } + throw new IdbUnavailableError( + failure.code, + this.#udid, + first.text ?? '', + { + cause: first.error, + mayHaveRun: true, + }, + ); + } + const second = await attempt(run, failureOf); + const again = + second.text === undefined ? undefined : classifyIdbFailure(second.text); + if (!again || !strict) { + return second.settle(); + } + throw new IdbUnavailableError(again.code, this.#udid, second.text ?? '', { + cause: second.error, + }); } /** @@ -226,6 +367,10 @@ export class IdbBackend implements DeviceBackend { try { return await this.#execStrict(this.#idbPath, args); } catch (error) { + if (error instanceof IdbUnavailableError) { + // Already reconnected and retried; another --api will not help. + throw error; + } lastError = error; } } @@ -808,6 +953,47 @@ export class IdbBackend implements DeviceBackend { } } +type Attempt = { + text: string | undefined; + error: unknown; + settle: () => Result; +}; + +async function attempt( + run: () => Promise, + failureOf: (value: Result) => string | undefined, +): Promise> { + try { + const value = await run(); + return { text: failureOf(value), error: undefined, settle: () => value }; + } catch (error) { + return { + text: error instanceof Error ? error.message : String(error), + error, + settle: (): never => { + throw error; + }, + }; + } +} + +const READ_ONLY_UI_COMMANDS = new Set([ + 'describe-all', + 'describe-point', + 'describe', + 'wait', +]); + +/** + * Whether an idb command can change the device (a resend could repeat it). + * + * @param args - The idb arguments. + * @returns True for `ui` input commands (tap, swipe, text, key, ...). + */ +function isMutatingIdbCommand(args: string[]): boolean { + return args[0] === 'ui' && !READ_ONLY_UI_COMMANDS.has(args[1] ?? ''); +} + function collectAlertTexts(elements: UIElement[]): string[] { const texts: string[] = []; for (const el of elements) { diff --git a/src/backends/idb-companion-sweep.test.ts b/src/backends/idb-companion-sweep.test.ts new file mode 100644 index 0000000..f09a367 --- /dev/null +++ b/src/backends/idb-companion-sweep.test.ts @@ -0,0 +1,232 @@ +import { spawn } from 'node:child_process'; +import type { ChildProcess } from 'node:child_process'; +import { + mkdir, + mkdtemp, + readdir, + rm, + utimes, + writeFile, +} from 'node:fs/promises'; +import { createServer } from 'node:net'; +import type { Server } from 'node:net'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { sweepStaleCompanionDirs } from './idb-companion-sweep.js'; + +const DEAD_PID = 2 ** 22 + 12_345; + +describe('sweepStaleCompanionDirs', () => { + let root: string; + const servers: Server[] = []; + const children: ChildProcess[] = []; + + beforeEach(async () => { + // A short root so the Unix socket paths stay under the macOS limit. + root = await mkdtemp(join('/tmp', 'dmcp-sweep-')); + }); + + afterEach(async () => { + await Promise.all( + servers + .splice(0) + .map( + async (server) => + new Promise((resolve) => server.close(() => resolve())), + ), + ); + for (const child of children.splice(0)) { + child.kill('SIGKILL'); + } + await rm(root, { recursive: true, force: true }); + }); + + async function makeDir( + name: string, + owner?: Record, + ageMs = 120_000, + ): Promise { + const directory = join(root, name); + await mkdir(directory); + if (owner) { + await writeFile(join(directory, 'owner.json'), JSON.stringify(owner)); + } + const time = (Date.now() - ageMs) / 1000; + await utimes(directory, time, time); + return directory; + } + + async function listen(directory: string): Promise { + const server = createServer(); + servers.push(server); + await new Promise((resolve) => + server.listen(join(directory, 'companion.sock'), resolve), + ); + } + + it('removes only directories whose owner is gone and whose socket is dead', async () => { + await makeDir('device-mcp-idb-dead', { ownerPid: DEAD_PID }); + await makeDir('device-mcp-idb-live-owner', { ownerPid: process.pid }); + const listening = await makeDir('device-mcp-idb-listening', { + ownerPid: DEAD_PID, + }); + await listen(listening); + await makeDir('device-mcp-idb-unowned-old'); + await makeDir('device-mcp-idb-unowned-new', undefined, 0); + await makeDir('idb'); + await makeDir('something-else', { ownerPid: DEAD_PID }); + + const removed = await sweepStaleCompanionDirs({ root }); + + expect(removed.sort()).toStrictEqual( + [ + join(root, 'device-mcp-idb-dead'), + join(root, 'device-mcp-idb-unowned-old'), + ].sort(), + ); + expect((await readdir(root)).sort()).toStrictEqual( + [ + 'device-mcp-idb-listening', + 'device-mcp-idb-live-owner', + 'device-mcp-idb-unowned-new', + 'idb', + 'something-else', + ].sort(), + ); + }); + + it('stops an orphaned companion only when it provably owns the directory socket', async () => { + const directory = await makeDir('device-mcp-idb-orphan', { + ownerPid: DEAD_PID, + companionPid: 4242, + }); + const terminate = vi.fn().mockResolvedValue(true); + const ownsSocket = vi.fn().mockResolvedValue(true); + + const removed = await sweepStaleCompanionDirs({ + root, + isAlive: (pid) => pid === 4242, + ownsSocket, + terminate, + socketAccepts: async () => false, + }); + + expect(ownsSocket).toHaveBeenCalledWith( + 4242, + join(directory, 'companion.sock'), + ); + expect(terminate).toHaveBeenCalledWith(4242); + expect(removed).toStrictEqual([directory]); + }); + + it('never signals a reused pid and keeps the directory while its socket answers', async () => { + await makeDir('device-mcp-idb-reused', { + ownerPid: DEAD_PID, + companionPid: 4242, + }); + const terminate = vi.fn(); + + const removed = await sweepStaleCompanionDirs({ + root, + isAlive: (pid) => pid === 4242, + ownsSocket: async () => false, + terminate, + socketAccepts: async () => true, + }); + + expect(terminate).not.toHaveBeenCalled(); + expect(removed).toStrictEqual([]); + }); + + it('keeps the directory when its companion does not stop', async () => { + await makeDir('device-mcp-idb-stuck', { + ownerPid: DEAD_PID, + companionPid: 4242, + }); + + const removed = await sweepStaleCompanionDirs({ + root, + isAlive: (pid) => pid === 4242, + ownsSocket: async () => true, + terminate: async () => false, + socketAccepts: async () => false, + }); + + expect(removed).toStrictEqual([]); + }); + + it('ignores a malformed owner record like a missing one', async () => { + const directory = await makeDir('device-mcp-idb-bad'); + await writeFile(join(directory, 'owner.json'), '{"ownerPid":"x"'); + const old = (Date.now() - 120_000) / 1000; + await utimes(directory, old, old); + expect(await sweepStaleCompanionDirs({ root })).toStrictEqual([directory]); + expect(await sweepStaleCompanionDirs({ root, now: () => 0 })).toStrictEqual( + [], + ); + }); + + it('skips non-directories and a missing root', async () => { + await writeFile(join(root, 'device-mcp-idb-file'), ''); + expect(await sweepStaleCompanionDirs({ root })).toStrictEqual([]); + expect( + await sweepStaleCompanionDirs({ root: join(root, 'missing') }), + ).toStrictEqual([]); + }); + + it('uses real pid, socket and ps probes by default', async () => { + // A real process that names the directory socket in its command line, + // standing in for an orphaned companion. + const directory = await makeDir('device-mcp-idb-real'); + const socket = join(directory, 'companion.sock'); + const child = spawn( + process.execPath, + [ + '-e', + 'setInterval(() => {}, 1000)', + 'idb_companion', + '--grpc-domain-sock', + socket, + ], + { stdio: 'ignore' }, + ); + children.push(child); + await writeFile( + join(directory, 'owner.json'), + JSON.stringify({ ownerPid: DEAD_PID, companionPid: child.pid }), + ); + const exited = new Promise((resolve) => child.once('exit', resolve)); + + const removed = await sweepStaleCompanionDirs({ root }); + + expect(removed).toStrictEqual([directory]); + await exited; + }, 10_000); + + it('does not treat an unrelated live process as the companion', async () => { + const directory = await makeDir('device-mcp-idb-unrelated', { + ownerPid: DEAD_PID, + companionPid: process.pid, + }); + // process.pid is alive but is not an idb_companion for this socket, and + // the socket is dead: the directory goes, the process is not signalled. + const removed = await sweepStaleCompanionDirs({ root }); + expect(removed).toStrictEqual([directory]); + }); +}); + +describe('sweepStaleCompanionDirsOnce', () => { + it('runs the sweep at most once per process and never rejects', async () => { + vi.resetModules(); + const { sweepStaleCompanionDirsOnce } = + await import('./idb-companion-sweep.js'); + const sweep = vi.fn().mockRejectedValue(new Error('EACCES')); + await Promise.all([ + sweepStaleCompanionDirsOnce(sweep), + sweepStaleCompanionDirsOnce(sweep), + ]); + expect(await sweepStaleCompanionDirsOnce(sweep)).toBeUndefined(); + expect(sweep).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/backends/idb-companion-sweep.ts b/src/backends/idb-companion-sweep.ts new file mode 100644 index 0000000..bfbcd5d --- /dev/null +++ b/src/backends/idb-companion-sweep.ts @@ -0,0 +1,215 @@ +import { lstat, readdir, readFile, rm } from 'node:fs/promises'; +import { createConnection } from 'node:net'; +import { join } from 'node:path'; + +import { exec } from '../utils/exec.js'; + +/** Parent directory of the private companion directories (short socket paths). */ +export const COMPANION_ROOT = '/tmp'; + +/** Prefix of every private companion directory created by `IdbConnection`. */ +export const COMPANION_DIR_PREFIX = 'device-mcp-idb-'; + +/** Socket file name inside a private companion directory. */ +export const COMPANION_SOCKET = 'companion.sock'; + +/** Ownership record inside a private companion directory. */ +export const COMPANION_OWNER_FILE = 'owner.json'; + +/** + * Directories without an ownership record (created by an older device-mcp, or + * by a process that has not written it yet) are left alone for this long. + */ +const UNOWNED_GRACE_MS = 60_000; + +const SOCKET_PROBE_TIMEOUT_MS = 1_000; + +const TERMINATE_WAIT_MS = 2_000; + +export type CompanionOwner = { + /** The process that owns the directory (the device-mcp host process). */ + ownerPid: number; + /** The private companion child, once spawned. */ + companionPid?: number; +}; + +export type SweepDependencies = { + root: string; + now: () => number; + isAlive: (pid: number) => boolean; + socketAccepts: (socket: string) => Promise; + ownsSocket: (pid: number, socket: string) => Promise; + terminate: (pid: number) => Promise; +}; + +/** + * Remove private companion directories left behind by a device-mcp process + * that died without disposal (for example SIGKILL). + * + * A directory is removed only when nobody can still use it: + * - its recorded owner process is gone (a live owner is never touched), and + * - its socket does not accept a connection. + * + * A companion child is stopped only when its owner is gone and its command + * line names this directory's socket (proof that it belongs to the + * directory). Shared companions (`/tmp/idb/...`) never match the prefix and + * are never touched. + * + * @param overrides - Test seams; defaults inspect the real system. + * @returns The directories that were removed. + */ +export async function sweepStaleCompanionDirs( + overrides: Partial = {}, +): Promise { + const deps: SweepDependencies = { ...defaultDependencies(), ...overrides }; + const entries = await readdir(deps.root).catch(() => [] as string[]); + const removed: string[] = []; + for (const entry of entries) { + if (!entry.startsWith(COMPANION_DIR_PREFIX)) { + continue; + } + const directory = join(deps.root, entry); + try { + if (await isStale(directory, deps)) { + await rm(directory, { recursive: true, force: true }); + removed.push(directory); + } + } catch { + // Another user's directory, or a race with its owner: leave it. + } + } + return removed; +} + +async function isStale( + directory: string, + deps: SweepDependencies, +): Promise { + const stats = await lstat(directory); + if ( + !stats.isDirectory() || + (typeof process.getuid === 'function' && stats.uid !== process.getuid()) + ) { + return false; + } + const owner = await readOwner(directory); + if (owner && deps.isAlive(owner.ownerPid)) { + return false; + } + if (!owner && deps.now() - stats.mtimeMs < UNOWNED_GRACE_MS) { + return false; + } + const socket = join(directory, COMPANION_SOCKET); + if (owner?.companionPid !== undefined && deps.isAlive(owner.companionPid)) { + if (!(await deps.ownsSocket(owner.companionPid, socket))) { + // The pid was reused by an unrelated process. Fall through to the + // socket probe; never signal that process. + } else if (!(await deps.terminate(owner.companionPid))) { + return false; + } + } + return !(await deps.socketAccepts(socket)); +} + +async function readOwner( + directory: string, +): Promise { + try { + const parsed = JSON.parse( + await readFile(join(directory, COMPANION_OWNER_FILE), 'utf8'), + ) as Partial; + if (typeof parsed.ownerPid !== 'number') { + return undefined; + } + return { + ownerPid: parsed.ownerPid, + companionPid: + typeof parsed.companionPid === 'number' + ? parsed.companionPid + : undefined, + }; + } catch { + return undefined; + } +} + +function isAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch (error) { + // EPERM: the process exists but belongs to someone else. + return (error as NodeJS.ErrnoException).code === 'EPERM'; + } +} + +async function socketAccepts(socket: string): Promise { + return new Promise((resolve) => { + const connection = createConnection(socket); + const timer = setTimeout(() => done(false), SOCKET_PROBE_TIMEOUT_MS); + function done(accepted: boolean): void { + clearTimeout(timer); + connection.destroy(); + resolve(accepted); + } + connection.once('connect', () => done(true)); + connection.once('error', () => done(false)); + }); +} + +async function ownsSocket(pid: number, socket: string): Promise { + const result = await exec('ps', ['-o', 'command=', '-p', String(pid)]).catch( + () => null, + ); + const command = result?.exitCode === 0 ? result.stdout : ''; + return ( + command.includes('idb_companion') && + command.includes(`--grpc-domain-sock ${socket}`) + ); +} + +async function terminate(pid: number): Promise { + try { + process.kill(pid, 'SIGTERM'); + } catch { + return !isAlive(pid); + } + const deadline = Date.now() + TERMINATE_WAIT_MS; + while (Date.now() < deadline) { + if (!isAlive(pid)) { + return true; + } + await new Promise((resolve) => setTimeout(resolve, 100)); + } + return !isAlive(pid); +} + +function defaultDependencies(): SweepDependencies { + return { + root: COMPANION_ROOT, + now: () => Date.now(), + isAlive, + socketAccepts, + ownsSocket, + terminate, + }; +} + +let sweeping: Promise | undefined; + +/** + * Run the sweep once per process, before the first private companion starts. + * Failures are ignored: a leftover directory must never block a session. + * + * @param sweep - The sweep to run (test seam). + * @returns A promise that settles when the sweep has finished. + */ +export async function sweepStaleCompanionDirsOnce( + sweep: () => Promise = sweepStaleCompanionDirs, +): Promise { + sweeping ??= sweep().then( + () => undefined, + () => undefined, + ); + return sweeping; +} diff --git a/src/backends/idb-connection.test.ts b/src/backends/idb-connection.test.ts index a3325ad..fd9a8b3 100644 --- a/src/backends/idb-connection.test.ts +++ b/src/backends/idb-connection.test.ts @@ -1,15 +1,24 @@ import { spawn } from 'node:child_process'; import { EventEmitter } from 'node:events'; -import { mkdtemp, rm } from 'node:fs/promises'; +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; import { PassThrough } from 'node:stream'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { sweepStaleCompanionDirsOnce } from './idb-companion-sweep.js'; import { IdbConnection } from './idb-connection.js'; import { exec } from '../utils/exec.js'; vi.mock('node:child_process', () => ({ spawn: vi.fn() })); -vi.mock('node:fs/promises', () => ({ mkdtemp: vi.fn(), rm: vi.fn() })); +vi.mock('node:fs/promises', () => ({ + mkdtemp: vi.fn(), + rm: vi.fn(), + writeFile: vi.fn(), +})); vi.mock('../utils/exec.js', () => ({ exec: vi.fn() })); +vi.mock('./idb-companion-sweep.js', async (importOriginal) => ({ + ...(await importOriginal()), + sweepStaleCompanionDirsOnce: vi.fn(), +})); function childProcess() { const child = Object.assign(new EventEmitter(), { @@ -32,6 +41,8 @@ describe('IdbConnection', () => { vi.resetAllMocks(); vi.mocked(exec).mockResolvedValue({ stdout: '', stderr: '', exitCode: 0 }); vi.mocked(rm).mockResolvedValue(undefined); + vi.mocked(writeFile).mockResolvedValue(undefined); + vi.mocked(sweepStaleCompanionDirsOnce).mockResolvedValue(undefined); let counter = 0; vi.mocked(mkdtemp).mockImplementation(async () => { counter += 1; @@ -75,6 +86,71 @@ describe('IdbConnection', () => { await second.dispose(); }); + it('starts the companion with lifetime flags so orphans exit on their own', async () => { + const connection = new IdbConnection('sim-a'); + await connection.connect('/opt/homebrew/bin/idb'); + const args = vi.mocked(spawn).mock.calls[0][1]; + expect(args).toStrictEqual([ + '--udid', + 'sim-a', + '--grpc-domain-sock', + '/tmp/device-mcp-idb-1/companion.sock', + '--terminate-offline', + 'true', + '--idle-shutdown-time', + '3600', + ]); + expect(sweepStaleCompanionDirsOnce).toHaveBeenCalledTimes(1); + expect( + vi.mocked(sweepStaleCompanionDirsOnce).mock.invocationCallOrder[0], + ).toBeLessThan(vi.mocked(mkdtemp).mock.invocationCallOrder[0]); + await connection.dispose(); + }); + + it('records the owner and companion pids in the private directory', async () => { + const connection = new IdbConnection('sim-a'); + await connection.connect('idb'); + expect(writeFile).toHaveBeenLastCalledWith( + '/tmp/device-mcp-idb-1/owner.json', + JSON.stringify({ ownerPid: process.pid, companionPid: 123 }), + { mode: 0o600 }, + ); + await connection.dispose(); + }); + + it('reports a companion that exits after readiness as not alive', async () => { + const connection = new IdbConnection('sim-a'); + expect(connection.alive).toBe(false); + await connection.connect('idb'); + expect(connection.alive).toBe(true); + const child = vi.mocked(spawn).mock.results[0].value; + child.exitCode = 0; + child.emit('exit', 0, null); + expect(connection.alive).toBe(false); + // The socket stays known until disposal so commands fail fast and get classified. + expect(connection.env.IDB_COMPANION).toBe( + '/tmp/device-mcp-idb-1/companion.sock', + ); + await connection.dispose(); + expect(rm).toHaveBeenCalledWith('/tmp/device-mcp-idb-1', { + recursive: true, + force: true, + }); + }); + + it('ignores a late exit from a replaced companion', async () => { + const connection = new IdbConnection('sim-a'); + await connection.connect('idb'); + const first = vi.mocked(spawn).mock.results[0].value; + await connection.dispose(); + await connection.connect('idb'); + expect(connection.alive).toBe(true); + first.emit('exit', 0, null); + expect(connection.alive).toBe(true); + await connection.dispose(); + expect(connection.alive).toBe(false); + }); + it('does not start resources when disposing an unused connection', async () => { await new IdbConnection('sim-a').dispose(); expect(spawn).not.toHaveBeenCalled(); diff --git a/src/backends/idb-connection.ts b/src/backends/idb-connection.ts index 235237d..1bbc69c 100644 --- a/src/backends/idb-connection.ts +++ b/src/backends/idb-connection.ts @@ -1,13 +1,28 @@ import { spawn } from 'node:child_process'; import type { ChildProcess } from 'node:child_process'; -import { mkdtemp, rm } from 'node:fs/promises'; +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; import { dirname, join } from 'node:path'; +import { + COMPANION_DIR_PREFIX, + COMPANION_OWNER_FILE, + COMPANION_ROOT, + COMPANION_SOCKET, + sweepStaleCompanionDirsOnce, +} from './idb-companion-sweep.js'; +import type { CompanionOwner } from './idb-companion-sweep.js'; import { exec } from '../utils/exec.js'; import { stopChildProcess } from '../utils/stop-child-process.js'; const START_TIMEOUT_MS = 10_000; +/** + * The companion exits after this many seconds without gRPC requests, so an + * orphan (owner killed with SIGKILL) does not live forever. The backend + * reconnects transparently on the next call when the companion is gone. + */ +export const COMPANION_IDLE_SHUTDOWN_SECONDS = 3600; + /** A private companion. Never registers or disconnects a shared UDID mapping. */ export class IdbConnection { #child: ChildProcess | undefined; @@ -16,12 +31,25 @@ export class IdbConnection { #socket: string | undefined; + #alive = false; + readonly #udid: string; constructor(udid: string) { this.#udid = udid; } + /** + * Whether the companion child is running. Becomes false when the child + * exits for any reason after readiness (crash, simulator shutdown with + * `--terminate-offline`, idle shutdown, external kill). + * + * @returns True while the private companion can serve requests. + */ + get alive(): boolean { + return this.#alive; + } + get env(): NodeJS.ProcessEnv { if (!this.#socket) { throw new Error('IDB companion is not connected'); @@ -50,18 +78,42 @@ export class IdbConnection { ); } + // Remove directories left by device-mcp processes that died without + // disposal. Runs once per process; never touches shared companions. + await sweepStaleCompanionDirsOnce(); + // macOS Unix sockets have a short path limit; the normal TMPDIR can be too long. - this.#directory = await mkdtemp('/tmp/device-mcp-idb-'); - this.#socket = join(this.#directory, 'companion.sock'); + this.#directory = await mkdtemp(join(COMPANION_ROOT, COMPANION_DIR_PREFIX)); + const directory = this.#directory; + await this.#writeOwner(directory); + this.#socket = join(directory, COMPANION_SOCKET); const socket = this.#socket; const child = spawn( executable, - ['--udid', this.#udid, '--grpc-domain-sock', socket], + [ + '--udid', + this.#udid, + '--grpc-domain-sock', + socket, + // Exit when the simulator shuts down (its HID connection is gone for + // good); the backend then starts a fresh companion on the next call. + '--terminate-offline', + 'true', + '--idle-shutdown-time', + String(COMPANION_IDLE_SHUTDOWN_SECONDS), + ], { stdio: ['ignore', 'pipe', 'pipe'], }, ); this.#child = child; + // Permanent listener: a companion that dies after readiness must be + // noticed, or every later call fails on a dead socket. + child.once('exit', () => { + if (this.#child === child) { + this.#alive = false; + } + }); await new Promise((resolve, reject) => { let output = ''; @@ -124,9 +176,25 @@ export class IdbConnection { // Keep draining output so a chatty companion cannot block on a full pipe. child.stdout?.resume(); child.stderr?.resume(); + this.#alive = child.exitCode === null && child.signalCode === null; + await this.#writeOwner(directory, child.pid); + } + + async #writeOwner(directory: string, companionPid?: number): Promise { + const owner: CompanionOwner = { ownerPid: process.pid, companionPid }; + // Best effort: the sweep treats a directory without a record as unknown + // and leaves it alone for a grace period. + await writeFile( + join(directory, COMPANION_OWNER_FILE), + JSON.stringify(owner), + { + mode: 0o600, + }, + ).catch(() => undefined); } async dispose(): Promise { + this.#alive = false; const child = this.#child; if (child) { await stopChildProcess(child); diff --git a/src/backends/idb-errors.test.ts b/src/backends/idb-errors.test.ts new file mode 100644 index 0000000..f4dd518 --- /dev/null +++ b/src/backends/idb-errors.test.ts @@ -0,0 +1,79 @@ +import { describe, expect, it } from 'vitest'; + +import { classifyIdbFailure, IdbUnavailableError } from './idb-errors.js'; + +describe('classifyIdbFailure', () => { + it.each([ + [ + 'The dtuhidd connection (com.apple.coredevice.feature.remote.hid.digitizer) has been invalidated; events sent on it would be discarded', + 'IDB_HID_UNAVAILABLE', + true, + ], + [ + 'dtuhidd connection (com.apple.coredevice.feature.remote.hid.digitizer) was interrupted', + 'IDB_HID_UNAVAILABLE', + true, + ], + [ + 'dtuhidd did not answer the liveness probe (attempt 3)', + 'IDB_HID_UNAVAILABLE', + true, + ], + [ + "Failed to connect to companion at address DomainSocketAddress(path='/tmp/device-mcp-idb-a/companion.sock'): [Errno 2] No such file or directory", + 'IDB_COMPANION_UNAVAILABLE', + true, + ], + [ + 'grpclib.exceptions.GRPCError: () StatusCode.UNAVAILABLE', + 'IDB_COMPANION_UNAVAILABLE', + false, + ], + [ + 'The dtuhidd connection (x) has been invalidated\nretry: unsafe (outcome_unknown)', + 'IDB_HID_UNAVAILABLE', + false, + ], + ])('classifies %s', (text, code, notDelivered) => { + expect(classifyIdbFailure(text)).toStrictEqual({ code, notDelivered }); + }); + + it.each(['Element not found', 'No such app', ''])( + 'leaves other failures unclassified: %s', + (text) => { + expect(classifyIdbFailure(text)).toBeUndefined(); + }, + ); +}); + +describe('IdbUnavailableError', () => { + it('carries a code, the device and a remediation', () => { + const cause = new Error('raw'); + const error = new IdbUnavailableError( + 'IDB_HID_UNAVAILABLE', + 'UDID-1', + 'stderr: first\nThe dtuhidd connection has been invalidated\n', + { cause }, + ); + expect(error.name).toBe('IdbUnavailableError'); + expect(error.code).toBe('IDB_HID_UNAVAILABLE'); + expect(error.udid).toBe('UDID-1'); + expect(error.cause).toBe(cause); + expect(error.message).toMatch( + /^IDB_HID_UNAVAILABLE: iOS simulator UDID-1: The dtuhidd connection has been invalidated\. /u, + ); + expect(error.message).toContain(error.remediation); + expect(error.message).not.toContain('may have run'); + }); + + it('warns when the action may have run', () => { + const error = new IdbUnavailableError( + 'IDB_COMPANION_UNAVAILABLE', + 'UDID-1', + '', + { mayHaveRun: true }, + ); + expect(error.message).toContain('may have run'); + expect(error.cause).toBeUndefined(); + }); +}); diff --git a/src/backends/idb-errors.ts b/src/backends/idb-errors.ts new file mode 100644 index 0000000..e2131ef --- /dev/null +++ b/src/backends/idb-errors.ts @@ -0,0 +1,92 @@ +/** Typed error codes for iOS (idb) transport failures. */ +export type IdbErrorCode = 'IDB_HID_UNAVAILABLE' | 'IDB_COMPANION_UNAVAILABLE'; + +export type IdbFailure = { + code: IdbErrorCode; + /** + * True when the failure proves the command was not delivered, so sending + * it again cannot repeat an action. HID events sent on an invalidated + * dtuhidd connection are discarded, and a companion that cannot be reached + * received nothing. + */ + notDelivered: boolean; +}; + +const HID_PATTERNS = [ + /dtuhidd .*(?:invalidated|interrupted|did not answer)/iu, + /events? sent (?:on|to) it would be discarded/iu, +]; + +const COMPANION_NOT_REACHED = /Failed to connect to companion/iu; + +const COMPANION_UNAVAILABLE = 'StatusCode.UNAVAILABLE'; + +const UNSAFE_VERDICT = /^retry: unsafe\b/mu; + +/** + * Classify an idb failure from its stderr (or an execStrict error message). + * + * @param text - The failure text. + * @returns The failure kind, or undefined for any other error. + */ +export function classifyIdbFailure(text: string): IdbFailure | undefined { + // An explicit companion verdict (idb --retry-verdict) always wins. + const unsafe = UNSAFE_VERDICT.test(text); + if (HID_PATTERNS.some((pattern) => pattern.test(text))) { + return { code: 'IDB_HID_UNAVAILABLE', notDelivered: !unsafe }; + } + if (COMPANION_NOT_REACHED.test(text)) { + return { code: 'IDB_COMPANION_UNAVAILABLE', notDelivered: !unsafe }; + } + if (text.includes(COMPANION_UNAVAILABLE)) { + // The channel failed, but possibly after the request was sent. + return { code: 'IDB_COMPANION_UNAVAILABLE', notDelivered: false }; + } + return undefined; +} + +const REMEDIATION: Record = { + IDB_HID_UNAVAILABLE: + 'The simulator HID (touch/keyboard) connection is invalid even after a fresh private companion. ' + + 'The simulator was probably shut down or rebooted during the session. ' + + 'Recreate the session (mm cleanup, then mm launch; or dispose and recreate the backend). ' + + 'If it persists, reboot the simulator: xcrun simctl shutdown && xcrun simctl boot .', + IDB_COMPANION_UNAVAILABLE: + 'The private idb_companion cannot be reached even after a restart. ' + + 'Check that the simulator is booted (xcrun simctl list devices booted) and that idb_companion is installed, ' + + 'then recreate the session.', +}; + +/** An idb transport failure that a reconnect and one retry did not fix. */ +export class IdbUnavailableError extends Error { + readonly code: IdbErrorCode; + + readonly remediation: string; + + readonly udid: string; + + constructor( + code: IdbErrorCode, + udid: string, + detail: string, + options?: { cause?: unknown; mayHaveRun?: boolean }, + ) { + const lastLine = + detail + .split('\n') + .map((line) => line.trim()) + .filter(Boolean) + .pop() ?? ''; + const ran = options?.mayHaveRun + ? ' The action may have run before the connection failed; check the screen before repeating it.' + : ''; + super( + `${code}: iOS simulator ${udid}: ${lastLine.slice(0, 300)}.${ran} ${REMEDIATION[code]}`, + options?.cause === undefined ? undefined : { cause: options.cause }, + ); + this.name = 'IdbUnavailableError'; + this.code = code; + this.remediation = REMEDIATION[code]; + this.udid = udid; + } +} diff --git a/src/backends/index.ts b/src/backends/index.ts index 402ef57..26564b7 100644 --- a/src/backends/index.ts +++ b/src/backends/index.ts @@ -208,6 +208,11 @@ export function createLazyBackend( await disposing; }, + async reconnect(): Promise { + const backend = await resolve(); + await backend.reconnect?.(); + }, + async listDevices(): Promise { if (pendingDevices) { return pendingDevices; diff --git a/src/backends/lazy-backend.test.ts b/src/backends/lazy-backend.test.ts index 76eef11..61e0201 100644 --- a/src/backends/lazy-backend.test.ts +++ b/src/backends/lazy-backend.test.ts @@ -23,11 +23,13 @@ vi.mock('../utils/platform.js', async () => { const mockEnsureConnected = vi.fn().mockResolvedValue(undefined); const mockDispose = vi.fn().mockResolvedValue(undefined); +const mockReconnect = vi.fn().mockResolvedValue(undefined); vi.mock('./idb-backend.js', () => ({ IdbBackend: vi.fn().mockImplementation((deviceId: string) => ({ ...createMockBackendFields('ios', deviceId), ensureConnected: mockEnsureConnected, dispose: mockDispose, + reconnect: mockReconnect, })), })); @@ -120,6 +122,30 @@ describe('createLazyBackend', () => { return createLazyBackend; } + describe('reconnect', () => { + it('forwards reconnect to the resolved backend', async () => { + const createLazyBackend = await getLazyBackend(); + mockDetectPlatform.mockResolvedValue({ + platform: 'ios', + deviceId: 'sim-a', + }); + const lazy = createLazyBackend(); + await lazy.reconnect?.(); + expect(mockReconnect).toHaveBeenCalledTimes(1); + }); + + it('is a no-op for backends without reconnect', async () => { + const createLazyBackend = await getLazyBackend(); + mockDetectPlatform.mockResolvedValue({ + platform: 'android', + deviceId: 'emulator-5554', + }); + const lazy = createLazyBackend(); + expect(await lazy.reconnect?.()).toBeUndefined(); + expect(mockReconnect).not.toHaveBeenCalled(); + }); + }); + describe('disposal', () => { it('does not resolve an unused lazy backend during teardown', async () => { const createLazyBackend = await getLazyBackend(); diff --git a/src/backends/types.ts b/src/backends/types.ts index 84ddc73..b27403d 100644 --- a/src/backends/types.ts +++ b/src/backends/types.ts @@ -116,6 +116,13 @@ export type DeviceBackend = { /** Release only resources owned by this backend. Idempotent; never shuts down a device. */ dispose?(): Promise; + /** + * Replace the backend's own device connection (for iOS, its private + * companion) with a fresh one. Optional; callers use it to recover after a + * typed transport error. Never shuts down or reboots a device. + */ + reconnect?(): Promise; + getDeviceInfo(): Promise; snapshot(): Promise; diff --git a/src/index.test.ts b/src/index.test.ts index 9608696..a9ed952 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -27,4 +27,10 @@ describe('public API surface', () => { confidence: 'high', }); }); + + it('exports the typed idb transport error', () => { + expect( + new publicApi.IdbUnavailableError('IDB_HID_UNAVAILABLE', 'u', 'x').code, + ).toBe('IDB_HID_UNAVAILABLE'); + }); }); diff --git a/src/index.ts b/src/index.ts index 410d18f..a2a4aba 100644 --- a/src/index.ts +++ b/src/index.ts @@ -8,6 +8,8 @@ export type { export { createMcpServer } from './server.js'; export { detectAllDevices, MultipleDevicesError } from './utils/platform.js'; +export { IdbUnavailableError } from './backends/idb-errors.js'; +export type { IdbErrorCode } from './backends/idb-errors.js'; export type { DetectedDevice } from './utils/platform.js'; export type { From bd88e915208cbd1b603d3b9da16147c6af189bf5 Mon Sep 17 00:00:00 2001 From: cryptotavares Date: Wed, 28 Oct 2026 22:43:49 +0000 Subject: [PATCH 4/4] chore: update changelog for IDB session recovery --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 673d58c..4e427ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Add optional backend disposal and private, session-owned IDB companions. Release owned resources on stdio close, stdin EOF, SIGINT, and SIGTERM. +- Recover iOS sessions from a lost private companion or an invalidated simulator HID connection: reconnect once and resend only when the command was not delivered. Add optional `DeviceBackend.reconnect()` and export `IdbUnavailableError` (`IDB_HID_UNAVAILABLE`, `IDB_COMPANION_UNAVAILABLE`). +- Start private companions with `--terminate-offline true` and a one-hour idle shutdown, and remove private companion directories left by dead processes. ### Changed