[PR #6520] [MERGED] fix(javascript): report colliding recovery inputs as input errors #7376

Closed
opened 2026-11-01 21:47:25 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6520
Author: @Adolanium
Created: 10/23/2026
Status: ✅ Merged
Merged: 10/23/2026
Merged by: @morluto

Base: main ← Head: fix/recovery-case-duplicate-inputs


📝 Commits (3)

  • 58e22a6 fix(javascript): report recovery inputs that collide on a case-insensitive filesystem
  • a675c78 test(javascript): cover a recovery input selected twice on any filesystem
  • 689403d test(javascript): cover snapshot collision causes and unrelated write failures

📊 Changes

2 files changed (+143 additions, -5 deletions)

View changed files

📝 src/javascript/recovery/RecoveryFiles.test.ts (+124 -3)
📝 src/javascript/recovery/RecoveryFiles.ts (+19 -2)

📄 Description

Summary

Fixes #6521.

recover_javascript_sources copies each selected input into a fresh snapshot directory before it starts the recovery tool. When two selected inputs map to the same snapshot path, the second exclusive write failed with a raw Error: EEXIST, not with a typed REA error. This happens when one file is selected twice, or when two names differ only in letter case on a case-insensitive filesystem (default Windows and macOS). The caller got an unexpected internal failure for what is an input problem. A collision during the snapshot write is now reported as an AnalysisInputError on additional_paths that names both inputs.

Problem and expected behavior

snapshotRecoveryInput in src/javascript/recovery/RecoveryFiles.ts creates the input's directory and writes the snapshot with flag: "wx". The snapshot directory starts empty, so EEXIST there means another selected input already took that relative path: the same file selected twice, or a name that this filesystem treats as the same. A file and a directory with names that fold to the same name give ENOTDIR in the same way. Neither error was handled, so the raw Node error escaped.

Expected: the caller learns which selected input collided and how to fix the selection, through REA's input error, as for other invalid selections.

Change and scope

  • The snapshot mkdir and writeFile are wrapped. EEXIST and ENOTDIR become recoveryInputFailure("additional_paths", ...): "Selected input collides with another selected input at on this filesystem. Select each file once." The original error is kept as the cause. Other failures still propagate unchanged.
  • No change to how inputs are selected, read, digested or recovered, and no change to the tool contract.

Follow the contribution scope.
Unresolved scope, authority, side effects, or required verification block
integration. Link the required maintainer scope decision for new providers or
capabilities involving target execution, injection, mutation, or protection
recovery; ordinary fixes within existing boundaries may state "not applicable".
Draft PRs may leave the scope decision pending and the readiness and merge checklists
unchecked. Maintainers can help resolve open questions.

Contribution scope: not applicable. This is a fix inside the existing JavaScript recovery input boundary.

Contract and boundary impact

  • Semantic owner and earliest changed stage: RecoveryFiles.ts snapshot creation for selected recovery inputs.
  • Contribution scope fit and linked maintainer decision: not applicable.
  • CLI and MCP/tool-catalog contract: no schema change. A colliding selection now returns the existing input-error shape on additional_paths instead of an untyped failure.
  • Provider, bridge, target-format, or platform compatibility: matters most on case-insensitive filesystems; a duplicate selection collides on every platform.
  • Evidence, artifact, provenance, or reconstruction contract: none; a colliding selection never reaches recovery.
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): none.
  • Generated metadata (docs/public/product-catalog.json), package, or installation impact (CI generates and validates ignored catalogs and skills/; edit authored instructions in .agents/skills/reverse-engineer-anything/ and never commit build outputs): none.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

  • Tests added or updated, in src/javascript/recovery/RecoveryFiles.test.ts:
    • "reports one file selected twice as an input error" passes the same path twice to snapshotRecoveryInputs and expects an AnalysisInputError on additional_paths. It runs on every filesystem.
    • "reports one file selected under two letter cases as an input error" selects bundle.js and BUNDLE.js. It skips itself unless the temporary filesystem is case-insensitive.
    • "reports a file and directory name clash as an input error" and "keeps an unrelated snapshot write failure unchanged" inject ENOTDIR from mkdir and ENOSPC from writeFile through a node:fs/promises wrapper in the test file. The first must become the input error with the injected error as its cause. The second must reject with the injected error itself. The twice-selected test also checks that its EEXIST stays attached as the cause.
  • Base reproduction or other evidence: with the snapshot write restored to upstream/main's unguarded mkdir/writeFile, the first test fails on Linux with expected Error: EEXIST: file already exists, open … to be an instance of AnalysisInputError. The case test was run with TMPDIR on a case-insensitive filesystem (WSL /mnt/c, NTFS). It passed with the fix and failed without it.
  • User-visible CLI/MCP output (if applicable): the input error message above, in place of the raw EEXIST failure.
  • Remaining proof gaps: no run on native macOS, and no end-to-end recover_javascript_sources run with Wakaru; the change is at the snapshot step, before the tool starts.

For evidence-bearing changes:

  • Observed, derived, and inferred claims remain distinguishable.
  • Artifact identity, source provenance, and failed attempts remain preserved.
  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

On Linux (WSL Ubuntu, Node 24.22.1, the version in .nvmrc) at upstream/main 28a33cae8 plus this branch, after npm ci and npm run build:cached:

  • npx tsc -p . --noEmit passed.
  • npx oxfmt --check on the two changed files: clean. npx oxlint: 1 warning, the same as on main.
  • npx vitest run src/javascript/recovery tests/boundary/javascript: 24 files, 296 tests passed (1 skipped: the case-insensitive test on Linux's case-sensitive temp directory), in two runs.
  • The negative checks above failed as described. Also: treating every snapshot failure as an input error fails the ENOSPC test, dropping ENOTDIR from the handled codes fails the clash test, and dropping the cause fails both cause assertions (checked at 689403d3c: 24 files, 298 tests passed in two runs).

Compatibility, safety, and release

  • Breaking changes or migration steps: none.
  • Real Hopper/Ghidra, browser, or OS coverage: Linux, plus a case-insensitive NTFS temp directory through WSL.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: none; the selection is rejected before any recovery work runs.

Author readiness

Human owner: @Adolanium

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/6520 **Author:** [@Adolanium](https://github.com/Adolanium) **Created:** 10/23/2026 **Status:** ✅ Merged **Merged:** 10/23/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/recovery-case-duplicate-inputs` --- ### 📝 Commits (3) - [`58e22a6`](https://github.com/morluto/rea/commit/58e22a61bee0ce8cdfc408e5d33adc21d26234c4) fix(javascript): report recovery inputs that collide on a case-insensitive filesystem - [`a675c78`](https://github.com/morluto/rea/commit/a675c78fcffbce7213ea07ac255e94d2411135c0) test(javascript): cover a recovery input selected twice on any filesystem - [`689403d`](https://github.com/morluto/rea/commit/689403d3c5a640825d3e08f4d859e576fda1192b) test(javascript): cover snapshot collision causes and unrelated write failures ### 📊 Changes **2 files changed** (+143 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `src/javascript/recovery/RecoveryFiles.test.ts` (+124 -3) 📝 `src/javascript/recovery/RecoveryFiles.ts` (+19 -2) </details> ### 📄 Description ## Summary Fixes #6521. `recover_javascript_sources` copies each selected input into a fresh snapshot directory before it starts the recovery tool. When two selected inputs map to the same snapshot path, the second exclusive write failed with a raw `Error: EEXIST`, not with a typed REA error. This happens when one file is selected twice, or when two names differ only in letter case on a case-insensitive filesystem (default Windows and macOS). The caller got an unexpected internal failure for what is an input problem. A collision during the snapshot write is now reported as an `AnalysisInputError` on `additional_paths` that names both inputs. ## Problem and expected behavior `snapshotRecoveryInput` in `src/javascript/recovery/RecoveryFiles.ts` creates the input's directory and writes the snapshot with `flag: "wx"`. The snapshot directory starts empty, so `EEXIST` there means another selected input already took that relative path: the same file selected twice, or a name that this filesystem treats as the same. A file and a directory with names that fold to the same name give `ENOTDIR` in the same way. Neither error was handled, so the raw Node error escaped. Expected: the caller learns which selected input collided and how to fix the selection, through REA's input error, as for other invalid selections. ## Change and scope - The snapshot `mkdir` and `writeFile` are wrapped. `EEXIST` and `ENOTDIR` become `recoveryInputFailure("additional_paths", ...)`: "Selected input <path> collides with another selected input at <relative path> on this filesystem. Select each file once." The original error is kept as the cause. Other failures still propagate unchanged. - No change to how inputs are selected, read, digested or recovered, and no change to the tool contract. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). Unresolved scope, authority, side effects, or required verification block integration. Link the required maintainer scope decision for new providers or capabilities involving target execution, injection, mutation, or protection recovery; ordinary fixes within existing boundaries may state "not applicable". Draft PRs may leave the scope decision pending and the readiness and merge checklists unchecked. Maintainers can help resolve open questions. Contribution scope: not applicable. This is a fix inside the existing JavaScript recovery input boundary. ## Contract and boundary impact - Semantic owner and earliest changed stage: `RecoveryFiles.ts` snapshot creation for selected recovery inputs. - Contribution scope fit and linked maintainer decision: not applicable. - CLI and MCP/tool-catalog contract: no schema change. A colliding selection now returns the existing input-error shape on `additional_paths` instead of an untyped failure. - Provider, bridge, target-format, or platform compatibility: matters most on case-insensitive filesystems; a duplicate selection collides on every platform. - Evidence, artifact, provenance, or reconstruction contract: none; a colliding selection never reaches recovery. - Target selection, authority, upstream behavior, and cleanup details (if applicable): none. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact (CI generates and validates ignored catalogs and `skills/`; edit authored instructions in `.agents/skills/reverse-engineer-anything/` and never commit build outputs): none. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage <!-- State whether evidence is executed, source-derived, or proposed. For fixes, show the original failure and explain how the regression would fail on the affected base revision when practical. Include the most useful proof inline; link long logs or artifacts, or put them in optional details. Documentation-only changes can use source, link, or rendered-document checks. --> - Tests added or updated, in `src/javascript/recovery/RecoveryFiles.test.ts`: - "reports one file selected twice as an input error" passes the same path twice to `snapshotRecoveryInputs` and expects an `AnalysisInputError` on `additional_paths`. It runs on every filesystem. - "reports one file selected under two letter cases as an input error" selects `bundle.js` and `BUNDLE.js`. It skips itself unless the temporary filesystem is case-insensitive. - "reports a file and directory name clash as an input error" and "keeps an unrelated snapshot write failure unchanged" inject `ENOTDIR` from `mkdir` and `ENOSPC` from `writeFile` through a `node:fs/promises` wrapper in the test file. The first must become the input error with the injected error as its cause. The second must reject with the injected error itself. The twice-selected test also checks that its `EEXIST` stays attached as the cause. - Base reproduction or other evidence: with the snapshot write restored to upstream/main's unguarded `mkdir`/`writeFile`, the first test fails on Linux with `expected Error: EEXIST: file already exists, open … to be an instance of AnalysisInputError`. The case test was run with `TMPDIR` on a case-insensitive filesystem (WSL `/mnt/c`, NTFS). It passed with the fix and failed without it. - User-visible CLI/MCP output (if applicable): the input error message above, in place of the raw `EEXIST` failure. - Remaining proof gaps: no run on native macOS, and no end-to-end `recover_javascript_sources` run with Wakaru; the change is at the snapshot step, before the tool starts. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed On Linux (WSL Ubuntu, Node 24.22.1, the version in `.nvmrc`) at upstream/main 28a33cae8 plus this branch, after `npm ci` and `npm run build:cached`: - `npx tsc -p . --noEmit` passed. - `npx oxfmt --check` on the two changed files: clean. `npx oxlint`: 1 warning, the same as on main. - `npx vitest run src/javascript/recovery tests/boundary/javascript`: 24 files, 296 tests passed (1 skipped: the case-insensitive test on Linux's case-sensitive temp directory), in two runs. - The negative checks above failed as described. Also: treating every snapshot failure as an input error fails the `ENOSPC` test, dropping `ENOTDIR` from the handled codes fails the clash test, and dropping the cause fails both cause assertions (checked at 689403d3c: 24 files, 298 tests passed in two runs). ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: Linux, plus a case-insensitive NTFS temp directory through WSL. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none; the selection is rejected before any recovery work runs. ## Author readiness <!-- The human owner completes these declarations before requesting review. The PR author is the owner by default. A different owner must explicitly accept ownership on this PR. AI agents must leave these declarations to a human to dictate to them or there could be catastrophic consequences for many users of REA. Draft PRs may leave them unchecked. These declarations record initial readiness when opening the PR for review; later pushes do not require a new human sign-off. Keep the description and validation evidence current as the work changes. See CONTRIBUTING.md#pull-request-ownership-and-readiness. --> Human owner: @Adolanium - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge <!-- Draft PRs may leave pending items unchecked. Resolve material scope and authority questions before integration. --> - [ ] This PR addresses a concrete problem or an agreed enhancement. - [ ] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [ ] The PR has one focused outcome and the title follows `type(scope): outcome`. - [ ] Related issue is linked, or the reason for not linking one is stated above. - [ ] Tests cover changed observable behavior and meaningful failure paths. - [ ] Owning docs, contracts, and generated metadata are updated where needed. - [ ] User-visible CLI/MCP changes include representative output. - [ ] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:47:25 +00:00
Sign in to join this conversation.
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#7376
No description provided.