[PR #5780] [MERGED] fix(hopper): preserve caller abort reasons on cancellation #7057

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

📋 Pull Request Information

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

Base: main ← Head: cursor/preserve-hopper-abort-reason-d4ac


📝 Commits (3)

  • af1b649 fix(hopper): preserve caller abort reasons on cancellation
  • ab854d7 style(hopper): format launcher cancellation return
  • 7839f78 Merge remote-tracking branch 'origin/main' into cursor/preserve-hopper-abort-reason-d4ac

📊 Changes

10 files changed (+195 additions, -42 deletions)

View changed files

➕ src/domain/hopperErrors.test.ts (+28 -0)
📝 src/domain/hopperErrors.ts (+27 -6)
📝 src/hopper/BridgeLauncher.ts (+17 -6)
📝 src/hopper/HopperClient.ts (+9 -8)
📝 src/hopper/HopperRequestQueue.test.ts (+58 -4)
📝 src/hopper/HopperRequestQueue.ts (+8 -4)
➕ src/hopper/HopperSocketConnection.test.ts (+22 -0)
📝 src/hopper/HopperSocketConnection.ts (+3 -2)
📝 tests/boundary/providers/hopper/bridgeBootstrapCleanup.test.ts (+9 -6)
📝 tests/boundary/providers/hopper/hopperClient.startupWaiters.test.ts (+14 -6)

📄 Description

Summary

Fixes #5714. Hopper cancellation kept the HopperCancelledError classification but dropped the caller's abort reason, so the queue caller could not recover the object it passed to abort.

Sibling triage reports #5393, #5377, and #5656 were rechecked on main 71b696582 and already match their expected outcomes. They are being closed as already fixed from the parent audit. Evidence:

  • #5393: npm run test:focused -- tests/boundary/javascript/workerOrderAudit.test.ts — 35 passed, including an owned Node peer that exits 7 and is then aborted. The result stays ProviderAdapterError with diagnostics.exit_code: 7, and the abort reason is a secondary AnalysisCancelledError. Fixed by f01dccab7.
  • #5377: npm run test:local -- src/domain/javascript/javascriptSemanticCallResolution.test.ts — 25 passed. The nested cb() before var cb=target stays complete: false; the same-activation call after the initializer stays complete: true. The graph owner maps a non-exact call to candidate. Fixed by 3b0217956. The public CLI was not re-executed.
  • #5656: npm run test:local -- src/browser/CdpCaptureEvents.consoleTimestamp.test.ts — 6 passed. Missing or invalid console timestamps are null with an invalid_protocol_value exclusion; 0 and -1 are preserved. Fixed by 690fc5185. The built CLI/MCP acceptance test was not re-run.

Problem and expected behavior

An inert HopperRequestQueue.run whose signal was aborted with { source: "caller", opaque: 7 } returned HopperCancelledError with no cause. The constructor also treated an explicit undefined cause as omission. The lifecycle contract requires the caller's abort reason as the typed cancellation error's cause, while cancellation classification, late-reply ownership, and cleanup stay intact.

Change and scope

HopperCancelledError now records a supplied cause whenever the argument is present, including undefined, and still omits cause when the caller does not pass one. Queue admission, the abort observer, and pre-send cancellation pass signal.reason. The same conversion is applied at Hopper client startup, socket connection, and launcher preparation, including a publisher-verification cancellation that already carried its own cause.

Not included: changing which failure wins when a physical error and a later abort overlap. That ordering is a separate contract from reason retention.

Follow the contribution scope.
Unresolved scope, authority, side effects, or required verification block integration. Ordinary fix within existing Hopper cancellation boundaries. Maintainer scope decision: not applicable.

Contract and boundary impact

  • Semantic owner and earliest changed stage: HopperCancelledError owns cause presence. The earliest changed stage is cancellation classification at the Hopper queue, client, socket, and launcher.
  • Contribution scope fit and linked maintainer decision: ordinary fix inside existing boundaries. Not applicable.
  • CLI and MCP/tool-catalog contract: none. Public projections already omit error causes.
  • Provider, bridge, target-format, or platform compatibility: Hopper typed cancellation errors now retain the caller reason. No bridge protocol or target-format change.
  • Evidence, artifact, provenance, or reconstruction contract: internal typed-error provenance only. No Evidence document change.
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): no target execution, mutation, or cleanup-ownership change. Late replies still settle only the first caller outcome.
  • 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: constructor omission versus explicit undefined; queue admission, active abort, queued abort, and synchronous progress abort; already-aborted socket connect; launcher bootstrap cleanup; startup join and independent startup waiters.
  • Base reproduction or other evidence: on this change, the queue regression expects error.cause to be the opaque abort reason. The previous constructor compared cause === undefined and the queue built new HopperCancelledError() with no argument, so that assertion failed on the base revision.
  • User-visible CLI/MCP output (if applicable): not applicable. Cause is not part of the public error projection.
  • Remaining proof gaps: no real Hopper process. Public CLI/MCP delivery of opaque causes was not exercised. AbortController.abort(undefined) still substitutes a DOMException; explicit undefined was tested on the constructor.

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

  • npm run test:local -- src/domain/hopperErrors.test.ts src/hopper/HopperRequestQueue.test.ts src/hopper/HopperSocketConnection.test.ts src/hopper/HopperMachOImage.test.ts src/hopper/HopperMachOImage.copyLifecycle.test.ts — 49 passed.
  • npm run test:local -- src/hopper/HopperRequestQueue.test.ts src/domain/hopperErrors.test.ts src/hopper/HopperSocketConnection.test.ts — 22 passed after the final assertion edit.
  • npm run test:focused -- tests/boundary/providers/hopper/bridgeBootstrapCleanup.test.ts tests/boundary/providers/hopper/hopperClient.startupWaiters.test.ts — 10 passed on the final tree.
  • npm run check:fast — typecheck and lint passed.
  • Sibling checks on main 71b696582, before this branch: console timestamp 6 passed, call resolution 25 passed, worker order audit 35 passed.
  • Shard 5/8 on head ab854d77d (run 39548377541) failed tests/boundary/mcp/nativeUiAvailability.test.ts. That pull_request merge included main 7c6c25dda (2026-10-21T12:59:27+08:00), which marks decode_interface_builder target_kind_unsupported unless the open target is an .app bundle. composedAvailabilityFor maps every unavailable code other than unsupported_host to provider_unavailable and lists decode_interface_builder in missing_operations. The matching test update dcdab56ac (2026-10-21T13:33:15+08:00) landed after that run started (2026-10-21T05:13:53Z). The Hopper diff does not change capability inventory. Merged origin/main at d36cb6121. On 7839f783e: npm run test:focused -- tests/boundary/mcp/nativeUiAvailability.test.ts — 11 passed; npm run check:fast — passed.

Real Hopper was not available. These checks use inert queue, socket, launcher, and client fixtures. The native-UI rerun uses the MCP session fixture provider.

Compatibility, safety, and release

  • Breaking changes or migration steps: none. Callers that already passed a reason keep it. Callers that omitted the cause still have no cause property. An explicit undefined is now an own cause property rather than omission.
  • Real Hopper/Ghidra, browser, or OS coverage: not run. No Hopper installation was used.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: abort reasons are caller-supplied values already present on the signal. They are retained on the internal error and are not added to the public projection. No new process or network authority.

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. Not applicable: cancellation causes stay off the public error projection, so there is no CLI or MCP output change.
  • 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/5780 **Author:** [@morluto](https://github.com/morluto) **Created:** 10/21/2026 **Status:** ✅ Merged **Merged:** 10/21/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `cursor/preserve-hopper-abort-reason-d4ac` --- ### 📝 Commits (3) - [`af1b649`](https://github.com/morluto/rea/commit/af1b649cb2ab688f19a362e5c82a971d5f82b0aa) fix(hopper): preserve caller abort reasons on cancellation - [`ab854d7`](https://github.com/morluto/rea/commit/ab854d77d669173cf854af40324a16ea49e402a6) style(hopper): format launcher cancellation return - [`7839f78`](https://github.com/morluto/rea/commit/7839f783eda6a52dccd0bcbba890f05f8b778c99) Merge remote-tracking branch 'origin/main' into cursor/preserve-hopper-abort-reason-d4ac ### 📊 Changes **10 files changed** (+195 additions, -42 deletions) <details> <summary>View changed files</summary> ➕ `src/domain/hopperErrors.test.ts` (+28 -0) 📝 `src/domain/hopperErrors.ts` (+27 -6) 📝 `src/hopper/BridgeLauncher.ts` (+17 -6) 📝 `src/hopper/HopperClient.ts` (+9 -8) 📝 `src/hopper/HopperRequestQueue.test.ts` (+58 -4) 📝 `src/hopper/HopperRequestQueue.ts` (+8 -4) ➕ `src/hopper/HopperSocketConnection.test.ts` (+22 -0) 📝 `src/hopper/HopperSocketConnection.ts` (+3 -2) 📝 `tests/boundary/providers/hopper/bridgeBootstrapCleanup.test.ts` (+9 -6) 📝 `tests/boundary/providers/hopper/hopperClient.startupWaiters.test.ts` (+14 -6) </details> ### 📄 Description <!-- CURSOR_AGENT_PR_BODY_BEGIN --> ## Summary Fixes #5714. Hopper cancellation kept the `HopperCancelledError` classification but dropped the caller's abort reason, so the queue caller could not recover the object it passed to `abort`. Sibling triage reports #5393, #5377, and #5656 were rechecked on main `71b696582` and already match their expected outcomes. They are being closed as already fixed from the parent audit. Evidence: - #5393: `npm run test:focused -- tests/boundary/javascript/workerOrderAudit.test.ts` — 35 passed, including an owned Node peer that exits 7 and is then aborted. The result stays `ProviderAdapterError` with `diagnostics.exit_code: 7`, and the abort reason is a secondary `AnalysisCancelledError`. Fixed by `f01dccab7`. - #5377: `npm run test:local -- src/domain/javascript/javascriptSemanticCallResolution.test.ts` — 25 passed. The nested `cb()` before `var cb=target` stays `complete: false`; the same-activation call after the initializer stays `complete: true`. The graph owner maps a non-exact call to `candidate`. Fixed by `3b0217956`. The public CLI was not re-executed. - #5656: `npm run test:local -- src/browser/CdpCaptureEvents.consoleTimestamp.test.ts` — 6 passed. Missing or invalid console timestamps are `null` with an `invalid_protocol_value` exclusion; `0` and `-1` are preserved. Fixed by `690fc5185`. The built CLI/MCP acceptance test was not re-run. ## Problem and expected behavior An inert `HopperRequestQueue.run` whose signal was aborted with `{ source: "caller", opaque: 7 }` returned `HopperCancelledError` with no `cause`. The constructor also treated an explicit `undefined` cause as omission. The lifecycle contract requires the caller's abort reason as the typed cancellation error's cause, while cancellation classification, late-reply ownership, and cleanup stay intact. ## Change and scope `HopperCancelledError` now records a supplied cause whenever the argument is present, including `undefined`, and still omits `cause` when the caller does not pass one. Queue admission, the abort observer, and pre-send cancellation pass `signal.reason`. The same conversion is applied at Hopper client startup, socket connection, and launcher preparation, including a publisher-verification cancellation that already carried its own cause. Not included: changing which failure wins when a physical error and a later abort overlap. That ordering is a separate contract from reason retention. 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. Ordinary fix within existing Hopper cancellation boundaries. Maintainer scope decision: not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: `HopperCancelledError` owns cause presence. The earliest changed stage is cancellation classification at the Hopper queue, client, socket, and launcher. - Contribution scope fit and linked maintainer decision: ordinary fix inside existing boundaries. Not applicable. - CLI and MCP/tool-catalog contract: none. Public projections already omit error causes. - Provider, bridge, target-format, or platform compatibility: Hopper typed cancellation errors now retain the caller reason. No bridge protocol or target-format change. - Evidence, artifact, provenance, or reconstruction contract: internal typed-error provenance only. No Evidence document change. - Target selection, authority, upstream behavior, and cleanup details (if applicable): no target execution, mutation, or cleanup-ownership change. Late replies still settle only the first caller outcome. - 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: constructor omission versus explicit `undefined`; queue admission, active abort, queued abort, and synchronous progress abort; already-aborted socket connect; launcher bootstrap cleanup; startup join and independent startup waiters. - Base reproduction or other evidence: on this change, the queue regression expects `error.cause` to be the opaque abort reason. The previous constructor compared `cause === undefined` and the queue built `new HopperCancelledError()` with no argument, so that assertion failed on the base revision. - User-visible CLI/MCP output (if applicable): not applicable. Cause is not part of the public error projection. - Remaining proof gaps: no real Hopper process. Public CLI/MCP delivery of opaque causes was not exercised. `AbortController.abort(undefined)` still substitutes a `DOMException`; explicit `undefined` was tested on the constructor. 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 - `npm run test:local -- src/domain/hopperErrors.test.ts src/hopper/HopperRequestQueue.test.ts src/hopper/HopperSocketConnection.test.ts src/hopper/HopperMachOImage.test.ts src/hopper/HopperMachOImage.copyLifecycle.test.ts` — 49 passed. - `npm run test:local -- src/hopper/HopperRequestQueue.test.ts src/domain/hopperErrors.test.ts src/hopper/HopperSocketConnection.test.ts` — 22 passed after the final assertion edit. - `npm run test:focused -- tests/boundary/providers/hopper/bridgeBootstrapCleanup.test.ts tests/boundary/providers/hopper/hopperClient.startupWaiters.test.ts` — 10 passed on the final tree. - `npm run check:fast` — typecheck and lint passed. - Sibling checks on main `71b696582`, before this branch: console timestamp 6 passed, call resolution 25 passed, worker order audit 35 passed. - Shard 5/8 on head `ab854d77d` (run 39548377541) failed `tests/boundary/mcp/nativeUiAvailability.test.ts`. That pull_request merge included main `7c6c25dda` (2026-10-21T12:59:27+08:00), which marks `decode_interface_builder` `target_kind_unsupported` unless the open target is an `.app` bundle. `composedAvailabilityFor` maps every unavailable code other than `unsupported_host` to `provider_unavailable` and lists `decode_interface_builder` in `missing_operations`. The matching test update `dcdab56ac` (2026-10-21T13:33:15+08:00) landed after that run started (2026-10-21T05:13:53Z). The Hopper diff does not change capability inventory. Merged `origin/main` at `d36cb6121`. On `7839f783e`: `npm run test:focused -- tests/boundary/mcp/nativeUiAvailability.test.ts` — 11 passed; `npm run check:fast` — passed. Real Hopper was not available. These checks use inert queue, socket, launcher, and client fixtures. The native-UI rerun uses the MCP session fixture provider. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Callers that already passed a reason keep it. Callers that omitted the cause still have no `cause` property. An explicit `undefined` is now an own `cause` property rather than omission. - Real Hopper/Ghidra, browser, or OS coverage: not run. No Hopper installation was used. - Package or release metadata impact: none. - Security, privacy, process, or containment review: abort reasons are caller-supplied values already present on the signal. They are retained on the internal error and are not added to the public projection. No new process or network authority. ## 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. - [ ] User-visible CLI/MCP changes include representative output. Not applicable: cancellation causes stay off the public error projection, so there is no CLI or MCP output change. - [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-72f4672f-a77e-57b2-98f5-6859c442d4ac?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-72f4672f-a77e-57b2-98f5-6859c442d4ac&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:45:02 +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#7057
No description provided.