[PR #4745] [MERGED] fix(native): retain Apple dispatch metadata descriptor cleanup ownership #6385

Closed
opened 2026-10-31 01:14:04 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4745
Author: @kaoru0822-kitauji
Created: 10/17/2026
Status: ✅ Merged
Merged: 10/17/2026
Merged by: @kaoru0822-kitauji

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


📝 Commits (4)

  • c061d21 fix(native): retain Apple dispatch metadata descriptor cleanup ownership
  • 38e96fd Merge origin/main into fix/apple-dispatch-descriptor-ownership
  • 07788d8 fix(verify): give the Apple dispatch verifier an owned file scope
  • 2b50a76 Merge origin/main into fix/apple-dispatch-descriptor-ownership

📊 Changes

4 files changed (+242 additions, -62 deletions)

View changed files

📝 scripts/verify/apple/dispatch.mjs (+15 -4)
📝 src/native/AppleDispatchMetadata.ts (+117 -32)
📝 src/native/NativeMacOSProvider.ts (+18 -26)
➕ tests/boundary/providers/native/nativeDispatchMetadataFileCleanup.test.ts (+92 -0)

📄 Description

Summary

Fixes #4743. inspect_native_dispatch_metadata now keeps a target descriptor whose close fails owned. Integrity, capacity and cancellation failures are no longer replaced by the close rejection, and client close retries the descriptor.

Problem and expected behavior

inspectAppleDispatchMetadata opened the target with raw openRegularFile and ran finally { await handle.close() }. If the body threw (for example EvidenceIntegrityError on a digest mismatch) and the close also rejected, the close rejection replaced the primary failure. The client then returned an ordinary ProviderAdapterError with cleanupIncomplete: false, and nothing retained the descriptor for retry.

Expected: keep the physical failure, report the cleanup failure next to it, retain the exact descriptor, retry it on client close, and clear the original error's cleanup uncertainty once release is confirmed.

Change and scope

  • AppleDispatchMetadata.ts: the bounded read and digest check run inside withArtifactRegularFile(..., "reject") with a caller-supplied ArtifactResourceScope. Decoding now runs after the descriptor is released, so parse work no longer holds it open. A cancellation rejected by the bounded reader is recognised only when it is the caller's exact signal.reason.
  • New appleDispatchMetadataFailure projection, following main's nativePlistFileFailure:
    • An owned cleanup failure becomes a ProviderCleanupError with cleanupPrimary set to the physical failure (and previous_error in diagnostics). It is bound to the exact leases.
    • Owner-recorded cancellation keeps its exact reason.
    • A closed scope maps to capability-unavailable.
    • Everything else stays ProviderAdapterError with the same diagnostics as before.
  • NativeMacOSProvider.ts: a new #dispatchResources scope is admitted through scope.run and retried in close() via the existing closeNativeFileScope. The client-side guess options.signal.aborted ? AnalysisCancelledError and the regularFileCancellation fallback are removed, because cancellation now arrives typed from the file owner.

Contribution scope: ordinary lifecycle fix within an existing read-only boundary; maintainer scope decision not applicable.

Contract and boundary impact

  • Semantic owner and earliest changed stage: Apple dispatch metadata target read (src/native/AppleDispatchMetadata.ts).

  • Contribution scope fit and linked maintainer decision: not applicable.

  • CLI and MCP/tool-catalog contract: unchanged schemas; only error classification under close failure changes (now cleanup_incomplete with the physical primary).

  • Provider, bridge, target-format, or platform compatibility: native macOS provider; unchanged results on success.

  • Evidence, artifact, provenance, or reconstruction contract: none.

  • Target selection, authority, upstream behavior, and cleanup details: read-only; failed descriptor closes are retained and retried.

  • Generated metadata, package, or installation impact: none.

  • Read existing files or captured data.

Evidence and regression coverage

  • Tests added: tests/boundary/providers/native/nativeDispatchMetadataFileCleanup.test.ts. It opens a real target file through the production NativeMacOSProvider client, and only that admitted descriptor's close is made to reject.
    1. Digest mismatch plus close rejection: asserts cleanupIncomplete: true, the target path in cleanupResources, and cleanupPrimary._tag === "EvidenceIntegrityError". While close still rejects, each client close retries the same descriptor (the attempt count goes 1→2 with no reopen). After release it asserts cleanupIncomplete becomes false.
    2. Completed read plus close rejection: asserts a ProviderCleanupError with no primary, and that it resolves after release.
  • Base reproduction: with both source files restored to origin/main, both new tests fail (expected false to be true on cleanupIncomplete, and ProviderAdapterError instead of ProviderCleanupError). With the fix, both pass.
  • Existing coverage still passes, including native dispatch metadata error results (tagged cancellation and EvidenceIntegrityError).
  • Remaining proof gaps: no real Mach-O dispatch decode under a close fault, no naturally occurring OS close failure, and no dedicated test for cancellation during final close.

Validation performed

  • npm run test:focused -- tests/boundary/providers/native/nativeDispatchMetadataFileCleanup.test.ts tests/boundary/providers/native/nativeProvider.test.ts src/native/NativeMacOSProviderInput.test.ts tests/boundary/providers/native/nativePlistFileCleanup.test.ts: 4 files, 63 tests passed.
  • New test file against base source: 2 failed.
  • npx tsc --noEmit -p .: clean. Pre-commit oxfmt/oxlint --fix: passed.

Compatibility, safety, and release

  • Breaking changes or migration steps: none.
  • Real Hopper/Ghidra, browser, or OS coverage: macOS arm64 / Node 24 controlled boundary only.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: read-only; no new authority.

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.

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.

🤖 Generated with 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/4745 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/17/2026 **Status:** ✅ Merged **Merged:** 10/17/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/apple-dispatch-descriptor-ownership` --- ### 📝 Commits (4) - [`c061d21`](https://github.com/morluto/rea/commit/c061d21cb52133a1c1e23641c4eb8bcf2808087a) fix(native): retain Apple dispatch metadata descriptor cleanup ownership - [`38e96fd`](https://github.com/morluto/rea/commit/38e96fd8496e621a0b3c6b5d7a08b371c41b2acb) Merge origin/main into fix/apple-dispatch-descriptor-ownership - [`07788d8`](https://github.com/morluto/rea/commit/07788d80ab37b81c5a3b818d0fcad1b440c1c6c5) fix(verify): give the Apple dispatch verifier an owned file scope - [`2b50a76`](https://github.com/morluto/rea/commit/2b50a76c9f0e78bf3d482c7e3469dd54be8dbf82) Merge origin/main into fix/apple-dispatch-descriptor-ownership ### 📊 Changes **4 files changed** (+242 additions, -62 deletions) <details> <summary>View changed files</summary> 📝 `scripts/verify/apple/dispatch.mjs` (+15 -4) 📝 `src/native/AppleDispatchMetadata.ts` (+117 -32) 📝 `src/native/NativeMacOSProvider.ts` (+18 -26) ➕ `tests/boundary/providers/native/nativeDispatchMetadataFileCleanup.test.ts` (+92 -0) </details> ### 📄 Description ## Summary Fixes #4743. `inspect_native_dispatch_metadata` now keeps a target descriptor whose close fails owned. Integrity, capacity and cancellation failures are no longer replaced by the close rejection, and client close retries the descriptor. ## Problem and expected behavior `inspectAppleDispatchMetadata` opened the target with raw `openRegularFile` and ran `finally { await handle.close() }`. If the body threw (for example `EvidenceIntegrityError` on a digest mismatch) and the close also rejected, the close rejection replaced the primary failure. The client then returned an ordinary `ProviderAdapterError` with `cleanupIncomplete: false`, and nothing retained the descriptor for retry. Expected: keep the physical failure, report the cleanup failure next to it, retain the exact descriptor, retry it on client close, and clear the original error's cleanup uncertainty once release is confirmed. ## Change and scope - `AppleDispatchMetadata.ts`: the bounded read and digest check run inside `withArtifactRegularFile(..., "reject")` with a caller-supplied `ArtifactResourceScope`. Decoding now runs after the descriptor is released, so parse work no longer holds it open. A cancellation rejected by the bounded reader is recognised only when it is the caller's exact `signal.reason`. - New `appleDispatchMetadataFailure` projection, following main's `nativePlistFileFailure`: - An owned cleanup failure becomes a `ProviderCleanupError` with `cleanupPrimary` set to the physical failure (and `previous_error` in diagnostics). It is bound to the exact leases. - Owner-recorded cancellation keeps its exact reason. - A closed scope maps to capability-unavailable. - Everything else stays `ProviderAdapterError` with the same diagnostics as before. - `NativeMacOSProvider.ts`: a new `#dispatchResources` scope is admitted through `scope.run` and retried in `close()` via the existing `closeNativeFileScope`. The client-side guess `options.signal.aborted ? AnalysisCancelledError` and the `regularFileCancellation` fallback are removed, because cancellation now arrives typed from the file owner. Contribution scope: ordinary lifecycle fix within an existing read-only boundary; maintainer scope decision not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: Apple dispatch metadata target read (`src/native/AppleDispatchMetadata.ts`). - Contribution scope fit and linked maintainer decision: not applicable. - CLI and MCP/tool-catalog contract: unchanged schemas; only error classification under close failure changes (now `cleanup_incomplete` with the physical primary). - Provider, bridge, target-format, or platform compatibility: native macOS provider; unchanged results on success. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details: read-only; failed descriptor closes are retained and retried. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added: `tests/boundary/providers/native/nativeDispatchMetadataFileCleanup.test.ts`. It opens a real target file through the production `NativeMacOSProvider` client, and only that admitted descriptor's close is made to reject. 1. Digest mismatch plus close rejection: asserts `cleanupIncomplete: true`, the target path in `cleanupResources`, and `cleanupPrimary._tag === "EvidenceIntegrityError"`. While close still rejects, each client close retries the same descriptor (the attempt count goes 1→2 with no reopen). After release it asserts `cleanupIncomplete` becomes `false`. 2. Completed read plus close rejection: asserts a `ProviderCleanupError` with no primary, and that it resolves after release. - Base reproduction: with both source files restored to `origin/main`, both new tests fail (`expected false to be true` on `cleanupIncomplete`, and `ProviderAdapterError` instead of `ProviderCleanupError`). With the fix, both pass. - Existing coverage still passes, including `native dispatch metadata error results` (tagged cancellation and `EvidenceIntegrityError`). - Remaining proof gaps: no real Mach-O dispatch decode under a close fault, no naturally occurring OS close failure, and no dedicated test for cancellation during final close. ## Validation performed - `npm run test:focused -- tests/boundary/providers/native/nativeDispatchMetadataFileCleanup.test.ts tests/boundary/providers/native/nativeProvider.test.ts src/native/NativeMacOSProviderInput.test.ts tests/boundary/providers/native/nativePlistFileCleanup.test.ts`: 4 files, 63 tests passed. - New test file against base source: 2 failed. - `npx tsc --noEmit -p .`: clean. Pre-commit `oxfmt`/`oxlint --fix`: passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: macOS arm64 / Node 24 controlled boundary only. - Package or release metadata impact: none. - Security, privacy, process, or containment review: read-only; no new authority. ## Author readiness Human owner: <!-- @GitHub-handle --> - [ ] 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <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:04 +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#6385
No description provided.