fix: correct IDB recovery error reporting and resend safety

README.md

@@ -84,12 +84,13 @@ try {
 
 #### 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. |
+| 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 connection ... has been invalidated`, or `... would be discarded`, which also covers a failed dtuhidd liveness check) | The backend replaces its private companion and sends the command **once** more. The companion refuses the HID event that hits an invalid connection, and an unreachable companion received nothing. When the connection was already stale (the usual case), nothing landed and the resend is safe. If the simulator died partway through a multi-event command (`ui text`, `ui swipe`), a leading part of it may already have landed. |
+| The channel fails after the request may have been sent (`StatusCode.UNAVAILABLE`)                                                                                                                                                               | Read-only commands (`describe`, `list-apps`, `screenshot`, `log`, `ui describe-all`, `ui describe-point`, `ui describe`, `ui wait`) are resent once. All other commands (`ui tap`, `ui swipe`, `ui text`, `ui key`, `launch`, `terminate`, ...) 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.                                                                                                                                           |
+| The backend is disposed while a command is running, and that command fails                                                                                                                                                                      | The command is not resent. The call rejects with `IDB command interrupted: the backend was disposed during the call`; `err.cause` is the original idb error.                                                                                                                                                                                                                                                                          |
 
 `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.
 

src/backends/idb-backend.test.ts

@@ -1302,16 +1302,29 @@ describe('IdbBackend companion recovery', () => {
     expect(connectionMocks.connect).toHaveBeenCalledTimes(2);
   });
 
-  it('honours an explicit unsafe retry verdict', async () => {
-    mockExecStrict.mockRejectedValue(
-      new Error(`${hidInvalidated}\nretry: unsafe (outcome_unknown)`),
-    );
+  it.each([
+    ['launch', async (): Promise<void> => backend.openApp('io.metamask')],
+    ['terminate', async (): Promise<void> => backend.closeApp('io.metamask')],
+  ])(
+    'does not resend idb %s when delivery is uncertain',
+    async (command, run) => {
+      mockExecStrict.mockImplementation(async (cmd) => {
+        if (cmd === 'xcrun') {
+          return '';
+        }
+        throw new Error('stderr: StatusCode.UNAVAILABLE: Socket closed');
+      });
 
-    await expect(backend.tapCoordinates(1, 1)).rejects.toMatchObject({
-      code: 'IDB_HID_UNAVAILABLE',
-    });
-    expect(tapCalls()).toHaveLength(1);
-  });
+      await run();
+
+      const idbCalls = mockExecStrict.mock.calls.filter(
+        ([cmd, args]) => cmd !== 'xcrun' && args?.[0] === command,
+      );
+      expect(idbCalls).toHaveLength(1);
+      // The companion was still replaced for the next command.
+      expect(connectionMocks.connect).toHaveBeenCalledTimes(2);
+    },
+  );
 
   it('resends a read-only command when delivery is uncertain', async () => {
     mockExecStrict
@@ -1323,6 +1336,145 @@ describe('IdbBackend companion recovery', () => {
     expect(connectionMocks.connect).toHaveBeenCalledTimes(2);
   });
 
+  it.each([
+    [
+      'describe',
+      async (): Promise<unknown> => backend.getDeviceInfo(),
+      '{"name":"iPhone"}',
+    ],
+    [
+      'list-apps',
+      async (): Promise<unknown> => backend.getAppState('io.metamask'),
+      '',
+    ],
+    [
+      'screenshot',
+      async (): Promise<unknown> =>
+        backend.screenshot('/tmp/shot.png', { encode: false }),
+      '',
+    ],
+    ['log', async (): Promise<unknown> => backend.getLogs(5), ''],
+  ])(
+    'resends read-only idb %s when delivery is uncertain',
+    async (command, run, output) => {
+      mockExecStrict
+        .mockRejectedValueOnce(new Error('StatusCode.UNAVAILABLE'))
+        .mockResolvedValueOnce(output);
+
+      await run();
+
+      const idbCalls = mockExecStrict.mock.calls.filter(
+        ([, args]) => args?.[0] === command,
+      );
+      expect(idbCalls).toHaveLength(2);
+      expect(connectionMocks.connect).toHaveBeenCalledTimes(2);
+    },
+  );
+
+  it('reports disposal during a command as an interruption, without a resend', async () => {
+    await backend.ensureConnected();
+    let fail!: (error: Error) => void;
+    mockExecStrict.mockReturnValueOnce(
+      new Promise<string>((_resolve, reject) => {
+        fail = reject;
+      }),
+    );
+    const tapping = backend.tapCoordinates(1, 2).catch((e: unknown) => e);
+    await vi.waitFor(() => expect(tapCalls()).toHaveLength(1));
+
+    await backend.dispose();
+    const cause = new Error(companionGone);
+    fail(cause);
+    const error = await tapping;
+
+    expect(error).not.toBeInstanceOf(IdbUnavailableError);
+    expect(error).toBeInstanceOf(Error);
+    expect((error as Error).message).toBe(
+      'IDB command interrupted: the backend was disposed during the call',
+    );
+    expect((error as Error).message).not.toContain(
+      'idb_companion is installed',
+    );
+    expect((error as Error).cause).toBe(cause);
+    expect(tapCalls()).toHaveLength(1);
+    expect(connectionMocks.connect).toHaveBeenCalledTimes(1);
+  });
+
+  it('returns the first non-strict failure when disposed during a command', async () => {
+    await backend.ensureConnected();
+    let finish!: (result: execModule.ExecResult) => void;
+    mockExec.mockImplementation(async (cmd, args) =>
+      cmd !== 'xcrun' && args?.[1] === 'key-sequence'
+        ? new Promise((resolve) => {
+            finish = resolve;
+          })
+        : { exitCode: 0, stdout: '', stderr: '' },
+    );
+    const typing = backend.typeText('abc').catch((e: unknown) => e);
+    await vi.waitFor(() => expect(finish).toBeDefined());
+
+    await backend.dispose();
+    finish({ exitCode: 1, stdout: '', stderr: hidInvalidated });
+
+    // The failed key-sequence is returned, not resent; the next idb command
+    // then fails fast because the backend is disposed.
+    expect(await typing).toMatchObject({
+      message: 'IDB backend has been disposed',
+    });
+    expect(
+      mockExec.mock.calls.filter(([, args]) => args?.[1] === 'key-sequence'),
+    ).toHaveLength(1);
+    expect(connectionMocks.connect).toHaveBeenCalledTimes(1);
+  });
+
+  it('does not resend when disposed after joining a newer companion', async () => {
+    await backend.ensureConnected();
+    let fail!: (error: Error) => void;
+    mockExecStrict.mockReturnValueOnce(
+      new Promise<string>((_resolve, reject) => {
+        fail = reject;
+      }),
+    );
+    const tapping = backend.tapCoordinates(1, 2).catch((e: unknown) => e);
+    await vi.waitFor(() => expect(tapCalls()).toHaveLength(1));
+    // Another caller replaces the companion while the tap is in flight.
+    await backend.reconnect();
+    // Dispose exactly when the failed tap checks the new companion, so
+    // ensureConnected() itself succeeds.
+    let disposing: Promise<void> | undefined;
+    Object.defineProperty(connectionState, 'alive', {
+      configurable: true,
+      get: (): boolean => {
+        disposing ??= backend.dispose();
+        return true;
+      },
+    });
+    try {
+      fail(new Error(hidInvalidated));
+      expect(await tapping).toMatchObject({
+        message:
+          'IDB command interrupted: the backend was disposed during the call',
+      });
+      await disposing;
+    } finally {
+      Object.defineProperty(connectionState, 'alive', {
+        configurable: true,
+        writable: true,
+        value: true,
+      });
+    }
+    expect(disposing).toBeDefined();
+    expect(tapCalls()).toHaveLength(1);
+  });
+
+  it('keeps the connect error when cleanup after a failed connect also fails', async () => {
+    connectionMocks.connect.mockRejectedValueOnce(new Error('startup failed'));
+    connectionMocks.dispose.mockRejectedValueOnce(new Error('stop failed'));
+
+    await expect(backend.ensureConnected()).rejects.toThrow('startup failed');
+    expect(connectionMocks.dispose).toHaveBeenCalledTimes(1);
+  });
+
   it('reports a failed reconnect as IDB_COMPANION_UNAVAILABLE', async () => {
     await backend.ensureConnected();
     mockExecStrict.mockRejectedValue(new Error(hidInvalidated));

src/backends/idb-backend.ts

@@ -146,7 +146,8 @@ export class IdbBackend implements DeviceBackend {
       this.#generation += 1;
       this.#connected = true;
     } catch (error) {
-      await this.#connection.dispose();
+      // Best-effort cleanup: a failed stop must not hide the connect error.
+      await this.#connection.dispose().catch(() => undefined);
       this.#connecting = undefined;
       throw error;
     }
@@ -206,10 +207,16 @@ export class IdbBackend implements DeviceBackend {
    * 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.
+   * command once more.
+   *
+   * The companion refuses the HID event that hits an invalidated dtuhidd
+   * connection, so that event was not delivered, and an unreachable companion
+   * received nothing. When the HID connection was already stale (the observed
+   * failure), the first event fails and the resend is safe. If the simulator
+   * died partway through a multi-event command (`ui text`, `ui swipe`), a
+   * leading part of it may already have landed. When delivery is uncertain
+   * (`StatusCode.UNAVAILABLE`), only read-only commands are resent. A command
+   * is never resent after the backend is disposed.
    *
    * @param args - The idb arguments (used to decide whether a resend is safe).
    * @param run - Sends the command once.
@@ -244,6 +251,9 @@ export class IdbBackend implements DeviceBackend {
         await this.ensureConnected();
       }
     } catch (error) {
+      if (this.#disposed) {
+        return interrupted(first, strict);
+      }
       if (!strict) {
         return first.settle();
       }
@@ -254,6 +264,10 @@ export class IdbBackend implements DeviceBackend {
         { cause: error, mayHaveRun: !resend },
       );
     }
+    if (this.#disposed) {
+      // Disposal started while another call's reconnect was finishing.
+      return interrupted(first, strict);
+    }
     if (!resend) {
       if (!strict) {
         return first.settle();
@@ -977,6 +991,34 @@ async function attempt<Result>(
   }
 }
 
+/**
+ * Settle a failed first attempt when the backend was disposed before the
+ * recovery could run. Disposal is a teardown, not a device fault, so the
+ * command is not resent and no device remediation is given.
+ *
+ * @param first - The failed first attempt.
+ * @param strict - Throw instead of returning the failed result.
+ * @returns The first result, for non-strict callers.
+ */
+function interrupted<Result>(first: Attempt<Result>, strict: boolean): Result {
+  if (!strict) {
+    return first.settle();
+  }
+  throw new Error(
+    'IDB command interrupted: the backend was disposed during the call',
+    { cause: first.error },
+  );
+}
+
+/** Top-level idb commands that only read device state. */
+const READ_ONLY_COMMANDS = new Set([
+  'describe',
+  'list-apps',
+  'screenshot',
+  'log',
+]);
+
+/** `idb ui` subcommands that only read device state. */
 const READ_ONLY_UI_COMMANDS = new Set([
   'describe-all',
   'describe-point',
@@ -986,12 +1028,18 @@ const READ_ONLY_UI_COMMANDS = new Set([
 
 /**
  * Whether an idb command can change the device (a resend could repeat it).
+ * Only commands listed as read-only are safe; any other command, including
+ * one added later, counts as mutating.
  *
  * @param args - The idb arguments.
- * @returns True for `ui` input commands (tap, swipe, text, key, ...).
+ * @returns False only for known read-only commands.
  */
 function isMutatingIdbCommand(args: string[]): boolean {
-  return args[0] === 'ui' && !READ_ONLY_UI_COMMANDS.has(args[1] ?? '');
+  const [command = '', subcommand = ''] = args;
+  if (command === 'ui') {
+    return !READ_ONLY_UI_COMMANDS.has(subcommand);
+  }
+  return !READ_ONLY_COMMANDS.has(command);
 }
 
 function collectAlertTexts(elements: UIElement[]): string[] {

src/backends/idb-errors.test.ts

@@ -10,12 +10,7 @@ describe('classifyIdbFailure', () => {
       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)',
+      'dtuhidd did not answer a liveness probe in 5 attempts (timed out) — the daemon is not running and launchd is not keeping it up, so every HID event sent to it would be discarded without error',
       'IDB_HID_UNAVAILABLE',
       true,
     ],
@@ -30,20 +25,24 @@ describe('classifyIdbFailure', () => {
       false,
     ],
     [
-      'The dtuhidd connection (x) has been invalidated\nretry: unsafe (outcome_unknown)',
+      'stderr: The dtuhidd connection (x) has been invalidated',
       'IDB_HID_UNAVAILABLE',
-      false,
+      true,
     ],
   ])('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();
-    },
-  );
+  it.each([
+    'Element not found',
+    'No such app',
+    '',
+    // Companion log lines only; they never reach idb stderr.
+    'dtuhidd connection (com.apple.coredevice.feature.remote.hid.digitizer) was interrupted',
+    'dtuhidd did not answer the liveness probe (attempt 3 of 5): timed out',
+  ])('leaves other failures unclassified: %s', (text) => {
+    expect(classifyIdbFailure(text)).toBeUndefined();
+  });
 });
 
 describe('IdbUnavailableError', () => {