morluto/rea

Fork 0

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

[GH-ISSUE #5135] [Bug] Replacing an observed reference symlink with a file aborts the whole inventory #3158

New issue

Closed

opened 2026-10-31 00:37:19 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-31 00:37:19 +00:00

Owner

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

Area / interface / earliest failure

Evidence, artifacts, or reconstruction / Other (production filesystem reader) / Capture or observation.

Observed behavior

readReferenceSource rejects with real OS EINVAL if a child already observed as a symlink is replaced by a regular file just before readlink. The traversal returns no inventory, including an otherwise readable sibling.

Reproduction actually run

At source revision 95c8f57aa2eafab4c8d7d43482b33ceefaf96b16, create a temporary source directory containing dangling link and readable sibling.txt. In an isolated filesystem-module wrapper, immediately before delegating that link's readlink to the real OS, unlink the symlink and write a regular file at the same path. Call the production readReferenceSource(root, resources) and await resource cleanup.

Executed Node 24.18.0 exact Vitest case tests/process-global/referenceSourceSymlinkRace.test.ts: 1 failed. The scheduling hook is injected; the replacement and errno are real filesystem operations. One deterministic run in this location (plus the same test before moving it into the isolated-fork project) failed at the expected partial-inventory assertion.

Expected behavior / typed recovery

Return the existing partial-inventory representation: a failed symlink entry with code: changed (or an explicit entry-level I/O outcome), retaining the readable sibling. Existing readStableSymlinkTarget already models observed replacement as an entry failure rather than a traversal failure.

Relevant evidence and source-derived cause

Smallest observed error: EINVAL: invalid argument, readlink '<fixture>/link'; code: EINVAL, syscall: readlink. Assertion: expected traversal not to throw; actual error escaped.

ReferenceSourceReaderEntries.ts at the tested revision: the guarded observer handles ENOENT/ENOTDIR and recognized I/O failures, but readSymlinkTarget lets this expected readlink failure escape. The generic filesystem error set omits EINVAL, so traversal rethrows it. The fix should classify this at the readlink owner, without treating unrelated EINVAL as a normal filesystem outcome.

Impact, environment, and limits

Observed loss of the complete traversal result in this controlled race. Node v24.18.0, macOS/Darwin 25.3.0 arm64; temporary directory with two entries, no analysis provider. Natural race frequency, Windows/Linux runtime behavior, and compiled CLI/MCP reproduction: NOT_ENOUGH_INFO (not measured/run). This is distinct from first-lookup absence.

Found while checking analogous producer/consumer races for #4575. Searched open and closed issues for readlink EINVAL and reference-source symlink reports; #4013 is the related unreadable-target case, not this replacement failure. No credentials or sensitive target data included.

Originally created by @morluto on GitHub (Oct 19, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5135 ### Area / interface / earliest failure Evidence, artifacts, or reconstruction / Other (production filesystem reader) / Capture or observation. ### Observed behavior `readReferenceSource` rejects with real OS `EINVAL` if a child already observed as a symlink is replaced by a regular file just before `readlink`. The traversal returns no inventory, including an otherwise readable sibling. ### Reproduction actually run At source revision `95c8f57aa2eafab4c8d7d43482b33ceefaf96b16`, create a temporary source directory containing dangling `link` and readable `sibling.txt`. In an isolated filesystem-module wrapper, immediately before delegating that link's `readlink` to the real OS, unlink the symlink and write a regular file at the same path. Call the production `readReferenceSource(root, resources)` and await resource cleanup. Executed Node 24.18.0 exact Vitest case `tests/process-global/referenceSourceSymlinkRace.test.ts`: **1 failed**. The scheduling hook is injected; the replacement and errno are real filesystem operations. One deterministic run in this location (plus the same test before moving it into the isolated-fork project) failed at the expected partial-inventory assertion. ### Expected behavior / typed recovery Return the existing partial-inventory representation: a failed symlink entry with `code: changed` (or an explicit entry-level I/O outcome), retaining the readable sibling. Existing `readStableSymlinkTarget` already models observed replacement as an entry failure rather than a traversal failure. ### Relevant evidence and source-derived cause Smallest observed error: `EINVAL: invalid argument, readlink '<fixture>/link'`; `code: EINVAL`, `syscall: readlink`. Assertion: expected traversal not to throw; actual error escaped. [`ReferenceSourceReaderEntries.ts` at the tested revision](https://github.com/morluto/rea/blob/95c8f57aa2eafab4c8d7d43482b33ceefaf96b16/src/reference/ReferenceSourceReaderEntries.ts): the guarded observer handles ENOENT/ENOTDIR and recognized I/O failures, but `readSymlinkTarget` lets this expected readlink failure escape. The generic filesystem error set omits EINVAL, so traversal rethrows it. The fix should classify this at the readlink owner, without treating unrelated EINVAL as a normal filesystem outcome. ### Impact, environment, and limits Observed loss of the complete traversal result in this controlled race. Node v24.18.0, macOS/Darwin 25.3.0 arm64; temporary directory with two entries, no analysis provider. Natural race frequency, Windows/Linux runtime behavior, and compiled CLI/MCP reproduction: **NOT_ENOUGH_INFO** (not measured/run). This is distinct from first-lookup absence. Found while checking analogous producer/consumer races for #4575. Searched open and closed issues for `readlink EINVAL` and reference-source symlink reports; #4013 is the related unreadable-target case, not this replacement failure. No credentials or sensitive target data included.

bsakaguchi

2026-10-31 00:37:19 +00:00

closed this issue
added the
triage

bug
labels

bsakaguchi commented

2026-10-31 00:37:20 +00:00

Author

Owner

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

Fixed on main in 6296fdf56, addressing #5135 and #5136 together.

The producer now handles replacement EINVAL at the actual readlink boundary rather than widening the generic I/O catch. An admitted root's later enumeration failure has its own typed rootFailure field; it is no longer fabricated as a child entry at .. The shared reader uses that fact for unresolved symlink coverage, and the importer projects it as a root-level graph limitation. Revalidation preserves the validator's parsed failure code/reason. Initial admission, cancellation and cleanup errors keep their separate outcomes; previously observed child facts survive.

Validation: the final isolated regression file failed all 4 cases on unchanged baseline 95c8f57aa, then passed after the fix. Cases exercise reader and importer with a real nested symlink→regular replacement and host EINVAL, plus real child/root renames after the actual root directory handle closes. Nesting guarantees the sibling was observed before descent rather than assuming OS enumeration order. The final focused run passed 106 checks across 14 reader/application/domain/compiled-CLI files, with 2 Windows-specific cases skipped on macOS. This includes existing mixed read/close failure and cancellation/cleanup ownership tests, and a root-coverage regression retaining nearer completed directories/exclusions. Full static/architecture checks, docs checks and 1604-file discovery passed.

Six stable compiled import-reference-source --json controls preserved the exact full graph digest. Repeated-process medians: elapsed 0.385→0.387 s, CPU 0.599→0.601 s, sampled process-family RSS 401.5→401.3 MB; arguments 33 and full response 281 cl100k-estimated tokens in both versions. One first and two repeated fresh processes per variant; OS caches were not flushed. No optimization claim, actual model usage, large/fault costs, or cross-host result is inferred.

The race scheduling is injected around real filesystem operations. Compiled CLI stable imports and ordinary failure projections were exercised; the race regressions themselves run through the production reader/application boundary, not a compiled CLI race. Native engines and Windows/Linux race behavior were not qualified. Existing directory-handle cleanup ownership remains intact. The breaking marker documents the partial-reader migration from the invalid . child to rootFailure.

@morluto commented on GitHub (Oct 19, 2026): Fixed on main in [6296fdf56](https://github.com/morluto/rea/commit/6296fdf560f1ac4aa55985558edaa98300aacdbc), addressing #5135 and #5136 together. The producer now handles replacement `EINVAL` at the actual `readlink` boundary rather than widening the generic I/O catch. An admitted root's later enumeration failure has its own typed `rootFailure` field; it is no longer fabricated as a child entry at `.`. The shared reader uses that fact for unresolved symlink coverage, and the importer projects it as a root-level graph limitation. Revalidation preserves the validator's parsed failure code/reason. Initial admission, cancellation and cleanup errors keep their separate outcomes; previously observed child facts survive. Validation: the final isolated regression file failed **all 4 cases on unchanged baseline `95c8f57aa`**, then passed after the fix. Cases exercise reader and importer with a real nested symlink→regular replacement and host `EINVAL`, plus real child/root renames after the actual root directory handle closes. Nesting guarantees the sibling was observed before descent rather than assuming OS enumeration order. The final focused run passed **106 checks across 14 reader/application/domain/compiled-CLI files**, with 2 Windows-specific cases skipped on macOS. This includes existing mixed read/close failure and cancellation/cleanup ownership tests, and a root-coverage regression retaining nearer completed directories/exclusions. Full static/architecture checks, docs checks and 1604-file discovery passed. Six stable compiled `import-reference-source --json` controls preserved the exact full graph digest. Repeated-process medians: elapsed 0.385→0.387 s, CPU 0.599→0.601 s, sampled process-family RSS 401.5→401.3 MB; arguments 33 and full response 281 cl100k-estimated tokens in both versions. One first and two repeated fresh processes per variant; OS caches were not flushed. No optimization claim, actual model usage, large/fault costs, or cross-host result is inferred. The race scheduling is injected around real filesystem operations. Compiled CLI stable imports and ordinary failure projections were exercised; the race regressions themselves run through the production reader/application boundary, not a compiled CLI race. Native engines and Windows/Linux race behavior were not qualified. Existing directory-handle cleanup ownership remains intact. The breaking marker documents the partial-reader migration from the invalid `.` child to `rootFailure`.

bsakaguchi referenced this issue

2026-10-31 01:08:15 +00:00

[PR #3158] [CLOSED] test(mcp): add regression coverage for Hopper app-exit session recovery #5648

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

No description provided.

Rows
Columns