[PR #6357] [MERGED] fix(lifecycle): retain exact cleanup owners when close or prelaunch fails #7304

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

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6357
Author: @morluto
Created: 10/22/2026
Status: ✅ Merged
Merged: 10/22/2026
Merged by: @morluto

Base: main ← Head: cursor/cleanup-ownership-transfer-68f2


📝 Commits (4)

  • 18d7ffa fix(hopper): retain file-offset reads when descriptor close fails
  • 752f4e9 fix(native): retain LLDB prelaunch cleanup ownership
  • 66e1e0c fix(hopper): bind combined close failures to both owners
  • 1ec1a1e fix(native): retain resolver preparation cleanup ownership

📊 Changes

5 files changed (+1137 additions, -123 deletions)

View changed files

📝 src/hopper/HopperFileOffset.ts (+258 -78)
📝 src/hopper/HopperProvider.ts (+100 -24)
📝 src/native/LldbCallTracer.ts (+269 -21)
➕ tests/boundary/providers/hopper/hopperFileOffsetClose.test.ts (+287 -0)
➕ tests/boundary/providers/native/lldbPrelaunchCleanup.test.ts (+223 -0)

📄 Description

Summary

Fixes #6030. Fixes #6056.

A rejected close or a prelaunch failure was replacing the completed observation or primary failure, reporting cleanupIncomplete: false, and dropping the exact descriptor or supervisor. Later close() then succeeded without retrying that owner.

#6047 and #6052 are the same finally-close family on Android snapshot copy and Kuna span reads. They stay open. Their session retirement and resource-scope admission are separate owners, and this change does not take them on.

Problem and expected behavior

Two production boundaries lost cleanup ownership:

  • Hopper address_to_file_offset awaited a raw FileHandle.close() in finally. Close EIO became BinaryTargetError and close EPERM became AnalysisAccessDeniedError. A non-regular source failure was replaced by that close error. The descriptor stayed open after client close() returned success.
  • LLDB prelaunch turned an ownership-helper preparation failure into LLDB exited without a usable tracer result with exit_code: null, and dropped the helper's unresolved cleanup marker. A discovery NativeCommandFailure that still owned its resolver supervisor was replaced by a capability error with cleanupIncomplete: false. Tracer close() did not call stop(), and another trace entered discovery again.

Completed reads and primary failures stay in place. Close and unresolved resolver cleanup are typed cleanup bound to the exact owner. A successful retry clears only that owner. Preparation failures keep compiler facts and do not invent an LLDB exit. The shared Darwin helper root stays with its reader; tracer close does not adopt it.

Change and scope

Follow the contribution scope.
Unresolved scope, authority, side effects, or required verification block
integration. Link the required maintainer scope decision for new providers or
capabilities involving target execution, injection, mutation, or protection
recovery; ordinary fixes within existing boundaries may state "not applicable".
Draft PRs may leave the scope decision pending and the readiness and merge checklists
unchecked. Maintainers can help resolve open questions.

Hopper file-offset reads now go through withOwnedFileHandle. The client keeps the rejected handle on an ArtifactResourceScope and retries it before the next operation and on close(). Identity checks still run on a completed snapshot; verified coordinates are retained on the cleanup error and are not returned as success while the descriptor is open. When descriptor cleanup and session close fail together, the returned aggregate is bound to both errors, so it stays incomplete until each owner resolves.

LLDB preparation failures return before finish() can rewrite them as an exit. The foreign ProviderCleanupError stays bound with bindCleanupDependencies, so the reader's later resolveCleanup() clears it. Resolver supervisors are retained on the tracer and stopped before another trace or a successful close. One owner's release does not clear another owner's marker. Discovery uses the same preparation projection when resolveXcrunTool fails while preparing the Darwin helper, including that helper's cleanup marker and compiler facts. Tracer close still does not call the helper.

Not applicable: this is an ordinary lifecycle fix inside existing Hopper and LLDB boundaries. No new provider, target execution, injection, mutation, or protection recovery.

Contract and boundary impact

  • Semantic owner and earliest changed stage: Hopper source-descriptor close after address_to_file_offset mapping, and LLDB tracer prelaunch (preparation and tool discovery), before a post-launch supervisor exists.
  • Contribution scope fit and linked maintainer decision: ordinary fix inside existing boundaries. Maintainer scope decision not applicable.
  • CLI and MCP/tool-catalog contract: no schema or catalog change. Both adapters already project AnalysisError. These failures now project as cleanup_incomplete instead of a source, access, or invented-exit failure. The primary diagnosis remains on cleanupPrimary / diagnostics.primary_error.
  • Provider, bridge, target-format, or platform compatibility: Hopper file-offset mapping and the macOS LLDB tracer. No bridge protocol change. Database targets still stop before the source read.
  • Evidence, artifact, provenance, or reconstruction contract: completed native mapping and read facts stay on the cleanup error. Verified file_offset is diagnostic evidence, not a successful coordinate result, until close succeeds and the normal identity checks pass.
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): no new target authority. Failed close retains the exact descriptor. Failed resolver stop retains that supervisor. The Darwin helper root is not adopted by the LLDB client.
  • Generated metadata (docs/public/product-catalog.json), package, or installation impact (CI generates and validates ignored catalogs and skills/; edit authored instructions in .agents/skills/reverse-engineer-anything/ and never commit build outputs): none.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

  • 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/providers/hopper/hopperFileOffsetClose.test.ts
    • tests/boundary/providers/native/lldbPrelaunchCleanup.test.ts
  • Base reproduction or other evidence: adapted from the issue repros and the two Codex comments on the first review. On this branch the Hopper case keeps file_offset: 8 as cleanup diagnostics after a completed read, blocks another open while close still fails, and resolves only that descriptor after close starts succeeding. A non-regular source stays AnalysisArtifactChangedError as cleanupPrimary. A combined descriptor and session close stays incomplete after only the session marker resolves, and clears only after the descriptor close succeeds. The LLDB preparation case reports phase: process-preparation and leaves the helper close uncalled until the supplied owner resolves the marker. Discovery does the same when the resolver failure's cause is that preparation error. The discovery supervisor case calls stop on close, refuses another resolver launch while stop fails, and admits discovery again only after that supervisor is released.
  • User-visible CLI/MCP output (if applicable): projectAnalysisError for these failures returns code: "cleanup_incomplete". Hopper non-regular plus close keeps details.diagnostics.primary_error.code: "artifact_changed". No separate CLI or MCP adapter change.
  • Remaining proof gaps: no real Hopper, LLDB, or xcrun process was launched. Android snapshot close (#6047) and Kuna span close (#6052) were not changed. Natural OS close-failure frequency was not measured.

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

  • PATH="/usr/local/bin:$PATH" node node_modules/vitest/vitest.mjs run tests/boundary/providers/hopper/hopperFileOffsetClose.test.ts tests/boundary/providers/native/lldbPrelaunchCleanup.test.ts --maxWorkers 1 — 5 passed.
  • PATH="/usr/local/bin:$PATH" node node_modules/vitest/vitest.mjs run tests/boundary/providers/native/lldbCallTracerFailures.test.ts tests/boundary/providers/native/LldbCallTracer.cleanup.test.ts --maxWorkers 1 — 32 passed.
  • PATH="/usr/local/bin:$PATH" npx tsc --noEmit --pretty false — passed.
  • PATH="/usr/local/bin:$PATH" npm run check:fast — typecheck and lint passed.

npm run test:focused was not run as a separate script. The vitest command above is the boundary-project run those tests use; they import src/ directly. No real Hopper or LLDB installation was available.

Compatibility, safety, and release

  • Breaking changes or migration steps: none. Callers that matched BinaryTargetError or AnalysisAccessDeniedError for a close-after-read, or LLDB exited without a usable tracer result for a preparation failure, now receive typed cleanup or a preparation-stage error. Successful coordinates are unchanged when close succeeds.
  • Real Hopper/Ghidra, browser, or OS coverage: not exercised. Linux Node handles and simulated LLDB/xcrun failures only.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: no new process authority. The LLDB client does not close the shared Darwin helper. Retained descriptors and resolver supervisors stay owned until their own retry confirms release.

Author readiness

Human owner: @morluto

  • 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.
Open in Web Open in Cursor 

🔄 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/6357 **Author:** [@morluto](https://github.com/morluto) **Created:** 10/22/2026 **Status:** ✅ Merged **Merged:** 10/22/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `cursor/cleanup-ownership-transfer-68f2` --- ### 📝 Commits (4) - [`18d7ffa`](https://github.com/morluto/rea/commit/18d7ffa7a351260d4caec9c835c2463373c040cc) fix(hopper): retain file-offset reads when descriptor close fails - [`752f4e9`](https://github.com/morluto/rea/commit/752f4e97ec5e43d0ce7cb9dd0182c7176bb9057d) fix(native): retain LLDB prelaunch cleanup ownership - [`66e1e0c`](https://github.com/morluto/rea/commit/66e1e0ccf362332256cae932899c954d416bcc27) fix(hopper): bind combined close failures to both owners - [`1ec1a1e`](https://github.com/morluto/rea/commit/1ec1a1ec7b900c8c16274df52c84adee71b698f2) fix(native): retain resolver preparation cleanup ownership ### 📊 Changes **5 files changed** (+1137 additions, -123 deletions) <details> <summary>View changed files</summary> 📝 `src/hopper/HopperFileOffset.ts` (+258 -78) 📝 `src/hopper/HopperProvider.ts` (+100 -24) 📝 `src/native/LldbCallTracer.ts` (+269 -21) ➕ `tests/boundary/providers/hopper/hopperFileOffsetClose.test.ts` (+287 -0) ➕ `tests/boundary/providers/native/lldbPrelaunchCleanup.test.ts` (+223 -0) </details> ### 📄 Description <!-- CURSOR_AGENT_PR_BODY_BEGIN --> ## Summary Fixes #6030. Fixes #6056. A rejected close or a prelaunch failure was replacing the completed observation or primary failure, reporting `cleanupIncomplete: false`, and dropping the exact descriptor or supervisor. Later `close()` then succeeded without retrying that owner. #6047 and #6052 are the same finally-close family on Android snapshot copy and Kuna span reads. They stay open. Their session retirement and resource-scope admission are separate owners, and this change does not take them on. ## Problem and expected behavior Two production boundaries lost cleanup ownership: - Hopper `address_to_file_offset` awaited a raw `FileHandle.close()` in `finally`. Close `EIO` became `BinaryTargetError` and close `EPERM` became `AnalysisAccessDeniedError`. A non-regular source failure was replaced by that close error. The descriptor stayed open after client `close()` returned success. - LLDB prelaunch turned an ownership-helper preparation failure into `LLDB exited without a usable tracer result` with `exit_code: null`, and dropped the helper's unresolved cleanup marker. A discovery `NativeCommandFailure` that still owned its resolver supervisor was replaced by a capability error with `cleanupIncomplete: false`. Tracer `close()` did not call `stop()`, and another trace entered discovery again. Completed reads and primary failures stay in place. Close and unresolved resolver cleanup are typed cleanup bound to the exact owner. A successful retry clears only that owner. Preparation failures keep compiler facts and do not invent an LLDB exit. The shared Darwin helper root stays with its reader; tracer close does not adopt it. ## Change and scope Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). Unresolved scope, authority, side effects, or required verification block integration. Link the required maintainer scope decision for new providers or capabilities involving target execution, injection, mutation, or protection recovery; ordinary fixes within existing boundaries may state "not applicable". Draft PRs may leave the scope decision pending and the readiness and merge checklists unchecked. Maintainers can help resolve open questions. Hopper file-offset reads now go through `withOwnedFileHandle`. The client keeps the rejected handle on an `ArtifactResourceScope` and retries it before the next operation and on `close()`. Identity checks still run on a completed snapshot; verified coordinates are retained on the cleanup error and are not returned as success while the descriptor is open. When descriptor cleanup and session close fail together, the returned aggregate is bound to both errors, so it stays incomplete until each owner resolves. LLDB preparation failures return before `finish()` can rewrite them as an exit. The foreign `ProviderCleanupError` stays bound with `bindCleanupDependencies`, so the reader's later `resolveCleanup()` clears it. Resolver supervisors are retained on the tracer and stopped before another trace or a successful close. One owner's release does not clear another owner's marker. Discovery uses the same preparation projection when `resolveXcrunTool` fails while preparing the Darwin helper, including that helper's cleanup marker and compiler facts. Tracer close still does not call the helper. Not applicable: this is an ordinary lifecycle fix inside existing Hopper and LLDB boundaries. No new provider, target execution, injection, mutation, or protection recovery. ## Contract and boundary impact - Semantic owner and earliest changed stage: Hopper source-descriptor close after `address_to_file_offset` mapping, and LLDB tracer prelaunch (preparation and tool discovery), before a post-launch supervisor exists. - Contribution scope fit and linked maintainer decision: ordinary fix inside existing boundaries. Maintainer scope decision not applicable. - CLI and MCP/tool-catalog contract: no schema or catalog change. Both adapters already project `AnalysisError`. These failures now project as `cleanup_incomplete` instead of a source, access, or invented-exit failure. The primary diagnosis remains on `cleanupPrimary` / `diagnostics.primary_error`. - Provider, bridge, target-format, or platform compatibility: Hopper file-offset mapping and the macOS LLDB tracer. No bridge protocol change. Database targets still stop before the source read. - Evidence, artifact, provenance, or reconstruction contract: completed native mapping and read facts stay on the cleanup error. Verified `file_offset` is diagnostic evidence, not a successful coordinate result, until close succeeds and the normal identity checks pass. - Target selection, authority, upstream behavior, and cleanup details (if applicable): no new target authority. Failed close retains the exact descriptor. Failed resolver stop retains that supervisor. The Darwin helper root is not adopted by the LLDB client. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact (CI generates and validates ignored catalogs and `skills/`; edit authored instructions in `.agents/skills/reverse-engineer-anything/` and never commit build outputs): none. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [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/providers/hopper/hopperFileOffsetClose.test.ts` - `tests/boundary/providers/native/lldbPrelaunchCleanup.test.ts` - Base reproduction or other evidence: adapted from the issue repros and the two Codex comments on the first review. On this branch the Hopper case keeps `file_offset: 8` as cleanup diagnostics after a completed read, blocks another open while close still fails, and resolves only that descriptor after close starts succeeding. A non-regular source stays `AnalysisArtifactChangedError` as `cleanupPrimary`. A combined descriptor and session close stays incomplete after only the session marker resolves, and clears only after the descriptor close succeeds. The LLDB preparation case reports `phase: process-preparation` and leaves the helper `close` uncalled until the supplied owner resolves the marker. Discovery does the same when the resolver failure's cause is that preparation error. The discovery supervisor case calls `stop` on close, refuses another resolver launch while stop fails, and admits discovery again only after that supervisor is released. - User-visible CLI/MCP output (if applicable): `projectAnalysisError` for these failures returns `code: "cleanup_incomplete"`. Hopper non-regular plus close keeps `details.diagnostics.primary_error.code: "artifact_changed"`. No separate CLI or MCP adapter change. - Remaining proof gaps: no real Hopper, LLDB, or xcrun process was launched. Android snapshot close (#6047) and Kuna span close (#6052) were not changed. Natural OS close-failure frequency was not measured. 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 - `PATH="/usr/local/bin:$PATH" node node_modules/vitest/vitest.mjs run tests/boundary/providers/hopper/hopperFileOffsetClose.test.ts tests/boundary/providers/native/lldbPrelaunchCleanup.test.ts --maxWorkers 1` — 5 passed. - `PATH="/usr/local/bin:$PATH" node node_modules/vitest/vitest.mjs run tests/boundary/providers/native/lldbCallTracerFailures.test.ts tests/boundary/providers/native/LldbCallTracer.cleanup.test.ts --maxWorkers 1` — 32 passed. - `PATH="/usr/local/bin:$PATH" npx tsc --noEmit --pretty false` — passed. - `PATH="/usr/local/bin:$PATH" npm run check:fast` — typecheck and lint passed. `npm run test:focused` was not run as a separate script. The vitest command above is the boundary-project run those tests use; they import `src/` directly. No real Hopper or LLDB installation was available. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Callers that matched `BinaryTargetError` or `AnalysisAccessDeniedError` for a close-after-read, or `LLDB exited without a usable tracer result` for a preparation failure, now receive typed cleanup or a preparation-stage error. Successful coordinates are unchanged when close succeeds. - Real Hopper/Ghidra, browser, or OS coverage: not exercised. Linux Node handles and simulated LLDB/xcrun failures only. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new process authority. The LLDB client does not close the shared Darwin helper. Retained descriptors and resolver supervisors stay owned until their own retry confirms release. ## Author readiness Human owner: @morluto - [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. <!-- CURSOR_AGENT_PR_BODY_END --> <div><a href="https://cursor.com/agents/bc-2010f79b-ea28-5b0c-897c-8a468b6168f2?cursor_ref=pr_footer&cursor_cta=open_in_web"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-web-light.png"><img alt="Open in Web" width="114" height="28" src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a>&nbsp;<a href="https://cursor.com/background-agent?bcId=bc-2010f79b-ea28-5b0c-897c-8a468b6168f2&cursor_ref=pr_footer&cursor_cta=open_in_cursor"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img alt="Open in Cursor" width="131" height="28" src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a>&nbsp;</div> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:46:53 +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#7304
No description provided.