morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[GH-ISSUE #6121] [Bug] Binary layout and recorded crash admission lose the caller's abort reason #3751

New issue

Closed

opened 2026-10-31 00:43:40 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:43:40 +00:00

Owner

Originally created by @coygeek on GitHub (Oct 21, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/6121

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, tokens, private paths, and sensitive target data.

Area

Native analysis or decompilation

Observed behavior

BinaryLayoutService and RecordedCrashService return AnalysisCancelledError with cause === undefined when their admission guard observes an already-aborted signal. The signal still retains the exact object supplied to abort(reason). Both inspect and inspectAndClose lose the reason, before calling their provider.

I executed four in-process service cases with a frozen object reason and a simulated provider that throws if invoked. All four returned reasonless cancellation with zero provider calls. A sibling PeResourcesService control preserved the identical object. These observations concern the typed application result; they do not establish a missing field in public CLI or MCP JSON.

Reproduction

In a source checkout with its locked dependencies installed, save this focused test as src/application/binaryDiagnostics/admissionReason.test.ts, then run:

node node_modules/vitest/vitest.mjs run src/application/binaryDiagnostics/admissionReason.test.ts --maxWorkers=1 --reporter=verbose

The following reduced test uses the same service calls and assertions as the executed cases. Assertions characterize the current behavior; no native decoder runs.

import { expect, it, vi } from "vitest";
import { BinaryLayoutService } from "./BinaryLayoutService.js";
import { RecordedCrashService } from "./RecordedCrashService.js";

for (const Service of [BinaryLayoutService, RecordedCrashService]) {
  for (const method of ["inspect", "inspectAndClose"] as const) {
    it(`${Service.name}.${method} loses the admission reason`, async () => {
      const reason = Object.freeze({ origin: "caller-pre-aborted" });
      const controller = new AbortController();
      controller.abort(reason);
      const inspect = vi.fn(() => {
        throw new Error("Provider must not run");
      });
      const service = new Service({
        identity: { id: "fixture", name: "Fixture", version: "1" },
        inspect,
        inspectAndClose: inspect,
      });
      const result = await service[method](
        { path: "/selected.elf" },
        { signal: controller.signal },
      );
      expect(result.ok).toBe(false);
      if (result.ok) throw new Error("Expected cancellation");
      expect(result.error._tag).toBe("AnalysisCancelledError");
      expect(result.error.cause).toBeUndefined();
      expect(result.error.cause).not.toBe(reason);
      expect(inspect).not.toHaveBeenCalled();
    });
  }
}

Expected behavior

Preserve signal.reason by identity as the typed cancellation error's cause, while keeping the operation name and avoiding provider invocation.

The lifecycle contract requires preserving the caller's abort reason. The regression guidance requires admission reason-identity assertions. PE resources admission already implements the expected behavior.

Impact and scope

P2: cancellation provenance disappears from the typed result at these two diagnostics owners. The same cancellation reason is retained by the PE resources control, so the result depends on which service observes the abort. Callers that retain their own signal can recover the reason independently.

The observed scope is four service/method combinations with a pre-aborted opaque object reason. Cancellation remains correctly classified, no provider executes, and no partial observation or cleanup leak was observed. Public transport timing, arbitrary cause serialization and real Python/provider behavior were not exercised. MCP may reject a request at its earlier session admission boundary; this report does not claim that every pre-aborted MCP request reaches the service.

REA version

rea-agents 6.7.0; source revision 52415975942a4b9c53a817915b313ccc404f280e.

Runtime and platform

macOS 27.0.1 arm64; Node.js v24.18.0, matching the repository development pin. Used existing locked dependencies and one Vitest worker; no rebuild or dependency changes.

Target and artifact context

An absolute inert selection string. Admission returns before schema validation or any file read, so no ELF or core artifact, Python environment, debugger or native provider is needed for this reproduction.

Interface

Other

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

The executed focused file passed six characterization/control tests: the four diagnostic cases, the PE reason-preservation control, and a separate snapshot lifecycle case. Reduced output for the diagnostic cases was:

{"service":"BinaryLayoutService","method":"inspect","tag":"AnalysisCancelledError","causeAbsent":true,"exact":false,"calls":0}
{"service":"BinaryLayoutService","method":"inspectAndClose","tag":"AnalysisCancelledError","causeAbsent":true,"exact":false,"calls":0}
{"service":"RecordedCrashService","method":"inspect","tag":"AnalysisCancelledError","causeAbsent":true,"exact":false,"calls":0}
{"service":"RecordedCrashService","method":"inspectAndClose","tag":"AnalysisCancelledError","causeAbsent":true,"exact":false,"calls":0}

These are diagnostic projections of the typed result and provider-call count, not REA wire payload fields. The PE control asserted error.cause === reason successfully.

The two omitted options are at BinaryLayoutService admission and RecordedCrashService admission.

Typed outcome or recovery expected

AnalysisCancelledError.cause === signal.reason for both methods of each service, with operation inspect_binary_layout or inspect_recorded_crash respectively. Preserve existing no-provider-invocation behavior. This does not require serializing an arbitrary opaque value into public JSON or granting cleanup ownership to that value.

Analysis or suspected cause

Source-derived: both shared admission helpers construct new AnalysisCancelledError(OPERATION) without passing error options. Their existing shared provider-operation admission counterpart passes { cause: options.signal.reason }. Repairing that counterpart alone cannot change these independent guards.

The production CLI registrations pass command signals to inspectAndClose. The layout MCP handler and recorded-crash MCP handler bind request signals to inspect. These adapter paths were inspected, rather than executed for cancellation in this report.

Additional context

Related #4945 covers the lower PwntoolsDecoder, PwntoolsFailures and PwndbgCoreContext producers, including layout/core provider cancellation. The reported service guards return before invoking those providers, so that repair cannot supply their lost cause. #4929 explicitly covers Rust, Go and PE service admission; the repaired PE branch supplies the passing control here. #4261 concerns completed Evidence on late abort. #5944, #5960, #5985 and #5670 concern other cancellation owners and do not change either guard.

Open PR #5288 refactors layout validation and summary preflight, but its diff retains the same reasonless cancellation constructors in the raw entry points. It does not repair the missing cause.

Open PR #6059 also touches both service files, but its current diff only projects timeout parameters into Evidence and leaves the admission guards unchanged.

Fetched upstream main aa51e4343963fdd0b97ec2ebdc3c41579fd9cb38 and compared both service files and their shared completion helper: the relevant source is unchanged. The newer runtime was not built or executed. No regression range or natural failure frequency was established.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/6121 ### Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ### Area Native analysis or decompilation ### Observed behavior `BinaryLayoutService` and `RecordedCrashService` return `AnalysisCancelledError` with `cause === undefined` when their admission guard observes an already-aborted signal. The signal still retains the exact object supplied to `abort(reason)`. Both `inspect` and `inspectAndClose` lose the reason, before calling their provider. I executed four in-process service cases with a frozen object reason and a simulated provider that throws if invoked. All four returned reasonless cancellation with zero provider calls. A sibling `PeResourcesService` control preserved the identical object. These observations concern the typed application result; they do not establish a missing field in public CLI or MCP JSON. ### Reproduction In a source checkout with its locked dependencies installed, save this focused test as `src/application/binaryDiagnostics/admissionReason.test.ts`, then run: ```sh node node_modules/vitest/vitest.mjs run src/application/binaryDiagnostics/admissionReason.test.ts --maxWorkers=1 --reporter=verbose ``` The following reduced test uses the same service calls and assertions as the executed cases. Assertions characterize the current behavior; no native decoder runs. ```ts import { expect, it, vi } from "vitest"; import { BinaryLayoutService } from "./BinaryLayoutService.js"; import { RecordedCrashService } from "./RecordedCrashService.js"; for (const Service of [BinaryLayoutService, RecordedCrashService]) { for (const method of ["inspect", "inspectAndClose"] as const) { it(`${Service.name}.${method} loses the admission reason`, async () => { const reason = Object.freeze({ origin: "caller-pre-aborted" }); const controller = new AbortController(); controller.abort(reason); const inspect = vi.fn(() => { throw new Error("Provider must not run"); }); const service = new Service({ identity: { id: "fixture", name: "Fixture", version: "1" }, inspect, inspectAndClose: inspect, }); const result = await service[method]( { path: "/selected.elf" }, { signal: controller.signal }, ); expect(result.ok).toBe(false); if (result.ok) throw new Error("Expected cancellation"); expect(result.error._tag).toBe("AnalysisCancelledError"); expect(result.error.cause).toBeUndefined(); expect(result.error.cause).not.toBe(reason); expect(inspect).not.toHaveBeenCalled(); }); } } ``` ### Expected behavior Preserve `signal.reason` by identity as the typed cancellation error's `cause`, while keeping the operation name and avoiding provider invocation. The [lifecycle contract](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/tool-design.md#L101-L110) requires preserving the caller's abort reason. The [regression guidance](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/testing.md#L268-L275) requires admission reason-identity assertions. [PE resources admission](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/application/binaryDiagnostics/PeResourcesService.ts#L34-L42) already implements the expected behavior. ### Impact and scope P2: cancellation provenance disappears from the typed result at these two diagnostics owners. The same cancellation reason is retained by the PE resources control, so the result depends on which service observes the abort. Callers that retain their own signal can recover the reason independently. The observed scope is four service/method combinations with a pre-aborted opaque object reason. Cancellation remains correctly classified, no provider executes, and no partial observation or cleanup leak was observed. Public transport timing, arbitrary cause serialization and real Python/provider behavior were not exercised. MCP may reject a request at its earlier session admission boundary; this report does not claim that every pre-aborted MCP request reaches the service. ### REA version rea-agents 6.7.0; source revision `52415975942a4b9c53a817915b313ccc404f280e`. ### Runtime and platform macOS 27.0.1 arm64; Node.js v24.18.0, matching the repository development pin. Used existing locked dependencies and one Vitest worker; no rebuild or dependency changes. ### Target and artifact context An absolute inert selection string. Admission returns before schema validation or any file read, so no ELF or core artifact, Python environment, debugger or native provider is needed for this reproduction. ### Interface Other ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence The executed focused file passed six characterization/control tests: the four diagnostic cases, the PE reason-preservation control, and a separate snapshot lifecycle case. Reduced output for the diagnostic cases was: ```json {"service":"BinaryLayoutService","method":"inspect","tag":"AnalysisCancelledError","causeAbsent":true,"exact":false,"calls":0} {"service":"BinaryLayoutService","method":"inspectAndClose","tag":"AnalysisCancelledError","causeAbsent":true,"exact":false,"calls":0} {"service":"RecordedCrashService","method":"inspect","tag":"AnalysisCancelledError","causeAbsent":true,"exact":false,"calls":0} {"service":"RecordedCrashService","method":"inspectAndClose","tag":"AnalysisCancelledError","causeAbsent":true,"exact":false,"calls":0} ``` These are diagnostic projections of the typed result and provider-call count, not REA wire payload fields. The PE control asserted `error.cause === reason` successfully. The two omitted options are at [BinaryLayoutService admission](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/application/binaryDiagnostics/BinaryLayoutService.ts#L48-L58) and [RecordedCrashService admission](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/application/binaryDiagnostics/RecordedCrashService.ts#L49-L59). ### Typed outcome or recovery expected `AnalysisCancelledError.cause === signal.reason` for both methods of each service, with operation `inspect_binary_layout` or `inspect_recorded_crash` respectively. Preserve existing no-provider-invocation behavior. This does not require serializing an arbitrary opaque value into public JSON or granting cleanup ownership to that value. ### Analysis or suspected cause Source-derived: both shared admission helpers construct `new AnalysisCancelledError(OPERATION)` without passing error options. Their existing [shared provider-operation admission counterpart](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/application/ProviderOperationExecution.ts#L47-L60) passes `{ cause: options.signal.reason }`. Repairing that counterpart alone cannot change these independent guards. The [production CLI registrations](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/cli/binaryDiagnosticsCommands.ts#L59-L103) pass command signals to `inspectAndClose`. The [layout MCP handler](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/server/registerBinaryDiagnosticsTools.ts#L23-L48) and [recorded-crash MCP handler](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/server/registerRecordedCrashTools.ts#L19-L35) bind request signals to `inspect`. These adapter paths were inspected, rather than executed for cancellation in this report. ### Additional context Related [#4945](https://github.com/morluto/rea/issues/4945) covers the lower `PwntoolsDecoder`, `PwntoolsFailures` and `PwndbgCoreContext` producers, including layout/core provider cancellation. The reported service guards return before invoking those providers, so that repair cannot supply their lost cause. [#4929](https://github.com/morluto/rea/issues/4929) explicitly covers Rust, Go and PE service admission; the repaired PE branch supplies the passing control here. [#4261](https://github.com/morluto/rea/issues/4261) concerns completed Evidence on late abort. [#5944](https://github.com/morluto/rea/issues/5944), [#5960](https://github.com/morluto/rea/issues/5960), [#5985](https://github.com/morluto/rea/issues/5985) and [#5670](https://github.com/morluto/rea/issues/5670) concern other cancellation owners and do not change either guard. Open [PR #5288](https://github.com/morluto/rea/pull/5288) refactors layout validation and summary preflight, but its diff retains the same reasonless cancellation constructors in the raw entry points. It does not repair the missing cause. Open [PR #6059](https://github.com/morluto/rea/pull/6059) also touches both service files, but its current diff only projects timeout parameters into Evidence and leaves the admission guards unchanged. Additional related cancellation reports cover separate producers: [JavaScript artifact reading (#5935)](https://github.com/morluto/rea/issues/5935), [Rizin (#5959)](https://github.com/morluto/rea/issues/5959), [JavaScript recovery (#5964)](https://github.com/morluto/rea/issues/5964), [JavaScript application checkpoints (#5987)](https://github.com/morluto/rea/issues/5987), [asset catalogs (#5998)](https://github.com/morluto/rea/issues/5998), [ZIP extraction (#5999)](https://github.com/morluto/rea/issues/5999), [WABT (#6002)](https://github.com/morluto/rea/issues/6002), [DMG commands (#6032)](https://github.com/morluto/rea/issues/6032), [LLDB discovery (#6037)](https://github.com/morluto/rea/issues/6037), [web-script export (#6042)](https://github.com/morluto/rea/issues/6042), [managed snapshot finalization (#6065)](https://github.com/morluto/rea/issues/6065), [detached PGP verification (#6100)](https://github.com/morluto/rea/issues/6100), [ASAR header listing (#6101)](https://github.com/morluto/rea/issues/6101), [Dart AOT admission (#6110)](https://github.com/morluto/rea/issues/6110), [Evidence-file analysis views (#6113)](https://github.com/morluto/rea/issues/6113), [binary-switch retirement (#6115)](https://github.com/morluto/rea/issues/6115). These reports do not supply the omitted cause at either diagnostics service guard. Fetched upstream main `aa51e4343963fdd0b97ec2ebdc3c41579fd9cb38` and compared both service files and their shared completion helper: the relevant source is unchanged. The newer runtime was not built or executed. No regression range or natural failure frequency was established.

bsakaguchi closed this issue

2026-10-31 00:43:40 +00:00

bsakaguchi referenced this issue

2026-10-31 00:44:30 +00:00

[GH-ISSUE #6405] [Feature] Ghidra: export analyst names and comments from a session and re-apply them when the target is reopened #3835

bsakaguchi referenced this issue

2026-10-31 00:52:31 +00:00

[GH-ISSUE #8165] [Design] Preserve and reopen owned Ghidra analysis with a packed-program checkpoint #4691

bsakaguchi referenced this issue

2026-10-31 01:10:28 +00:00

[PR #3818] feat(ghidra): apply a caller-selected PDB that matches the target #5923

bsakaguchi referenced this issue