[PR #6960] [CLOSED] fix(cancellation): preserve the caller abort reason at remaining checkpoints #7475

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

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6960
Author: @viniciusdsandrade
Created: 10/24/2026
Status: ❌ Closed

Base: main ← Head: fix/6958-remaining-abort-reasons


📝 Commits (2)

  • 24b5e71 fix(cancellation): preserve the caller abort reason at remaining checkpoints
  • ed37f98 test(cancellation): assert the caller abort reason at refused checkpoints

📊 Changes

7 files changed (+98 additions, -19 deletions)

View changed files

📝 src/application/android/AndroidPackageInspection.ts (+3 -1)
📝 src/application/binary/BinarySession.ts (+26 -6)
📝 src/application/native/NativeValueTrace.test.ts (+14 -0)
📝 src/application/native/NativeValueTrace.ts (+5 -1)
📝 src/server/registerSessionRecordTools.ts (+5 -1)
📝 tests/boundary/mcp/androidPackageSummaryMcp.test.ts (+12 -8)
📝 tests/composition/analysis-sessions/binarySession.test.ts (+33 -2)

📄 Description

Summary

After #6143 and #6144, four producing checkpoints still built AnalysisCancelledError for an observed abort without the caller's signal.reason as the cause: the binary session's transition wait and external-mutation guards, the native value trace loop checkpoint, the Android package inspection admission guard, and the import_evidence_bundle checkpoint after the bundle read. Each now passes the exact reason, as the neighbouring checkpoints in the same owners already do. Cancellation classification and cleanup behavior are unchanged.

Fixes #6958

Problem and expected behavior

Smallest trigger: pass an AbortSignal aborted with a non-undefined reason to one of the affected entry points, either already aborted or aborted while the operation waits. For the session, withAdmittedAnalysis returns the transition wait result directly (src/application/binary/BinarySession.ts line 1143), so a session-backed MCP server refused a cancelled tool call without the reason while the fixed-analysis admission in src/server/analysisAdmission.ts (lines 27 to 32) included it.

Expected: the cancellation error's cause is that exact reason. Observed before this change: cause === undefined. Violated invariant: a caller-supplied abort reason survives into the typed cancellation error.

Change and scope

  • src/application/binary/BinarySession.ts: #waitForTransition (pre-aborted signal, abort listener, and a transition settling while aborted) and both guards in withAdmittedExternalMutation pass the caller's reason. The shutdown-induced refusal in #targetAcquisitionRefused has no caller signal and is unchanged.
  • src/application/native/NativeValueTrace.ts: the loop checkpoint passes cause: signal.reason, matching the final checkpoint.
  • src/application/android/AndroidPackageInspection.ts: the admission guard passes cause: options.signal.reason.
  • src/server/registerSessionRecordTools.ts: the import_evidence_bundle checkpoint after the bundle read passes cause: signal.reason.

Intentionally not included: cancellations caused by provider or session closure, which carry no caller signal; sites whose public boundary already rewraps the result with the caller's reason (for example FlutterBuildProvider.execute); and any schema change. No complexity was added for a speculative race.

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.

Contract and boundary impact

  • Semantic owner and earliest changed stage: each boundary that constructs the cancellation error; the change is at that construction site.
  • Contribution scope fit and linked maintainer decision: not applicable (ordinary fix within existing boundaries).
  • CLI and MCP/tool-catalog contract: no schema change. The typed error's cause is now populated; whether cause reaches wire output was not examined.
  • Provider, bridge, target-format, or platform compatibility: none. Tests use the repository's fixture providers only.
  • Evidence, artifact, provenance, or reconstruction contract: unchanged.
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): unchanged; existing cleanup and retirement assertions in the touched tests are untouched.
  • Generated metadata (docs/public/product-catalog.json), package, or installation impact: 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.

No target interaction: the change affects only the construction of typed errors.

Evidence and regression coverage

  • Tests added or updated:
    • tests/composition/analysis-sessions/binarySession.test.ts: the queued admission behind an in-flight open now aborts with a reason and asserts the identical cause; a new case covers a pre-aborted admission and a pre-aborted external mutation.
    • src/application/native/NativeValueTrace.test.ts: "preserves the caller abort reason when the trace starts cancelled".
    • tests/boundary/mcp/androidPackageSummaryMcp.test.ts: the existing pre-provider cancellation case aborts with a reason and asserts the identical cause.
    • Every assertion uses toBe on the reason object. On the base revision each fails because cause is undefined.
  • Base reproduction or other evidence: source-derived; the issue lists the file and line references at 1d896e03b.
  • User-visible CLI/MCP output (if applicable): none intended.
  • Remaining proof gaps: the import_evidence_bundle checkpoint is reached only when the abort lands after the bundle read completes; no deterministic trigger exists at the MCP boundary, so that one-line change is established by reading. The existing cancellation test for that tool (tests/boundary/mcp/evidenceImportCancellation.test.ts) exercises the reader-owned path, which already carried the reason. Wire projection of cause was not examined.

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.

Not an evidence-bearing change.

Validation performed

  • No commands were run locally. The repository CI on this PR (source tests, built-dependent shards, PR readiness) is the executed validation; its results are the checks on this PR.

Compatibility, safety, and release

  • Breaking changes or migration steps: none.
  • Real Hopper/Ghidra, browser, or OS coverage: not exercised; the touched tests use fixture providers.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: none; the cause is the caller's own reason object.

Author readiness

Human owner: @viniciusdsandrade

  • 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.

🔄 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/6960 **Author:** [@viniciusdsandrade](https://github.com/viniciusdsandrade) **Created:** 10/24/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/6958-remaining-abort-reasons` --- ### 📝 Commits (2) - [`24b5e71`](https://github.com/morluto/rea/commit/24b5e71f6ad69d504fc7cd9a43f16097957a5f7b) fix(cancellation): preserve the caller abort reason at remaining checkpoints - [`ed37f98`](https://github.com/morluto/rea/commit/ed37f982a80728a97e3b5cefd1565c55044a8158) test(cancellation): assert the caller abort reason at refused checkpoints ### 📊 Changes **7 files changed** (+98 additions, -19 deletions) <details> <summary>View changed files</summary> 📝 `src/application/android/AndroidPackageInspection.ts` (+3 -1) 📝 `src/application/binary/BinarySession.ts` (+26 -6) 📝 `src/application/native/NativeValueTrace.test.ts` (+14 -0) 📝 `src/application/native/NativeValueTrace.ts` (+5 -1) 📝 `src/server/registerSessionRecordTools.ts` (+5 -1) 📝 `tests/boundary/mcp/androidPackageSummaryMcp.test.ts` (+12 -8) 📝 `tests/composition/analysis-sessions/binarySession.test.ts` (+33 -2) </details> ### 📄 Description ## Summary After #6143 and #6144, four producing checkpoints still built `AnalysisCancelledError` for an observed abort without the caller's `signal.reason` as the `cause`: the binary session's transition wait and external-mutation guards, the native value trace loop checkpoint, the Android package inspection admission guard, and the `import_evidence_bundle` checkpoint after the bundle read. Each now passes the exact reason, as the neighbouring checkpoints in the same owners already do. Cancellation classification and cleanup behavior are unchanged. Fixes #6958 ## Problem and expected behavior Smallest trigger: pass an `AbortSignal` aborted with a non-undefined reason to one of the affected entry points, either already aborted or aborted while the operation waits. For the session, `withAdmittedAnalysis` returns the transition wait result directly (`src/application/binary/BinarySession.ts` line 1143), so a session-backed MCP server refused a cancelled tool call without the reason while the fixed-analysis admission in `src/server/analysisAdmission.ts` (lines 27 to 32) included it. Expected: the cancellation error's `cause` is that exact reason. Observed before this change: `cause === undefined`. Violated invariant: a caller-supplied abort reason survives into the typed cancellation error. ## Change and scope - `src/application/binary/BinarySession.ts`: `#waitForTransition` (pre-aborted signal, abort listener, and a transition settling while aborted) and both guards in `withAdmittedExternalMutation` pass the caller's reason. The shutdown-induced refusal in `#targetAcquisitionRefused` has no caller signal and is unchanged. - `src/application/native/NativeValueTrace.ts`: the loop checkpoint passes `cause: signal.reason`, matching the final checkpoint. - `src/application/android/AndroidPackageInspection.ts`: the admission guard passes `cause: options.signal.reason`. - `src/server/registerSessionRecordTools.ts`: the `import_evidence_bundle` checkpoint after the bundle read passes `cause: signal.reason`. Intentionally not included: cancellations caused by provider or session closure, which carry no caller signal; sites whose public boundary already rewraps the result with the caller's reason (for example `FlutterBuildProvider.execute`); and any schema change. No complexity was added for a speculative race. 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. ## Contract and boundary impact - Semantic owner and earliest changed stage: each boundary that constructs the cancellation error; the change is at that construction site. - Contribution scope fit and linked maintainer decision: not applicable (ordinary fix within existing boundaries). - CLI and MCP/tool-catalog contract: no schema change. The typed error's `cause` is now populated; whether `cause` reaches wire output was not examined. - Provider, bridge, target-format, or platform compatibility: none. Tests use the repository's fixture providers only. - Evidence, artifact, provenance, or reconstruction contract: unchanged. - Target selection, authority, upstream behavior, and cleanup details (if applicable): unchanged; existing cleanup and retirement assertions in the touched tests are untouched. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: 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. No target interaction: the change affects only the construction of typed errors. ## Evidence and regression coverage - Tests added or updated: - `tests/composition/analysis-sessions/binarySession.test.ts`: the queued admission behind an in-flight `open` now aborts with a reason and asserts the identical `cause`; a new case covers a pre-aborted admission and a pre-aborted external mutation. - `src/application/native/NativeValueTrace.test.ts`: "preserves the caller abort reason when the trace starts cancelled". - `tests/boundary/mcp/androidPackageSummaryMcp.test.ts`: the existing pre-provider cancellation case aborts with a reason and asserts the identical `cause`. - Every assertion uses `toBe` on the reason object. On the base revision each fails because `cause` is `undefined`. - Base reproduction or other evidence: source-derived; the issue lists the file and line references at `1d896e03b`. - User-visible CLI/MCP output (if applicable): none intended. - Remaining proof gaps: the `import_evidence_bundle` checkpoint is reached only when the abort lands after the bundle read completes; no deterministic trigger exists at the MCP boundary, so that one-line change is established by reading. The existing cancellation test for that tool (`tests/boundary/mcp/evidenceImportCancellation.test.ts`) exercises the reader-owned path, which already carried the reason. Wire projection of `cause` was not examined. 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. Not an evidence-bearing change. ## Validation performed - No commands were run locally. The repository CI on this PR (source tests, built-dependent shards, PR readiness) is the executed validation; its results are the checks on this PR. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: not exercised; the touched tests use fixture providers. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none; the cause is the caller's own reason object. ## Author readiness Human owner: @viniciusdsandrade - [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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:48:07 +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#7475
No description provided.