fix(process): settle owned cleanup with the launcher's own exit proof (#6205)

* fix(process): settle owned cleanup with the launcher's own exit proof

When owned process-group verification fails while the supervised
launcher is still mid-exit, the supervisor now waits up to the kill grace
for its own child-exit observation and verifies once more after it
arrives. Verification is not repeated when the exit was already observed
before cleanup started, so there is no new evidence and the original
failure is kept. Nothing is signaled based on the wait.

Fixes #6134

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

* test(process): bound Hopper session cleanup attempts by exit evidence

The fake Hopper launcher exits after its shutdown acknowledgement. When
the supervisor first observes that exit during a failed verification, it
verifies once more, so the first close makes one or two attempts depending
on timing. The test now asserts that bound. Later session retries, made
after the exit was already observed, make exactly one attempt each.

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

---------

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

src/process/ProviderProcess.launcherExit.test.ts

@@ -0,0 +1,240 @@
+import { EventEmitter } from "node:events";
+import { PassThrough } from "node:stream";
+import { afterEach, expect, it, vi } from "vitest";
+import {
+  cleanupOwnedProcessGroup,
+  type OwnedProcessGroup,
+  type ProcessCleanupResult,
+  type ProcessIdentityObservation,
+  type ProcessOwnershipHost,
+  type ProcessTableEntry,
+} from "./ProcessOwnership.js";
+import { ProcessOwnershipEvidenceOwner } from "./ProcessOwnershipEvidence.js";
+import { ProviderProcessSupervisor } from "./ProviderProcess.js";
+
+const JAVA = "/usr/lib/jvm/java-21-openjdk-amd64/bin/java";
+
+const ownership = (): OwnedProcessGroup => ({
+  runId: "run-6134",
+  leaderPid: 910,
+  processGroupId: 910,
+  expectedParentPid: 17,
+  expectedCommand: JAVA,
+  sweepTokenOwnedProcesses: true,
+  evidenceOwner: new ProcessOwnershipEvidenceOwner(),
+});
+
+const launcherRow = (command: string): ProcessTableEntry => ({
+  pid: 910,
+  parentPid: 17,
+  processGroupId: 910,
+  state: "R",
+  command,
+});
+
+const nativeIdentity = (
+  lifecycle: "non-zombie" | "exiting",
+): ProcessIdentityObservation => ({
+  state: "readable",
+  identity: "linux-starttime:4242",
+  lifecycle: { state: "observed", lifecycle },
+  relation: { state: "observed", parentPid: 17, processGroupId: 910 },
+});
+
+/** A launcher child whose exit the supervisor observes only when the test says so. */
+const launcherChild = () => {
+  const child = Object.assign(new EventEmitter(), {
+    stdout: new PassThrough(),
+    stderr: new PassThrough(),
+    exitCode: null,
+    signalCode: null,
+    kill: vi.fn(() => false),
+  });
+  let exited = false;
+  return {
+    child,
+    get exited() {
+      return exited;
+    },
+    exit() {
+      exited = true;
+      child.emit("exit", 0, null);
+      child.stdout.end();
+      child.stderr.end();
+      child.emit("close", 0, null);
+    },
+  };
+};
+
+/** Linux-like host: the launcher row stays listed until the child exits. */
+const hostFor = (
+  launcher: ReturnType<typeof launcherChild>,
+  row: ProcessTableEntry,
+  lifecycle?: "non-zombie" | "exiting",
+): ProcessOwnershipHost => {
+  const host: ProcessOwnershipHost = {
+    platform: "linux",
+    listProcesses: vi.fn(async () => (launcher.exited ? [] : [row])),
+    environment: vi.fn(async () => ({ REA_PROCESS_RUN_ID: "run-6134" })),
+    signalGroup: vi.fn(),
+  };
+  if (lifecycle === undefined) return host;
+  return {
+    ...host,
+    processIdentities: vi.fn(async (rows: readonly ProcessTableEntry[]) => {
+      const observation: ProcessIdentityObservation = launcher.exited
+        ? { state: "absent", reason: "not-found-in-caller-pid-namespace" }
+        : nativeIdentity(lifecycle);
+      return new Map(rows.map(({ pid }) => [pid, observation] as const));
+    }),
+  };
+};
+
+const supervise = (
+  launcher: ReturnType<typeof launcherChild>,
+  host: ProcessOwnershipHost,
+) => {
+  const results: ProcessCleanupResult[] = [];
+  const supervisor = new ProviderProcessSupervisor({
+    process: launcher.child,
+    ownsProcessLifetime: true,
+    ownership: ownership(),
+    cleanup: async () => {
+      const result = await cleanupOwnedProcessGroup(ownership(), host);
+      results.push(result);
+      return result;
+    },
+  });
+  return { supervisor, results };
+};
+
+afterEach(() => {
+  vi.restoreAllMocks();
+});
+
+it("re-verifies after the launcher's own exit settles an exiting procps row", async () => {
+  const launcher = launcherChild();
+  const host = hostFor(launcher, launcherRow("[java]"));
+  const { supervisor, results } = supervise(launcher, host);
+  // The JVM finishes tearing down only after the single table refresh.
+  vi.mocked(host.listProcesses).mockImplementation(async () => {
+    if (launcher.exited) return [];
+    setTimeout(() => launcher.exit(), 20);
+    return [launcherRow("[java]")];
+  });
+
+  await expect(supervisor.stop()).resolves.toEqual({
+    status: "verified-cleanup",
+  });
+  expect(results).toEqual([
+    {
+      cleaned: false,
+      reason: `owned launcher command identity did not match (observed=[java]; expected=${JAVA})`,
+    },
+    { cleaned: true, signaled: false },
+  ]);
+  expect(host.signalGroup).not.toHaveBeenCalled();
+  expect(launcher.child.kill).not.toHaveBeenCalled();
+});
+
+it("re-verifies after native exit-in-progress outlasts the settlement deadline", async () => {
+  const launcher = launcherChild();
+  const host = hostFor(launcher, launcherRow("[java]"), "exiting");
+  const { supervisor, results } = supervise(launcher, host);
+  const exitTimer = setTimeout(() => launcher.exit(), 1_200);
+  try {
+    await expect(supervisor.stop({ killGraceMs: 2_000 })).resolves.toEqual({
+      status: "verified-cleanup",
+    });
+  } finally {
+    clearTimeout(exitTimer);
+  }
+  expect(results).toEqual([
+    { cleaned: false, reason: "owned launcher exit could not be confirmed" },
+    { cleaned: true, signaled: false },
+  ]);
+  expect(host.signalGroup).not.toHaveBeenCalled();
+});
+
+it.each([
+  ["identityless", undefined],
+  ["native", "non-zombie"],
+] as const)(
+  "keeps a still-live mismatched %s launcher incomplete without signaling",
+  async (_host, lifecycle) => {
+    const launcher = launcherChild();
+    const host = hostFor(launcher, launcherRow("/usr/bin/other"), lifecycle);
+    const { supervisor, results } = supervise(launcher, host);
+    const started = performance.now();
+
+    const stopped = await supervisor.stop({ killGraceMs: 50 });
+
+    expect(performance.now() - started).toBeGreaterThanOrEqual(45);
+    expect(stopped).toEqual({
+      status: "incomplete",
+      reason: `owned launcher command identity did not match (observed=/usr/bin/other; expected=${JAVA})`,
+      cleanupResult: results[0],
+    });
+    expect(results).toHaveLength(1);
+    expect(host.signalGroup).not.toHaveBeenCalled();
+    expect(launcher.child.kill).not.toHaveBeenCalled();
+    supervisor.dispose();
+  },
+);
+
+it("does not repeat verification without new launcher exit evidence", async () => {
+  const launcher = launcherChild();
+  const cleanup = vi.fn(async (): Promise<ProcessCleanupResult> => ({
+    cleaned: false,
+    reason: "process table could not be inspected: spawn EPERM",
+  }));
+  const supervisor = new ProviderProcessSupervisor({
+    process: launcher.child,
+    ownsProcessLifetime: true,
+    ownership: ownership(),
+    cleanup,
+  });
+  launcher.exit();
+  const started = performance.now();
+
+  await expect(supervisor.stop()).resolves.toMatchObject({
+    status: "incomplete",
+    reason: "process table could not be inspected: spawn EPERM",
+  });
+  expect(cleanup).toHaveBeenCalledOnce();
+  expect(performance.now() - started).toBeLessThan(500);
+});
+
+it("retains the post-exit verification failure for a later owned retry", async () => {
+  const launcher = launcherChild();
+  const outcomes: ProcessCleanupResult[] = [
+    { cleaned: false, reason: "owned launcher exit could not be confirmed" },
+    {
+      cleaned: false,
+      reason: "process ownership could not be revalidated for PID 911: EACCES",
+    },
+    { cleaned: true, signaled: true },
+  ];
+  const cleanup = vi.fn(async (): Promise<ProcessCleanupResult> => {
+    const next = outcomes.shift();
+    if (next === undefined) throw new Error("unexpected cleanup attempt");
+    if (cleanup.mock.calls.length === 1) setTimeout(() => launcher.exit(), 10);
+    return next;
+  });
+  const supervisor = new ProviderProcessSupervisor({
+    process: launcher.child,
+    ownsProcessLifetime: true,
+    ownership: ownership(),
+    cleanup,
+  });
+
+  await expect(supervisor.stop()).resolves.toMatchObject({
+    status: "incomplete",
+    reason: "process ownership could not be revalidated for PID 911: EACCES",
+  });
+  expect(cleanup).toHaveBeenCalledTimes(2);
+  await expect(supervisor.stop()).resolves.toEqual({
+    status: "verified-cleanup",
+  });
+  expect(cleanup).toHaveBeenCalledTimes(3);
+});

src/process/ProviderProcess.ts

@@ -225,6 +225,14 @@ export type ProviderProcessStopResult =
       readonly cleanupResult?: ProcessCleanupFailure;
     };
 
+const incompleteCleanup = (
+  failure: ProcessCleanupFailure,
+): ProviderProcessStopResult => ({
+  status: "incomplete",
+  reason: failure.reason,
+  cleanupResult: failure,
+});
+
 /**
  * Spawn a provider in an owned Windows job or dedicated POSIX process group.
  *
@@ -675,23 +683,24 @@ export class ProviderProcessSupervisor {
     try {
       if (!this.launch.ownsProcessLifetime) return { status: "not-owned" };
       if (this.launch.cleanup !== undefined) {
-        const cleanup = await this.#ownedCleanup(options.cleanupResult);
-        if (!cleanup.cleaned)
-          return {
-            status: "incomplete",
-            reason: cleanup.reason,
-            cleanupResult: cleanup,
-          };
         const graceMs = options.killGraceMs ?? DEFAULT_KILL_GRACE_MS;
+        const exitObservedBeforeCleanup = this.#exitObservation !== undefined;
+        const cleanup = await this.#ownedCleanup(options.cleanupResult);
+        if (!cleanup.cleaned) {
+          // Process tables can show the launcher mid-exit until verification
+          // gives up. The supervisor's own exit observation is positive proof
+          // that verification lacked, so verify again only after it arrives.
+          if (exitObservedBeforeCleanup || !(await this.waitForExit(graceMs)))
+            return incompleteCleanup(cleanup);
+          const settled = await this.launch.cleanup();
+          return settled.cleaned
+            ? { status: "verified-cleanup" }
+            : incompleteCleanup(settled);
+        }
         if (await this.waitForExit(graceMs))
           return { status: "verified-cleanup" };
         const retried = await this.launch.cleanup();
-        if (!retried.cleaned)
-          return {
-            status: "incomplete",
-            reason: retried.reason,
-            cleanupResult: retried,
-          };
+        if (!retried.cleaned) return incompleteCleanup(retried);
         if (await this.waitForExit(graceMs))
           return { status: "verified-cleanup" };
         return {

tests/boundary/process/binarySessionCleanupRetry.test.ts

@@ -129,7 +129,12 @@ it("retains Hopper process handles, private files, and lease until an actual ses
   if (runtime === undefined)
     throw new Error("Fixture did not capture its runtime");
   await access(runtime);
-  expect(launcher.cleanupAttempts).toBe(1);
+  // The fixture exits after its shutdown acknowledgement. When the supervisor
+  // observes that exit only after a failed verification began, it verifies
+  // once more; a failed stop never runs more than two verifications.
+  const firstCloseAttempts = launcher.cleanupAttempts;
+  expect(firstCloseAttempts).toBeGreaterThanOrEqual(1);
+  expect(firstCloseAttempts).toBeLessThanOrEqual(2);
   expect(await launcher.acquire("other-owner")).toMatchObject({
     acquired: false,
   });
@@ -138,10 +143,11 @@ it("retains Hopper process handles, private files, and lease until an actual ses
     error: { cleanupIncomplete: true },
   });
   expect(clientsCreated).toBe(1);
-  expect(launcher.cleanupAttempts).toBe(2);
+  // The launcher exit was already observed, so the retry has no new evidence.
+  expect(launcher.cleanupAttempts).toBe(firstCloseAttempts + 1);
   launcher.cleanupAllowed = true;
   expect(await session.close()).toEqual(resultOk(null));
-  expect(launcher.cleanupAttempts).toBe(3);
+  expect(launcher.cleanupAttempts).toBe(firstCloseAttempts + 2);
   await expect(access(runtime)).rejects.toMatchObject({ code: "ENOENT" });
   const replacement = await launcher.acquire("replacement-owner");
   expect(replacement.acquired).toBe(true);