morluto/rea

Fork 0

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

[PR #4756] [CLOSED] fix(native): retain Apple dispatch metadata descriptor cleanup ownership #6390

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4756
Author: @Swatantra-66
Created: 10/17/2026
Status: ❌ Closed

Base: main ← Head: fix/apple-dispatch-descriptor-ownership

📝 Commits (1)

a4d934c fix(native): retain Apple dispatch metadata descriptor cleanup ownership

📊 Changes

3 files changed (+437 additions, -59 deletions)

View changed files

📝 src/native/AppleDispatchMetadata.ts (+199 -34)
📝 src/native/NativeMacOSProvider.ts (+18 -25)
➕ tests/boundary/providers/native/nativeDispatchMetadataFileCleanup.test.ts (+220 -0)

📄 Description

Summary

Fixes #4743.
Retains target descriptor cleanup ownership during Apple dispatch metadata inspection, keeps the physical primary failure when descriptor close rejects, retries descriptor cleanup on client close, and separates Mach-O decoding failures from file adapter errors while preserving complete backward compatibility for direct callers.

Problem and expected behavior

inspectAppleDispatchMetadata previously opened the target executable binary with raw openRegularFile and closed it in a naive finally { await handle.close() } block without tying it to an ArtifactResourceScope.

If a physical failure occurred (such as EvidenceIntegrityError from a changed binary digest) and the subsequent close also rejected:

The close rejection replaced the primary failure.
The client returned an ordinary ProviderAdapterError with cleanupIncomplete: false.
The descriptor was leaked without being retained for retry on client close.

Expected behavior:

Preserve the physical failure as the primary error.
Report descriptor close failure via ProviderCleanupError with cleanupIncomplete: true and cleanupResources: [path].
Retain the exact open descriptor within the provider client's #dispatchResources scope and retry it on client.close().
Clear cleanupIncomplete uncertainty upon successful release.

Change and scope

AppleDispatchMetadata.ts:
- Migrated regular file access to withArtifactRegularFile(target.path, resources, signal, ..., "reject").
- Preserved backward compatibility for direct callers that do not supply an ArtifactResourceScope (such as scripts/verify/apple/dispatch.mjs:57). Fallback calls instantiate an owned ArtifactResourceScope, run inside owned.run(...), and close cleanly. If both inspection and scope close fail, an AggregateError retains the inspection failure as primary (cause and errors[0]).
- Separated decoding errors from file errors: wrapped decodeAppleDispatchMetadata in a try...catch throwing AnalysisOutputError so malformed Mach-O inputs (e.g. misaligned Swift metadata sections) are correctly classified as output/decode failures rather than file adapter errors.
- Added explicit return type Promise<NativeDispatchMetadataResult> to inspectAppleDispatchMetadata.
- Added nativeDispatchMetadataFailure(cause, path) helper that translates cleanup failures to ProviderCleanupError, binds cleanup leases via bindArtifactFileCleanup(cause, failure), and extracts primary physical errors via artifactFilePrimaryFailure.
NativeMacOSProvider.ts:
- Added #dispatchResources = new ArtifactResourceScope() to NativeMacOSClient.
- Wrapped inspect_native_dispatch_metadata in this.#dispatchResources.run(...).
- In NativeMacOSClient.close(), retries the dispatch resource scope via closeNativeFileScope(this.#dispatchResources, "native-dispatch-metadata-target") and includes it in cleanupErrors.
tests/boundary/providers/native/nativeDispatchMetadataFileCleanup.test.ts:
- Added boundary tests verifying:
  1. Integrity failure is preserved as primary when close rejects, reporting cleanupIncomplete: true and retrying descriptor close upon client.close().
  2. Completed read whose close rejects reports ProviderCleanupError and clears upon retry.
  3. Direct caller backward compatibility without resources (both 2-argument and 3-argument forms).
  4. Decoding failure reporting as AnalysisOutputError rather than a file error.
  5. Direct fallback scope close rejection preserving the primary error identity and EvidenceIntegrityError in AggregateError.

Validation performed

node ./node_modules/vitest/vitest.mjs run tests/boundary/providers/native/nativeDispatchMetadataFileCleanup.test.ts src/native/AppleDispatchMetadata.test.ts: 57 tests passed (100% green).
npx oxlint src/native/AppleDispatchMetadata.ts src/native/NativeMacOSProvider.ts tests/boundary/providers/native/nativeDispatchMetadataFileCleanup.test.ts: 0 errors.
npx tsc --noEmit: 0 errors.
npm run check:fast: 2/2 tasks successful (turbo typecheck & lint passed).

Compatibility, safety, and release

Breaking changes or migration steps: None; full backward compatibility preserved.
Package or release metadata impact: None.
Security, privacy, process, or containment review: Read-only metadata inspection; no mutation authority.

Author readiness

Human owner: @Swatantra-66

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/4756 **Author:** [@Swatantra-66](https://github.com/Swatantra-66) **Created:** 10/17/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/apple-dispatch-descriptor-ownership` --- ### 📝 Commits (1) - [`a4d934c`](https://github.com/morluto/rea/commit/a4d934c078ad71da84b852d015a1fd62de6b6d10) fix(native): retain Apple dispatch metadata descriptor cleanup ownership ### 📊 Changes **3 files changed** (+437 additions, -59 deletions) <details> <summary>View changed files</summary> 📝 `src/native/AppleDispatchMetadata.ts` (+199 -34) 📝 `src/native/NativeMacOSProvider.ts` (+18 -25) ➕ `tests/boundary/providers/native/nativeDispatchMetadataFileCleanup.test.ts` (+220 -0) </details> ### 📄 Description ## Summary Fixes #4743. Retains target descriptor cleanup ownership during Apple dispatch metadata inspection, keeps the physical primary failure when descriptor close rejects, retries descriptor cleanup on client close, and separates Mach-O decoding failures from file adapter errors while preserving complete backward compatibility for direct callers. ## Problem and expected behavior `inspectAppleDispatchMetadata` previously opened the target executable binary with raw `openRegularFile` and closed it in a naive `finally { await handle.close() }` block without tying it to an `ArtifactResourceScope`. If a physical failure occurred (such as `EvidenceIntegrityError` from a changed binary digest) and the subsequent close also rejected: 1. The close rejection replaced the primary failure. 2. The client returned an ordinary `ProviderAdapterError` with `cleanupIncomplete: false`. 3. The descriptor was leaked without being retained for retry on client close. Expected behavior: - Preserve the physical failure as the primary error. - Report descriptor close failure via `ProviderCleanupError` with `cleanupIncomplete: true` and `cleanupResources: [path]`. - Retain the exact open descriptor within the provider client's `#dispatchResources` scope and retry it on `client.close()`. - Clear `cleanupIncomplete` uncertainty upon successful release. ## Change and scope - **`AppleDispatchMetadata.ts`**: - Migrated regular file access to `withArtifactRegularFile(target.path, resources, signal, ..., "reject")`. - Preserved backward compatibility for direct callers that do not supply an `ArtifactResourceScope` (such as `scripts/verify/apple/dispatch.mjs:57`). Fallback calls instantiate an owned `ArtifactResourceScope`, run inside `owned.run(...)`, and close cleanly. If both inspection and scope close fail, an `AggregateError` retains the inspection failure as primary (`cause` and `errors[0]`). - Separated decoding errors from file errors: wrapped `decodeAppleDispatchMetadata` in a `try...catch` throwing `AnalysisOutputError` so malformed Mach-O inputs (e.g. misaligned Swift metadata sections) are correctly classified as output/decode failures rather than file adapter errors. - Added explicit return type `Promise<NativeDispatchMetadataResult>` to `inspectAppleDispatchMetadata`. - Added `nativeDispatchMetadataFailure(cause, path)` helper that translates cleanup failures to `ProviderCleanupError`, binds cleanup leases via `bindArtifactFileCleanup(cause, failure)`, and extracts primary physical errors via `artifactFilePrimaryFailure`. - **`NativeMacOSProvider.ts`**: - Added `#dispatchResources = new ArtifactResourceScope()` to `NativeMacOSClient`. - Wrapped `inspect_native_dispatch_metadata` in `this.#dispatchResources.run(...)`. - In `NativeMacOSClient.close()`, retries the dispatch resource scope via `closeNativeFileScope(this.#dispatchResources, "native-dispatch-metadata-target")` and includes it in `cleanupErrors`. - **`tests/boundary/providers/native/nativeDispatchMetadataFileCleanup.test.ts`**: - Added boundary tests verifying: 1. Integrity failure is preserved as primary when close rejects, reporting `cleanupIncomplete: true` and retrying descriptor close upon `client.close()`. 2. Completed read whose close rejects reports `ProviderCleanupError` and clears upon retry. 3. Direct caller backward compatibility without resources (both 2-argument and 3-argument forms). 4. Decoding failure reporting as `AnalysisOutputError` rather than a file error. 5. Direct fallback scope close rejection preserving the primary error identity and `EvidenceIntegrityError` in `AggregateError`. ## Validation performed - `node ./node_modules/vitest/vitest.mjs run tests/boundary/providers/native/nativeDispatchMetadataFileCleanup.test.ts src/native/AppleDispatchMetadata.test.ts`: 57 tests passed (100% green). - `npx oxlint src/native/AppleDispatchMetadata.ts src/native/NativeMacOSProvider.ts tests/boundary/providers/native/nativeDispatchMetadataFileCleanup.test.ts`: 0 errors. - `npx tsc --noEmit`: 0 errors. - `npm run check:fast`: 2/2 tasks successful (turbo typecheck & lint passed). ## Compatibility, safety, and release - Breaking changes or migration steps: None; full backward compatibility preserved. - Package or release metadata impact: None. - Security, privacy, process, or containment review: Read-only metadata inspection; no mutation authority. ## Author readiness Human owner: @Swatantra-66 - [ ] 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:14:06 +00:00

closed this issue
added the
pull-request
label

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

No description provided.

Rows
Columns