[PR #5383] [CLOSED] fix(evm): keep the caller's abort reason as the cancellation cause #6913

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

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5383
Author: @XiaoBinGan
Created: 10/20/2026
Status: ❌ Closed

Base: main ← Head: fix/evm-artifact-cancellation-cause


📝 Commits (1)

  • 2e45e63 fix(evm): keep the caller's abort reason as the cancellation cause

📊 Changes

3 files changed (+110 additions, -1 deletions)

View changed files

📝 src/evm/EvmoleFailures.test.ts (+24 -0)
📝 src/evm/EvmoleFailures.ts (+1 -1)
➕ tests/boundary/evm/evmArtifactCancellationCause.test.ts (+85 -0)

📄 Description

Summary

evmInterfaceFailure projected a cancelled ArtifactReaderFailure with the wrapper itself as the typed cause, so a caller that aborts during the EVM carrier read received ArtifactReaderFailure instead of the reason it passed to abort(). The OwnedCommandFailure branch beside it already unwraps the recorded reason.

Fixes #5332

Problem and expected behavior

  • Smallest trigger: cancel inspect_evm_interface after the entry checks, either while the inherited-limits preflight runs or while the carrier descriptor closes. The reader raises a cancelled ArtifactReaderFailure, and evmInterfaceFailure returned new AnalysisCancelledError(..., { cause }) with the wrapper.
  • Violated invariant: docs/tool-design.md requires the caller's exact abort reason as the typed cancellation cause, through awaited cleanup and failure wrapping.
  • Expected: result.error.cause === reason for both timings, while the operation still cancels, the descriptor still closes, cleanupIncomplete stays false and no worker runtime starts.

Change and scope

  • src/evm/EvmoleFailures.ts: the cancelled ArtifactReaderFailure branch now projects cause.cancellation.reason, matching the OwnedCommandFailure branch in the same function, and keeps the wrapper only when the reader recorded no reason.
  • src/evm/EvmoleFailures.test.ts: a projection-level regression over opaque reasons (string and object) that also asserts the reader's cleanup resources survive on the typed error.
  • tests/boundary/evm/evmArtifactCancellationCause.test.ts: the report's service/provider reproduction, reduced to the three timings (pre-aborted, admission, close) with exact cause identity, no spawned worker and confirmed descriptor close.
  • Not included: no change to the cancellation tag, the cleanup contract or the other providers' projections.

Contract and boundary impact

  • Semantic owner and earliest changed stage: EVM provider failure projection in src/evm/EvmoleFailures.ts.
  • Contribution scope fit and linked maintainer decision: not applicable — an ordinary fix inside an existing boundary.
  • CLI and MCP/tool-catalog contract: no schema or tool-catalog change; only the typed in-process cause becomes the caller's reason.
  • Provider, bridge, target-format, or platform compatibility: EVM interface provider only; Linux/x64 admission and limits untouched.
  • Evidence, artifact, provenance, or reconstruction contract: unchanged; cleanup resources and the partial observation keep their fields.
  • Target selection, authority, upstream behavior, and cleanup details: unchanged. No carrier read, descriptor close or cleanup decision changes.
  • Generated metadata, package, or installation impact: none.

Expected effects (check known effects; use documentation-only on its own).

  • 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: the projection regression in EvmoleFailures.test.ts and the three-timing boundary reproduction.
  • Base reproduction or other evidence: on 799e8265b and on current main the boundary file reports 2 failed | 1 passed with expected ArtifactReaderFailure: Artifact read canc… to be { requested: 'caller stop' }, exactly the report's shape. With the projection change it reports 3 passed. The failure is reached through the real service/provider path and the real 64 MiB-independent carrier read, not by asserting on a hand-built result.
  • User-visible CLI/MCP output (if applicable): none; the change is in-process typed-result metadata.
  • Remaining proof gaps: no real Linux/x64 host with util-linux prlimit and EVMole was used; Linux admission and the /proc/self/limits read are emulated as in the report. CLI and MCP transports were not exercised.

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

  • npx vitest run tests/boundary/evm/evmArtifactCancellationCause.test.ts — 3 passed; with the projection change reverted, 2 failed | 1 passed with the report's assertion error.
  • npx vitest run src/evm tests/boundary/evm — 69 passed, 30 Linux-gated skipped (99 tests across 8 files).
  • npx vitest run src/evm/EvmoleFailures.test.ts — 10 passed.
  • npm run format:check — all matched files use the correct format.
  • npm run verify:test-discovery — all 1719 test files discovered exactly once.
  • npm run typecheck — no diagnostic in the changed files; the only errors come from this checkout's install predating the @bufbuild/protobuf dependency on current main.
  • npm run lint — 0 errors, module boundaries resolved in 2262 files.

Compatibility, safety, and release

  • Breaking changes or migration steps: the typed cause for an aborted carrier read changes from the internal wrapper to the caller's reason, which is the documented contract. In-repo callers that unwrapped cause.cancellation.reason can read cause directly.
  • Real Hopper/Ghidra, browser, or OS coverage: none claimed.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: none; no spawn, descriptor or cleanup behaviour changes.

Note: independent PRs #5369 and #5370 target the same report.

Author readiness

Human owner: @XiaoBinGan

  • 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/5383 **Author:** [@XiaoBinGan](https://github.com/XiaoBinGan) **Created:** 10/20/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/evm-artifact-cancellation-cause` --- ### 📝 Commits (1) - [`2e45e63`](https://github.com/morluto/rea/commit/2e45e63fbc08849746d86dde758031a09f9012c8) fix(evm): keep the caller's abort reason as the cancellation cause ### 📊 Changes **3 files changed** (+110 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/evm/EvmoleFailures.test.ts` (+24 -0) 📝 `src/evm/EvmoleFailures.ts` (+1 -1) ➕ `tests/boundary/evm/evmArtifactCancellationCause.test.ts` (+85 -0) </details> ### 📄 Description ## Summary `evmInterfaceFailure` projected a cancelled `ArtifactReaderFailure` with the wrapper itself as the typed `cause`, so a caller that aborts during the EVM carrier read received `ArtifactReaderFailure` instead of the reason it passed to `abort()`. The `OwnedCommandFailure` branch beside it already unwraps the recorded reason. Fixes #5332 ## Problem and expected behavior - Smallest trigger: cancel `inspect_evm_interface` after the entry checks, either while the inherited-limits preflight runs or while the carrier descriptor closes. The reader raises a cancelled `ArtifactReaderFailure`, and `evmInterfaceFailure` returned `new AnalysisCancelledError(..., { cause })` with the wrapper. - Violated invariant: `docs/tool-design.md` requires the caller's exact abort reason as the typed cancellation cause, through awaited cleanup and failure wrapping. - Expected: `result.error.cause === reason` for both timings, while the operation still cancels, the descriptor still closes, `cleanupIncomplete` stays `false` and no worker runtime starts. ## Change and scope - `src/evm/EvmoleFailures.ts`: the cancelled `ArtifactReaderFailure` branch now projects `cause.cancellation.reason`, matching the `OwnedCommandFailure` branch in the same function, and keeps the wrapper only when the reader recorded no reason. - `src/evm/EvmoleFailures.test.ts`: a projection-level regression over opaque reasons (string and object) that also asserts the reader's cleanup resources survive on the typed error. - `tests/boundary/evm/evmArtifactCancellationCause.test.ts`: the report's service/provider reproduction, reduced to the three timings (`pre-aborted`, `admission`, `close`) with exact cause identity, no spawned worker and confirmed descriptor close. - Not included: no change to the cancellation tag, the cleanup contract or the other providers' projections. ## Contract and boundary impact - Semantic owner and earliest changed stage: EVM provider failure projection in `src/evm/EvmoleFailures.ts`. - Contribution scope fit and linked maintainer decision: not applicable — an ordinary fix inside an existing boundary. - CLI and MCP/tool-catalog contract: no schema or tool-catalog change; only the typed in-process `cause` becomes the caller's reason. - Provider, bridge, target-format, or platform compatibility: EVM interface provider only; Linux/x64 admission and limits untouched. - Evidence, artifact, provenance, or reconstruction contract: unchanged; cleanup resources and the partial observation keep their fields. - Target selection, authority, upstream behavior, and cleanup details: unchanged. No carrier read, descriptor close or cleanup decision changes. - Generated metadata, package, or installation impact: none. Expected effects (check known effects; use documentation-only on its own). - [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: the projection regression in `EvmoleFailures.test.ts` and the three-timing boundary reproduction. - Base reproduction or other evidence: on `799e8265b` and on current main the boundary file reports `2 failed | 1 passed` with `expected ArtifactReaderFailure: Artifact read canc… to be { requested: 'caller stop' }`, exactly the report's shape. With the projection change it reports `3 passed`. The failure is reached through the real service/provider path and the real 64 MiB-independent carrier read, not by asserting on a hand-built result. - User-visible CLI/MCP output (if applicable): none; the change is in-process typed-result metadata. - Remaining proof gaps: no real Linux/x64 host with util-linux `prlimit` and EVMole was used; Linux admission and the `/proc/self/limits` read are emulated as in the report. CLI and MCP transports were not exercised. 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 - `npx vitest run tests/boundary/evm/evmArtifactCancellationCause.test.ts` — 3 passed; with the projection change reverted, `2 failed | 1 passed` with the report's assertion error. - `npx vitest run src/evm tests/boundary/evm` — 69 passed, 30 Linux-gated skipped (99 tests across 8 files). - `npx vitest run src/evm/EvmoleFailures.test.ts` — 10 passed. - `npm run format:check` — all matched files use the correct format. - `npm run verify:test-discovery` — all 1719 test files discovered exactly once. - `npm run typecheck` — no diagnostic in the changed files; the only errors come from this checkout's install predating the `@bufbuild/protobuf` dependency on current main. - `npm run lint` — 0 errors, module boundaries resolved in 2262 files. ## Compatibility, safety, and release - Breaking changes or migration steps: the typed `cause` for an aborted carrier read changes from the internal wrapper to the caller's reason, which is the documented contract. In-repo callers that unwrapped `cause.cancellation.reason` can read `cause` directly. - Real Hopper/Ghidra, browser, or OS coverage: none claimed. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none; no spawn, descriptor or cleanup behaviour changes. Note: independent PRs #5369 and #5370 target the same report. ## Author readiness Human owner: @XiaoBinGan - [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:44:01 +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#6913
No description provided.