[GH-ISSUE #2543] fix(process): retain withheld-process observations after cleanup errors #861

Closed
opened 2026-10-15 18:29:04 +00:00 by bsakaguchi · 1 comment
Owner

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

Summary

Owned-process cleanup observes an unrelated process whose environment the operating system withholds, then loses that PID and diagnostic if later ownership revalidation or signaling fails. Successful cleanup retains the same observation. As a result, the documented uncertainty evidence depends on which cleanup branch finishes the attempt.

Steps to reproduce

Use the source runtime below. Add this test as src/process/cleanup-withheld.regression.test.ts beside the ownership module and run npm run test:focused -- src/process/cleanup-withheld.regression.test.ts.

import { expect, it, vi } from "vitest";
import { cleanupOwnedProcessGroup, type ProcessOwnershipHost } from "./ProcessOwnership.js";

it.each(["revalidation", "signal-error", "success"])("retains observation on %s", async (mode) => {
  const rows = [
    {pid: 100, parentPid: 1, processGroupId: 100, state: "S", command: "fixture"},
    {pid: 900, parentPid: 1, processGroupId: 900, state: "S", command: "opaque"},
  ];
  let rootReads = 0;
  const signalGroup = vi.fn(() => {
    if (mode === "signal-error") throw Object.assign(new Error("signal denied"), {code: "EACCES"});
  });
  const host: ProcessOwnershipHost = {
    listProcesses: async () => rows,
    environment: async () => ({}),
    runTokens: async (members) => new Map(members.map(({pid}) => {
      if (pid === 900) return [pid, {state: "unavailable" as const,
        reason: "platform_binary_environment_withheld"}];
      rootReads += 1;
      return [pid, {state: "readable" as const,
        runId: mode === "revalidation" && rootReads > 2 ? "other-run" : "run-token"}];
    })),
    signalGroup,
  };
  const result = await cleanupOwnedProcessGroup({runId: "run-token",
    leaderPid: 100, processGroupId: 100, sweepTokenOwnedProcesses: true,
    captureBaseline: []}, host);
  expect(result.cleaned).toBe(mode === "success");
  expect(signalGroup).toHaveBeenCalledTimes(mode === "revalidation" ? 0 : 1);
  expect(result.unverified).toEqual([{pid: 900,
    diagnostic: "platform_binary_environment_withheld"}]);
});

Both processes remain live across all reads. PID 900 has no ancestry or process-group relation to the captured launcher. The revalidation case changes the launcher's token only after planning; the signal-error case keeps the token valid and throws from the signal stub. The success case provides a control. No operating-system signal is sent by this fixture.

Expected behavior

Once the sweep records an unrelated withheld process, later cleanup errors must retain its PID and diagnostic in the cleanup result, so the capture report can preserve its uncertainty evidence. Revalidation failure must continue refusing the signal, and a signal error must continue reporting incomplete cleanup.

Actual behavior

The revalidation probe returned process ownership token mismatch for PID(s): 100 with no unverified observation and zero signal calls. The signal-error probe returned owned process group signal failed: signal denied, also without the observation. The success control returned the expected PID 900 and its withheld-environment diagnostic. The two retention assertions failed; the control passed.

Evidence

  • Expected source: docs/process-capture.md:406-410 promises the withheld PID and reason in cleanup observations and residual unknowns; AGENTS.md, Architecture and Cleanup, requires uncertainty and partial facts to survive boundaries.
  • Failure source: src/process/ProcessOwnership.ts:255 returns a revalidation failure without the plan's observations, and src/process/ProcessOwnership.ts:263-269 returns a signal error without them; the success return at src/process/ProcessOwnership.ts:273-277 retains them.
  • Evidence provenance: observed
  • Local verification: reproduced
  • Reproduction completeness: complete

The capture consumer copies the returned observations before checking the failure status in src/process/capture/ProcessCaptureLifecycle.ts:1009-1021. That propagation was checked from source; runtime probes exercised the ownership helper with a simulated host. The earlier withheld-token cleanup change explicitly includes preservation in partial cleanup reports. This report concerns observation retention after classification rather than the classification of an unreadable process.

Restoration check

All three fixture cases must retain PID 900 with platform_binary_environment_withheld. Preserve each case's cleanup status, mismatch or signal-error reason, and signal-call count. A control with no withheld candidate must not invent an unrelated-process observation.

Runtime or environment

Source revision 8b181716aca2cb467be499cba226abfb5ae19141, Node v26.10.0, npm 11.19.1, installed lockfile dependencies and Vitest v5.0.3. The focused source tests used a simulated process host.

Triage assessment

  • Impact level: P2
  • Assessment status: supported
  • Impact basis: Later cleanup errors discard a documented process uncertainty observation, leaving callers without a PID and diagnostic that successful cleanup preserves. Both error branches reproduced the omission without changing ownership authority.
  • Workaround status: unverified
  • Workaround basis: A new process table does not recover the earlier withheld-token diagnostic. No practical caller-facing method to reconstruct the omitted observation was verified.
Originally created by @coygeek on GitHub (Oct 13, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/2543 ## Summary Owned-process cleanup observes an unrelated process whose environment the operating system withholds, then loses that PID and diagnostic if later ownership revalidation or signaling fails. Successful cleanup retains the same observation. As a result, the documented uncertainty evidence depends on which cleanup branch finishes the attempt. ## Steps to reproduce Use the source runtime below. Add this test as `src/process/cleanup-withheld.regression.test.ts` beside the ownership module and run `npm run test:focused -- src/process/cleanup-withheld.regression.test.ts`. ```ts import { expect, it, vi } from "vitest"; import { cleanupOwnedProcessGroup, type ProcessOwnershipHost } from "./ProcessOwnership.js"; it.each(["revalidation", "signal-error", "success"])("retains observation on %s", async (mode) => { const rows = [ {pid: 100, parentPid: 1, processGroupId: 100, state: "S", command: "fixture"}, {pid: 900, parentPid: 1, processGroupId: 900, state: "S", command: "opaque"}, ]; let rootReads = 0; const signalGroup = vi.fn(() => { if (mode === "signal-error") throw Object.assign(new Error("signal denied"), {code: "EACCES"}); }); const host: ProcessOwnershipHost = { listProcesses: async () => rows, environment: async () => ({}), runTokens: async (members) => new Map(members.map(({pid}) => { if (pid === 900) return [pid, {state: "unavailable" as const, reason: "platform_binary_environment_withheld"}]; rootReads += 1; return [pid, {state: "readable" as const, runId: mode === "revalidation" && rootReads > 2 ? "other-run" : "run-token"}]; })), signalGroup, }; const result = await cleanupOwnedProcessGroup({runId: "run-token", leaderPid: 100, processGroupId: 100, sweepTokenOwnedProcesses: true, captureBaseline: []}, host); expect(result.cleaned).toBe(mode === "success"); expect(signalGroup).toHaveBeenCalledTimes(mode === "revalidation" ? 0 : 1); expect(result.unverified).toEqual([{pid: 900, diagnostic: "platform_binary_environment_withheld"}]); }); ``` Both processes remain live across all reads. PID 900 has no ancestry or process-group relation to the captured launcher. The revalidation case changes the launcher's token only after planning; the signal-error case keeps the token valid and throws from the signal stub. The success case provides a control. No operating-system signal is sent by this fixture. ## Expected behavior Once the sweep records an unrelated withheld process, later cleanup errors must retain its PID and diagnostic in the cleanup result, so the capture report can preserve its uncertainty evidence. Revalidation failure must continue refusing the signal, and a signal error must continue reporting incomplete cleanup. ## Actual behavior The revalidation probe returned `process ownership token mismatch for PID(s): 100` with no `unverified` observation and zero signal calls. The signal-error probe returned `owned process group signal failed: signal denied`, also without the observation. The success control returned the expected PID 900 and its withheld-environment diagnostic. The two retention assertions failed; the control passed. ## Evidence - Expected source: `docs/process-capture.md:406-410` promises the withheld PID and reason in cleanup observations and residual unknowns; `AGENTS.md`, Architecture and Cleanup, requires uncertainty and partial facts to survive boundaries. - Failure source: `src/process/ProcessOwnership.ts:255` returns a revalidation failure without the plan's observations, and `src/process/ProcessOwnership.ts:263-269` returns a signal error without them; the success return at `src/process/ProcessOwnership.ts:273-277` retains them. - Evidence provenance: observed - Local verification: reproduced - Reproduction completeness: complete The capture consumer copies the returned observations before checking the failure status in `src/process/capture/ProcessCaptureLifecycle.ts:1009-1021`. That propagation was checked from source; runtime probes exercised the ownership helper with a simulated host. The earlier [withheld-token cleanup change](https://github.com/morluto/rea/pull/1057) explicitly includes preservation in partial cleanup reports. This report concerns observation retention after classification rather than the classification of an unreadable process. ## Restoration check All three fixture cases must retain PID 900 with `platform_binary_environment_withheld`. Preserve each case's cleanup status, mismatch or signal-error reason, and signal-call count. A control with no withheld candidate must not invent an unrelated-process observation. ## Runtime or environment Source revision `8b181716aca2cb467be499cba226abfb5ae19141`, Node v26.10.0, npm 11.19.1, installed lockfile dependencies and Vitest v5.0.3. The focused source tests used a simulated process host. ## Triage assessment - Impact level: P2 - Assessment status: supported - Impact basis: Later cleanup errors discard a documented process uncertainty observation, leaving callers without a PID and diagnostic that successful cleanup preserves. Both error branches reproduced the omission without changing ownership authority. - Workaround status: unverified - Workaround basis: A new process table does not recover the earlier withheld-token diagnostic. No practical caller-facing method to reconstruct the omitted observation was verified.
bsakaguchi 2026-10-15 18:29:04 +00:00
Author
Owner

@morluto commented on GitHub (Oct 14, 2026):

Cleanup results now retain previously observed withheld-process uncertainty when ownership revalidation or signaling fails. Both failure-path regressions pass and preserve the no-signal behavior for unresolved ownership.

Root cause: Cleanup revalidation and signal-error returns discarded previously recorded unrelated withheld-process observations.

Validation: src/process/ProcessOwnership.test.ts: 'retains withheld-process evidence after revalidation failure' and '... after signal-error failure' assert PID 900 and its diagnostic; adjacent 'cleans verified run-owned groups and records an unrelated ... neighbor' is the success control. These assertions passed in CI at 500e32399; the reviewed implementation is present on current main. Fix: 599a8ee11de6b1a7fb270e669b88d78b6be982d2.

Verification scope: Controlled process-table adapter only; no live process signaling is claimed.

<!-- gh-comment-id:6161944712 --> @morluto commented on GitHub (Oct 14, 2026): Cleanup results now retain previously observed withheld-process uncertainty when ownership revalidation or signaling fails. Both failure-path regressions pass and preserve the no-signal behavior for unresolved ownership. Root cause: Cleanup revalidation and signal-error returns discarded previously recorded unrelated withheld-process observations. Validation: src/process/ProcessOwnership.test.ts: 'retains withheld-process evidence after revalidation failure' and '... after signal-error failure' assert PID 900 and its diagnostic; adjacent 'cleans verified run-owned groups and records an unrelated ... neighbor' is the success control. These assertions passed in [CI at 500e32399](https://github.com/morluto/rea/actions/runs/38514789942); the reviewed implementation is present on current main. Fix: [599a8ee11de6b1a7fb270e669b88d78b6be982d2](https://github.com/morluto/rea/commit/599a8ee11de6b1a7fb270e669b88d78b6be982d2). Verification scope: Controlled process-table adapter only; no live process signaling is claimed.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#861
No description provided.