morluto/rea

Fork 0

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

[PR #3468] fix(target): report targets held open by another process as retryable #5759

New issue

Open

opened 2026-10-31 01:09:13 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3468
Author: @InspectRM
Created: 10/14/2026
Status: 🔄 Open

Base: main ← Head: fix/retryable-busy-targets

📝 Commits (7)

1f78f33 fix(target): report targets held open by another process as retryable
3cb73db fix(doctor): classify a busy target as a runtime failure
e7651bc Merge branch 'main' of https://github.com/morluto/rea into fix/retryable-busy-targets
8e355eb Merge branch 'main' of https://github.com/morluto/rea into fix/retryable-busy-targets
93665df Merge branch 'main' of https://github.com/morluto/rea into fix/retryable-busy-targets
e043f4a Merge branch 'main' of https://github.com/morluto/rea into fix/retryable-busy-targets
5859ee2 Merge branch 'main' of https://github.com/morluto/rea into fix/retryable-busy-targets

📊 Changes

7 files changed (+291 additions, -5 deletions)

View changed files

📝 src/application/BinaryTargetResolver.ts (+8 -1)
📝 src/application/Doctor.ts (+6 -3)
📝 src/domain/analysisErrorPresentation.ts (+5 -0)
📝 src/domain/analysisErrorProjection.ts (+2 -0)
📝 src/domain/configurationErrors.ts (+6 -1)
📝 src/native/pe/PeResourcesProvider.ts (+15 -0)
➕ tests/boundary/filesystem/targetBusy.test.ts (+249 -0)

📄 Description

Summary

On Windows, a selected file that another process holds open without read sharing fails with EBUSY. Target admission and inspect_pe_resources reported that as a non-retryable failure, although the unchanged request succeeds as soon as the other process releases the file. The SQLite provider already handles this case (#2822); this applies the same classification to the remaining surfaces I could reproduce.

Refs #2822 (the issue's SQLite case is already fixed on main; this covers the other entry points that hit the same sharing violation).

Problem and expected behavior

Smallest trigger, on Windows: hold a PE file open with [System.IO.File]::Open(path, 'Open', 'ReadWrite', 'None'), then run rea inspect-artifact <path>.

Before: target_unavailable, retryable: false, with "Check that the path exists, is readable, and points to a supported file." inspect-pe-resources returned artifact_operation_failed, retryable: false, whose message nonetheless says "try again".
Expected: a retryable result that names the sharing violation, since retrying the unchanged request succeeds once the holder releases the file.
After: target_unavailable, category unavailable, retryable: true, details.constraint: "target_in_use", with a message saying another process holds the target without read sharing.

Change and scope

classifyBinaryTargetFilesystemFailure marks EBUSY with a new BinaryTargetError constraint, target_in_use. The projection treats that constraint as retryable, and the presentation gives it a sharing-specific message (remediation falls back to the same text). Every caller of the classifier picks this up: target resolution (inspect-artifact, analyze, open_binary), managed snapshots (inspect-managed-artifact), and managed member comparison.
inspect_pe_resources reads through its own stable-artifact path, so PeResourcesProvider maps EBUSY to a retryable AnalysisCapabilityUnavailableError, mirroring SqliteDatabaseFailures beside its existing EACCES/ENOENT handling.
rea doctor --target classifies a target_in_use target as runtime_failure instead of unsupported_target: the file is supported, only its current read failed (review feedback).
Not included: other providers that read targets through their own paths. I only changed surfaces where I reproduced the failure. The error code set is unchanged; no new code was added.

Contract and boundary impact

Semantic owner and earliest changed stage: target filesystem admission (BinaryTargetResolver) and the PE resource provider's acquisition failure mapping.
Contribution scope fit and linked maintainer decision: ordinary fix within existing boundaries; not applicable.
CLI and MCP/tool-catalog contract: no schema or catalog change. retryable is already a boolean for target_unavailable and capability_unavailable in docs/error-contract.schema.json; the new details.constraint value is target_in_use. node scripts/generate-error-schema.mjs --check passes.
Provider, bridge, target-format, or platform compatibility: the trigger is Windows-specific (sharing violations surface as EBUSY); other platforms are unaffected unless they report EBUSY for a target read.
Evidence, artifact, provenance, or reconstruction contract: none; failures only.
Target selection, authority, upstream behavior, and cleanup details: none changed. REA still does not wait or retry by itself; it only reports the failure as retryable.
Generated metadata, package, or installation impact: none. npm run metadata:check passes.

Expected effects:

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: tests/boundary/filesystem/targetBusy.test.ts, following tests/boundary/sqlite/sqliteDatabaseBusy.test.ts:
- injected EBUSY for target resolution, then success after release (all platforms);
- injected EBUSY for rea doctor --target (runtime_failure, then healthy after release; all platforms);
- injected EBUSY for inspect_pe_resources, then success after release (all platforms);
- a Windows-only case where a separate PowerShell process holds both files open without sharing (resolver, doctor and PE resources). Each holder is registered for cleanup at spawn and its readiness wait is bounded.
Base reproduction: all three tests fail on cbc0e97c for the expected reason (constraint: undefined / retryable: false / "Artifact could not be read or written"), including the real-holder Windows case.

User-visible CLI output, Windows 11 Pro (build 26200), built CLI from this branch, with the target held by another process (and then released):

command	base: code / retryable	this branch: code / retryable	after release
`inspect-artifact`	`target_unavailable` / false	`target_unavailable` / true	succeeds
`inspect-managed-artifact`	`target_unavailable` / false	`target_unavailable` / true	succeeds
`analyze`	`target_unavailable` / false	`target_unavailable` / true	passes admission
`inspect-pe-resources`	`artifact_operation_failed` / false	`capability_unavailable` / true	succeeds

Projected error on this branch:

{
  "code": "target_unavailable",
  "category": "unavailable",
  "message": "The selected target is held open by another process without read sharing. Close that application or select a copy, then retry.",
  "retryable": true,
  "remediation": { "action": "The selected target is held open by another process without read sharing. Close that application or select a copy, then retry." },
  "details": { "path": "…", "reason": "target is in use by another process (EBUSY): …", "constraint": "target_in_use" }
}

Remaining proof gaps: no macOS/Linux run (the trigger is Windows-specific). analyze after release stopped at provider_unavailable because that shell had no Ghidra configured; that is past target admission and unchanged by this PR. I did not audit providers that read targets outside the two paths changed here.

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

All on Windows 11 Pro 26200, Node 22.23.1, from a dev checkout without the CI-built Windows native addon (native/windows/build is absent locally).

npx vitest run --project boundary tests/boundary/filesystem/targetBusy.test.ts: 3 failed on base; 4 passed on this branch. Reverting only the Doctor.ts change makes the two doctor assertions fail with unsupported_target.
npx vitest run --project services src/application/Doctor.test.ts: 1 failure (checks the canonical user install after the system launcher, POSIX path fixture), identical on base.
npx vitest run --project boundary --project domain tests/boundary/filesystem/targetBusy.test.ts tests/boundary/filesystem/binaryTarget.test.ts src/domain/analysisErrorProjection.test.ts: 106 passed, 8 skipped.
npm run check:changed selection (domain, services, adapters, composition, conformance, evaluation; --changed against the base), run file-for-file on base and on this branch: 1452 tests, 1164 passed, 183 failed, 105 skipped on both, with 0 differing outcomes. The 183 base failures are local-environment ones: the missing native addon (PrivateRuntimeRoot unavailable, which fails the IDA/Ghidra session fixtures), symlink creation without Developer Mode (EPERM), and POSIX-path fixtures.
tests/boundary/filesystem/peResources.test.ts and tests/boundary/sqlite/sqliteDatabaseBusy.test.ts fail identically on base and branch here (symlink EPERM; missing native addon).
npx tsc --noEmit: passes.
npm run lint: 0 errors; warning count unchanged from base (1381).
npx oxfmt --check on changed files: passes. Pre-commit oxfmt and oxlint --fix made no changes.
npm run verify:test-discovery: all 1275 test files discovered exactly once.
npm run metadata:check and node scripts/generate-error-schema.mjs --check: pass.

Compatibility, safety, and release

Breaking changes or migration steps: none. Callers that branched on retryable: false for this case will now see true; code is unchanged for target admission. inspect_pe_resources changes from artifact_operation_failed to capability_unavailable for this case, matching the SQLite provider.
Real Hopper/Ghidra, browser, or OS coverage: Windows 11 only.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no new process, file, or network effects. The message includes only the caller-selected path, which was already reported.

Author readiness

Human owner: @InspectRM

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.

_{✨ Crafted with ❤️ by Roberto & 🤖 Claude Code ✨}

_{🔄 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/3468 **Author:** [@InspectRM](https://github.com/InspectRM) **Created:** 10/14/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/retryable-busy-targets` --- ### 📝 Commits (7) - [`1f78f33`](https://github.com/morluto/rea/commit/1f78f33f7e80177bec6bce4cd8f21ee823598e93) fix(target): report targets held open by another process as retryable - [`3cb73db`](https://github.com/morluto/rea/commit/3cb73db72bef751394f0e8c615f841be45392ec0) fix(doctor): classify a busy target as a runtime failure - [`e7651bc`](https://github.com/morluto/rea/commit/e7651bceb1063e3328621922798b390a2c6c6285) Merge branch 'main' of https://github.com/morluto/rea into fix/retryable-busy-targets - [`8e355eb`](https://github.com/morluto/rea/commit/8e355eb73c383793a0dc94c13469fad93c7805ad) Merge branch 'main' of https://github.com/morluto/rea into fix/retryable-busy-targets - [`93665df`](https://github.com/morluto/rea/commit/93665df819d0a455207139d1c923d68b14b774c2) Merge branch 'main' of https://github.com/morluto/rea into fix/retryable-busy-targets - [`e043f4a`](https://github.com/morluto/rea/commit/e043f4ae6c1928f192b971f0b1b6379fd49c7f3b) Merge branch 'main' of https://github.com/morluto/rea into fix/retryable-busy-targets - [`5859ee2`](https://github.com/morluto/rea/commit/5859ee25a1b79f5b68d5607fcc19ad2b6f884b10) Merge branch 'main' of https://github.com/morluto/rea into fix/retryable-busy-targets ### 📊 Changes **7 files changed** (+291 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `src/application/BinaryTargetResolver.ts` (+8 -1) 📝 `src/application/Doctor.ts` (+6 -3) 📝 `src/domain/analysisErrorPresentation.ts` (+5 -0) 📝 `src/domain/analysisErrorProjection.ts` (+2 -0) 📝 `src/domain/configurationErrors.ts` (+6 -1) 📝 `src/native/pe/PeResourcesProvider.ts` (+15 -0) ➕ `tests/boundary/filesystem/targetBusy.test.ts` (+249 -0) </details> ### 📄 Description ## Summary On Windows, a selected file that another process holds open without read sharing fails with `EBUSY`. Target admission and `inspect_pe_resources` reported that as a non-retryable failure, although the unchanged request succeeds as soon as the other process releases the file. The SQLite provider already handles this case (#2822); this applies the same classification to the remaining surfaces I could reproduce. Refs #2822 (the issue's SQLite case is already fixed on `main`; this covers the other entry points that hit the same sharing violation). ## Problem and expected behavior Smallest trigger, on Windows: hold a PE file open with `[System.IO.File]::Open(path, 'Open', 'ReadWrite', 'None')`, then run `rea inspect-artifact <path>`. - Before: `target_unavailable`, `retryable: false`, with "Check that the path exists, is readable, and points to a supported file." `inspect-pe-resources` returned `artifact_operation_failed`, `retryable: false`, whose message nonetheless says "try again". - Expected: a retryable result that names the sharing violation, since retrying the unchanged request succeeds once the holder releases the file. - After: `target_unavailable`, category `unavailable`, `retryable: true`, `details.constraint: "target_in_use"`, with a message saying another process holds the target without read sharing. ## Change and scope - `classifyBinaryTargetFilesystemFailure` marks `EBUSY` with a new `BinaryTargetError` constraint, `target_in_use`. The projection treats that constraint as retryable, and the presentation gives it a sharing-specific message (remediation falls back to the same text). Every caller of the classifier picks this up: target resolution (`inspect-artifact`, `analyze`, `open_binary`), managed snapshots (`inspect-managed-artifact`), and managed member comparison. - `inspect_pe_resources` reads through its own stable-artifact path, so `PeResourcesProvider` maps `EBUSY` to a retryable `AnalysisCapabilityUnavailableError`, mirroring `SqliteDatabaseFailures` beside its existing `EACCES`/`ENOENT` handling. - `rea doctor --target` classifies a `target_in_use` target as `runtime_failure` instead of `unsupported_target`: the file is supported, only its current read failed (review feedback). - Not included: other providers that read targets through their own paths. I only changed surfaces where I reproduced the failure. The error code set is unchanged; no new code was added. ## Contract and boundary impact - Semantic owner and earliest changed stage: target filesystem admission (`BinaryTargetResolver`) and the PE resource provider's acquisition failure mapping. - Contribution scope fit and linked maintainer decision: ordinary fix within existing boundaries; not applicable. - CLI and MCP/tool-catalog contract: no schema or catalog change. `retryable` is already a boolean for `target_unavailable` and `capability_unavailable` in `docs/error-contract.schema.json`; the new `details.constraint` value is `target_in_use`. `node scripts/generate-error-schema.mjs --check` passes. - Provider, bridge, target-format, or platform compatibility: the trigger is Windows-specific (sharing violations surface as `EBUSY`); other platforms are unaffected unless they report `EBUSY` for a target read. - Evidence, artifact, provenance, or reconstruction contract: none; failures only. - Target selection, authority, upstream behavior, and cleanup details: none changed. REA still does not wait or retry by itself; it only reports the failure as retryable. - Generated metadata, package, or installation impact: none. `npm run metadata:check` passes. Expected effects: - [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 - Tests added or updated: `tests/boundary/filesystem/targetBusy.test.ts`, following `tests/boundary/sqlite/sqliteDatabaseBusy.test.ts`: - injected `EBUSY` for target resolution, then success after release (all platforms); - injected `EBUSY` for `rea doctor --target` (`runtime_failure`, then `healthy` after release; all platforms); - injected `EBUSY` for `inspect_pe_resources`, then success after release (all platforms); - a Windows-only case where a separate PowerShell process holds both files open without sharing (resolver, doctor and PE resources). Each holder is registered for cleanup at spawn and its readiness wait is bounded. - Base reproduction: all three tests fail on `cbc0e97c` for the expected reason (`constraint: undefined` / `retryable: false` / "Artifact could not be read or written"), including the real-holder Windows case. - User-visible CLI output, Windows 11 Pro (build 26200), built CLI from this branch, with the target held by another process (and then released): | command | base: code / retryable | this branch: code / retryable | after release | | --- | --- | --- | --- | | `inspect-artifact` | `target_unavailable` / false | `target_unavailable` / true | succeeds | | `inspect-managed-artifact` | `target_unavailable` / false | `target_unavailable` / true | succeeds | | `analyze` | `target_unavailable` / false | `target_unavailable` / true | passes admission | | `inspect-pe-resources` | `artifact_operation_failed` / false | `capability_unavailable` / true | succeeds | Projected error on this branch: ```json { "code": "target_unavailable", "category": "unavailable", "message": "The selected target is held open by another process without read sharing. Close that application or select a copy, then retry.", "retryable": true, "remediation": { "action": "The selected target is held open by another process without read sharing. Close that application or select a copy, then retry." }, "details": { "path": "…", "reason": "target is in use by another process (EBUSY): …", "constraint": "target_in_use" } } ``` - Remaining proof gaps: no macOS/Linux run (the trigger is Windows-specific). `analyze` after release stopped at `provider_unavailable` because that shell had no Ghidra configured; that is past target admission and unchanged by this PR. I did not audit providers that read targets outside the two paths changed here. 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 All on Windows 11 Pro 26200, Node 22.23.1, from a dev checkout without the CI-built Windows native addon (`native/windows/build` is absent locally). - `npx vitest run --project boundary tests/boundary/filesystem/targetBusy.test.ts`: 3 failed on base; 4 passed on this branch. Reverting only the `Doctor.ts` change makes the two doctor assertions fail with `unsupported_target`. - `npx vitest run --project services src/application/Doctor.test.ts`: 1 failure (`checks the canonical user install after the system launcher`, POSIX path fixture), identical on base. - `npx vitest run --project boundary --project domain tests/boundary/filesystem/targetBusy.test.ts tests/boundary/filesystem/binaryTarget.test.ts src/domain/analysisErrorProjection.test.ts`: 106 passed, 8 skipped. - `npm run check:changed` selection (domain, services, adapters, composition, conformance, evaluation; `--changed` against the base), run file-for-file on base and on this branch: 1452 tests, 1164 passed, 183 failed, 105 skipped on both, with 0 differing outcomes. The 183 base failures are local-environment ones: the missing native addon (`PrivateRuntimeRoot` unavailable, which fails the IDA/Ghidra session fixtures), symlink creation without Developer Mode (`EPERM`), and POSIX-path fixtures. - `tests/boundary/filesystem/peResources.test.ts` and `tests/boundary/sqlite/sqliteDatabaseBusy.test.ts` fail identically on base and branch here (symlink `EPERM`; missing native addon). - `npx tsc --noEmit`: passes. - `npm run lint`: 0 errors; warning count unchanged from base (1381). - `npx oxfmt --check` on changed files: passes. Pre-commit `oxfmt` and `oxlint --fix` made no changes. - `npm run verify:test-discovery`: all 1275 test files discovered exactly once. - `npm run metadata:check` and `node scripts/generate-error-schema.mjs --check`: pass. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Callers that branched on `retryable: false` for this case will now see `true`; `code` is unchanged for target admission. `inspect_pe_resources` changes from `artifact_operation_failed` to `capability_unavailable` for this case, matching the SQLite provider. - Real Hopper/Ghidra, browser, or OS coverage: Windows 11 only. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new process, file, or network effects. The message includes only the caller-selected path, which was already reported. ## Author readiness Human owner: @InspectRM - [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 - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>✨ Crafted with ❤️ by **Roberto** & 🤖 [Claude Code](https://claude.com/claude-code) ✨</sub> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

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

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

No description provided.

Rows
Columns