[GH-ISSUE #7273] [Bug] doctor Hopper probe reports cleanup released after a child changes groups #4315

New issue

Closed

opened 2026-10-31 00:48:51 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:48:51 +00:00

Owner

Originally created by @coygeek on GitHub (Oct 26, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/7273

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, tokens, private paths, and sensitive target data.

Area

Process capture or runtime observation

Observed behavior

A controlled production-boundary test of a timed-out private-display probe reported cleanup.status: "released" while a token-owned child remained alive in another process group. The production Linux process inspector, cleanup implementation and supervisor ran; process-table, procfs, spawn and signal system calls were simulated. The public CLI route was source-traced rather than executed. No real Hopper, Xvfb or operating-system group move was reproduced.

Reproduction

Prepare the baseline checkout with its locked dependencies. Save the reproduction source at the end of this report as src/process/cleanupMovement.test.ts. Its imports are relative to that directory.
Run npm run test:local -- src/process/cleanupMovement.test.ts, then remove that temporary test. The verifier executed identical companion bytes under a disposable filename; the final focused run passed one test. An earlier attempt exposed fixture-control errors, which were corrected before the passing reruns.
The test calls the production direct private-display probe runner with a one-millisecond timeout and no cancellation. Its helper is PID 100/group 100; its child is PID 101/group 100. Both have readable matching tokens, stable start identities and non-zombie native state. After the plan's relation snapshot, the child changes to group 101 during its token read. Parentage changes only after helper termination.
The probe releases cleanup after signaling only group 100, and the companion asserts that child 101 still lives. The recovery control enables the existing token sweep, finds group 101 and removes that survivor.

The source-traced entry point is node scripts/rea.mjs doctor --provider hopper on Linux with a discovered executable Hopper whose linked dependencies are available. The real CLI timing case remains unexecuted. The companion launches and signals no actual processes.

Expected behavior

An owned child must remain represented until release is established. If it changes groups during cleanup, either confirm its termination or return pending cleanup with retained retry ownership. AGENTS.md, Architecture and Cleanup, requires retaining resources after failed cleanup. docs/tool-design.md, Own the lifecycle, distinguishes launcher exit from resource release and requires retryable unresolved cleanup.

Impact and scope

P2: a bounded owned-resource leak and incorrect release claim under the demonstrated event schedule. Natural frequency, real Linux/provider behavior and severe host effects are unknown. The internal sweep control recovers the simulated survivor, but no public workaround was demonstrated. Other production callers were inspected; this report's executed evidence is restricted to the private-display probe boundary.

REA version

615fbbbdd7 (package.json version 6.8.0)

Runtime and platform

Node v26.12.1, Vitest 5.0.3, macOS arm64. The production Linux host is selected before module import and its native inputs are simulated. The pinned Node 24.22.1 runtime was not exercised.

Target and artifact context

Simulated owned Python probe and its Xvfb-like child. No target binary, provider installation or device was used.

Interface

CLI

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

The executed probe returned this subset, with only the helper group signaled:

{"result":{"outcome":"timed-out","cleanup":{"status":"released"}},"signals":[100],"survivor":{"live":true,"pid":101,"group":101,"tokenRetained":true}}

The recovery control then returned:

{"control":{"cleaned":true,"signaled":true},"signals":[100,101],"survivorLive":false}

Source-derived reachability: src/cli/setupCommands.ts:117–148 composes doctor with the system host; src/application/Doctor.ts:225–280 collects diagnostics; src/application/DoctorDiagnostics.ts:164–187 and 301–307 admit the discovered executable and Linux demo check; src/doctorRuntime.ts:109–131 reaches the probe. src/hopper/LinuxPrivateDisplayProbe.ts:120–167 acquires the helper, registers group cleanup and releases the lease when its supervisor reports verified cleanup. The companion exercises the production Linux host, following the procfs fixture style in src/process/ProcessOwnership.procIdentity.test.ts; it injects no ownership host or cleanup result.

Typed outcome or recovery expected

The same moved-child case must leave cleanup pending with retry ownership, or stop the child and establish release. Helper exit by itself must not release the surviving child. The token-sweep recovery control must continue to remove the survivor, and foreign-token or unknown-identity cases must remain fail-closed.

Analysis or suspected cause

Source-derived: src/process/ProcessOwnership.ts:1284–1288 skips a member after a native group change without retaining a failure or adding its new group to the planned queue. Revalidation at 1126–1144 only considers current members of the old group, and the non-swept branch returns success at 470–474. This explains the executed simulated-boundary result; it does not establish the frequency of such a transition in a real provider.

Additional context

The reproduction source is included below. Real CLI execution, real group-movement timing and the pinned Node runtime remain unverified. Closed issue #5206 covers a different exit-refresh candidate loss. Closed issue #3985 covers discarding a probe owner after cleanup had already failed. Open issue #4322 is the opposite result, a false mismatch after an owned process exits. Open issue #7272 classifies an unresolved unrelated process as a token mismatch. This report is the false released result after a child changes groups. No same-behavior open issue was found.

import { afterEach, expect, it, vi } from "vitest";
import { EventEmitter } from "node:events";
import { PassThrough } from "node:stream";

const originalPlatform = vi.hoisted(() => {
  const descriptor = Object.getOwnPropertyDescriptor(process, "platform")!;
  Object.defineProperty(process, "platform", { value: "linux", configurable: true });
  return descriptor;
});
const state = vi.hoisted(() => ({
  readFile: vi.fn(),
  spawn: vi.fn(),
  ps: vi.fn(),
}));
vi.mock("node:fs/promises", async (original) => ({
  ...await original<typeof import("node:fs/promises")>(),
  readFile: state.readFile,
}));
vi.mock("node:child_process", async (original) => ({
  ...await original<typeof import("node:child_process")>(),
  spawn: state.spawn,
}));
vi.mock("./ExecFileOutput.js", () => ({ execFileOutput: state.ps }));

import { runLinuxPrivateDisplayProbe } from "../hopper/LinuxPrivateDisplayProbe.js";
import { cleanupOwnedProcessGroup } from "./ProcessOwnership.js";
import { systemProcessOwnershipHost } from "./ProcessOwnershipObservation.js";
import { ProcessOwnershipEvidenceOwner } from "./ProcessOwnershipEvidence.js";

afterEach(() => { vi.restoreAllMocks(); Object.defineProperty(process, "platform", originalPlatform); });

it("runs the production private-display timeout cleanup through the Linux host", async () => {
  expect(systemProcessOwnershipHost.platform).toBe("linux");
  const signals: number[] = [];
  let token = "";
  let moved = false;
  let rootLive = true;
  let childLive = true;
  const helper = Object.assign(new EventEmitter(), {
    pid: 100,
    exitCode: null as number | null,
    signalCode: null as NodeJS.Signals | null,
    stdout: new PassThrough(),
    stderr: new PassThrough(),
    kill: vi.fn(),
    unref: vi.fn(),
  });
  state.spawn.mockImplementation((_command, _arguments, options) => {
    token = options.env.REA_PROCESS_RUN_ID;
    queueMicrotask(() => helper.emit("spawn"));
    return helper;
  });
  const rows = () => [
    `${process.pid} ${process.ppid} ${process.pid} 1000 S verifier`,
    ...(rootLive ? [`100 ${process.pid} 100 1000 S /usr/bin/python3 helper.py`] : []),
    ...(childLive ? [`101 ${rootLive ? 100 : 1} ${moved ? 101 : 100} 1000 S Xvfb`] : []),
  ];
  state.ps.mockImplementation(async (command) => {
    expect(command).toBe("ps");
    return { stdout: rows().join("\n"), stderr: "" };
  });
  state.readFile.mockImplementation(async (path) => {
    const match = /^\/proc\/(\d+)\/(stat|environ)$/u.exec(String(path));
    if (match === null) throw new Error(`Unexpected read: ${String(path)}`);
    const pid = Number(match[1]);
    if ((pid === 100 && !rootLive) || (pid === 101 && !childLive))
      throw Object.assign(new Error("ESRCH"), { code: "ESRCH" });
    if (match[2] === "environ") {
      // A same-generation child moves after the native relation snapshot,
      // during the identity/token/identity bracket used by the real host.
      if (pid === 101) moved = true;
      return pid === process.pid ? "FOREIGN=value\0" : `REA_PROCESS_RUN_ID=${token}\0`;
    }
    const parent = pid === 100 ? process.pid : pid === 101 ? (rootLive ? 100 : 1) : process.ppid;
    const group = pid === 101 ? (moved ? 101 : 100) : pid === 100 ? 100 : process.pid;
    const fields = ["S", String(parent), String(group), "100", "0", "0", "0"];
    while (fields.length < 19) fields.push("0");
    fields.push(String(pid * 10));
    return `${pid} (fixture) ${fields.join(" ")}`;
  });
  vi.spyOn(process, "kill").mockImplementation((pid, signal) => {
    if (signal === 0 && ((pid === 100 && !rootLive) || (pid === 101 && !childLive))) throw Object.assign(new Error("ESRCH"), { code: "ESRCH" });
    if (pid >= 0 || signal !== "SIGKILL") throw new Error(`Unexpected signal ${pid}`);
    signals.push(-pid);
    if (pid === -100) {
      rootLive = false;
      if (!moved) childLive = false;
      helper.signalCode = "SIGKILL";
      helper.stdout.end(); helper.stderr.end();
      helper.emit("exit", null, "SIGKILL");
      helper.emit("close", null, "SIGKILL");
    } else if (pid === -101) childLive = false;
    return true;
  });
  const result = await runLinuxPrivateDisplayProbe("direct", {
    helperPath: "scripts/hopper-demo-x11.py",
    timeoutMs: 1,
    environment: {},
  });
  console.log(JSON.stringify({ boundary: "production Linux private-display probe", result, signals, survivor: { live: childLive, pid: 101, group: moved ? 101 : 100, tokenRetained: token.length > 0 } }));
  expect(result).toMatchObject({ outcome: "timed-out", cleanup: { status: "released" } });
  expect(signals).toEqual([100]);
  expect(childLive).toBe(true);
  expect(moved).toBe(true);
  // Recovery control: the production token-swept owner discovers the survivor.
  const control = await cleanupOwnedProcessGroup({ runId: token, leaderPid: 100, processGroupId: 100, sweepTokenOwnedProcesses: true, evidenceOwner: new ProcessOwnershipEvidenceOwner() });
  console.log(JSON.stringify({ boundary: "token-sweep recovery control", control, signals, survivorLive: childLive }));
  expect(control.cleaned).toBe(true);
  expect(signals).toEqual([100, 101]);
  expect(childLive).toBe(false);
});

Originally created by @coygeek on GitHub (Oct 26, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/7273 ### Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ### Area Process capture or runtime observation ### Observed behavior A controlled production-boundary test of a timed-out private-display probe reported `cleanup.status: "released"` while a token-owned child remained alive in another process group. The production Linux process inspector, cleanup implementation and supervisor ran; process-table, procfs, spawn and signal system calls were simulated. The public CLI route was source-traced rather than executed. No real Hopper, Xvfb or operating-system group move was reproduced. ### Reproduction 1. Prepare the baseline checkout with its locked dependencies. Save the reproduction source at the end of this report as `src/process/cleanupMovement.test.ts`. Its imports are relative to that directory. 2. Run `npm run test:local -- src/process/cleanupMovement.test.ts`, then remove that temporary test. The verifier executed identical companion bytes under a disposable filename; the final focused run passed one test. An earlier attempt exposed fixture-control errors, which were corrected before the passing reruns. 3. The test calls the production direct private-display probe runner with a one-millisecond timeout and no cancellation. Its helper is PID 100/group 100; its child is PID 101/group 100. Both have readable matching tokens, stable start identities and non-zombie native state. After the plan's relation snapshot, the child changes to group 101 during its token read. Parentage changes only after helper termination. 4. The probe releases cleanup after signaling only group 100, and the companion asserts that child 101 still lives. The recovery control enables the existing token sweep, finds group 101 and removes that survivor. The source-traced entry point is `node scripts/rea.mjs doctor --provider hopper` on Linux with a discovered executable Hopper whose linked dependencies are available. The real CLI timing case remains unexecuted. The companion launches and signals no actual processes. ### Expected behavior An owned child must remain represented until release is established. If it changes groups during cleanup, either confirm its termination or return pending cleanup with retained retry ownership. `AGENTS.md`, Architecture and Cleanup, requires retaining resources after failed cleanup. `docs/tool-design.md`, Own the lifecycle, distinguishes launcher exit from resource release and requires retryable unresolved cleanup. ### Impact and scope P2: a bounded owned-resource leak and incorrect release claim under the demonstrated event schedule. Natural frequency, real Linux/provider behavior and severe host effects are unknown. The internal sweep control recovers the simulated survivor, but no public workaround was demonstrated. Other production callers were inspected; this report's executed evidence is restricted to the private-display probe boundary. ### REA version 615fbbbdd750de2df5d92b9827d0c7c4422c1d03 (package.json version 6.8.0) ### Runtime and platform Node v26.12.1, Vitest 5.0.3, macOS arm64. The production Linux host is selected before module import and its native inputs are simulated. The pinned Node 24.22.1 runtime was not exercised. ### Target and artifact context Simulated owned Python probe and its Xvfb-like child. No target binary, provider installation or device was used. ### Interface CLI ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence The executed probe returned this subset, with only the helper group signaled: ```json {"result":{"outcome":"timed-out","cleanup":{"status":"released"}},"signals":[100],"survivor":{"live":true,"pid":101,"group":101,"tokenRetained":true}} ``` The recovery control then returned: ```json {"control":{"cleaned":true,"signaled":true},"signals":[100,101],"survivorLive":false} ``` Source-derived reachability: `src/cli/setupCommands.ts:117–148` composes doctor with the system host; `src/application/Doctor.ts:225–280` collects diagnostics; `src/application/DoctorDiagnostics.ts:164–187` and `301–307` admit the discovered executable and Linux demo check; `src/doctorRuntime.ts:109–131` reaches the probe. `src/hopper/LinuxPrivateDisplayProbe.ts:120–167` acquires the helper, registers group cleanup and releases the lease when its supervisor reports verified cleanup. The companion exercises the production Linux host, following the procfs fixture style in `src/process/ProcessOwnership.procIdentity.test.ts`; it injects no ownership host or cleanup result. ### Typed outcome or recovery expected The same moved-child case must leave cleanup pending with retry ownership, or stop the child and establish release. Helper exit by itself must not release the surviving child. The token-sweep recovery control must continue to remove the survivor, and foreign-token or unknown-identity cases must remain fail-closed. ### Analysis or suspected cause Source-derived: `src/process/ProcessOwnership.ts:1284–1288` skips a member after a native group change without retaining a failure or adding its new group to the planned queue. Revalidation at `1126–1144` only considers current members of the old group, and the non-swept branch returns success at `470–474`. This explains the executed simulated-boundary result; it does not establish the frequency of such a transition in a real provider. ### Additional context The reproduction source is included below. Real CLI execution, real group-movement timing and the pinned Node runtime remain unverified. Closed [issue #5206](https://github.com/morluto/rea/issues/5206) covers a different exit-refresh candidate loss. Closed [issue #3985](https://github.com/morluto/rea/issues/3985) covers discarding a probe owner after cleanup had already failed. Open [issue #4322](https://github.com/morluto/rea/issues/4322) is the opposite result, a false mismatch after an owned process exits. Open [issue #7272](https://github.com/morluto/rea/issues/7272) classifies an unresolved unrelated process as a token mismatch. This report is the false `released` result after a child changes groups. No same-behavior open issue was found. ```ts import { afterEach, expect, it, vi } from "vitest"; import { EventEmitter } from "node:events"; import { PassThrough } from "node:stream"; const originalPlatform = vi.hoisted(() => { const descriptor = Object.getOwnPropertyDescriptor(process, "platform")!; Object.defineProperty(process, "platform", { value: "linux", configurable: true }); return descriptor; }); const state = vi.hoisted(() => ({ readFile: vi.fn(), spawn: vi.fn(), ps: vi.fn(), })); vi.mock("node:fs/promises", async (original) => ({ ...await original<typeof import("node:fs/promises")>(), readFile: state.readFile, })); vi.mock("node:child_process", async (original) => ({ ...await original<typeof import("node:child_process")>(), spawn: state.spawn, })); vi.mock("./ExecFileOutput.js", () => ({ execFileOutput: state.ps })); import { runLinuxPrivateDisplayProbe } from "../hopper/LinuxPrivateDisplayProbe.js"; import { cleanupOwnedProcessGroup } from "./ProcessOwnership.js"; import { systemProcessOwnershipHost } from "./ProcessOwnershipObservation.js"; import { ProcessOwnershipEvidenceOwner } from "./ProcessOwnershipEvidence.js"; afterEach(() => { vi.restoreAllMocks(); Object.defineProperty(process, "platform", originalPlatform); }); it("runs the production private-display timeout cleanup through the Linux host", async () => { expect(systemProcessOwnershipHost.platform).toBe("linux"); const signals: number[] = []; let token = ""; let moved = false; let rootLive = true; let childLive = true; const helper = Object.assign(new EventEmitter(), { pid: 100, exitCode: null as number | null, signalCode: null as NodeJS.Signals | null, stdout: new PassThrough(), stderr: new PassThrough(), kill: vi.fn(), unref: vi.fn(), }); state.spawn.mockImplementation((_command, _arguments, options) => { token = options.env.REA_PROCESS_RUN_ID; queueMicrotask(() => helper.emit("spawn")); return helper; }); const rows = () => [ `${process.pid} ${process.ppid} ${process.pid} 1000 S verifier`, ...(rootLive ? [`100 ${process.pid} 100 1000 S /usr/bin/python3 helper.py`] : []), ...(childLive ? [`101 ${rootLive ? 100 : 1} ${moved ? 101 : 100} 1000 S Xvfb`] : []), ]; state.ps.mockImplementation(async (command) => { expect(command).toBe("ps"); return { stdout: rows().join("\n"), stderr: "" }; }); state.readFile.mockImplementation(async (path) => { const match = /^\/proc\/(\d+)\/(stat|environ)$/u.exec(String(path)); if (match === null) throw new Error(`Unexpected read: ${String(path)}`); const pid = Number(match[1]); if ((pid === 100 && !rootLive) || (pid === 101 && !childLive)) throw Object.assign(new Error("ESRCH"), { code: "ESRCH" }); if (match[2] === "environ") { // A same-generation child moves after the native relation snapshot, // during the identity/token/identity bracket used by the real host. if (pid === 101) moved = true; return pid === process.pid ? "FOREIGN=value\0" : `REA_PROCESS_RUN_ID=${token}\0`; } const parent = pid === 100 ? process.pid : pid === 101 ? (rootLive ? 100 : 1) : process.ppid; const group = pid === 101 ? (moved ? 101 : 100) : pid === 100 ? 100 : process.pid; const fields = ["S", String(parent), String(group), "100", "0", "0", "0"]; while (fields.length < 19) fields.push("0"); fields.push(String(pid * 10)); return `${pid} (fixture) ${fields.join(" ")}`; }); vi.spyOn(process, "kill").mockImplementation((pid, signal) => { if (signal === 0 && ((pid === 100 && !rootLive) || (pid === 101 && !childLive))) throw Object.assign(new Error("ESRCH"), { code: "ESRCH" }); if (pid >= 0 || signal !== "SIGKILL") throw new Error(`Unexpected signal ${pid}`); signals.push(-pid); if (pid === -100) { rootLive = false; if (!moved) childLive = false; helper.signalCode = "SIGKILL"; helper.stdout.end(); helper.stderr.end(); helper.emit("exit", null, "SIGKILL"); helper.emit("close", null, "SIGKILL"); } else if (pid === -101) childLive = false; return true; }); const result = await runLinuxPrivateDisplayProbe("direct", { helperPath: "scripts/hopper-demo-x11.py", timeoutMs: 1, environment: {}, }); console.log(JSON.stringify({ boundary: "production Linux private-display probe", result, signals, survivor: { live: childLive, pid: 101, group: moved ? 101 : 100, tokenRetained: token.length > 0 } })); expect(result).toMatchObject({ outcome: "timed-out", cleanup: { status: "released" } }); expect(signals).toEqual([100]); expect(childLive).toBe(true); expect(moved).toBe(true); // Recovery control: the production token-swept owner discovers the survivor. const control = await cleanupOwnedProcessGroup({ runId: token, leaderPid: 100, processGroupId: 100, sweepTokenOwnedProcesses: true, evidenceOwner: new ProcessOwnershipEvidenceOwner() }); console.log(JSON.stringify({ boundary: "token-sweep recovery control", control, signals, survivorLive: childLive })); expect(control.cleaned).toBe(true); expect(signals).toEqual([100, 101]); expect(childLive).toBe(false); }); ```

bsakaguchi closed this issue

2026-10-31 00:48:52 +00:00

bsakaguchi referenced this issue

2026-10-31 01:12:21 +00:00

[PR #4315] [MERGED] fix(package): parse tar listings without platform line endings #6164