fix(scripts): remove the ADB verifier's pushed probe file from the device (#5256)

* fix(scripts): remove the ADB verifier's pushed probe file from the device

The verifier pushed `/data/local/tmp/rea-verify-<timestamp>.txt` for its
transfer roundtrip but never deleted it, so every successful run left a
probe behind while reporting that all checks passed.

After a successful no-overwrite push, the verifier now removes that exact
destination with the provider-reported adb client and confirms its absence
through `list_adb_directory`. Removal runs even when the pull fails, and a
removal failure is reported as a failed check instead of a pass.
`pull_adb_file` still retains device originals.

Fixes #5190

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(scripts): cover ADB probe cleanup after pull failure and leftover probes

Add verifier cases where the pull returns an error or throws after a
successful push, and where `adb shell rm` exits 0 but the listing still
shows the probe. Pull failures must still remove the probe, and a leftover
probe must fail the run.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

scripts/verify-adb.mjs

@@ -1,12 +1,15 @@
+import { execFile } from "node:child_process";
 import { createHash } from "node:crypto";
 import { mkdtemp, readFile, rm, stat } from "node:fs/promises";
 import { tmpdir } from "node:os";
-import { join } from "node:path";
+import { join, posix } from "node:path";
+import { promisify } from "node:util";
 
 import { AdbDeviceAnalysisService } from "../dist/application/adb/AdbDeviceAnalysisService.js";
 import { createAdbDeviceAnalysisProvider } from "../dist/composition/adb.js";
 import { closeVerifierResources } from "./lib/verifier-cleanup.mjs";
 
+const execFileAsync = promisify(execFile);
 const options = parseArguments(process.argv.slice(2));
 if (options.help) {
   process.stdout.write(
@@ -40,6 +43,80 @@ const run = async (operation, input) => {
   return result.value.normalized_result;
 };
 
+// Remove the verifier's own transfer probe with the provider-reported adb
+// client, then confirm through the directory listing that it is gone.
+const removeDeviceProbe = async (binary, serial, devicePath) => {
+  const name = "verifier transfer probe removed";
+  if (typeof binary !== "string" || binary === "") {
+    check(name, false, `${devicePath}: adb client path unavailable`);
+    return;
+  }
+  try {
+    await execFileAsync(
+      binary,
+      ["-s", serial, "shell", "rm", "--", devicePath],
+      {
+        timeout: 30_000,
+      },
+    );
+  } catch (cause) {
+    check(name, false, `${devicePath}: ${formatFailure(cause)}`);
+    return;
+  }
+  const listing = await run("list_adb_directory", {
+    serial,
+    device_path: posix.dirname(devicePath),
+  });
+  if (listing !== null)
+    check(
+      name,
+      listing.coverage === "complete" &&
+        !listing.entries.some(
+          (entry) => entry.name === posix.basename(devicePath),
+        ),
+      devicePath,
+    );
+};
+
+const verifyTransferRoundtrip = async ({
+  adbBinary,
+  serial,
+  payload,
+  devicePath,
+  outputDirectory,
+}) => {
+  const pushed = await run("push_adb_file", {
+    serial,
+    local_path: payload,
+    device_path: devicePath,
+    overwrite: false,
+  });
+  if (pushed === null) return;
+  // A successful no-overwrite push proves the verifier created this exact
+  // destination; pull_adb_file deliberately retains device originals.
+  try {
+    check(
+      "push reports matching device-side digest",
+      pushed.device_digest_source === "device_sha256sum" &&
+        pushed.device_sha256 === pushed.sha256,
+      `${pushed.bytes} bytes`,
+    );
+    const pulled = await run("pull_adb_file", {
+      serial,
+      device_path: devicePath,
+      output_directory: outputDirectory,
+    });
+    if (pulled !== null)
+      check(
+        "pulled file digest matches the pushed digest",
+        pulled.sha256 === pushed.sha256,
+        `${pulled.bytes} bytes`,
+      );
+  } finally {
+    await removeDeviceProbe(adbBinary, serial, devicePath);
+  }
+};
+
 let verifierFailed = false;
 let verifierFailure;
 try {
@@ -245,31 +322,13 @@ try {
         fs.writeFile(payload, payloadBytes),
       );
       const deviceTarget = `/data/local/tmp/rea-verify-${Date.now()}.txt`;
-      const pushed = await run("push_adb_file", {
+      await verifyTransferRoundtrip({
+        adbBinary: client?.client.binary_path,
         serial,
-        local_path: payload,
-        device_path: deviceTarget,
-        overwrite: false,
+        payload,
+        devicePath: deviceTarget,
+        outputDirectory: transferRoot,
       });
-      if (pushed !== null) {
-        check(
-          "push reports matching device-side digest",
-          pushed.device_digest_source === "device_sha256sum" &&
-            pushed.device_sha256 === pushed.sha256,
-          `${pushed.bytes} bytes`,
-        );
-        const pulled = await run("pull_adb_file", {
-          serial,
-          device_path: deviceTarget,
-          output_directory: transferRoot,
-        });
-        if (pulled !== null)
-          check(
-            "pulled file digest matches the pushed digest",
-            pulled.sha256 === pushed.sha256,
-            `${pulled.bytes} bytes`,
-          );
-      }
       const screenRoot = await mkdtemp(join(tmpdir(), "rea-adb-screen-"));
       try {
         const screen = await run("capture_adb_screen", {

tests/boundary/process/adbVerifierCleanup.test.ts

@@ -1,5 +1,13 @@
 import { spawnSync } from "node:child_process";
-import { mkdtemp, mkdir, readFile, rm, writeFile } from "node:fs/promises";
+import {
+  chmod,
+  mkdtemp,
+  mkdir,
+  readFile,
+  readdir,
+  rm,
+  writeFile,
+} from "node:fs/promises";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { afterEach, expect, it } from "vitest";
@@ -12,7 +20,12 @@ afterEach(async () => {
 });
 
 const runVerifier = async (
-  settings: { closeFails?: boolean; thrown?: "undefined" | "null" } = {},
+  settings: {
+    closeFails?: boolean;
+    thrown?: "undefined" | "null";
+    removal?: "fails" | "leaves_probe";
+    pull?: "error" | "throw";
+  } = {},
 ) => {
   const root = await mkdtemp(join(tmpdir(), "rea-adb-verifier-test-"));
   roots.push(root);
@@ -29,6 +42,24 @@ const runVerifier = async (
       new URL("../../../scripts/lib/verifier-cleanup.mjs", import.meta.url),
     ),
   );
+  // The simulated device directory stands in for /data/local/tmp; the fake
+  // adb client records its argv and removes only the named probe.
+  const device = join(root, "device");
+  await mkdir(device);
+  const adb = join(root, "adb");
+  await writeFile(
+    adb,
+    [
+      "#!/bin/sh",
+      `printf '%s\\n' "$*" >> "$REA_FAKE_ADB_LOG"`,
+      `[ "$REA_FAKE_ADB_REMOVAL" = "fails" ] && exit 1`,
+      `[ "$REA_FAKE_ADB_REMOVAL" = "leaves_probe" ] && exit 0`,
+      `[ "$1 $3 $4 $5" = "-s shell rm --" ] || exit 2`,
+      `rm -- "$REA_FAKE_DEVICE/$(basename -- "$6")"`,
+      "",
+    ].join("\n"),
+  );
+  await chmod(adb, 0o755);
   await writeFile(
     join(root, "dist", "composition", "adb.js"),
     `import { writeFile } from "node:fs/promises";
@@ -46,12 +77,21 @@ export const createAdbDeviceAnalysisProvider = () => ({
   );
   await writeFile(
     join(root, "dist", "application", "adb", "AdbDeviceAnalysisService.js"),
-    `export class AdbDeviceAnalysisService {
+    `import { readdir, writeFile } from "node:fs/promises";
+import { basename, join } from "node:path";
+export class AdbDeviceAnalysisService {
   async execute(operation, input) {
     if (process.env.REA_ADB_THROW === "undefined") throw undefined;
     if (process.env.REA_ADB_THROW === "null") throw null;
+    const device = process.env.REA_FAKE_DEVICE;
+    if (operation === "push_adb_file")
+      await writeFile(join(device, basename(input.device_path)), "probe");
+    if (operation === "pull_adb_file" && process.env.REA_ADB_PULL === "throw")
+      throw new Error("pull transport crashed");
+    if (operation === "pull_adb_file" && process.env.REA_ADB_PULL === "error")
+      return { ok: false, error: { message: "pull refused" } };
     const result = {
-      inspect_adb_client: { client: { version: "1", binary_path: "/tmp/adb" } },
+      inspect_adb_client: { client: { version: "1", binary_path: process.env.REA_FAKE_ADB } },
       list_adb_devices: { devices: [{ serial: "test", state: "device", kind: "emulator", kind_basis: "stub" }] },
       inspect_adb_device: { properties: [{ name: "ro.build.version.sdk", value: "35" }], emulator_observed: true },
       list_adb_processes: { processes: [{ name: "probe" }], columns: "pid name" },
@@ -61,7 +101,7 @@ export const createAdbDeviceAnalysisProvider = () => ({
       read_adb_logcat: { lines: [], coverage: "complete" },
       list_adb_features: { features: ["feature"], coverage: "complete" },
       list_adb_services: { services: ["service"], coverage: "complete" },
-      list_adb_directory: { entries: [], coverage: "complete" },
+      list_adb_directory: { entries: (await readdir(device)).map((name) => ({ name })), coverage: "complete" },
       list_adb_packages: { packages: input.scope === "all" ? [{ package_name: "com.test" }] : [], coverage: "complete" },
       push_adb_file: { bytes: 3, sha256: "${"a".repeat(64)}", device_sha256: "${"a".repeat(64)}", device_digest_source: "device_sha256sum" },
       pull_adb_file: { bytes: 3, sha256: "${"a".repeat(64)}" },
@@ -72,6 +112,7 @@ export const createAdbDeviceAnalysisProvider = () => ({
 }\n`,
   );
   const marker = join(root, "provider-closed");
+  const adbLog = join(root, "adb.log");
   const run = spawnSync(
     process.execPath,
     [join(root, "scripts", "verify-adb.mjs")],
@@ -82,6 +123,11 @@ export const createAdbDeviceAnalysisProvider = () => ({
         ...process.env,
         REA_ADB_CLOSE_MARKER: marker,
         REA_ADB_CLOSE_FAIL: settings.closeFails ? "1" : "0",
+        REA_FAKE_ADB: adb,
+        REA_FAKE_ADB_LOG: adbLog,
+        REA_FAKE_ADB_REMOVAL: settings.removal ?? "removes",
+        REA_ADB_PULL: settings.pull ?? "succeeds",
+        REA_FAKE_DEVICE: device,
         ...(settings.thrown === undefined
           ? {}
           : { REA_ADB_THROW: settings.thrown }),
@@ -89,16 +135,91 @@ export const createAdbDeviceAnalysisProvider = () => ({
     },
   );
   const closed = await readFile(marker, "utf8").catch(() => null);
-  return { ...run, closed };
+  const adbCalls = await readFile(adbLog, "utf8").then(
+    (text) => text.trim().split("\n"),
+    () => [],
+  );
+  return { ...run, closed, adbCalls, deviceFiles: await readdir(device) };
 };
 
-it("runs the production verifier successfully and closes its provider", async () => {
-  const result = await runVerifier();
+// The fake adb client is a POSIX shell script.
+const posix = it.skipIf(process.platform === "win32");
 
-  expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(0);
-  expect(result.stdout).toContain("all adb device checks passed");
-  expect(result.closed).toBe("closed");
-});
+posix(
+  "runs the production verifier successfully and closes its provider",
+  async () => {
+    const result = await runVerifier();
+
+    expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(0);
+    expect(result.stdout).toContain("all adb device checks passed");
+    expect(result.closed).toBe("closed");
+  },
+);
+
+posix(
+  "removes exactly its own pushed probe before reporting success",
+  async () => {
+    const result = await runVerifier();
+
+    expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(0);
+    expect(result.adbCalls).toHaveLength(1);
+    expect(result.adbCalls[0]).toMatch(
+      /^-s test shell rm -- \/data\/local\/tmp\/rea-verify-\d+\.txt$/u,
+    );
+    expect(result.stdout).toContain("[ok] verifier transfer probe removed");
+    expect(result.deviceFiles).toEqual([]);
+  },
+);
+
+posix(
+  "fails visibly and keeps the probe when device removal fails",
+  async () => {
+    const result = await runVerifier({ removal: "fails" });
+
+    expect(result.status).toBe(1);
+    expect(result.stdout).toMatch(
+      /\[FAIL\] verifier transfer probe removed — \/data\/local\/tmp\/rea-verify-\d+\.txt: /u,
+    );
+    expect(result.stdout).not.toContain("all adb device checks passed");
+    expect(result.deviceFiles).toHaveLength(1);
+    expect(result.closed).toBe("closed");
+  },
+);
+
+posix(
+  "fails when removal reports success but the listing still shows the probe",
+  async () => {
+    const result = await runVerifier({ removal: "leaves_probe" });
+
+    expect(result.status).toBe(1);
+    expect(result.adbCalls).toHaveLength(1);
+    expect(result.stdout).toMatch(
+      /\[FAIL\] verifier transfer probe removed — \/data\/local\/tmp\/rea-verify-\d+\.txt\n/u,
+    );
+    expect(result.stdout).not.toContain("all adb device checks passed");
+    expect(result.deviceFiles).toHaveLength(1);
+  },
+);
+
+posix.each([
+  ["returns an error", "error", "[FAIL] pull_adb_file — pull refused"],
+  ["throws", "throw", "verifier aborted — Error: pull transport crashed"],
+] as const)(
+  "still removes the probe when the pull %s",
+  async (_label, pull, failure) => {
+    const result = await runVerifier({ pull });
+
+    expect(result.status).toBe(1);
+    expect(`${result.stdout}${result.stderr}`).toContain(failure);
+    expect(result.adbCalls).toHaveLength(1);
+    expect(result.adbCalls[0]).toMatch(
+      /^-s test shell rm -- \/data\/local\/tmp\/rea-verify-\d+\.txt$/u,
+    );
+    expect(result.stdout).toContain("[ok] verifier transfer probe removed");
+    expect(result.deviceFiles).toEqual([]);
+    expect(result.closed).toBe("closed");
+  },
+);
 
 it("makes provider cleanup failure fail the production verifier with resource details", async () => {
   const result = await runVerifier({ closeFails: true });