morluto/rea

Fork 0

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

[PR #4503] [CLOSED] fix(ownership): retain failed artifact and native handles #6278

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4503
Author: @Elements2720
Created: 10/16/2026
Status: ❌ Closed

Base: main ← Head: fix/stable-artifact-descriptor-ownership

📝 Commits (5)

845a74a fix(artifacts): retain stable read ownership on close failure
5f8b315 fix(windows): retain failed native handle ownership
9bdd269 fix(windows): address cleanup review findings
211952b fix(merge): resolve origin main conflicts
6b1c706 fix(jeb): retain cleanup ownership after acquisition failure

📊 Changes

27 files changed (+1184 additions, -306 deletions)

View changed files

📝 native/windows/src/authority.hpp (+41 -5)
📝 native/windows/src/filesystem.cc (+265 -101)
📝 native/windows/src/process.cc (+21 -9)
📝 scripts/build-windows-native.mjs (+4 -0)
📝 scripts/verify-windows-native-inspection.mjs (+41 -5)
📝 src/artifacts/AnalysisCleanupLease.ts (+23 -1)
📝 src/browser/execution/CdpRuntimeSession.ts (+31 -24)
📝 src/browser/execution/CdpWebRuntimeProvider.ts (+23 -14)
📝 src/domain/analysisErrorBase.ts (+35 -2)
📝 src/domain/browserObservationError.ts (+3 -3)
📝 src/domain/providerCleanupError.ts (+1 -2)
📝 src/domain/webScriptExportError.ts (+2 -3)
📝 src/ida/IdaSessionClient.ts (+1 -2)
📝 src/jeb/JebMcpConnection.test.ts (+106 -1)
📝 src/jeb/JebMcpConnection.ts (+74 -126)
➕ src/jeb/JebMcpConnectionOwner.ts (+158 -0)
➕ src/jeb/JebMcpDeadline.ts (+58 -0)
📝 src/jeb/JebProvider.test.ts (+66 -0)
📝 src/jeb/JebProvider.ts (+17 -2)
📝 src/process/SharedStartupAttempt.ts (+12 -3)

...and 7 more files

📄 Description

Summary

Fixes #4458 and #4505.

This PR contains two focused ownership fixes already present on the branch:

Stable artifact reads retain descriptor ownership and operation outcomes when close rejects.
Windows native handles retain exact unresolved ownership when CloseHandle fails.

Windows lifecycle flow:

Handle close
checked CloseHandle
retain exact owner on failure
move assignment closes destination before releasing source

Snapshot/runtime/process cleanup
retain source, destination, temporary cleanup, parent, and walk handles
preserve primary failures with structured cleanup details
retry only unresolved owners
retry process release directly after polling failure

No schema, database, provider authority, target execution, or deployment behavior changes are introduced.

Evidence

Focused Windows wrapper tests: 30 tests passed.
npm run check:fast: passed during the push hook.
Scoped formatting, JavaScript syntax, and diff checks passed.
Native inspection coverage now injects two CloseHandle failures against the exact snapshot destination and requires cleanup retry through the retained owner.
Full npm test is blocked before Vitest because this Windows source checkout lacks native/windows/build/manifest.json and rea-windows-x64.node.
Native fixture compilation is also unavailable here because build:windows-native requires Linux and MinGW-w64.

Merge Danger

Door: Two-way; the commits can be reverted without a schema or public contract migration.

Blast Radius: Filesystem and Windows lifecycle

The native Windows/MinGW lane must run before merge. The existing artifact ownership changes remain part of this branch and are included in the PR diff.

Author readiness

Human owner:

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.

_{🔄 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/4503 **Author:** [@Elements2720](https://github.com/Elements2720) **Created:** 10/16/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/stable-artifact-descriptor-ownership` --- ### 📝 Commits (5) - [`845a74a`](https://github.com/morluto/rea/commit/845a74a71906fc29ab915d61ebf8dfc7340a291e) fix(artifacts): retain stable read ownership on close failure - [`5f8b315`](https://github.com/morluto/rea/commit/5f8b315839479293246cb64272be97ba6ce6793d) fix(windows): retain failed native handle ownership - [`9bdd269`](https://github.com/morluto/rea/commit/9bdd2695eacf646fb1d5e7e78761d0bac4c3be1c) fix(windows): address cleanup review findings - [`211952b`](https://github.com/morluto/rea/commit/211952b60da040366597c4ba052e49a17342f92f) fix(merge): resolve origin main conflicts - [`6b1c706`](https://github.com/morluto/rea/commit/6b1c706210eeade8ddc5a754209b5915716c97f8) fix(jeb): retain cleanup ownership after acquisition failure ### 📊 Changes **27 files changed** (+1184 additions, -306 deletions) <details> <summary>View changed files</summary> 📝 `native/windows/src/authority.hpp` (+41 -5) 📝 `native/windows/src/filesystem.cc` (+265 -101) 📝 `native/windows/src/process.cc` (+21 -9) 📝 `scripts/build-windows-native.mjs` (+4 -0) 📝 `scripts/verify-windows-native-inspection.mjs` (+41 -5) 📝 `src/artifacts/AnalysisCleanupLease.ts` (+23 -1) 📝 `src/browser/execution/CdpRuntimeSession.ts` (+31 -24) 📝 `src/browser/execution/CdpWebRuntimeProvider.ts` (+23 -14) 📝 `src/domain/analysisErrorBase.ts` (+35 -2) 📝 `src/domain/browserObservationError.ts` (+3 -3) 📝 `src/domain/providerCleanupError.ts` (+1 -2) 📝 `src/domain/webScriptExportError.ts` (+2 -3) 📝 `src/ida/IdaSessionClient.ts` (+1 -2) 📝 `src/jeb/JebMcpConnection.test.ts` (+106 -1) 📝 `src/jeb/JebMcpConnection.ts` (+74 -126) ➕ `src/jeb/JebMcpConnectionOwner.ts` (+158 -0) ➕ `src/jeb/JebMcpDeadline.ts` (+58 -0) 📝 `src/jeb/JebProvider.test.ts` (+66 -0) 📝 `src/jeb/JebProvider.ts` (+17 -2) 📝 `src/process/SharedStartupAttempt.ts` (+12 -3) _...and 7 more files_ </details> ### 📄 Description ## Summary Fixes #4458 and #4505. This PR contains two focused ownership fixes already present on the branch: 1. Stable artifact reads retain descriptor ownership and operation outcomes when close rejects. 2. Windows native handles retain exact unresolved ownership when CloseHandle fails. Windows lifecycle flow: Handle close checked CloseHandle retain exact owner on failure move assignment closes destination before releasing source Snapshot/runtime/process cleanup retain source, destination, temporary cleanup, parent, and walk handles preserve primary failures with structured cleanup details retry only unresolved owners retry process release directly after polling failure No schema, database, provider authority, target execution, or deployment behavior changes are introduced. ## Evidence - Focused Windows wrapper tests: 30 tests passed. - npm run check:fast: passed during the push hook. - Scoped formatting, JavaScript syntax, and diff checks passed. - Native inspection coverage now injects two CloseHandle failures against the exact snapshot destination and requires cleanup retry through the retained owner. - Full npm test is blocked before Vitest because this Windows source checkout lacks native/windows/build/manifest.json and rea-windows-x64.node. - Native fixture compilation is also unavailable here because build:windows-native requires Linux and MinGW-w64. ## Merge Danger **Door:** Two-way; the commits can be reverted without a schema or public contract migration. **Blast Radius:** Filesystem and Windows lifecycle The native Windows/MinGW lane must run before merge. The existing artifact ownership changes remain part of this branch and are included in the PR diff. ## Author readiness Human owner:  - [ ] 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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

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

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:46:30 +00:00

[PR #6278] [MERGED] feat(native): inspect PE and ELF export tables without an engine #7252

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

No description provided.

Rows
Columns