test(chromium): stop electron boot tests depending on host CDP ports

packages/tool-server/test/boot-electron-spawn-error.test.ts

@@ -17,6 +17,24 @@ import * as fs from "node:fs";
 import * as os from "node:os";
 import * as path from "node:path";
 
+/**
+ * CDP port for the boots below that must never reach a live endpoint.
+ *
+ * Every test that drives a spawn-error / early-exit / no-pid path needs
+ * `waitForCdpReady` to keep failing, so the readiness probe cannot win
+ * `bootElectronApp`'s `Promise.race`. When it does win, the boot resolves and
+ * detaches the child's boot listeners — the synthetic `'error'` emit that
+ * follows then hits an EventEmitter with no `'error'` listener and throws
+ * instead of rejecting, failing the test.
+ *
+ * Port 1 is privileged, so no unprivileged process on the host can bind it.
+ * A port in the unprivileged range is bindable by anything: argent's own
+ * examples and sibling tests use `chromium-cdp-19222`, so a real Chromium /
+ * Electron endpoint on 19222-19226 is exactly the kind of neighbour that turns
+ * these tests into host-dependent coin flips.
+ */
+const UNREACHABLE_CDP_PORT = 1;
+
 const spawnMock = vi.fn();
 
 vi.mock("node:child_process", async () => {
@@ -50,7 +68,19 @@ function makeFakeChild(opts: { pid?: number | undefined } = {}): FakeChild {
 }
 
 let appDir: string;
+let prevPortsFile: string | undefined;
+const TEST_PORTS_FILE = path.join(
+  os.tmpdir(),
+  `argent-boot-electron-spawn-ports-${process.pid}.json`
+);
 beforeAll(() => {
+  // The successful boot below calls trackChromiumPort, which persists the
+  // booted port to ~/.argent/chromium-cdp-ports.json. Redirect that to a
+  // throwaway file so tests never touch the real state on a developer machine
+  // or CI runner — the port they would leave behind is an ephemeral one that
+  // list-devices then probes on every call.
+  prevPortsFile = process.env.ARGENT_CHROMIUM_PORTS_FILE;
+  process.env.ARGENT_CHROMIUM_PORTS_FILE = TEST_PORTS_FILE;
   // resolveLauncher() fs-checks the app path before spawn, so the test needs
   // a real directory on disk. The spawn itself is mocked, so the contents
   // don't matter — only the path's existence.
@@ -62,6 +92,9 @@ beforeAll(() => {
   fs.writeFileSync(path.join(appDir, "main.js"), "// fake\n");
 });
 afterAll(() => {
+  if (prevPortsFile === undefined) delete process.env.ARGENT_CHROMIUM_PORTS_FILE;
+  else process.env.ARGENT_CHROMIUM_PORTS_FILE = prevPortsFile;
+  fs.rmSync(TEST_PORTS_FILE, { force: true });
   if (appDir) fs.rmSync(appDir, { recursive: true, force: true });
 });
 
@@ -76,7 +109,7 @@ describe("bootElectronApp — spawn error handling", () => {
 
     const promise = bootElectronApp({
       appPath: appDir,
-      port: 19222,
+      port: UNREACHABLE_CDP_PORT,
       readyTimeoutMs: 100,
     });
     promise.catch(() => {}); // detach so the test doesn't hang after assertion
@@ -107,7 +140,7 @@ describe("bootElectronApp — spawn error handling", () => {
         appPath: appDir,
         // Unreachable port → the readiness race rejects fast; we only care about
         // the spawn env, so detach and swallow the rejection.
-        port: 1,
+        port: UNREACHABLE_CDP_PORT,
         readyTimeoutMs: 50,
       });
       promise.catch(() => {});
@@ -134,7 +167,7 @@ describe("bootElectronApp — spawn error handling", () => {
 
     const promise = bootElectronApp({
       appPath: appDir,
-      port: 1,
+      port: UNREACHABLE_CDP_PORT,
       readyTimeoutMs: 50,
       extraArgs: ["--user-flag"],
     });
@@ -147,7 +180,7 @@ describe("bootElectronApp — spawn error handling", () => {
     expect(args).toContain("--disable-renderer-backgrounding");
     // User extras survive alongside the defaults.
     expect(args).toContain("--user-flag");
-    expect(args).toContain("--remote-debugging-port=1");
+    expect(args).toContain(`--remote-debugging-port=${UNREACHABLE_CDP_PORT}`);
 
     await promise.catch(() => {});
   });
@@ -158,7 +191,7 @@ describe("bootElectronApp — spawn error handling", () => {
 
     const promise = bootElectronApp({
       appPath: appDir,
-      port: 19223,
+      port: UNREACHABLE_CDP_PORT,
       readyTimeoutMs: 30_000,
     });
 
@@ -180,7 +213,7 @@ describe("bootElectronApp — spawn error handling", () => {
 
     const promise = bootElectronApp({
       appPath: appDir,
-      port: 19224,
+      port: UNREACHABLE_CDP_PORT,
       readyTimeoutMs: 30_000,
     });
     await new Promise((r) => setTimeout(r, 10));
@@ -200,7 +233,7 @@ describe("bootElectronApp — spawn error handling", () => {
     await expect(
       bootElectronApp({
         appPath: appDir,
-        port: 19225,
+        port: UNREACHABLE_CDP_PORT,
         readyTimeoutMs: 100,
       })
     ).rejects.toThrow(/spawn returned without a pid/);
@@ -313,10 +346,10 @@ describe("bootElectronApp — spawn error handling", () => {
       await expect(
         bootElectronApp({
           appPath: appDir,
-          // Pick an unbound port; ensureCdpReachable will fail repeatedly
-          // until readyTimeoutMs elapses. Don't pick 0 — we want a real
-          // unreachable port, not OS-assigned ephemeral.
-          port: 1,
+          // ensureCdpReachable fails repeatedly until readyTimeoutMs elapses.
+          // Don't pick 0 — we want a real unreachable port, not an OS-assigned
+          // ephemeral one.
+          port: UNREACHABLE_CDP_PORT,
           readyTimeoutMs: 100,
         })
       ).rejects.toBeInstanceOf(Error);
@@ -358,7 +391,7 @@ describe("bootElectronApp — spawn error handling", () => {
       await expect(
         bootElectronApp({
           appPath: appDir,
-          port: 19226,
+          port: UNREACHABLE_CDP_PORT,
           readyTimeoutMs: 100,
         })
       ).rejects.toThrow(/spawn returned without a pid/);

packages/tool-server/test/chromium-discovery.test.ts

@@ -186,14 +186,19 @@ describe("port persistence across tool-server restarts", () => {
   });
 
   it("a dead persisted port is pruned from the file after a failed probe", async () => {
-    trackChromiumPort(43211);
-    portsToCleanup.push(43211);
-    expect(JSON.parse(fs.readFileSync(TEST_PORTS_FILE, "utf8"))).toContain(43211);
-
-    // Nothing listens on 43211 — the probe fails and prunes it everywhere.
-    await discoverChromiumDevices({ timeoutMs: 300, ports: [43211] });
-    expect(JSON.parse(fs.readFileSync(TEST_PORTS_FILE, "utf8"))).not.toContain(43211);
-    expect(getCandidateChromiumPorts()).not.toContain(43211);
+    // Port 1 is privileged, so no unprivileged process on the host can bind it
+    // and the probe below is guaranteed to fail. A port in the unprivileged
+    // range would be bindable by anything — including a `listen(0)` fake CDP
+    // server in a sibling test file, which serves the very /json responses that
+    // would make this probe succeed and leave the port tracked.
+    const deadPort = 1;
+    trackChromiumPort(deadPort);
+    portsToCleanup.push(deadPort);
+    expect(JSON.parse(fs.readFileSync(TEST_PORTS_FILE, "utf8"))).toContain(deadPort);
+
+    await discoverChromiumDevices({ timeoutMs: 300, ports: [deadPort] });
+    expect(JSON.parse(fs.readFileSync(TEST_PORTS_FILE, "utf8"))).not.toContain(deadPort);
+    expect(getCandidateChromiumPorts()).not.toContain(deadPort);
   });
 
   it("untrackChromiumPort removes the port from the persisted file", () => {