Merge pull request #233 from morluto/agent/process-zombie-settlement

fix(process): settle exited zombie groups

src/application/ProcessOwnership.ts

@@ -19,6 +19,7 @@ interface ProcessGroupMember {
   readonly pid: number;
   readonly parentPid: number;
   readonly processGroupId: number;
+  readonly state: string;
   readonly command: string;
 }
 
@@ -30,9 +31,19 @@ export interface ProcessOwnershipHost {
 }
 
 /** Cleanup succeeds only when the group is absent or every member is owned. */
+interface ProcessOwnershipValidationFailure {
+  readonly pid: number;
+  readonly reason: "environment-unreadable" | "run-token-mismatch";
+}
+
+/** Cleanup outcome with per-member diagnostics when ownership is uncertain. */
 export type ProcessCleanupResult =
   | { readonly cleaned: true; readonly signaled: boolean }
-  | { readonly cleaned: false; readonly reason: string };
+  | {
+      readonly cleaned: false;
+      readonly reason: string;
+      readonly failures?: readonly ProcessOwnershipValidationFailure[];
+    };
 
 /** Token-verified liveness result used by post-root-exit settlement. */
 export type ProcessGroupObservation =
@@ -59,17 +70,18 @@ const systemHost: ProcessOwnershipHost = {
     if (process.platform === "win32") return [];
     const { stdout } = await execFileAsync("ps", [
       "-axo",
-      "pid=,ppid=,pgid=,command=",
+      "pid=,ppid=,pgid=,stat=,command=",
     ]);
     return stdout
       .split("\n")
-      .map((line) => /\s*(\d+)\s+(\d+)\s+(\d+)\s+(.*)/u.exec(line))
+      .map((line) => /\s*(\d+)\s+(\d+)\s+(\d+)\s+(\S+)\s+(.*)/u.exec(line))
       .filter((match): match is RegExpExecArray => match !== null)
       .map((match) => ({
         pid: Number(match[1]),
         parentPid: Number(match[2]),
         processGroupId: Number(match[3]),
-        command: match[4] ?? "",
+        state: match[4] ?? "",
+        command: match[5] ?? "",
       }))
       .filter((member) => member.processGroupId === processGroupId);
   },
@@ -109,8 +121,11 @@ export const cleanupOwnedProcessGroup = async (
   } catch {
     return { cleaned: false, reason: "process group could not be inspected" };
   }
-  if (members.length === 0) return { cleaned: true, signaled: false };
-  const leader = members.find((member) => member.pid === ownership.leaderPid);
+  const liveMembers = liveProcessGroupMembers(members);
+  if (liveMembers.length === 0) return { cleaned: true, signaled: false };
+  const leader = liveMembers.find(
+    (member) => member.pid === ownership.leaderPid,
+  );
   if (leader !== undefined) {
     if (
       ownership.expectedParentPid !== undefined &&
@@ -129,23 +144,24 @@ export const cleanupOwnedProcessGroup = async (
         reason: "owned launcher command identity did not match",
       };
   }
-  for (const member of members) {
-    let environment: Readonly<Record<string, string>>;
+  const failures: ProcessOwnershipValidationFailure[] = [];
+  for (const member of liveMembers) {
     try {
-      environment = await host.environment(member.pid);
+      const environment = await host.environment(member.pid);
+      if (environment.REA_PROCESS_RUN_ID !== ownership.runId)
+        failures.push({ pid: member.pid, reason: "run-token-mismatch" });
     } catch {
-      return {
-        cleaned: false,
-        reason: "process ownership could not be revalidated",
-      };
-    }
-    if (environment.REA_PROCESS_RUN_ID !== ownership.runId) {
-      return {
-        cleaned: false,
-        reason: "process group contains an unowned or PID-reused process",
-      };
+      failures.push({ pid: member.pid, reason: "environment-unreadable" });
     }
   }
+  if (failures.length > 0)
+    return {
+      cleaned: false,
+      reason: failures.some(({ reason }) => reason === "run-token-mismatch")
+        ? "process group contains an unowned or PID-reused process"
+        : "process ownership could not be revalidated",
+      failures,
+    };
   try {
     host.signalGroup(ownership.processGroupId, "SIGKILL");
   } catch (cause: unknown) {
@@ -171,8 +187,9 @@ export const observeOwnedProcessGroup = async (
       reason: "process group could not be inspected",
     };
   }
-  if (members.length === 0) return { state: "empty" };
-  for (const member of members) {
+  const liveMembers = liveProcessGroupMembers(members);
+  if (liveMembers.length === 0) return { state: "empty" };
+  for (const member of liveMembers) {
     try {
       if (
         (await host.environment(member.pid)).REA_PROCESS_RUN_ID !==
@@ -192,6 +209,11 @@ export const observeOwnedProcessGroup = async (
   return { state: "alive" };
 };
 
+const liveProcessGroupMembers = (
+  members: readonly ProcessGroupMember[],
+): readonly ProcessGroupMember[] =>
+  members.filter(({ state }) => !state.startsWith("Z"));
+
 const commandMatches = (actual: string, expected: string): boolean => {
   const normalizedActual = actual.trim();
   const normalizedExpected = expected.trim();

tests/processOwnership.test.ts

@@ -1,6 +1,7 @@
 import { describe, expect, it, vi } from "vitest";
 import {
   cleanupOwnedProcessGroup,
+  observeOwnedProcessGroup,
   parseProcessEnvironment,
   type ProcessOwnershipHost,
 } from "../src/application/ProcessOwnership.js";
@@ -26,6 +27,7 @@ const host = (
             pid: Number(pid),
             parentPid: Number(pid) === 100 ? 1 : 100,
             processGroupId: 100,
+            state: "S",
             command: "fixture",
           })),
         ),
@@ -63,10 +65,108 @@ describe("owned process-group cleanup", () => {
     expect(await cleanupOwnedProcessGroup(ownership, adapter)).toEqual({
       cleaned: false,
       reason: "process group contains an unowned or PID-reused process",
+      failures: [{ pid: 101, reason: "run-token-mismatch" }],
     });
     expect(signalGroup).not.toHaveBeenCalled();
   });
 
+  it("checks every member and aggregates ownership read failures", async () => {
+    const environment = vi.fn((pid: number) => {
+      if (pid === 100)
+        return Promise.reject(new Error("transient procfs read"));
+      return Promise.resolve(
+        pid === 101
+          ? { REA_PROCESS_RUN_ID: "different-run" }
+          : { REA_PROCESS_RUN_ID: "run-token" },
+      );
+    });
+    const signalGroup = vi.fn();
+    const adapter: ProcessOwnershipHost = {
+      listMembers: () =>
+        Promise.resolve(
+          [100, 101, 102].map((pid) => ({
+            pid,
+            parentPid: pid === 100 ? 1 : 100,
+            processGroupId: 100,
+            state: "S",
+            command: "fixture",
+          })),
+        ),
+      environment,
+      signalGroup,
+    };
+
+    expect(await cleanupOwnedProcessGroup(ownership, adapter)).toEqual({
+      cleaned: false,
+      reason: "process group contains an unowned or PID-reused process",
+      failures: [
+        { pid: 100, reason: "environment-unreadable" },
+        { pid: 101, reason: "run-token-mismatch" },
+      ],
+    });
+    expect(environment.mock.calls.map(([pid]) => pid)).toEqual([100, 101, 102]);
+    expect(signalGroup).not.toHaveBeenCalled();
+  });
+
+  it("ignores exited zombie members during live ownership checks", async () => {
+    const environment = vi.fn((pid: number) =>
+      Promise.resolve(pid === 101 ? {} : { REA_PROCESS_RUN_ID: "run-token" }),
+    );
+    const signalGroup = vi.fn();
+    const adapter: ProcessOwnershipHost = {
+      listMembers: () =>
+        Promise.resolve([
+          {
+            pid: 100,
+            parentPid: 1,
+            processGroupId: 100,
+            state: "S",
+            command: "fixture",
+          },
+          {
+            pid: 101,
+            parentPid: 100,
+            processGroupId: 100,
+            state: "Z",
+            command: "[node] <defunct>",
+          },
+        ]),
+      environment,
+      signalGroup,
+    };
+
+    expect(await cleanupOwnedProcessGroup(ownership, adapter)).toEqual({
+      cleaned: true,
+      signaled: true,
+    });
+    expect(environment).toHaveBeenCalledTimes(1);
+    expect(environment).toHaveBeenCalledWith(100);
+    expect(signalGroup).toHaveBeenCalledWith(100, "SIGKILL");
+  });
+
+  it("observes a zombie-only group as settled", async () => {
+    const environment = vi.fn(() => Promise.resolve({}));
+    const adapter: ProcessOwnershipHost = {
+      listMembers: () =>
+        Promise.resolve([
+          {
+            pid: 101,
+            parentPid: 1,
+            processGroupId: 100,
+            state: "Z+",
+            command: "[node] <defunct>",
+          },
+        ]),
+      environment,
+      signalGroup: vi.fn(),
+    };
+
+    expect(await observeOwnedProcessGroup(ownership, adapter)).toEqual({
+      state: "empty",
+    });
+    expect(environment).not.toHaveBeenCalled();
+  });
+
   it("is idempotent when the owned group has already exited", async () => {
     const { adapter } = host({});
     expect(await cleanupOwnedProcessGroup(ownership, adapter)).toEqual({