morluto/rea

Fork 0

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

[GH-ISSUE #5841] [Bug] Linux Hopper publisher close failure skips cleanup of retained publishers #3538

New issue

Closed

opened 2026-10-31 00:41:38 +00:00 by bsakaguchi · 2 comments

bsakaguchi commented

2026-10-31 00:41:38 +00:00

Owner

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

Before submitting

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

Area

Setup, packaging, or upgrade

Observed behavior

A controlled source-boundary reproduction observed that inspectLinuxHopperLauncher returns cleanup_failed after its publisher close rejects, without running the following global retained-cleanup retry. The current descriptor remains open. In a mixed-owner case, another publisher's already-retained descriptor receives no retry either, even though its next close succeeds. Calling retryLinuxHopperPublisherCleanup explicitly then closes both exact descriptors.

A separate test observed the same sequencing in systemLinuxHopperInstallHost(...).close(): closure rejected while its descriptor remained open, and explicit retained cleanup closed it. These tests injected descriptor-close failures into real owned file handles. They did not execute a public CLI command, a Linux package manager, Hopper, or a naturally occurring filesystem failure.

Reproduction

From the stated source revision with its lockfile dependencies installed, save the following mixed-owner case as src/application/repro.test.ts. It is the essential case from a three-test characterization run. It uses the existing temporary-directory fixture, real inert files and real publisher ownership; the seam controls only close failures. Hash cleanup fails before any package-cache or network lookup.

import { writeFile } from "node:fs/promises";
import { join } from "node:path";
import { expect, it, vi } from "vitest";
import { inspectLinuxHopperLauncher } from "./LinuxHopper.js";
import {
  LinuxHopperPublisher,
  retryLinuxHopperPublisherCleanup,
} from "./LinuxHopperPublisher.js";
import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js";
import { createTestTempDirectory } from "../../tests/fixtures/temporaryDirectory.js";

it("a failing current publisher close also skips a different recoverable publisher", async () => {
  const directory = await createTestTempDirectory("rea-publisher-repro-");
  const earlierPath = join(directory, "earlier");
  const currentPath = join(directory, "current");
  await writeFile(earlierPath, "inert earlier fixture");
  await writeFile(currentPath, "inert current fixture");
  const earlier = new LinuxHopperPublisher({}, async () => undefined);
  const originalClose = OwnedFileHandle.prototype.close;
  const attempts = new Map<OwnedFileHandle, number>();
  const close = vi.spyOn(OwnedFileHandle.prototype, "close").mockImplementation(async function (this: OwnedFileHandle) {
    const count = (attempts.get(this) ?? 0) + 1;
    attempts.set(this, count);
    const isEarlier = this === attempts.keys().next().value;
    if (count <= (isEarlier ? 3 : 4)) throw new Error("controlled descriptor close denial");
    return originalClose.call(this);
  });
  try {
    await expect(earlier.hashLauncher(earlierPath)).rejects.toMatchObject({ reason: "cleanup_failed" });
    const oldOwner = [...attempts.keys()][0]!;
    expect(attempts.get(oldOwner)).toBe(3);
    const result = await inspectLinuxHopperLauncher(currentPath, {});
    expect(result).toMatchObject({ ok: false, error: { reason: "cleanup_failed" } });
    expect(attempts.size).toBe(2);
    expect(attempts.get(oldOwner)).toBe(3);
    expect(oldOwner.handle.fd).toBeGreaterThanOrEqual(0);
    console.log("other retained publisher: no retry; attempts=3; descriptor still open=true");
    await retryLinuxHopperPublisherCleanup();
    expect([...attempts.values()]).toEqual([4, 5]);
    expect([...attempts.keys()].every(owner => owner.handle.fd === -1)).toBe(true);
    console.log("explicit retry: earlier attempts=4; current attempts=5; both descriptors closed=true");
  } finally {
    close.mockRestore();
    await earlier.close();
    await retryLinuxHopperPublisherCleanup();
  }
});

Run npm run test:local -- src/application/repro.test.ts with HOME, XDG_CONFIG_HOME and TMPDIR set to disposable directories, then delete the test. The test deliberately asserts the current failure and demonstrates recovery; passing means the defect was reproduced.

The three-test run also exercised a single inspection and the installation host's launcher-status/close boundary, with a stubbed ldd reply. All three cases reproduced in the final focused run. The installation host case denied the first four closes: launcher-status processing made three attempts, host closure made the fourth and rejected, and the explicit global retry made the fifth and closed the descriptor. A single initial close failure is insufficient because existing inner retries may recover it.

Expected behavior

Complete the retained-cleanup phase even if closing the current publisher fails, including attempts for other pending publishers. Keep the primary operation diagnostic and report only resources that remain unresolved after cleanup. A successful release should not require an otherwise skipped global phase to be called separately by test code.

The lifecycle contract requires retrying unresolved children and preserving remaining resource ownership during one-shot cleanup: docs/tool-design.md, lifecycle completion rules. The required mixed-outcome and cleanup-retry scenarios are explicit in docs/testing.md.

Impact and scope

P2: a bounded cleanup/recovery defect under close failures. The tests observed real descriptors remaining open after the affected wrappers completed; an earlier publisher's recoverable descriptor was also skipped. No data loss, target execution or security-boundary crossing was observed. Natural frequency is NOT_ENOUGH_INFO.

Source-derived affected routes include Linux Hopper inspection during rea doctor --provider hopper, approved Linux Hopper setup, and publisher verification before Linux demo launch. Doctor calls inspection when it has an executable Hopper path on Linux; the production mapping is in DoctorDiagnostics.ts and Doctor.ts. Setup closes the affected installation host through SetupHost.ts.

Ownership remains retained; permanent loss is not claimed. The explicit internal retry recovered all test handles. Linux bridge cleanup also calls that retry independently, so later session cleanup may recover resources skipped by the inspection wrapper. No end-to-end CLI recovery workaround or real Linux behavior was tested.

REA version

rea-agents 6.7.0; source revision a5f982de8d

Runtime and platform

Node.js 24.22.0; macOS/Darwin arm64; Vitest 5.0.3; portable Linux publisher boundary exercised on macOS; Hopper was not used.

Target and artifact context

Inert authored launcher-byte files; real owned descriptors; no analysis target, signed package, persistent cache publication or package installation.

Interface

Other

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

Executed command: npm run test:local -- src/application/repro.test.ts. Trimmed output from the three-case run:

inspection: cleanup_failed; close attempts=4; exact descriptor still open=true
explicit retry: close attempts=5; exact descriptor closed=true
setup host: close rejected; close attempts=4; exact descriptor still open=true
setup host explicit retry: close attempts=5; exact descriptor closed=true
other retained publisher: no retry; attempts=3; descriptor still open=true
explicit retry: earlier attempts=4; current attempts=5; both descriptors closed=true
Test Files  1 passed (1)
Tests  3 passed (3)

The characterization verified descriptor identity, an open file descriptor after the skipped phase, and fd === -1 after the retained owner's successful retry. Finalizers restored the close method and drained pending publishers. The temporary test was deleted and git status --porcelain was empty.

The sequential awaits are at LinuxHopper.ts, installation-host close and launcher inspection. The publisher retains its scope before rethrowing at LinuxHopperPublisher.ts, close. The skipped global retry attempts every pending scope and collects failures at LinuxHopperPublisher.ts, retry.

Typed outcome or recovery expected

Retain the operation's original failure. Attempt all pending cleanup owners despite a local close error; if cleanup still fails, preserve its remaining resources and recovery authority. This report does not require converting an earlier failed operation into success merely because a later cleanup attempt succeeds.

Analysis or suspected cause

Source-derived: a rejection from await publisher.close() transfers control past the next await. The global retry therefore runs only when the local close succeeds. Collecting the two cleanup outcomes independently would allow the intended global phase to run while preserving both diagnostics.

Additional context

Read-only open/closed issue and pull-request searches for publisher cleanup, retained cleanup and the retry symbol found no matching report or fix. Related installer-outcome, Mach-O preflight, private-display probe and macOS handshake reports concern different owners or failure stages. This report is limited to the skipped publisher retry phase; it does not claim those reports are unresolved instances of this defect.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5841 ### 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 Setup, packaging, or upgrade ### Observed behavior A controlled source-boundary reproduction observed that `inspectLinuxHopperLauncher` returns `cleanup_failed` after its publisher close rejects, without running the following global retained-cleanup retry. The current descriptor remains open. In a mixed-owner case, another publisher's already-retained descriptor receives no retry either, even though its next close succeeds. Calling `retryLinuxHopperPublisherCleanup` explicitly then closes both exact descriptors. A separate test observed the same sequencing in `systemLinuxHopperInstallHost(...).close()`: closure rejected while its descriptor remained open, and explicit retained cleanup closed it. These tests injected descriptor-close failures into real owned file handles. They did not execute a public CLI command, a Linux package manager, Hopper, or a naturally occurring filesystem failure. ### Reproduction From the stated source revision with its lockfile dependencies installed, save the following mixed-owner case as `src/application/repro.test.ts`. It is the essential case from a three-test characterization run. It uses the existing temporary-directory fixture, real inert files and real publisher ownership; the seam controls only close failures. Hash cleanup fails before any package-cache or network lookup. ```ts import { writeFile } from "node:fs/promises"; import { join } from "node:path"; import { expect, it, vi } from "vitest"; import { inspectLinuxHopperLauncher } from "./LinuxHopper.js"; import { LinuxHopperPublisher, retryLinuxHopperPublisherCleanup, } from "./LinuxHopperPublisher.js"; import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js"; import { createTestTempDirectory } from "../../tests/fixtures/temporaryDirectory.js"; it("a failing current publisher close also skips a different recoverable publisher", async () => { const directory = await createTestTempDirectory("rea-publisher-repro-"); const earlierPath = join(directory, "earlier"); const currentPath = join(directory, "current"); await writeFile(earlierPath, "inert earlier fixture"); await writeFile(currentPath, "inert current fixture"); const earlier = new LinuxHopperPublisher({}, async () => undefined); const originalClose = OwnedFileHandle.prototype.close; const attempts = new Map<OwnedFileHandle, number>(); const close = vi.spyOn(OwnedFileHandle.prototype, "close").mockImplementation(async function (this: OwnedFileHandle) { const count = (attempts.get(this) ?? 0) + 1; attempts.set(this, count); const isEarlier = this === attempts.keys().next().value; if (count <= (isEarlier ? 3 : 4)) throw new Error("controlled descriptor close denial"); return originalClose.call(this); }); try { await expect(earlier.hashLauncher(earlierPath)).rejects.toMatchObject({ reason: "cleanup_failed" }); const oldOwner = [...attempts.keys()][0]!; expect(attempts.get(oldOwner)).toBe(3); const result = await inspectLinuxHopperLauncher(currentPath, {}); expect(result).toMatchObject({ ok: false, error: { reason: "cleanup_failed" } }); expect(attempts.size).toBe(2); expect(attempts.get(oldOwner)).toBe(3); expect(oldOwner.handle.fd).toBeGreaterThanOrEqual(0); console.log("other retained publisher: no retry; attempts=3; descriptor still open=true"); await retryLinuxHopperPublisherCleanup(); expect([...attempts.values()]).toEqual([4, 5]); expect([...attempts.keys()].every(owner => owner.handle.fd === -1)).toBe(true); console.log("explicit retry: earlier attempts=4; current attempts=5; both descriptors closed=true"); } finally { close.mockRestore(); await earlier.close(); await retryLinuxHopperPublisherCleanup(); } }); ``` Run `npm run test:local -- src/application/repro.test.ts` with HOME, XDG_CONFIG_HOME and TMPDIR set to disposable directories, then delete the test. The test deliberately asserts the current failure and demonstrates recovery; passing means the defect was reproduced. The three-test run also exercised a single inspection and the installation host's launcher-status/close boundary, with a stubbed `ldd` reply. All three cases reproduced in the final focused run. The installation host case denied the first four closes: launcher-status processing made three attempts, host closure made the fourth and rejected, and the explicit global retry made the fifth and closed the descriptor. A single initial close failure is insufficient because existing inner retries may recover it. ### Expected behavior Complete the retained-cleanup phase even if closing the current publisher fails, including attempts for other pending publishers. Keep the primary operation diagnostic and report only resources that remain unresolved after cleanup. A successful release should not require an otherwise skipped global phase to be called separately by test code. The lifecycle contract requires retrying unresolved children and preserving remaining resource ownership during one-shot cleanup: [docs/tool-design.md, lifecycle completion rules](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/docs/tool-design.md#L119-L140). The required mixed-outcome and cleanup-retry scenarios are explicit in [docs/testing.md](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/docs/testing.md#L247-L259). ### Impact and scope P2: a bounded cleanup/recovery defect under close failures. The tests observed real descriptors remaining open after the affected wrappers completed; an earlier publisher's recoverable descriptor was also skipped. No data loss, target execution or security-boundary crossing was observed. Natural frequency is NOT_ENOUGH_INFO. Source-derived affected routes include Linux Hopper inspection during `rea doctor --provider hopper`, approved Linux Hopper setup, and publisher verification before Linux demo launch. Doctor calls inspection when it has an executable Hopper path on Linux; the production mapping is in [DoctorDiagnostics.ts](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/application/DoctorDiagnostics.ts#L272-L303) and [Doctor.ts](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/application/Doctor.ts#L614-L618). Setup closes the affected installation host through [SetupHost.ts](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/application/SetupHost.ts#L450-L465). Ownership remains retained; permanent loss is not claimed. The explicit internal retry recovered all test handles. Linux [bridge cleanup](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/hopper/BridgeLauncher.ts#L193-L214) also calls that retry independently, so later session cleanup may recover resources skipped by the inspection wrapper. No end-to-end CLI recovery workaround or real Linux behavior was tested. ### REA version rea-agents 6.7.0; source revision a5f982de8dbe4b092cc20ab89d492b7e7028b758 ### Runtime and platform Node.js 24.22.0; macOS/Darwin arm64; Vitest 5.0.3; portable Linux publisher boundary exercised on macOS; Hopper was not used. ### Target and artifact context Inert authored launcher-byte files; real owned descriptors; no analysis target, signed package, persistent cache publication or package installation. ### Interface Other ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence Executed command: `npm run test:local -- src/application/repro.test.ts`. Trimmed output from the three-case run: ```text inspection: cleanup_failed; close attempts=4; exact descriptor still open=true explicit retry: close attempts=5; exact descriptor closed=true setup host: close rejected; close attempts=4; exact descriptor still open=true setup host explicit retry: close attempts=5; exact descriptor closed=true other retained publisher: no retry; attempts=3; descriptor still open=true explicit retry: earlier attempts=4; current attempts=5; both descriptors closed=true Test Files 1 passed (1) Tests 3 passed (3) ``` The characterization verified descriptor identity, an open file descriptor after the skipped phase, and `fd === -1` after the retained owner's successful retry. Finalizers restored the close method and drained pending publishers. The temporary test was deleted and `git status --porcelain` was empty. The sequential awaits are at [LinuxHopper.ts, installation-host close](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/application/LinuxHopper.ts#L500-L504) and [launcher inspection](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/application/LinuxHopper.ts#L517-L541). The publisher retains its scope before rethrowing at [LinuxHopperPublisher.ts, close](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/application/LinuxHopperPublisher.ts#L603-L610). The skipped global retry attempts every pending scope and collects failures at [LinuxHopperPublisher.ts, retry](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/application/LinuxHopperPublisher.ts#L1066-L1107). ### Typed outcome or recovery expected Retain the operation's original failure. Attempt all pending cleanup owners despite a local close error; if cleanup still fails, preserve its remaining resources and recovery authority. This report does not require converting an earlier failed operation into success merely because a later cleanup attempt succeeds. ### Analysis or suspected cause Source-derived: a rejection from `await publisher.close()` transfers control past the next await. The global retry therefore runs only when the local close succeeds. Collecting the two cleanup outcomes independently would allow the intended global phase to run while preserving both diagnostics. ### Additional context Read-only open/closed issue and pull-request searches for publisher cleanup, retained cleanup and the retry symbol found no matching report or fix. Related installer-outcome, Mach-O preflight, private-display probe and macOS handshake reports concern different owners or failure stages. This report is limited to the skipped publisher retry phase; it does not claim those reports are unresolved instances of this defect.

bsakaguchi closed this issue

2026-10-31 00:41:39 +00:00

bsakaguchi commented

2026-10-31 00:41:39 +00:00

Author

Owner

@XinwangTao commented on GitHub (Oct 21, 2026):

Confirmed — the mixed-owner case is the strongest evidence: a failing close on the current publisher returns early and skips the retained-cleanup phase entirely, leaving an earlier recoverable publisher's descriptor open with zero retry attempts. That violates the one-shot cleanup contract (retry unresolved children regardless of the current operation's outcome). The fix should run the global retained-cleanup phase unconditionally in the cleanup_failed path, keeping the primary diagnostic but aggregating whatever remains unresolved after that pass.

@XinwangTao commented on GitHub (Oct 21, 2026): Confirmed — the mixed-owner case is the strongest evidence: a failing close on the current publisher returns early and skips the retained-cleanup phase entirely, leaving an earlier recoverable publisher's descriptor open with zero retry attempts. That violates the one-shot cleanup contract (retry unresolved children regardless of the current operation's outcome). The fix should run the global retained-cleanup phase unconditionally in the cleanup_failed path, keeping the primary diagnostic but aggregating whatever remains unresolved after that pass.

bsakaguchi commented

2026-10-31 00:41:40 +00:00

Author

Owner

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

Resolved on main in 25b28ffe8. Both inspection and installation-host closure drain retained publishers even after current publisher close fails. Executed mixed-owner controls release both exact real descriptors and retain the original operation diagnostic. Validation: src/application/LinuxHopper.cleanup.test.ts — retries every retained publisher after the inspection publisher close fails; recorded execution at d397ac742. Two owner-boundary cases passed with fault-injected descriptor close and simulated dependency command. No Hopper installation or real Linux run.

@morluto commented on GitHub (Oct 23, 2026): Resolved on main in [25b28ffe8](https://github.com/morluto/rea/commit/25b28ffe8b0efd5d8b3a59d2957e55d5a72b8255). Both inspection and installation-host closure drain retained publishers even after current publisher close fails. Executed mixed-owner controls release both exact real descriptors and retain the original operation diagnostic. Validation: `src/application/LinuxHopper.cleanup.test.ts` — retries every retained publisher after the inspection publisher close fails; recorded execution at `d397ac742`. Two owner-boundary cases passed with fault-injected descriptor close and simulated dependency command. No Hopper installation or real Linux run.

bsakaguchi referenced this issue

2026-10-31 00:43:33 +00:00

[GH-ISSUE #6109] [Bug] JavaScript checkpoint refusal recommends unavailable MCP transport recovery #3741

bsakaguchi referenced this issue

2026-10-31 01:09:38 +00:00

[PR #3545] [CLOSED] fix(javascript): explain parent budget exhaustion after a static checkpoint #5815

bsakaguchi referenced this issue

2026-10-31 01:09:39 +00:00

[PR #3550] [CLOSED] fix(javascript): keep parent budget exhaustion from advising more memory #5816

bsakaguchi referenced this issue

2026-11-01 21:45:19 +00:00

[PR #5860] [MERGED] fix(errors): name fixed plist decode allowances instead of host memory in recovery #7090