morluto/rea

Fork 0

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

[GH-ISSUE #7846] [Bug] Windows: five artifact tests fail because they rename a parent directory while its admitted file is open #4650

New issue

Closed

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

bsakaguchi commented

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

Owner

Originally created by @yuz19990106-ship-it on GitHub (Oct 28, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/7846

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

Five source tests fail deterministically on a Windows source checkout of main at 6c895eeb8. All five assert that a changed admitted source is rejected with reason: "integrity"; each instead observes reason: "io" carrying a Windows EPERM, because the test moves the parent directory of a file whose descriptor is still open.

src/artifacts/GodotPckArtifactReader.test.ts (37 tests | 2 failed)
  × reads located pack facts from the admitted descriptor after path replacement (embedded: false)
  × reads located pack facts from the admitted descriptor after path replacement (embedded: true)

src/artifacts/extraction/EmbeddedArtifactExtraction.test.ts (21 tests | 3 failed)
  × rejects a replacement at the admitted pyinstaller pathname
  × rejects a replacement at the admitted node-sea pathname
  × rejects a replacement at the admitted godot-pck pathname

Raw failures:

Error: EPERM: operation not permitted, rename
  'C:\Users\<user>\AppData\Local\Temp\rea-godot-borrowed-source-VQD4Yq\selected' ->
  'C:\Users\<user>\AppData\Local\Temp\rea-godot-borrowed-source-VQD4Yq\original'
  at src/artifacts/GodotPckArtifactReader.test.ts:402:9

AssertionError: expected ArtifactReaderFailure: EPERM: operation n… to match object { reason: 'integrity' }
- Expected  { "reason": "integrity" }
+ Received  { "reason": "io" }

Both results reproduced on two consecutive runs of the same two files (5 failed | 53 passed both times).

This is not a product defect claim: the readers behaved consistently with their own contracts on the failing path (a changed source surfaced as an I/O failure). The report concerns the test technique, which cannot express the intended scenario on Windows.

Reproduction

Clean source checkout at 6c895eeb8, npm ci, then:

node node_modules/vitest/vitest.mjs run \
  src/artifacts/GodotPckArtifactReader.test.ts \
  src/artifacts/extraction/EmbeddedArtifactExtraction.test.ts

Observed: 5 failed, 53 passed.

The failure mechanism is independent of REA. Windows refuses to rename a directory while any handle below it is open:

// node 24.22.1, win32 x64
const root = await fs.mkdtemp(join(os.tmpdir(), "rename-probe-"));
const selected = join(root, "selected");
await fs.mkdir(selected);
const path = join(selected, "game");
await fs.writeFile(path, "x");
const handle = await fs.open(path, "r");
await fs.rename(selected, join(root, "original"));
// EPERM: operation not permitted
await handle.close();
await fs.rename(selected, join(root, "original"));
// OK

Both failing tests hold descriptor.handle open across the rename:

GodotPckArtifactReader.test.ts:402 — await rename(selected, join(root, "original")) after openRegularFile(path, …) and before reader.close().
EmbeddedArtifactExtraction.test.ts:324 — the rename runs inside a close spy that wraps originalClose.call(this), so the admitted handle is closed but the enclosing scope still owns a live admitted descriptor for the replaced path.

Neither file is in the curated Windows CI lane (.github/workflows/ci.yml), so CI does not exercise them on win32.

Expected behavior

Either outcome would be reasonable; the report does not prescribe one:

Guard the affected cases the way the repository already guards platform-bound scenarios, e.g. it.skipIf(process.platform === "win32") (944 existing occurrences of that form across src/ and tests/), or
Replace the parent-directory rename with a Windows-valid equivalent that still changes the selected pathname without altering the admitted file's metadata.

CONTRIBUTING.md → "Windows source checkouts" already states that some source tests fail on Windows because fixtures use POSIX rename semantics, so this may be accepted as expected. The concrete file/test list is not documented, so a Windows contributor cannot tell whether these five belong to that class or represent a regression.

Impact and scope

Impact is limited to Windows source contributors running npm run test:local / test:changed: 5 of 58 tests in these two files fail, which makes change-scoped runs look red for artifact changes that did not touch them. No runtime, Evidence, or extraction behavior is affected; the same five tests pass on Linux CI.

Frequency: 2/2 runs, deterministic. Untested: macOS, WSL, and the curated Windows lane (these files are not selected there). Not investigated: whether other src/artifacts tests share the rename-under-open-handle pattern; a grep for rename(selected found additional candidates in tests/acceptance/asarSelectedCompanionCli.test.ts, tests/boundary/filesystem/asarSelectedCompanion.test.ts, tests/boundary/providers/kuna/kunaProjectCache.test.ts, and tests/boundary/providers/native/nativeSignatureTargetBinding.test.ts, which were not run.

REA version

6.8.0 (source revision 6c895eeb8, main)

Runtime and platform

Node 24.22.1 (.nvmrc); Windows 10.0.19045 x64; npm 11.19.1; npm ci + npm run build:cached completed

Target and artifact context

Not applicable — test-harness behavior. The affected fixtures build synthetic Godot PCK, PyInstaller, and Node.js SEA carriers.

Interface

Other

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

Commands actually run:

npm run test:local -- src/artifacts/GodotPckArtifactReader.test.ts
# 2 failed | 35 passed (37)

npm run test:local -- src/artifacts/extraction/EmbeddedArtifactExtraction.test.ts
# 3 failed | 18 passed (21)

npm run test:local -- src/artifacts/GodotPckArtifactReader.test.ts src/artifacts/extraction/EmbeddedArtifactExtraction.test.ts
# 5 failed | 53 passed (58)   [twice]

npm run test:local -- src/artifacts/UImageArtifactReader.test.ts src/artifacts/UImageArtifactReader.boundaries.test.ts \
  src/artifacts/UnityFsArtifactReader.test.ts src/artifacts/UnityFsArtifactReader.boundaries.test.ts \
  src/artifacts/HexRecordArtifactReader.test.ts
# 82 passed (82) — other new readers are Windows-clean

Evidence supporting the mechanism: a standalone Node script (no REA code) reproduced EPERM on rename-with-open-handle and success after handle.close() on the same Windows host.

Provenance of the two files, to help distinguish new from long-standing:

src/artifacts/GodotPckArtifactReader.test.ts — the failing case was introduced by a891bb80a (refactor(filesystem): preserve admitted file facts, 2026-10-25), which predates main at aa6c1d49f. It fails on Windows in both revisions.
src/artifacts/extraction/EmbeddedArtifactExtraction.test.ts — new file introduced by cb0c73246 (refactor(artifacts): reuse admitted sources for extraction, 2026-10-27), first failing on Windows in this revision.

Missing evidence: no run on macOS, Linux, WSL, or the curated Windows CI lane; no attempt to determine whether maintainers consider these cases intentionally Linux-only.

Typed outcome or recovery expected

Optional. If these cases stay Linux-only, a short note listing them (or the general rule "tests that rename a parent directory of an open admitted file are POSIX-only") in CONTRIBUTING.md → "Windows source checkouts" would let a Windows contributor classify the failures without bisecting.

Analysis or suspected cause

Source-derived, not a confirmed root cause. Windows holds a directory against rename while any child handle is open without FILE_SHARE_DELETE; fs.rename on the parent therefore fails with EPERM. The tests assert an integrity outcome from a path-replacement scenario, and on Windows the replacement step itself fails before the reader can compare admitted metadata, so the observed error surfaces as io rather than integrity. The product code appears to handle the actual failure correctly; only the test setup cannot reach the intended state.

Additional context

The five failures are unrelated to the change that surfaced them locally (a fast-forward from aa6c1d49f to 6c895eeb8); they were attributed by running the same files on the base revision.

Originally created by @yuz19990106-ship-it on GitHub (Oct 28, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/7846 ### 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 Five source tests fail deterministically on a Windows source checkout of `main` at `6c895eeb8`. All five assert that a changed admitted source is rejected with `reason: "integrity"`; each instead observes `reason: "io"` carrying a Windows `EPERM`, because the test moves the parent directory of a file whose descriptor is still open. ``` src/artifacts/GodotPckArtifactReader.test.ts (37 tests | 2 failed) × reads located pack facts from the admitted descriptor after path replacement (embedded: false) × reads located pack facts from the admitted descriptor after path replacement (embedded: true) src/artifacts/extraction/EmbeddedArtifactExtraction.test.ts (21 tests | 3 failed) × rejects a replacement at the admitted pyinstaller pathname × rejects a replacement at the admitted node-sea pathname × rejects a replacement at the admitted godot-pck pathname ``` Raw failures: ``` Error: EPERM: operation not permitted, rename 'C:\Users\<user>\AppData\Local\Temp\rea-godot-borrowed-source-VQD4Yq\selected' -> 'C:\Users\<user>\AppData\Local\Temp\rea-godot-borrowed-source-VQD4Yq\original' at src/artifacts/GodotPckArtifactReader.test.ts:402:9 AssertionError: expected ArtifactReaderFailure: EPERM: operation n… to match object { reason: 'integrity' } - Expected { "reason": "integrity" } + Received { "reason": "io" } ``` Both results reproduced on two consecutive runs of the same two files (5 failed | 53 passed both times). This is not a product defect claim: the readers behaved consistently with their own contracts on the failing path (a changed source surfaced as an I/O failure). The report concerns the test technique, which cannot express the intended scenario on Windows. ### Reproduction Clean source checkout at `6c895eeb8`, `npm ci`, then: ```sh node node_modules/vitest/vitest.mjs run \ src/artifacts/GodotPckArtifactReader.test.ts \ src/artifacts/extraction/EmbeddedArtifactExtraction.test.ts ``` Observed: 5 failed, 53 passed. The failure mechanism is independent of REA. Windows refuses to rename a directory while any handle below it is open: ```js // node 24.22.1, win32 x64 const root = await fs.mkdtemp(join(os.tmpdir(), "rename-probe-")); const selected = join(root, "selected"); await fs.mkdir(selected); const path = join(selected, "game"); await fs.writeFile(path, "x"); const handle = await fs.open(path, "r"); await fs.rename(selected, join(root, "original")); // EPERM: operation not permitted await handle.close(); await fs.rename(selected, join(root, "original")); // OK ``` Both failing tests hold `descriptor.handle` open across the rename: - `GodotPckArtifactReader.test.ts:402` — `await rename(selected, join(root, "original"))` after `openRegularFile(path, …)` and before `reader.close()`. - `EmbeddedArtifactExtraction.test.ts:324` — the rename runs inside a `close` spy that wraps `originalClose.call(this)`, so the admitted handle is closed but the enclosing scope still owns a live admitted descriptor for the replaced path. Neither file is in the curated Windows CI lane (`.github/workflows/ci.yml`), so CI does not exercise them on `win32`. ### Expected behavior Either outcome would be reasonable; the report does not prescribe one: 1. Guard the affected cases the way the repository already guards platform-bound scenarios, e.g. `it.skipIf(process.platform === "win32")` (944 existing occurrences of that form across `src/` and `tests/`), or 2. Replace the parent-directory rename with a Windows-valid equivalent that still changes the selected pathname without altering the admitted file's metadata. `CONTRIBUTING.md` → "Windows source checkouts" already states that some source tests fail on Windows because fixtures use POSIX rename semantics, so this may be accepted as expected. The concrete file/test list is not documented, so a Windows contributor cannot tell whether these five belong to that class or represent a regression. ### Impact and scope Impact is limited to Windows source contributors running `npm run test:local` / `test:changed`: 5 of 58 tests in these two files fail, which makes change-scoped runs look red for artifact changes that did not touch them. No runtime, Evidence, or extraction behavior is affected; the same five tests pass on Linux CI. Frequency: 2/2 runs, deterministic. Untested: macOS, WSL, and the curated Windows lane (these files are not selected there). Not investigated: whether other `src/artifacts` tests share the rename-under-open-handle pattern; a `grep` for `rename(selected` found additional candidates in `tests/acceptance/asarSelectedCompanionCli.test.ts`, `tests/boundary/filesystem/asarSelectedCompanion.test.ts`, `tests/boundary/providers/kuna/kunaProjectCache.test.ts`, and `tests/boundary/providers/native/nativeSignatureTargetBinding.test.ts`, which were not run. ### REA version `6.8.0` (source revision `6c895eeb8`, `main`) ### Runtime and platform Node 24.22.1 (`.nvmrc`); Windows 10.0.19045 x64; npm 11.19.1; `npm ci` + `npm run build:cached` completed ### Target and artifact context Not applicable — test-harness behavior. The affected fixtures build synthetic Godot PCK, PyInstaller, and Node.js SEA carriers. ### Interface Other ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence Commands actually run: ```sh npm run test:local -- src/artifacts/GodotPckArtifactReader.test.ts # 2 failed | 35 passed (37) npm run test:local -- src/artifacts/extraction/EmbeddedArtifactExtraction.test.ts # 3 failed | 18 passed (21) npm run test:local -- src/artifacts/GodotPckArtifactReader.test.ts src/artifacts/extraction/EmbeddedArtifactExtraction.test.ts # 5 failed | 53 passed (58) [twice] npm run test:local -- src/artifacts/UImageArtifactReader.test.ts src/artifacts/UImageArtifactReader.boundaries.test.ts \ src/artifacts/UnityFsArtifactReader.test.ts src/artifacts/UnityFsArtifactReader.boundaries.test.ts \ src/artifacts/HexRecordArtifactReader.test.ts # 82 passed (82) — other new readers are Windows-clean ``` Evidence supporting the mechanism: a standalone Node script (no REA code) reproduced `EPERM` on rename-with-open-handle and success after `handle.close()` on the same Windows host. Provenance of the two files, to help distinguish new from long-standing: - `src/artifacts/GodotPckArtifactReader.test.ts` — the failing case was introduced by `a891bb80a` (`refactor(filesystem): preserve admitted file facts`, 2026-10-25), which predates `main` at `aa6c1d49f`. It fails on Windows in both revisions. - `src/artifacts/extraction/EmbeddedArtifactExtraction.test.ts` — new file introduced by `cb0c73246` (`refactor(artifacts): reuse admitted sources for extraction`, 2026-10-27), first failing on Windows in this revision. Missing evidence: no run on macOS, Linux, WSL, or the curated Windows CI lane; no attempt to determine whether maintainers consider these cases intentionally Linux-only. ### Typed outcome or recovery expected Optional. If these cases stay Linux-only, a short note listing them (or the general rule "tests that rename a parent directory of an open admitted file are POSIX-only") in `CONTRIBUTING.md` → "Windows source checkouts" would let a Windows contributor classify the failures without bisecting. ### Analysis or suspected cause Source-derived, not a confirmed root cause. Windows holds a directory against rename while any child handle is open without `FILE_SHARE_DELETE`; `fs.rename` on the parent therefore fails with `EPERM`. The tests assert an integrity outcome from a path-replacement scenario, and on Windows the replacement step itself fails before the reader can compare admitted metadata, so the observed error surfaces as `io` rather than `integrity`. The product code appears to handle the actual failure correctly; only the test setup cannot reach the intended state. ### Additional context The five failures are unrelated to the change that surfaced them locally (a fast-forward from `aa6c1d49f` to `6c895eeb8`); they were attributed by running the same files on the base revision.

bsakaguchi closed this issue

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

bsakaguchi referenced this issue

2026-10-31 01:13:43 +00:00

[PR #4650] [MERGED] fix(ghidra): keep listing facts when one function fails to decompile #6343

bsakaguchi referenced this issue

2026-11-03 20:41:07 +00:00

[GH-ISSUE #487] [Feature] Improve Ghidra AArch64 jump-table recovery #261