morluto/rea

Fork 0

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

[GH-ISSUE #4379] [Bug] Regular-file reads return success after cancellation during final metadata checks #1735

New issue

Closed

opened 2026-10-16 21:09:38 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-16 21:09:38 +00:00

Owner

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

Before submitting

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

Area

Evidence, artifacts, or reconstruction

Observed behavior

An executed production-boundary probe called withRegularFile on a real, unchanged fixture. Cancellation delivered while its final descriptor metadata request settled did not reject the operation: it returned the decoded fixture bytes with signal.aborted === true. This occurred once in one deterministic attempt per revision on both current main and the latest release source. The owned descriptor closed successfully in both cases. This is source execution of the production file-read boundary, not a compiled CLI/MCP or provider run.

Reproduction

Prerequisites: Node v24.18.0 and npm 11.16.0, matching the repository pins. For a fresh source checkout, install the locked dependencies:

git clone https://github.com/morluto/rea.git rea
cd rea
git checkout 8ab5024397ddbd3b37ab68a13cabedc2eb7ce7dd
npm ci
repro_dir="$(mktemp -d)"

Save this complete script as $repro_dir/repro.mts outside the checkout:

import assert from "node:assert/strict";
import { mkdtemp, rm, writeFile } from "node:fs/promises";
import { join } from "node:path";
import { tmpdir } from "node:os";
import { pathToFileURL } from "node:url";
const checkout = process.env.REA_CHECKOUT;
assert.ok(checkout, "REA_CHECKOUT required");
const { withRegularFile } = await import(pathToFileURL(join(checkout, "src/application/RegularFileRead.ts")).href);
const root = await mkdtemp(join(process.env.REPRO_TMPDIR ?? tmpdir(), "rea-final-stat-cancel-"));
try {
  const path = join(root, "fixture.txt");
  await writeFile(path, "fixture bytes");
  for (const abortAtFinalStat of [false, true]) {
    const controller = new AbortController();
    const reason = new Error("fixture cancellation during final metadata");
    let handleSeen; let finalStats = 0;
    const value = await withRegularFile(path, async (handle) => {
      handleSeen = handle;
      const decoded = await handle.readFile({ encoding: "utf8" });
      const originalStat = handle.stat.bind(handle);
      handle.stat = async (...args) => {
        finalStats++;
        const metadata = await originalStat(...args);
        if (abortAtFinalStat) controller.abort(reason);
        return metadata;
      };
      return decoded;
    }, { signal: controller.signal });
    assert.equal(value, "fixture bytes");
    assert.equal(finalStats, 1);
    assert.equal(controller.signal.aborted, abortAtFinalStat);
    assert.equal(handleSeen.fd, -1, "owned descriptor must close");
    console.log(JSON.stringify({ case: abortAtFinalStat ? "abort-during-final-stat" : "valid-control", aborted: controller.signal.aborted, outcome: "resolved", value, descriptorClosed: true }));
  }
  const controller = new AbortController();
  const reason = new Error("fixture pre-abort");
  controller.abort(reason);
  let called = false;
  await assert.rejects(withRegularFile(path, async () => { called = true; return "unexpected"; }, { signal: controller.signal }), error => error === reason);
  assert.equal(called, false);
  console.log(JSON.stringify({ case: "pre-aborted-control", outcome: "rejected-with-exact-abort-reason", readCalled: false }));
  console.log(JSON.stringify({ node: process.version, platform: process.platform, architecture: process.arch }));
} finally { await rm(root, { recursive: true, force: true }); }

From the checkout, execute:

REA_CHECKOUT="$PWD" ./node_modules/.bin/tsx "$repro_dir/repro.mts"

This script was executed against both revisions listed below. To repeat against 6.5.0 source, check out the release revision listed in REA version, run npm ci, and rerun the same script. It reads an unchanged real file and wraps only the admitted handle's final stat() so cancellation arrives deterministically after the real metadata query, before the wrapper resolves. Valid and pre-aborted controls distinguish this window from ordinary successful reads and early cancellation. Fixture cleanup runs in finally.

Expected behavior

After final metadata validation and awaited cleanup, a cancellable read should observe the signal before reporting success. The repository's lifecycle contract explicitly requires checking after the last awaited finalization step. The raw regular-file boundary should preserve the caller's cancellation reason, as the pre-aborted control already does.

Impact and scope

This is a bounded cancellation race in shared file-reading infrastructure. An unchanged file can yield a successful result after the caller cancels during final metadata gathering. Source inspection shows JSON and ordinary byte/text readers use this boundary; the probe directly exercised the shared helper. No file corruption, descriptor leak, provider execution, or end-to-end CLI/MCP consequence was observed; frequency under ordinary filesystem scheduling was not measured.

REA version

Main 8ab5024397ddbd3b37ab68a13cabedc2eb7ce7dd; latest published 6.5.0 source 3b79af28e80785afce9f5998d8f03289402a1847.

Runtime and platform

Node v24.18.0; npm 11.16.0; macOS (darwin), arm64. Source execution uses the locked TSX dependency.

Target and artifact context

An isolated regular UTF-8 fixture containing fixture bytes; no target application or native provider.

Interface

Other

Shared application file-read boundary; no CLI/MCP request was executed.

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

Exact stdout was identical on both tested revisions; both runs exited 0:

{"case":"valid-control","aborted":false,"outcome":"resolved","value":"fixture bytes","descriptorClosed":true}
{"case":"abort-during-final-stat","aborted":true,"outcome":"resolved","value":"fixture bytes","descriptorClosed":true}
{"case":"pre-aborted-control","outcome":"rejected-with-exact-abort-reason","readCalled":false}
{"node":"v24.18.0","platform":"darwin","architecture":"arm64"}

The assertions describe the current faulty behavior: a zero exit confirms that cancellation during final metadata gathering was missed.

RegularFileRead.ts:86-100 checks cancellation before the final Promise.all, then returns the value without another signal check. Lifecycle guidance establishes the expected completion contract.

Typed outcome or recovery expected

Reject with the cancellation reason while still completing owned descriptor cleanup.

Analysis or suspected cause

The metadata await opens a cancellation window after the last explicit signal check. withOwnedRegularFile preserves the callback's successful outcome through close and returns it without a later signal check. This explanation matches the observed boundary result; broader caller effects remain untested.

Additional context

Related open issue #3977 concerns cancellation during Ghidra snapshot descriptor cleanup in a separate reader. The broad lifecycle audit #4227 asks for focused reports. Neither covers this ordinary regular-file final metadata window. The full open/closed issue and PR bodies and relevant current PR diffs were checked; no matching focused duplicate or pending fix was found.

Triage assessment

Impact level: P2
Assessment status: supported
Impact basis: The executed shared-boundary probe returns success for an operation cancelled during final metadata gathering; valid and pre-aborted controls isolate the missed finalization check.
Workaround status: unverified
Workaround basis: A caller-side post-return signal check could discard the value, but availability and coverage of that recovery across production consumers were not verified.

Originally created by @coygeek on GitHub (Oct 16, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4379 ### 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 Evidence, artifacts, or reconstruction ### Observed behavior An executed production-boundary probe called `withRegularFile` on a real, unchanged fixture. Cancellation delivered while its final descriptor metadata request settled did not reject the operation: it returned the decoded fixture bytes with `signal.aborted === true`. This occurred once in one deterministic attempt per revision on both current main and the latest release source. The owned descriptor closed successfully in both cases. This is source execution of the production file-read boundary, not a compiled CLI/MCP or provider run. ### Reproduction Prerequisites: Node v24.18.0 and npm 11.16.0, matching the repository pins. For a fresh source checkout, install the locked dependencies: ```sh git clone https://github.com/morluto/rea.git rea cd rea git checkout 8ab5024397ddbd3b37ab68a13cabedc2eb7ce7dd npm ci repro_dir="$(mktemp -d)" ``` Save this complete script as `$repro_dir/repro.mts` outside the checkout: ```ts import assert from "node:assert/strict"; import { mkdtemp, rm, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { tmpdir } from "node:os"; import { pathToFileURL } from "node:url"; const checkout = process.env.REA_CHECKOUT; assert.ok(checkout, "REA_CHECKOUT required"); const { withRegularFile } = await import(pathToFileURL(join(checkout, "src/application/RegularFileRead.ts")).href); const root = await mkdtemp(join(process.env.REPRO_TMPDIR ?? tmpdir(), "rea-final-stat-cancel-")); try { const path = join(root, "fixture.txt"); await writeFile(path, "fixture bytes"); for (const abortAtFinalStat of [false, true]) { const controller = new AbortController(); const reason = new Error("fixture cancellation during final metadata"); let handleSeen; let finalStats = 0; const value = await withRegularFile(path, async (handle) => { handleSeen = handle; const decoded = await handle.readFile({ encoding: "utf8" }); const originalStat = handle.stat.bind(handle); handle.stat = async (...args) => { finalStats++; const metadata = await originalStat(...args); if (abortAtFinalStat) controller.abort(reason); return metadata; }; return decoded; }, { signal: controller.signal }); assert.equal(value, "fixture bytes"); assert.equal(finalStats, 1); assert.equal(controller.signal.aborted, abortAtFinalStat); assert.equal(handleSeen.fd, -1, "owned descriptor must close"); console.log(JSON.stringify({ case: abortAtFinalStat ? "abort-during-final-stat" : "valid-control", aborted: controller.signal.aborted, outcome: "resolved", value, descriptorClosed: true })); } const controller = new AbortController(); const reason = new Error("fixture pre-abort"); controller.abort(reason); let called = false; await assert.rejects(withRegularFile(path, async () => { called = true; return "unexpected"; }, { signal: controller.signal }), error => error === reason); assert.equal(called, false); console.log(JSON.stringify({ case: "pre-aborted-control", outcome: "rejected-with-exact-abort-reason", readCalled: false })); console.log(JSON.stringify({ node: process.version, platform: process.platform, architecture: process.arch })); } finally { await rm(root, { recursive: true, force: true }); } ``` From the checkout, execute: ```sh REA_CHECKOUT="$PWD" ./node_modules/.bin/tsx "$repro_dir/repro.mts" ``` This script was executed against both revisions listed below. To repeat against 6.5.0 source, check out the release revision listed in REA version, run `npm ci`, and rerun the same script. It reads an unchanged real file and wraps only the admitted handle's final `stat()` so cancellation arrives deterministically after the real metadata query, before the wrapper resolves. Valid and pre-aborted controls distinguish this window from ordinary successful reads and early cancellation. Fixture cleanup runs in `finally`. ### Expected behavior After final metadata validation and awaited cleanup, a cancellable read should observe the signal before reporting success. The repository's lifecycle contract explicitly requires checking after the last awaited finalization step. The raw regular-file boundary should preserve the caller's cancellation reason, as the pre-aborted control already does. ### Impact and scope This is a bounded cancellation race in shared file-reading infrastructure. An unchanged file can yield a successful result after the caller cancels during final metadata gathering. Source inspection shows JSON and ordinary byte/text readers use this boundary; the probe directly exercised the shared helper. No file corruption, descriptor leak, provider execution, or end-to-end CLI/MCP consequence was observed; frequency under ordinary filesystem scheduling was not measured. ### REA version Main `8ab5024397ddbd3b37ab68a13cabedc2eb7ce7dd`; latest published 6.5.0 source `3b79af28e80785afce9f5998d8f03289402a1847`. ### Runtime and platform Node v24.18.0; npm 11.16.0; macOS (`darwin`), arm64. Source execution uses the locked TSX dependency. ### Target and artifact context An isolated regular UTF-8 fixture containing `fixture bytes`; no target application or native provider. ### Interface Other Shared application file-read boundary; no CLI/MCP request was executed. ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence Exact stdout was identical on both tested revisions; both runs exited 0: ```jsonl {"case":"valid-control","aborted":false,"outcome":"resolved","value":"fixture bytes","descriptorClosed":true} {"case":"abort-during-final-stat","aborted":true,"outcome":"resolved","value":"fixture bytes","descriptorClosed":true} {"case":"pre-aborted-control","outcome":"rejected-with-exact-abort-reason","readCalled":false} {"node":"v24.18.0","platform":"darwin","architecture":"arm64"} ``` The assertions describe the current faulty behavior: a zero exit confirms that cancellation during final metadata gathering was missed. [RegularFileRead.ts:86-100](https://github.com/morluto/rea/blob/8ab5024397ddbd3b37ab68a13cabedc2eb7ce7dd/src/application/RegularFileRead.ts#L86-L100) checks cancellation before the final `Promise.all`, then returns the value without another signal check. [Lifecycle guidance](https://github.com/morluto/rea/blob/8ab5024397ddbd3b37ab68a13cabedc2eb7ce7dd/docs/tool-design.md#L91-L97) establishes the expected completion contract. ### Typed outcome or recovery expected Reject with the cancellation reason while still completing owned descriptor cleanup. ### Analysis or suspected cause The metadata await opens a cancellation window after the last explicit signal check. `withOwnedRegularFile` preserves the callback's successful outcome through close and returns it without a later signal check. This explanation matches the observed boundary result; broader caller effects remain untested. ### Additional context Related open issue [#3977](https://github.com/morluto/rea/issues/3977) concerns cancellation during Ghidra snapshot descriptor cleanup in a separate reader. The broad lifecycle audit [#4227](https://github.com/morluto/rea/issues/4227) asks for focused reports. Neither covers this ordinary regular-file final metadata window. The full open/closed issue and PR bodies and relevant current PR diffs were checked; no matching focused duplicate or pending fix was found. ## Triage assessment - Impact level: P2 - Assessment status: supported - Impact basis: The executed shared-boundary probe returns success for an operation cancelled during final metadata gathering; valid and pre-aborted controls isolate the missed finalization check. - Workaround status: unverified - Workaround basis: A caller-side post-return signal check could discard the value, but availability and coverage of that recovery across production consumers were not verified.

bsakaguchi closed this issue

2026-10-16 21:09:38 +00:00

bsakaguchi commented

2026-10-16 21:09:40 +00:00

Author

Owner

@N0zoM1z0 commented on GitHub (Oct 16, 2026):

Reproduced on Linux x64, Node 24.18.0/npm 11.16.0, fresh main 781d5751a, and opened #4443.

The original source probe reproduces exactly: cancellation after the final real descriptor stat still returns bytes, despite successful close. Compiled probes using a genuine public Evidence export additionally confirm the close window:

Observation	Base	#4443 `fce507850`
Abort during final metadata or descriptor close	resolves after abort	rejects with exact caller reason
Descriptor closed / no retained resource	yes	yes
Source file unchanged	yes	yes
Typed Evidence consumer retains original abort reason in its cause chain	no	yes

The fix belongs to the shared descriptor owner, with an additional check after application-owned close retries. Existing read errors stay primary; a failed close retains its exact owner and composed failures. The redundant forwarding wrapper is removed. The new regression fails 6/7 cases on base; its existing-primary-failure control passes.

Executed validation: 175 related tests, check:fast and docs:check passed. Actual CLI/stdio MCP import → filtered summary → malformed-input recovery → complete export → close/reimport passed twice per revision, retaining all 3 record IDs and artifact metadata. Median task base/fix wall 5.12/4.86s, CPU 6.21/6.04s, sampled peak 0.633/0.648GiB; shared-host samples establish no speedup. All 9 MCP request+reply token proxy totals were 2658–2668; schemas/control/CLI/billing excluded.

The deterministic cancellation proof wraps real filesystem calls at the production boundary. Public CLI/MCP runs verify compatibility and recovery; they do not claim reproduction of that exact cancellation window through the SDK. Other platforms/native engines were not exercised for this fix. Raw evidence and actual bundle SHA are retained locally in /tmp/rea-4379-file-finalization-20261016/. Related audit: #4227.

@N0zoM1z0 commented on GitHub (Oct 16, 2026): Reproduced on **Linux x64, Node 24.18.0/npm 11.16.0, fresh main `781d5751a`**, and opened #4443. The original source probe reproduces exactly: cancellation after the final real descriptor stat still returns bytes, despite successful close. Compiled probes using a genuine public Evidence export additionally confirm the close window: | Observation | Base | #4443 `fce507850` | |---|---|---| | Abort during final metadata or descriptor close | resolves after abort | rejects with exact caller reason | | Descriptor closed / no retained resource | yes | yes | | Source file unchanged | yes | yes | | Typed Evidence consumer retains original abort reason in its cause chain | no | yes | The fix belongs to the shared descriptor owner, with an additional check after application-owned close retries. Existing read errors stay primary; a failed close retains its exact owner and composed failures. The redundant forwarding wrapper is removed. The new regression fails 6/7 cases on base; its existing-primary-failure control passes. Executed validation: 175 related tests, check:fast and docs:check passed. Actual CLI/stdio MCP import → filtered summary → malformed-input recovery → complete export → close/reimport passed twice per revision, retaining all 3 record IDs and artifact metadata. Median task base/fix wall 5.12/4.86s, CPU 6.21/6.04s, sampled peak 0.633/0.648GiB; shared-host samples establish no speedup. All 9 MCP request+reply token proxy totals were 2658–2668; schemas/control/CLI/billing excluded. The deterministic cancellation proof wraps real filesystem calls at the production boundary. Public CLI/MCP runs verify compatibility and recovery; they do not claim reproduction of that exact cancellation window through the SDK. Other platforms/native engines were not exercised for this fix. Raw evidence and actual bundle SHA are retained locally in `/tmp/rea-4379-file-finalization-20261016/`. Related audit: #4227.

bsakaguchi referenced this issue

2026-10-31 00:50:31 +00:00

[GH-ISSUE #7597] [Bug] Bun startup failures receive registry-retry recovery advice #4528

bsakaguchi referenced this issue

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

[GH-ISSUE #7638] [Bug] Update failure remediation inserts a second terminal period #4562

bsakaguchi referenced this issue

2026-10-31 00:55:01 +00:00

[PR #1735] [MERGED] feat(update): update Bun global installations #4785