test: cover #148 (android)

packages/argent-installer/test/update.test.ts

@@ -4,6 +4,7 @@ import {
   detectPackageManager,
   globalInstallCommand,
   formatShellCommand,
+  isTempRunnerPath,
 } from "../src/utils.js";
 import { PACKAGE_NAME, NPM_REGISTRY } from "../src/constants.js";
 
@@ -54,6 +55,33 @@ describe("update — constants are correct", () => {
   });
 });
 
+describe("update — temp runner detection", () => {
+  // npx-cached argent shares the latest version, so without this filter the
+  // version compare would falsely match latest after the user uninstalled the
+  // global package via `npx @swmansion/argent uninstall`.
+  it("flags npx cache paths as transient", () => {
+    expect(isTempRunnerPath("/Users/me/.npm/_npx/abc123/node_modules/.bin/argent")).toBe(true);
+  });
+
+  it("flags pnpm dlx cache paths as transient", () => {
+    expect(isTempRunnerPath("/Users/me/.pnpm-store/dlx-1234/node_modules/.bin/argent")).toBe(true);
+  });
+
+  it("flags bun install cache paths as transient", () => {
+    expect(isTempRunnerPath("/Users/me/.bun/install/cache/argent")).toBe(true);
+  });
+
+  it("flags Windows dlx cache paths as transient", () => {
+    expect(isTempRunnerPath("C:\\Users\\me\\AppData\\Local\\dlx-abc\\argent.cmd")).toBe(true);
+  });
+
+  it("treats real global install paths as permanent", () => {
+    expect(isTempRunnerPath("/usr/local/bin/argent")).toBe(false);
+    expect(isTempRunnerPath("/opt/homebrew/bin/argent")).toBe(false);
+    expect(isTempRunnerPath("C:\\Users\\me\\AppData\\Roaming\\npm\\argent.cmd")).toBe(false);
+  });
+});
+
 describe("update — registry safety", () => {
   it("globalInstallCommand never includes --registry (relies on .npmrc scoped registry)", () => {
     for (const pm of ["npm", "yarn", "pnpm", "bun"] as const) {

packages/argent-mcp/test/auto-screenshot.test.ts

@@ -180,18 +180,33 @@ describe("getAutoScreenshotDelayMs", () => {
 });
 
 // ---------------------------------------------------------------------------
-// AUTO_SCREENSHOT_TOOLS consistency
+// shouldAutoScreenshot — unified tools trigger one screenshot regardless of platform
 // ---------------------------------------------------------------------------
-describe("AUTO_SCREENSHOT_TOOLS and delay map consistency", () => {
-  it("every tool in the allow-list has a corresponding delay", () => {
-    for (const tool of AUTO_SCREENSHOT_TOOLS) {
-      expect(AUTO_SCREENSHOT_DELAY_MS_BY_TOOL).toHaveProperty(tool);
-    }
+describe("shouldAutoScreenshot — unified surface", () => {
+  it("returns false for the screenshot tool itself (prevents recursion)", () => {
+    expect(shouldAutoScreenshot("screenshot")).toBe(false);
+    expect(shouldAutoScreenshot("mcp__argent__screenshot")).toBe(false);
   });
 
-  it("every tool in the delay map is in the allow-list", () => {
-    for (const tool of Object.keys(AUTO_SCREENSHOT_DELAY_MS_BY_TOOL)) {
-      expect(AUTO_SCREENSHOT_TOOLS.has(tool)).toBe(true);
+  it("returns true for unified interaction tools", () => {
+    for (const t of [
+      "gesture-tap",
+      "gesture-swipe",
+      "button",
+      "keyboard",
+      "rotate",
+      "launch-app",
+      "restart-app",
+      "open-url",
+      "describe",
+      "run-sequence",
+    ]) {
+      expect(shouldAutoScreenshot(t)).toBe(true);
     }
   });
+
+  it("normalizes MCP-prefixed names before looking up the allow-list", () => {
+    expect(shouldAutoScreenshot("mcp__argent__gesture-tap")).toBe(true);
+    expect(shouldAutoScreenshot("mcp__argent__launch-app")).toBe(true);
+  });
 });

packages/tool-server/test/adb-hardening.test.ts

@@ -0,0 +1,179 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+
+const execFileMock = vi.fn();
+
+vi.mock("node:child_process", async () => {
+  const actual = await vi.importActual<typeof import("node:child_process")>("node:child_process");
+  return {
+    ...actual,
+    execFile: (
+      cmd: string,
+      args: readonly string[],
+      opts: unknown,
+      cb?: (err: Error | null, out: { stdout: string; stderr: string }) => void
+    ) => {
+      const callback = typeof opts === "function" ? opts : cb!;
+      const options = typeof opts === "function" ? undefined : opts;
+      const result = execFileMock(cmd, args, options);
+      if (result instanceof Error) callback(result, { stdout: "", stderr: "" });
+      else callback(null, result ?? { stdout: "", stderr: "" });
+    },
+  };
+});
+
+// `runAdb` / `listAvds` now resolve adb / emulator to an absolute path before
+// spawning, so a bare `cmd === "adb"` matcher would never fire on real hosts.
+// Stub the resolver to return the bare name so existing test mocks keep
+// working, regardless of whether the host has the SDK installed.
+vi.mock("../src/utils/android-binary", () => ({
+  resolveAndroidBinary: vi.fn(async (name: "adb" | "emulator") => name),
+  __resetAndroidBinaryCacheForTesting: () => {},
+}));
+
+import { listAndroidDevices, listAvds } from "../src/utils/adb";
+
+beforeEach(() => {
+  execFileMock.mockReset();
+});
+
+describe("readAvdName — modern property, legacy fallback (review #3)", () => {
+  /**
+   * Emulator release 30 (Android 11+) moved the AVD name from
+   * `ro.kernel.qemu.avd_name` to `ro.boot.qemu.avd_name`. Reading only the
+   * old property makes modern images report `avdName: null`, which in turn
+   * breaks `findSerialByAvdName` disambiguation when two emulators boot
+   * concurrently.
+   *
+   * The fix probes the new prop first and falls back to the old one. These
+   * tests pin both paths.
+   */
+
+  function mockAdbGetProps(
+    serial: string,
+    props: Partial<{
+      "ro.product.model": string;
+      "ro.build.version.sdk": string;
+      "ro.boot.qemu.avd_name": string;
+      "ro.kernel.qemu.avd_name": string;
+    }>
+  ): void {
+    execFileMock.mockImplementation((cmd: string, args: string[]) => {
+      if (cmd === "adb" && args[0] === "devices" && args.length === 1) {
+        return { stdout: `List of devices attached\n${serial}\tdevice\n`, stderr: "" };
+      }
+      if (cmd === "adb" && args[0] === "-s" && args[2] === "shell") {
+        const shell = args[3] ?? "";
+        for (const [prop, value] of Object.entries(props)) {
+          if (shell === `getprop ${prop}`) return { stdout: `${value}\n`, stderr: "" };
+        }
+        return { stdout: "\n", stderr: "" };
+      }
+      return { stdout: "", stderr: "" };
+    });
+  }
+
+  it("reads ro.boot.qemu.avd_name on modern images (Android 11+)", async () => {
+    mockAdbGetProps("emulator-5554", {
+      "ro.product.model": "sdk_gphone64",
+      "ro.build.version.sdk": "34",
+      "ro.boot.qemu.avd_name": "Pixel_7_API_34",
+      "ro.kernel.qemu.avd_name": "",
+    });
+
+    const devices = await listAndroidDevices();
+    expect(devices).toHaveLength(1);
+    expect(devices[0]!.avdName).toBe("Pixel_7_API_34");
+  });
+
+  it("falls back to ro.kernel.qemu.avd_name on legacy images", async () => {
+    mockAdbGetProps("emulator-5554", {
+      "ro.product.model": "sdk_gphone",
+      "ro.build.version.sdk": "29",
+      "ro.boot.qemu.avd_name": "",
+      "ro.kernel.qemu.avd_name": "Pixel_3a_API_29",
+    });
+
+    const devices = await listAndroidDevices();
+    expect(devices[0]!.avdName).toBe("Pixel_3a_API_29");
+  });
+
+  it("prefers the modern property when both are present (some images double-set)", async () => {
+    mockAdbGetProps("emulator-5554", {
+      "ro.product.model": "sdk_gphone64",
+      "ro.build.version.sdk": "34",
+      "ro.boot.qemu.avd_name": "Pixel_7_API_34",
+      "ro.kernel.qemu.avd_name": "Pixel_7_API_34_stale",
+    });
+
+    const devices = await listAndroidDevices();
+    expect(devices[0]!.avdName).toBe("Pixel_7_API_34");
+  });
+
+  it("returns null when neither property is set (physical device)", async () => {
+    mockAdbGetProps("R5CT12345678", {
+      "ro.product.model": "SM-G990B",
+      "ro.build.version.sdk": "33",
+    });
+    execFileMock.mockImplementation((cmd: string, args: string[]) => {
+      if (cmd === "adb" && args[0] === "devices") {
+        return { stdout: `List of devices attached\nR5CT12345678\tdevice\n`, stderr: "" };
+      }
+      if (cmd === "adb" && args[0] === "-s" && args[2] === "shell") {
+        const shell = args[3] ?? "";
+        if (shell === "getprop ro.product.model") return { stdout: "SM-G990B\n", stderr: "" };
+        if (shell === "getprop ro.build.version.sdk") return { stdout: "33\n", stderr: "" };
+        return { stdout: "\n", stderr: "" };
+      }
+      return { stdout: "", stderr: "" };
+    });
+
+    const devices = await listAndroidDevices();
+    expect(devices[0]!.avdName).toBeNull();
+  });
+});
+
+describe("listAvds noise filter (review #9)", () => {
+  /**
+   * Old filter was prefix-only — any AVD name starting with INFO/HAX was
+   * silently dropped. Real `emulator -list-avds` noise is diagnostic
+   * header/footer lines that contain whitespace or colons (e.g.
+   * `INFO    | Android emulator version ...`), while AVD names are
+   * identifier-only. The new filter accepts identifier-shaped lines only.
+   */
+
+  it("accepts an AVD name that happens to start with HAX (e.g. HAX-Pixel-6)", async () => {
+    execFileMock.mockImplementation((cmd: string) => {
+      if (cmd === "emulator") {
+        return { stdout: "HAX-Pixel-6\nINFO_BuildBot_Pixel7\nPixel_7_API_34\n", stderr: "" };
+      }
+      return { stdout: "", stderr: "" };
+    });
+    const avds = await listAvds();
+    expect(avds.map((a) => a.name)).toEqual([
+      "HAX-Pixel-6",
+      "INFO_BuildBot_Pixel7",
+      "Pixel_7_API_34",
+    ]);
+  });
+
+  it("filters out genuine noise lines with whitespace / pipe characters", async () => {
+    // Real emulator output on at least some installs prints a log-format header.
+    execFileMock.mockImplementation((cmd: string) => {
+      if (cmd === "emulator") {
+        return {
+          stdout: [
+            "INFO    | Android emulator version 33.1.11.0",
+            "HAX is working and emulator runs in fast virt mode.",
+            "Pixel_7_API_34",
+            "Pixel_3a_API_29",
+            "",
+          ].join("\n"),
+          stderr: "",
+        };
+      }
+      return { stdout: "", stderr: "" };
+    });
+    const avds = await listAvds();
+    expect(avds.map((a) => a.name)).toEqual(["Pixel_7_API_34", "Pixel_3a_API_29"]);
+  });
+});

packages/tool-server/test/adb-terminal-error-format.test.ts

@@ -0,0 +1,75 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+
+const execFileMock = vi.fn();
+
+vi.mock("node:child_process", async () => {
+  const actual = await vi.importActual<typeof import("node:child_process")>("node:child_process");
+  return {
+    ...actual,
+    execFile: (
+      cmd: string,
+      args: readonly string[],
+      opts: unknown,
+      cb?: (err: Error | null, out: { stdout: string; stderr: string }) => void
+    ) => {
+      const callback = typeof opts === "function" ? opts : cb!;
+      const options = typeof opts === "function" ? undefined : opts;
+      const result = execFileMock(cmd, args, options);
+      if (result instanceof Error) {
+        // Mirror execFile's actual rejection contract: stderr/stdout are
+        // attached to the error object so describeAdbFailure can read them.
+        const e = result as Error & { stderr?: string; stdout?: string };
+        callback(e, { stdout: e.stdout ?? "", stderr: e.stderr ?? "" });
+      } else callback(null, result ?? { stdout: "", stderr: "" });
+    },
+  };
+});
+
+// `runAdb` resolves adb to an absolute path before spawning. Stub the
+// resolver to return the bare name so existing `cmd === "adb"` mocks fire.
+vi.mock("../src/utils/android-binary", () => ({
+  resolveAndroidBinary: vi.fn(async (name: "adb" | "emulator") => name),
+  __resetAndroidBinaryCacheForTesting: () => {},
+}));
+
+import { waitForBootCompleted } from "../src/utils/adb";
+
+beforeEach(() => {
+  execFileMock.mockReset();
+});
+
+/**
+ * `isTerminalAdbError` checks for the literal substring "device not found",
+ * but adb's actual stderr is `error: device 'emulator-5554' not found` —
+ * the serial appears between "device" and "not found", so the substring
+ * match never fires. Result: when a device drops off PATH mid-boot,
+ * `waitForBootCompleted` keeps spinning until the full timeoutMs elapses
+ * (default 120 s) instead of failing fast with the actionable error.
+ *
+ * Expected: the function should detect the terminal state and throw on
+ * the first failed poll (well before timeoutMs).
+ */
+describe("isTerminalAdbError matches adb's real `device 'X' not found` format", () => {
+  it("waitForBootCompleted should fail fast when adb says \"device 'X' not found\"", async () => {
+    execFileMock.mockImplementation((cmd: string, args: string[]) => {
+      if (cmd === "adb" && args[0] === "-s" && args[2] === "shell") {
+        const err = new Error("Command failed") as Error & { stderr?: string };
+        err.stderr = "error: device 'emulator-5554' not found";
+        return err;
+      }
+      return new Error("unexpected call");
+    });
+
+    const start = Date.now();
+    // Use a small budget so the test doesn't take 2 minutes; the bug
+    // produces a full-timeoutMs hang regardless of size.
+    await expect(waitForBootCompleted("emulator-5554", 4_000)).rejects.toThrow(
+      /terminal state|device.*not found/i
+    );
+    const elapsed = Date.now() - start;
+    // Fail-fast path: throw fires after the first failed poll (< 1 s).
+    // Bug path: loop spins until the deadline (~timeoutMs).
+    // Anything ≥ 3 s on the 4 s budget proves the bug.
+    expect(elapsed).toBeLessThan(2_500);
+  }, 8_000);
+});