fix(server): device_open finds a booted AVD by name and says when the device list failed

apps/server/src/device/DeviceService.test.ts

@@ -2028,12 +2028,14 @@ describe("locating a device id across hosts", () => {
   const macMini = { id: "mac-mini", label: "Mac Mini" };
   const listingHost = (
     host: { readonly id: string; readonly label: string },
-    ids: ReadonlyArray<string>,
+    ids: ReadonlyArray<string | { readonly id: string; readonly name: string }>,
   ): DeviceHost.DeviceHost["Service"] => {
-    const devices = ids.map((id) => ({
-      id,
-      name: id,
-      platform: id.startsWith("Pixel") ? ("android" as const) : ("ios" as const),
+    const devices = ids.map((entry) => ({
+      ...(typeof entry === "string" ? { id: entry, name: entry } : entry),
+      platform:
+        typeof entry !== "string" || entry.startsWith("Pixel")
+          ? ("android" as const)
+          : ("ios" as const),
       version: "test",
       booted: true,
       physical: false,
@@ -2145,6 +2147,77 @@ describe("locating a device id across hosts", () => {
     ),
   );
 
+  it.effect("finds a running emulator by the AVD name it was opened with", () =>
+    Effect.gen(function* () {
+      // A booted AVD is listed by its adb serial; its AVD name is only its name.
+      const service = yield* DeviceService.makeWithHosts(
+        new Map([
+          [
+            thisMachine.id,
+            listingHost(thisMachine, [{ id: "emulator-5554", name: "Medium_Phone" }]),
+          ],
+        ]),
+      ).pipe(Effect.provide(NodeCrypto.layer));
+      yield* service.list;
+      const located = yield* service.locateDevice({ deviceId: DeviceId.make("Medium_Phone") });
+      expect(Option.getOrUndefined(located)).toMatchObject({
+        hostId: LOCAL_DEVICE_HOST_ID,
+        id: "emulator-5554",
+      });
+    }).pipe(Effect.provide(ServerSettings.layerTest({ enableDeviceSupport: true })), Effect.scoped),
+  );
+
+  it.effect("does not guess between two emulators running the same AVD", () =>
+    Effect.gen(function* () {
+      const service = yield* DeviceService.makeWithHosts(
+        new Map([
+          [
+            thisMachine.id,
+            listingHost(thisMachine, [
+              { id: "emulator-5554", name: "Medium_Phone" },
+              { id: "emulator-5556", name: "Medium_Phone" },
+            ]),
+          ],
+          [macMini.id, listingHost(macMini, [{ id: "emulator-5554", name: "Medium_Phone" }])],
+        ]),
+      ).pipe(Effect.provide(NodeCrypto.layer));
+      yield* service.list;
+      const deviceId = DeviceId.make("Medium_Phone");
+      expect(yield* service.locateDevice({ deviceId }).pipe(Effect.flip)).toMatchObject({
+        _tag: "DeviceHostAmbiguousError",
+      });
+      expect(yield* service.locateDevice({ deviceId, hostId: LOCAL_DEVICE_HOST_ID })).toEqual(
+        Option.none(),
+      );
+      expect(
+        Option.getOrUndefined(yield* service.locateDevice({ deviceId, hostId: "mac-mini" })),
+      ).toMatchObject({ hostId: "mac-mini", id: "emulator-5554" });
+    }).pipe(Effect.provide(ServerSettings.layerTest({ enableDeviceSupport: true })), Effect.scoped),
+  );
+
+  it.effect("prefers a listed id to another host's emulator running that AVD", () =>
+    Effect.gen(function* () {
+      const service = yield* DeviceService.makeWithHosts(
+        new Map([
+          [
+            thisMachine.id,
+            listingHost(thisMachine, [{ id: "Medium_Phone", name: "Medium_Phone" }]),
+          ],
+          [macMini.id, listingHost(macMini, [{ id: "emulator-5554", name: "Medium_Phone" }])],
+        ]),
+      ).pipe(Effect.provide(NodeCrypto.layer));
+      yield* service.list;
+      const deviceId = DeviceId.make("Medium_Phone");
+      expect(Option.getOrUndefined(yield* service.locateDevice({ deviceId }))).toMatchObject({
+        hostId: LOCAL_DEVICE_HOST_ID,
+        id: "Medium_Phone",
+      });
+      expect(
+        Option.getOrUndefined(yield* service.locateDevice({ deviceId, hostId: "mac-mini" })),
+      ).toMatchObject({ hostId: "mac-mini", id: "emulator-5554" });
+    }).pipe(Effect.provide(ServerSettings.layerTest({ enableDeviceSupport: true })), Effect.scoped),
+  );
+
   it.effect("finds nothing for a device id that no host lists", () =>
     withTwoHosts((service) =>
       Effect.gen(function* () {

apps/server/src/device/DeviceService.ts

@@ -221,7 +221,8 @@ export class DeviceService extends Context.Service<
     /**
      * The device an id names in the last listing. Ids are unique per host, not across hosts (an
      * Android AVD name can exist on two), so a hostId decides, and without one the id has to be
-     * on a single host.
+     * on a single host. When no device in scope has the id, the one emulator running the AVD of
+     * that name on a host stands in for it.
      */
     readonly locateDevice: (input: {
       readonly hostId?: DeviceHostId | undefined;
@@ -1072,10 +1073,23 @@ export const makeWithHosts = Effect.fn("DeviceService.makeWithHosts")(function*
     "DeviceService.locateDevice",
   )(function* (input) {
     const { state } = yield* SynchronizedRef.get(stateRef);
-    return yield* locate(
-      state,
-      input,
-      state.devices.filter((device) => device.id === input.deviceId),
+    const byId = state.devices.filter((device) => device.id === input.deviceId);
+    if (byId.some((device) => input.hostId === undefined || device.hostId === input.hostId)) {
+      return yield* locate(state, input, byId);
+    }
+    // A shut-down AVD is listed by its name and a running one by its adb serial.
+    const byAvdName = state.devices.filter(
+      (device) =>
+        device.platform === "android" &&
+        !device.physical &&
+        device.booted &&
+        device.name === input.deviceId,
+    );
+    const located = yield* locate(state, input, [...byId, ...byAvdName]);
+    // Several emulators running one AVD: its name picks none of them.
+    return Option.filter(
+      located,
+      (device) => byAvdName.filter((other) => other.hostId === device.hostId).length === 1,
     );
   });
 

apps/server/src/mcp/McpDeviceToolkit.test.ts

@@ -591,6 +591,55 @@ describe("device targeting across hosts", () => {
     ),
   );
 
+  const failedRefresh = {
+    ...multiHostState,
+    hostStatuses: {
+      ...multiHostState.hostStatuses,
+      local: { status: "failed" as const, detail: "adb devices timed out" },
+    },
+  };
+  const withFailedRefresh = {
+    list: Effect.succeed(failedRefresh),
+    state: Effect.succeed(failedRefresh),
+  };
+
+  it.effect("says the asked host's device list failed even when another host has the id", () =>
+    run(
+      [],
+      ({ opened, call }) =>
+        Effect.gen(function* () {
+          const result = yield* call("device_open", { deviceId: "MAC-UDID", hostId: "local" });
+          expect(result.isError).toBe(true);
+          expect(text(result)).toContain("adb devices timed out");
+          expect(opened).toEqual([]);
+        }),
+      { ...withFailedRefresh, locateDevice: () => Effect.fail(onTheMac) },
+    ),
+  );
+
+  it.effect("does not open another host's emulator by AVD name while a device list failed", () =>
+    run(
+      [],
+      ({ opened, call }) =>
+        Effect.gen(function* () {
+          const result = yield* call("device_open", { deviceId: "Medium_Phone" });
+          expect(result.isError).toBe(true);
+          expect(text(result)).toContain("adb devices timed out");
+          expect(opened).toEqual([]);
+        }),
+      {
+        ...withFailedRefresh,
+        locateDevice: () =>
+          Effect.succeedSome({
+            ...macPixel,
+            id: "emulator-5554",
+            name: "Medium_Phone",
+            booted: true,
+          }),
+      },
+    ),
+  );
+
   it.effect("screenshots an open device by id on whichever host it is on", () =>
     run([session("mac-mini", "MAC-UDID", "ios")], ({ call }) =>
       Effect.gen(function* () {

apps/server/src/mcp/toolkits/device/handlers.ts

@@ -168,21 +168,28 @@ const pickDevice = Effect.fn("handlers.pickDevice")(function* (
   if (input.deviceId !== undefined) {
     const { deviceId, hostId } = input;
     const devices = yield* DeviceService.DeviceService;
-    const located = yield* devices.locateDevice({ deviceId, hostId }).pipe(mapLocateErrors);
-    if (Option.isSome(located)) return located.value;
     const target =
       hostId === undefined
         ? undefined
         : (state.hosts.find((host) => host.id === hostId) ?? { id: hostId });
-    const failures = failedHosts(state, target === undefined ? state.hosts : [target]).map(
-      ({ host, detail }) => `${hostName(host)} failed to start: ${detail}`,
+    // A failed refresh keeps the previous devices, so the device may be there unlisted.
+    const failures = failedHosts(state, target === undefined ? state.hosts : [target]);
+    const located = yield* devices.locateDevice({ deviceId, hostId }).pipe(
+      Effect.catchTags({
+        DeviceOnOtherHostError: (error) =>
+          failures.length > 0 ? Effect.succeedNone : Effect.fail(error),
+      }),
+      mapLocateErrors,
     );
+    // An AVD-name match could be on another host than the one whose list failed.
+    if (Option.isSome(located) && (failures.length === 0 || located.value.id === deviceId)) {
+      return located.value;
+    }
     return yield* new DeviceToolUnavailableError({
-      reason: [
-        `No device ${deviceId} on ${target === undefined ? "any host" : hostName(target)}.`,
-        ...failures,
-        "Call device_list for current ids.",
-      ].join(" "),
+      reason:
+        failures.length > 0
+          ? `Device ${deviceId} is not in the last device list, and listing devices failed on ${failures.map(({ host, detail }) => `${hostName(host)} (${detail})`).join(", ")}. Call device_list to retry.`
+          : `No device ${deviceId} on ${target === undefined ? "any host" : hostName(target)}. Call device_list for current ids.`,
     });
   }
   const hostId = input.hostId ?? LOCAL_DEVICE_HOST_ID;