morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[GH-ISSUE #2542] fix(process): preserve member evidence when releasing retained analysis resources #860

New issue

Closed

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

bsakaguchi commented

2026-10-15 18:29:04 +00:00

Owner

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

Summary

When a retained producer cannot be cleaned up, its supervisor returns the member failures and unrelated unverified processes that cleanup observed. Releasing the analysis resources discards those records and retains only a summary string. The caller cannot recover the complete verification evidence from the cleanup error, even when that summary already names a failed member.

Steps to reproduce

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

import { EventEmitter } from "node:events";
import { PassThrough } from "node:stream";
import { expect, it } from "vitest";
import { readStableArtifact } from "../artifacts/readStableArtifact.js";
import { projectAnalysisError } from "../domain/analysisErrorProjection.js";
import { ProviderAdapterError } from "../domain/providerAdapterError.js";
import { ArtifactAnalysisResources } from "./ArtifactAnalysisResources.js";
import { readLinuxResourceLimits } from "./LinuxResourceLimits.js";
import { ProviderProcessSupervisor } from "./ProviderProcess.js";

it("retains the cleanup member observations", async () => {
  const report = {
    cleaned: false as const,
    reason: "process ownership token could not be read for 1 live process(es): environment_unavailable=1; live candidates 4321=environment_unavailable",
    failures: [{ pid: 4321, reason: "environment-unreadable" as const,
      diagnostic: "environment_unavailable" }],
    unverified: [{ pid: 9000, diagnostic: "platform_binary_environment_withheld" }],
  };
  const producer = Object.assign(new EventEmitter(), {
    stdout: new PassThrough(), stderr: new PassThrough(),
    exitCode: null, signalCode: null, kill: () => true,
  });
  const owner = new ProviderProcessSupervisor({process: producer,
    ownsProcessLifetime: true, cleanup: async () => report});
  const stopped = await owner.stop();
  if (stopped.status !== "incomplete") throw new Error("expected incomplete stop");
  expect(stopped.cleanupResult).toEqual(report);
  const resources = new ArtifactAnalysisResources("fixture-provider", "fixture-operation",
    readStableArtifact, readLinuxResourceLimits, {message: "busy", details: {}});
  resources.retain({path: "runtime-root", close: async () => {}}, owner);
  try {
    await resources.release();
    throw new Error("expected cleanup failure");
  } catch (cause) {
    if (!(cause instanceof ProviderAdapterError)) throw cause;
    const projected = JSON.stringify(projectAnalysisError(cause));
    expect(projected).toContain("9000");
    expect(projected).toContain("environment-unreadable");
  } finally {
    owner.dispose();
  }
});

The report describes a related member whose token is unreadable and a separate unrelated process whose environment was withheld. The supervisor and finalizer are real production classes; the producer and cleanup observations are simulated, and no external process runs.

Expected behavior

An incomplete resource-cleanup error must preserve the observed member PID, typed failure reason and diagnostic, as well as the separate unverified-process observations. It must keep the resource retained and must not acquire additional signaling authority.

Actual behavior

The focused probe confirmed that the supervisor returned the entire cleanup report. The release error's projected diagnostics then contained only the reason string naming PID 4321. It omitted the typed member failure and the unverified observation for PID 9000. A separate probe through analysis finalization also lost the records.

Evidence

Expected source: AGENTS.md, Architecture and Cleanup, requires partial facts to survive boundaries; src/process/ProcessOwnership.ts:163-169 defines the retained cleanup ledger and src/process/ProviderProcess.ts:392-401 returns that ledger on an incomplete stop.
Failure source: src/process/ArtifactAnalysisResources.ts:115-127 replaces the incomplete stop report with a plain error containing only its reason; src/process/ArtifactAnalysisResources.ts:204-231 projects the subsequent cleanup error without recovering the discarded records.
Evidence provenance: observed
Local verification: reproduced
Reproduction completeness: complete

The existing owned-command diagnostic fix establishes that typed member failures and sweep uncertainty should survive the supervisor/command boundary. This failure occurs at its next resource boundary. Packet and signal decode finalizers retain failed command owners and use that boundary in src/captures/TsharkCaptureProvider.ts:349-366 and src/signals/SigrokRecordingProvider.ts:323-340; those call paths were checked from source.

Restoration check

With the same report, the projected release and finalization errors must expose PID 4321 with environment-unreadable and environment_unavailable, plus PID 9000 with its withheld-environment diagnostic. Keep cleanup incomplete and the retained runtime identified. A report without member arrays must not invent member observations. Preserving only the summary string is insufficient because it does not contain the separate unverified-process record.

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 test used a simulated producer.

Triage assessment

Impact level: P2
Assessment status: supported
Impact basis: Cleanup errors lose the typed reasons and process observations required to identify what remained unverified. The isolation probe loses a separate PID even while retaining the scalar reason and resource path.
Workaround status: unverified
Workaround basis: A fresh process table does not reconstruct historical verification failures or withheld-token observations. No practical caller-facing way to recover those records after projection was verified.

Originally created by @coygeek on GitHub (Oct 13, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/2542 ## Summary When a retained producer cannot be cleaned up, its supervisor returns the member failures and unrelated unverified processes that cleanup observed. Releasing the analysis resources discards those records and retains only a summary string. The caller cannot recover the complete verification evidence from the cleanup error, even when that summary already names a failed member. ## Steps to reproduce Use the source runtime below. Add this test as `src/process/cleanup-members.regression.test.ts` beside the resource module and run `npm run test:focused -- src/process/cleanup-members.regression.test.ts`. ```ts import { EventEmitter } from "node:events"; import { PassThrough } from "node:stream"; import { expect, it } from "vitest"; import { readStableArtifact } from "../artifacts/readStableArtifact.js"; import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import { ProviderAdapterError } from "../domain/providerAdapterError.js"; import { ArtifactAnalysisResources } from "./ArtifactAnalysisResources.js"; import { readLinuxResourceLimits } from "./LinuxResourceLimits.js"; import { ProviderProcessSupervisor } from "./ProviderProcess.js"; it("retains the cleanup member observations", async () => { const report = { cleaned: false as const, reason: "process ownership token could not be read for 1 live process(es): environment_unavailable=1; live candidates 4321=environment_unavailable", failures: [{ pid: 4321, reason: "environment-unreadable" as const, diagnostic: "environment_unavailable" }], unverified: [{ pid: 9000, diagnostic: "platform_binary_environment_withheld" }], }; const producer = Object.assign(new EventEmitter(), { stdout: new PassThrough(), stderr: new PassThrough(), exitCode: null, signalCode: null, kill: () => true, }); const owner = new ProviderProcessSupervisor({process: producer, ownsProcessLifetime: true, cleanup: async () => report}); const stopped = await owner.stop(); if (stopped.status !== "incomplete") throw new Error("expected incomplete stop"); expect(stopped.cleanupResult).toEqual(report); const resources = new ArtifactAnalysisResources("fixture-provider", "fixture-operation", readStableArtifact, readLinuxResourceLimits, {message: "busy", details: {}}); resources.retain({path: "runtime-root", close: async () => {}}, owner); try { await resources.release(); throw new Error("expected cleanup failure"); } catch (cause) { if (!(cause instanceof ProviderAdapterError)) throw cause; const projected = JSON.stringify(projectAnalysisError(cause)); expect(projected).toContain("9000"); expect(projected).toContain("environment-unreadable"); } finally { owner.dispose(); } }); ``` The report describes a related member whose token is unreadable and a separate unrelated process whose environment was withheld. The supervisor and finalizer are real production classes; the producer and cleanup observations are simulated, and no external process runs. ## Expected behavior An incomplete resource-cleanup error must preserve the observed member PID, typed failure reason and diagnostic, as well as the separate unverified-process observations. It must keep the resource retained and must not acquire additional signaling authority. ## Actual behavior The focused probe confirmed that the supervisor returned the entire cleanup report. The release error's projected diagnostics then contained only the reason string naming PID 4321. It omitted the typed member failure and the unverified observation for PID 9000. A separate probe through analysis finalization also lost the records. ## Evidence - Expected source: `AGENTS.md`, Architecture and Cleanup, requires partial facts to survive boundaries; `src/process/ProcessOwnership.ts:163-169` defines the retained cleanup ledger and `src/process/ProviderProcess.ts:392-401` returns that ledger on an incomplete stop. - Failure source: `src/process/ArtifactAnalysisResources.ts:115-127` replaces the incomplete stop report with a plain error containing only its reason; `src/process/ArtifactAnalysisResources.ts:204-231` projects the subsequent cleanup error without recovering the discarded records. - Evidence provenance: observed - Local verification: reproduced - Reproduction completeness: complete The existing [owned-command diagnostic fix](https://github.com/morluto/rea/pull/1907) establishes that typed member failures and sweep uncertainty should survive the supervisor/command boundary. This failure occurs at its next resource boundary. Packet and signal decode finalizers retain failed command owners and use that boundary in `src/captures/TsharkCaptureProvider.ts:349-366` and `src/signals/SigrokRecordingProvider.ts:323-340`; those call paths were checked from source. ## Restoration check With the same report, the projected release and finalization errors must expose PID 4321 with `environment-unreadable` and `environment_unavailable`, plus PID 9000 with its withheld-environment diagnostic. Keep cleanup incomplete and the retained runtime identified. A report without member arrays must not invent member observations. Preserving only the summary string is insufficient because it does not contain the separate unverified-process record. ## 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 test used a simulated producer. ## Triage assessment - Impact level: P2 - Assessment status: supported - Impact basis: Cleanup errors lose the typed reasons and process observations required to identify what remained unverified. The isolation probe loses a separate PID even while retaining the scalar reason and resource path. - Workaround status: unverified - Workaround basis: A fresh process table does not reconstruct historical verification failures or withheld-token observations. No practical caller-facing way to recover those records after projection was verified.

bsakaguchi

2026-10-15 18:29:04 +00:00

closed this issue
added the
area: runtime

bug
labels

bsakaguchi commented

2026-10-15 18:29:05 +00:00

Author

Owner

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

Verified the reported defect is addressed on main by 599a8ee11d.

Root cause and resolution: Cleanup failure details for individual process-group members were discarded while converting retained-resource cleanup into a projected provider error.

Validation: src/process/ArtifactAnalysisResources.test.ts — projects the specific reason and member evidence for retained cleanup preserves failed and unverified members through cleanup, typed error, and public error projection.

Evidence: CI run 500e: owner unchanged and assertion passed.

Verification scope: Controlled supervisor observations; no OS-specific cleanup failure reproduced.

Current main CI has separate failures under repair; this closure does not claim the complete main workflow is green.

@morluto commented on GitHub (Oct 14, 2026): Verified the reported defect is addressed on main by 599a8ee11de6b1a7fb270e669b88d78b6be982d2. Root cause and resolution: Cleanup failure details for individual process-group members were discarded while converting retained-resource cleanup into a projected provider error. Validation: `src/process/ArtifactAnalysisResources.test.ts` — `projects the specific reason and member evidence for retained cleanup` preserves failed and unverified members through cleanup, typed error, and public error projection. Evidence: CI run 500e: owner unchanged and assertion passed. Verification scope: Controlled supervisor observations; no OS-specific cleanup failure reproduced. Current main CI has separate failures under repair; this closure does not claim the complete main workflow is green.

bsakaguchi referenced this issue

2026-10-16 21:16:54 +00:00

[PR #860] [MERGED] fix(native): keep integral plist reals beyond the JSON range as reals #2297

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#860

No description provided.

Rows
Columns