fix(hopper): verify lease cleanup and repair the macOS test lane (#1111)

* fix(hopper): distinguish shared lease roots from leaked sessions

* fix(ci): follow the moved process CLI regression tests

scripts/lib/real-hopper-cleanup.d.mts

@@ -0,0 +1,6 @@
+/** Observe concrete Hopper runtime resources for a before/after cleanup check. */
+export function snapshotHopperRuntime(
+  parent: string,
+  targetLeaseDirectory: string,
+  ownedProcessIds?: ReadonlySet<number>,
+): Promise<Set<string>>;

scripts/lib/real-hopper-cleanup.mjs

@@ -0,0 +1,52 @@
+import { lstat, readFile, readdir } from "node:fs/promises";
+import { join } from "node:path";
+
+/** Observe session paths and target leases without treating their shared parent as a session. */
+export async function snapshotHopperRuntime(
+  parent,
+  targetLeaseDirectory,
+  ownedProcessIds,
+) {
+  const resources = new Set();
+  for (const entry of await readdir(parent, { withFileTypes: true })) {
+    const path = join(parent, entry.name);
+    if (!entry.name.startsWith("rea-") || path === targetLeaseDirectory)
+      continue;
+    if (
+      ownedProcessIds !== undefined &&
+      (!entry.isDirectory() ||
+        !(await belongsToOwnedProcess(path, ownedProcessIds)))
+    )
+      continue;
+    resources.add(path);
+  }
+  const leaseRoot = await lstat(targetLeaseDirectory).catch(missingIsAbsent);
+  if (leaseRoot === undefined) return resources;
+  if (!leaseRoot.isDirectory() || leaseRoot.isSymbolicLink()) {
+    resources.add(targetLeaseDirectory);
+    return resources;
+  }
+  const leases = await readdir(targetLeaseDirectory).catch(missingIsAbsent);
+  for (const name of leases ?? [])
+    resources.add(join(targetLeaseDirectory, name));
+  return resources;
+}
+
+const missingIsAbsent = (cause) => {
+  if (cause?.code === "ENOENT") return undefined;
+  throw cause;
+};
+
+const belongsToOwnedProcess = async (path, ownedProcessIds) => {
+  const encoded = await readFile(join(path, "ownership.json"), "utf8").catch(
+    missingIsAbsent,
+  );
+  if (encoded === undefined) return false;
+  const ownership = JSON.parse(encoded);
+  return (
+    ownership !== null &&
+    typeof ownership === "object" &&
+    Number.isSafeInteger(ownership.parent_pid) &&
+    ownedProcessIds.has(ownership.parent_pid)
+  );
+};

scripts/verify-real-hopper.mjs

@@ -1,7 +1,5 @@
 import { execFile } from "node:child_process";
-import { readFile, readdir } from "node:fs/promises";
 import { tmpdir } from "node:os";
-import { join } from "node:path";
 import { promisify } from "node:util";
 
 import { Client } from "@modelcontextprotocol/client";
@@ -14,6 +12,7 @@ import {
   requirePseudocode,
 } from "../dist/application/RealHopperAssertions.js";
 import { HOPPER_PROVIDER_IDENTITY } from "../dist/hopper/HopperProvider.js";
+import { HOPPER_TARGET_LEASE_DIRECTORY } from "../dist/hopper/HopperTargetLease.js";
 import { REA_WORKFLOW_PROVIDER } from "../dist/application/InvestigationProviders.js";
 import { loadRealHopperFixtureTargets } from "./lib/real-hopper-fixture.mjs";
 import {
@@ -40,6 +39,7 @@ import {
   verifyHopperLifecycleAndCli,
 } from "./lib/real-hopper-boundaries.mjs";
 import { startUnrelatedHopperSentinel } from "./lib/unrelated-hopper-sentinel.mjs";
+import { snapshotHopperRuntime } from "./lib/real-hopper-cleanup.mjs";
 const execFileAsync = promisify(execFile);
 const verifierRun = createVerifierRun();
 const timeout = 180_000;
@@ -54,7 +54,10 @@ const parseServerArgs = (encoded) => {
 };
 
 const runtimeParent = process.platform === "darwin" ? "/tmp" : tmpdir();
-const sessionsBefore = new Set(await readdir(runtimeParent));
+const sessionsBefore = await snapshotHopperRuntime(
+  runtimeParent,
+  HOPPER_TARGET_LEASE_DIRECTORY,
+);
 const ownedProcessIds = new Set();
 const textValue = mcpTextValue;
 const requireSuccessfulTool = requireMcpResult;
@@ -519,26 +522,25 @@ try {
 }
 
 await new Promise((resolve) => setTimeout(resolve, 500));
-for (const entry of await readdir(runtimeParent, { withFileTypes: true })) {
-  if (
-    !entry.isDirectory() ||
-    !entry.name.startsWith("rea-") ||
-    sessionsBefore.has(entry.name)
-  )
-    continue;
-  let ownership;
-  try {
-    ownership = JSON.parse(
-      await readFile(join(runtimeParent, entry.name, "ownership.json"), "utf8"),
-    );
-  } catch (cause) {
-    if (cause.code === "ENOENT") continue;
-    throw cause;
-  }
-  if (ownedProcessIds.has(ownership.parent_pid))
-    throw new Error(
-      `An owned Hopper runtime leaked its directory: ${entry.name}`,
-    );
+const sessionsAfter = [
+  ...(await snapshotHopperRuntime(
+    runtimeParent,
+    HOPPER_TARGET_LEASE_DIRECTORY,
+    ownedProcessIds,
+  )),
+]
+  .filter((path) => !sessionsBefore.has(path))
+  .sort();
+if (sessionsAfter.length > 0) {
+  await writeVerificationReport({
+    ...summary,
+    unrelatedHopperSurvived,
+    cleanShutdown: false,
+    retainedRuntimePaths: sessionsAfter,
+  });
+  throw new Error(
+    `The MCP runtime retained Hopper resources: ${sessionsAfter.join(", ")}`,
+  );
 }
 if (summary === undefined)
   throw new Error("Real-Hopper verification did not produce a summary");
@@ -550,12 +552,18 @@ if (
   throw new Error(
     "The verifier's process lineage did not prove owned child cleanup",
   );
-await new Promise((resolve, reject) => {
-  process.stdout.write(
-    `${JSON.stringify({ verifier_run: completedVerifierRun, ...summary, unrelatedHopperSurvived, cleanShutdown: true }, null, 2)}\n`,
-    (cause) => {
+await writeVerificationReport({
+  verifier_run: completedVerifierRun,
+  ...summary,
+  unrelatedHopperSurvived,
+  cleanShutdown: true,
+});
+
+async function writeVerificationReport(report) {
+  await new Promise((resolve, reject) => {
+    process.stdout.write(`${JSON.stringify(report, null, 2)}\n`, (cause) => {
       if (cause) reject(cause);
       else resolve();
-    },
-  );
-});
+    });
+  });
+}

src/hopper/HopperTargetLease.ts

@@ -23,7 +23,11 @@ export type HopperTargetLeaseAcquisition =
   | { readonly acquired: true; readonly lease: HopperTargetLease }
   | { readonly acquired: false; readonly owner: HopperTargetLeaseOwner };
 
-const leaseDirectory = join("/tmp", `rea-hopper-${process.getuid?.() ?? 0}`);
+/** Shared private parent of per-target lease sockets; it outlives individual sessions. */
+export const HOPPER_TARGET_LEASE_DIRECTORY = join(
+  "/tmp",
+  `rea-hopper-${process.getuid?.() ?? 0}`,
+);
 
 /** Keep two REA processes from opening duplicate Hopper documents for one target/profile. */
 export const acquireHopperTargetLease = async (input: {
@@ -33,7 +37,7 @@ export const acquireHopperTargetLease = async (input: {
   readonly runId: string;
   readonly directory?: string;
 }): Promise<HopperTargetLeaseAcquisition> => {
-  const directory = input.directory ?? leaseDirectory;
+  const directory = input.directory ?? HOPPER_TARGET_LEASE_DIRECTORY;
   await ensureLeaseDirectory(directory);
   // The canonical path only derives the lease key; it never authorizes access.
   // Common expected `realpath` failures include a missing target or path

tests/boundary/providers/hopper/hopperVerificationCleanup.test.ts

@@ -0,0 +1,103 @@
+import { mkdir, mkdtemp, readdir, rm, writeFile } from "node:fs/promises";
+import { join } from "node:path";
+import { expect, it, onTestFinished } from "vitest";
+
+import { snapshotHopperRuntime } from "../../../../scripts/lib/real-hopper-cleanup.mjs";
+import { acquireHopperTargetLease } from "../../../../src/hopper/HopperTargetLease.js";
+import { createTestTempDirectory } from "../../../fixtures/temporaryDirectory.js";
+
+it.skipIf(process.platform === "win32")(
+  "does not classify a released shared lease directory as a session leak",
+  async () => {
+    const parent = await createLeaseWorkspace();
+    const directory = join(parent, "rea-hopper-501");
+    const targetPath = join(parent, "target");
+    await writeFile(targetPath, "source-owned lease target");
+    const before = await snapshotHopperRuntime(parent, directory);
+    const acquired = await acquireHopperTargetLease({
+      targetPath,
+      targetKind: "executable",
+      loaderArgs: [],
+      runId: "released-verifier-lease",
+      directory,
+    });
+    if (!acquired.acquired) throw new Error("Expected the owned target lease");
+    await acquired.lease.release();
+
+    expect(await readdir(directory)).toEqual([]);
+    expect(await snapshotHopperRuntime(parent, directory)).toEqual(before);
+  },
+);
+
+it.skipIf(process.platform === "win32")(
+  "retains an unreleased real target-lease socket in cleanup observations",
+  async () => {
+    const parent = await createLeaseWorkspace();
+    const directory = join(parent, "rea-hopper-501");
+    const targetPath = join(parent, "target");
+    await writeFile(targetPath, "source-owned lease target");
+    const acquired = await acquireHopperTargetLease({
+      targetPath,
+      targetKind: "executable",
+      loaderArgs: [],
+      runId: "retained-verifier-lease",
+      directory,
+    });
+    if (!acquired.acquired) throw new Error("Expected the owned target lease");
+    try {
+      const children = await readdir(directory);
+      expect(children).toHaveLength(1);
+      expect(await snapshotHopperRuntime(parent, directory)).toEqual(
+        new Set(children.map((name) => join(directory, name))),
+      );
+    } finally {
+      await acquired.lease.release();
+    }
+  },
+);
+
+it("reports retained sessions belonging to observed MCP processes", async () => {
+  const parent = await createTestTempDirectory("rea-cleanup-retained-session-");
+  const directory = join(parent, "rea-hopper-501");
+  const previous = join(parent, "rea-existing-session");
+  await mkdir(previous);
+  const before = await snapshotHopperRuntime(parent, directory);
+  const retained = join(parent, "rea-retained-session");
+  const unrelated = join(parent, "rea-unrelated-session");
+  const unknown = join(parent, "rea-no-ownership-record");
+  for (const path of [retained, unrelated, unknown]) await mkdir(path);
+  await writeFile(join(retained, "bootstrap.py"), "source-owned fixture");
+  await writeFile(
+    join(retained, "ownership.json"),
+    JSON.stringify({ parent_pid: process.pid }),
+  );
+  await writeFile(
+    join(unrelated, "ownership.json"),
+    JSON.stringify({ parent_pid: process.pid + 1 }),
+  );
+  const after = await snapshotHopperRuntime(
+    parent,
+    directory,
+    new Set([process.pid]),
+  );
+
+  expect([...after].filter((path) => !before.has(path))).toEqual([retained]);
+});
+
+it("reports an unexpected file at the shared lease root", async () => {
+  const parent = await createTestTempDirectory("rea-cleanup-invalid-lease-");
+  const directory = join(parent, "rea-hopper-501");
+  await writeFile(directory, "unexpected lease root file");
+
+  expect(await snapshotHopperRuntime(parent, directory)).toEqual(
+    new Set([directory]),
+  );
+});
+
+const createLeaseWorkspace = async (): Promise<string> => {
+  // Unix socket paths must fit the host sockaddr_un limit, independently of
+  // the test runner's potentially long redirected temporary directory.
+  const parent = await mkdtemp("/tmp/rhv-");
+  onTestFinished(() => rm(parent, { recursive: true, force: true }));
+  return parent;
+};

tests/process-global/curatedWindowsLane.test.ts

@@ -4,23 +4,48 @@ import { join } from "node:path";
 import { describe, expect, it } from "vitest";
 
 import { parse } from "yaml";
+import { z } from "zod";
 
 const WORKFLOW = join(".github", "workflows", "ci.yml");
 
-const curatedPaths = async (): Promise<string[]> => {
-  const workflow = parse(await readFile(WORKFLOW, "utf8")) as {
-    jobs?: Record<string, { steps?: { run?: string }[] }>;
-  };
-  const run = workflow.jobs?.["windows-curated"]?.steps
-    ?.map((step) => step.run)
-    .find((value) => value?.includes("npx vitest run"));
-  expect(run, "expected a curated Windows vitest invocation").toBeDefined();
-  return (
-    run
-      ?.split(/\s+/)
-      .filter((token) => token.endsWith(".test.ts"))
-      .sort() ?? []
+const workflowSchema = z.object({
+  jobs: z.record(
+    z.string(),
+    z.object({
+      steps: z.array(z.object({ run: z.string().optional() })).optional(),
+    }),
+  ),
+});
+
+const explicitPaths = async (
+  lane: "windows" | "focused",
+): Promise<string[]> => {
+  const workflow = workflowSchema.parse(
+    parse(await readFile(WORKFLOW, "utf8")),
   );
+  const runs =
+    lane === "windows"
+      ? (workflow.jobs["windows-curated"]?.steps
+          ?.map((step) => step.run)
+          .filter((value) => value?.includes("npx vitest run")) ?? [])
+      : Object.values(workflow.jobs)
+          .flatMap((job) => job.steps ?? [])
+          .map((step) => step.run)
+          .filter((value) =>
+            value?.trim().startsWith("npm run test:focused -- "),
+          );
+  expect(
+    runs.length,
+    `expected explicit ${lane} CI test invocations`,
+  ).toBeGreaterThan(0);
+  return [
+    ...new Set(
+      runs.flatMap(
+        (run) =>
+          run?.split(/\s+/).filter((token) => token.endsWith(".test.ts")) ?? [],
+      ),
+    ),
+  ].sort();
 };
 
 const exists = async (path: string): Promise<boolean> =>
@@ -31,7 +56,7 @@ const exists = async (path: string): Promise<boolean> =>
 
 describe("curated Windows test lane", () => {
   it("references only test files that exist", async () => {
-    const paths = await curatedPaths();
+    const paths = await explicitPaths("windows");
     expect(paths.length).toBeGreaterThan(0);
     const missing = await Promise.all(
       paths.map(async (path) => ((await exists(path)) ? undefined : path)),
@@ -39,3 +64,12 @@ describe("curated Windows test lane", () => {
     expect(missing.filter((path) => path !== undefined)).toEqual([]);
   }, 10_000);
 });
+
+it("keeps every explicit focused CI test path executable after file moves", async () => {
+  const paths = await explicitPaths("focused");
+  expect(paths.length).toBeGreaterThan(0);
+  const missing = await Promise.all(
+    paths.map(async (path) => ((await exists(path)) ? undefined : path)),
+  );
+  expect(missing.filter((path) => path !== undefined)).toEqual([]);
+}, 10_000);