[PR #6062] [CLOSED] npm run test:focused -- tests/process-global/hopperFileOffsetDescriptorOwnership.test.ts - 4 passed. #7137

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

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6062
Author: @rkfshakti
Created: 10/21/2026
Status: ❌ Closed

Base: main ← Head: fix/hopper-file-offset-descriptor-ownership


📝 Commits (2)

  • 2d2d5c6 fix(hopper): retain failed-close file offset descriptors
  • d3e5dd0 fix(hopper): preserve source identity and cancellation cleanup

📊 Changes

3 files changed (+551 additions, -17 deletions)

View changed files

📝 src/hopper/HopperFileOffset.ts (+248 -16)
📝 src/hopper/HopperProvider.ts (+8 -1)
➕ tests/process-global/hopperFileOffsetDescriptorOwnership.test.ts (+295 -0)

📄 Description

Summary

Fixes #6030.

A rejected source-descriptor close during Hopper address_to_file_offset verification was translated as a source or access failure, hid the completed read and native mapping facts, and lost the descriptor owner needed for cleanup retry.

Problem and expected behavior

After a complete source read, an injected close failure became BinaryTargetError for EIO and AnalysisAccessDeniedError for EPERM. The descriptor remained open after the client closed. A nonregular-source control also lost its earlier AnalysisArtifactChangedError precedence.

The change keeps the completed read and mapping facts, reports descriptor cleanup separately as typed cleanup, and retains the exact failed-close owner for retry.

Change and scope

  • src/hopper/HopperFileOffset.ts: use withOwnedRegularFile; retain failed-close owners; translate cleanup failures into ProviderCleanupError with completed mapping diagnostics; preserve cancellation and nonregular-source precedence.
  • src/hopper/HopperProvider.ts: pass a retainFailedClose callback that retains the owner in the existing provider resource scope.
  • Scope: ordinary lifecycle fix within the existing Hopper file-offset boundary. No target mutation, new capability, or provider protocol change.
  • Not included: changing source-identity validation or FAT-slice semantics.

Contract and boundary impact

  • Semantic owner and earliest changed stage: mapHopperFileOffset source snapshot ownership.
  • Contribution scope fit: ordinary fix inside existing boundaries; no scope decision required.
  • CLI/MCP contract: no schema change; cleanup diagnostics and cancellation classification remain typed.
  • Provider/platform: Hopper on existing supported hosts; no bridge protocol change.
  • Evidence/provenance: completed read, native mapping, and prior failure precedence are retained.
  • Authority/cleanup: no target execution or mutation; failed-close descriptors are retained in provider cleanup.
  • Generated metadata/package: none.

Expected effects: reads existing files only; the focused test uses a real file handle and simulated bridge response.

Evidence and regression coverage

  • Tests added: tests/process-global/hopperFileOffsetDescriptorOwnership.test.ts covers completed mapping diagnostics, retained descriptor owner, retry success, and provider-scope cleanup.
  • Base reproduction: the test was applied alone to main and failed; it passes on this branch.
  • Remaining gaps: no real Hopper engine, GUI, bridge, or CLI/MCP transport run.

Validation performed

  • npm run test:focused -- tests/process-global/hopperFileOffsetDescriptorOwnership.test.ts — 2 passed.
  • npm run test:focused -- src/hopper/HopperProvider.test.ts — 3 passed.
  • npm run check:fast — passed.
  • git diff --check — passed.

Compatibility, safety, and release

  • Breaking changes: none.
  • Real Hopper coverage: not run; bridge response simulated.
  • Package/release impact: none.
  • Security/process review: no new target execution or mutation authority; cleanup ownership is retained rather than discarded.

Author readiness

Human owner: @rkfshakti

  • 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.
  • 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/6062 **Author:** [@rkfshakti](https://github.com/rkfshakti) **Created:** 10/21/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/hopper-file-offset-descriptor-ownership` --- ### 📝 Commits (2) - [`2d2d5c6`](https://github.com/morluto/rea/commit/2d2d5c6ad7d5844ea64f2a70f38826d2faf8c502) fix(hopper): retain failed-close file offset descriptors - [`d3e5dd0`](https://github.com/morluto/rea/commit/d3e5dd0201dfb2807f1e0cfecb45c7e161b74082) fix(hopper): preserve source identity and cancellation cleanup ### 📊 Changes **3 files changed** (+551 additions, -17 deletions) <details> <summary>View changed files</summary> 📝 `src/hopper/HopperFileOffset.ts` (+248 -16) 📝 `src/hopper/HopperProvider.ts` (+8 -1) ➕ `tests/process-global/hopperFileOffsetDescriptorOwnership.test.ts` (+295 -0) </details> ### 📄 Description ## Summary Fixes #6030. A rejected source-descriptor close during Hopper `address_to_file_offset` verification was translated as a source or access failure, hid the completed read and native mapping facts, and lost the descriptor owner needed for cleanup retry. ## Problem and expected behavior After a complete source read, an injected `close` failure became `BinaryTargetError` for `EIO` and `AnalysisAccessDeniedError` for `EPERM`. The descriptor remained open after the client closed. A nonregular-source control also lost its earlier `AnalysisArtifactChangedError` precedence. The change keeps the completed read and mapping facts, reports descriptor cleanup separately as typed cleanup, and retains the exact failed-close owner for retry. ## Change and scope - `src/hopper/HopperFileOffset.ts`: use `withOwnedRegularFile`; retain failed-close owners; translate cleanup failures into `ProviderCleanupError` with completed mapping diagnostics; preserve cancellation and nonregular-source precedence. - `src/hopper/HopperProvider.ts`: pass a `retainFailedClose` callback that retains the owner in the existing provider resource scope. - Scope: ordinary lifecycle fix within the existing Hopper file-offset boundary. No target mutation, new capability, or provider protocol change. - Not included: changing source-identity validation or FAT-slice semantics. ## Contract and boundary impact - Semantic owner and earliest changed stage: `mapHopperFileOffset` source snapshot ownership. - Contribution scope fit: ordinary fix inside existing boundaries; no scope decision required. - CLI/MCP contract: no schema change; cleanup diagnostics and cancellation classification remain typed. - Provider/platform: Hopper on existing supported hosts; no bridge protocol change. - Evidence/provenance: completed read, native mapping, and prior failure precedence are retained. - Authority/cleanup: no target execution or mutation; failed-close descriptors are retained in provider cleanup. - Generated metadata/package: none. Expected effects: reads existing files only; the focused test uses a real file handle and simulated bridge response. ## Evidence and regression coverage - Tests added: `tests/process-global/hopperFileOffsetDescriptorOwnership.test.ts` covers completed mapping diagnostics, retained descriptor owner, retry success, and provider-scope cleanup. - Base reproduction: the test was applied alone to `main` and failed; it passes on this branch. - Remaining gaps: no real Hopper engine, GUI, bridge, or CLI/MCP transport run. ## Validation performed - `npm run test:focused -- tests/process-global/hopperFileOffsetDescriptorOwnership.test.ts` — 2 passed. - `npm run test:focused -- src/hopper/HopperProvider.test.ts` — 3 passed. - `npm run check:fast` — passed. - `git diff --check` — passed. ## Compatibility, safety, and release - Breaking changes: none. - Real Hopper coverage: not run; bridge response simulated. - Package/release impact: none. - Security/process review: no new target execution or mutation authority; cleanup ownership is retained rather than discarded. ## Author readiness Human owner: @rkfshakti - [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. - [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>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:45:40 +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#7137
No description provided.