morluto/rea

Fork 0

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

[GH-ISSUE #5985] [Bug] Managed static analysis entry cancellation loses the caller abort reason #3652

New issue

Closed

opened 2026-10-31 00:42:44 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

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

Owner

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

Before submitting

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

Area

Other

Observed behavior

Managed static analysis returns AnalysisCancelledError with cause === undefined when cancellation is observed at its entry checkpoint, although the signal retains the exact object passed to abort(reason).

Executed production-client tests reproduced this for inspect_managed_artifact, inspect_managed_members, and inspect_managed_native_boundaries; an ordinary call to each operation succeeded first. Additional tests using the real managed session synchronized cancellation after transition admission and before provider dispatch. Call-through observation proved the managed provider was reached and the session returned that same reasonless provider error. Client and session closure succeeded.

These were in-process provider/session calls with actual local PE/CLI bytes, not public CLI or MCP cancellation requests.

Reproduction

In a source checkout with locked dependencies and prepared artifacts, save this reduced subset of the executed focused test as tests/boundary/providers/managed/cancellationReason.repro.test.ts, then run:

node node_modules/vitest/vitest.mjs run tests/boundary/providers/managed/cancellationReason.repro.test.ts --maxWorkers=1

The original observation run included three failing provider reason-identity assertions, each after a successful non-aborted control. A separate filtered run produced three failing session reason-identity assertions. The assertions below require the intended contract and therefore fail on the affected revision. Spies call through to production methods; no managed parser, file read, or error is replaced. The queued microtask synchronizes the real session admission/dispatch boundary.

import { writeFile } from "node:fs/promises";
import { join } from "node:path";
import { expect, it, vi } from "vitest";
import { createTestTempDirectory } from "../../../fixtures/temporaryDirectory.js";
import { parseBinaryTarget } from "../../../../src/application/BinaryTargetResolver.js";
import { ManagedStaticProvider } from "../../../../src/dotnet/ManagedStaticProvider.js";
import { buildManagedPeFixture } from "../../../../src/dotnet/ManagedPe.fixture.js";
import { createManagedBinarySession } from "../../../../src/composition/binary.js";

it.each(["inspect_managed_artifact", "inspect_managed_members", "inspect_managed_native_boundaries"] as const)(
  "managed admission preserves reason for %s", async operation => {
    const root = await createTestTempDirectory("rea-managed-reason-");
    const path = join(root, "assembly.dll");
    await writeFile(path, buildManagedPeFixture());
    const target = await parseBinaryTarget(path);
    if (!target.ok) throw target.error;
    const client = new ManagedStaticProvider().createClient(target.value);
    try {
      const control = await client.execute(operation, {});
      expect(control.ok).toBe(true);
      const controller = new AbortController();
      const reason = Object.freeze({ requested: "managed-stop", operation });
      controller.abort(reason);
      const result = await client.execute(operation, {}, { signal: controller.signal });
      if (result.ok) throw new Error("Expected cancellation");
      console.log(JSON.stringify({ producer: "managed-static", operation, controlOk: control.ok, tag: result.error._tag, causeAbsent: result.error.cause === undefined, signalRetainsReason: controller.signal.reason === reason }));
      expect(result.error._tag).toBe("AnalysisCancelledError");
      expect(result.error.cause).toBe(reason);
    } finally { expect((await client.close()).ok).toBe(true); }
});


it.each(["inspect_managed_artifact", "inspect_managed_members", "inspect_managed_native_boundaries"] as const)(
  "managed session reaches provider admission cancellation: %s", async operation => {
    const root = await createTestTempDirectory("rea-managed-session-reason-");
    const path = join(root, "assembly.dll");
    await writeFile(path, buildManagedPeFixture());
    const create = vi.spyOn(ManagedStaticProvider.prototype, "createClient");
    const session = createManagedBinarySession();
    try {
      const opened = await session.open(path);
      if (!opened.ok) throw opened.error;
      const client = create.mock.results.at(-1)?.value;
      if (!client) throw new Error("Provider client not created");
      const execute = vi.spyOn(client, "execute");
      const controller = new AbortController();
      const reason = Object.freeze({ requested: "managed-session-stop", operation });
      const pending = session.execute(operation, {}, { signal: controller.signal });
      queueMicrotask(() => controller.abort(reason));
      const result = await pending;
      expect(execute).toHaveBeenCalledWith(operation, {}, expect.objectContaining({ signal: controller.signal }));
      const providerResult = await execute.mock.results[0]?.value;
      if (result.ok || providerResult?.ok) throw new Error("Expected cancellation");
      expect(result.error).toBe(providerResult?.error);
      console.log(JSON.stringify({ producer: "managed-session", operation, providerReached: true, tag: result.error._tag, causeAbsent: result.error.cause === undefined, signalRetainsReason: controller.signal.reason === reason }));
      expect(result.error.cause).toBe(reason);
    } finally { expect((await session.close()).ok).toBe(true); vi.restoreAllMocks(); }
});

Expected behavior

Keep AnalysisCancelledError and retain the exact caller reason as its cause. The lifecycle contract explicitly requires that provenance, and the lifecycle testing guidance requires reason-identity assertions at admission and after provider completion.

Impact and scope

P2: a required diagnostic/provenance value disappears from typed errors for the three managed operations. Cancellation still occurs; the tests do not establish a changed public JSON field, incorrect successful analysis, or damaged artifact. A caller retaining its original signal can still read its reason; this was observed in every case.

Each of the three provider cases and three session cases reproduced once. Natural timing frequency, public CLI/MCP delivery, and other platforms were not measured. The tests use synthetic managed bytes as data without executing an assembly.

REA version

rea-agents 6.7.0; source revision 52415975942a4b9c53a817915b313ccc404f280e.

Runtime and platform

macOS 27.0.1 arm64; Node.js v26.12.0; Vitest 5.0.3. No external analysis engine.

Target and artifact context

Synthetic PE/CLI assembly built with the repository’s buildManagedPeFixture; real regular-file parsing and provider execution.

Interface

Other

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

Observed diagnostics, repeated for each of the three managed operation names:

{"producer":"managed-static","operation":"inspect_managed_artifact","controlOk":true,"tag":"AnalysisCancelledError","causeAbsent":true,"signalRetainsReason":true}
{"producer":"managed-session","operation":"inspect_managed_artifact","providerReached":true,"tag":"AnalysisCancelledError","causeAbsent":true,"signalRetainsReason":true}

AssertionError: expected undefined to be { …(2) } // Object.is equality

The session test also asserted identity between the provider’s error and the error returned by the session. Public reachability is source-derived: managed CLI/MCP workflow, MCP signal forwarding, and session dispatch feed these provider methods. Those adapters were inspected; their transports were not exercised.

Typed outcome or recovery expected

Preserve the existing cancellation code, operation, and cleanup semantics, with error.cause === signal.reason at the point where cancellation is observed. This does not require exposing arbitrary opaque values in public JSON.

Analysis or suspected cause

The managed entry checkpoint constructs AnalysisCancelledError without error options. The constructor accepts those options. Managed read normalization already retains reasons, so the value depends on which stage observes cancellation.

The catch fallback has another bare constructor; that branch was inspected only. It is a failure-handling branch, and no normal post-read checkpoint or physical-failure reclassification claim is made here.

Additional context

Related cancellation reports identify the same contract at different producers: #5935 concerns the JavaScript artifact reader, #5944 derived MCP operations, #5959 Rizin, #5960 call-path traversal, and #5964 JavaScript source recovery. None changes ManagedStaticClient's entry checkpoint. #4381 and merged PR #4446 repair managed hashing/descriptor ownership and ManagedSnapshot normalization, with a different fix.

Fetched upstream main 25e1a24a3ed3671a0148757555f18d16c29d936a and compared the relevant managed source: the entry checkpoint and catch constructor remain unchanged. That newer revision was not executed. No fix or regression range was established.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5985 ### 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 Other ### Observed behavior Managed static analysis returns `AnalysisCancelledError` with `cause === undefined` when cancellation is observed at its entry checkpoint, although the signal retains the exact object passed to `abort(reason)`. Executed production-client tests reproduced this for `inspect_managed_artifact`, `inspect_managed_members`, and `inspect_managed_native_boundaries`; an ordinary call to each operation succeeded first. Additional tests using the real managed session synchronized cancellation after transition admission and before provider dispatch. Call-through observation proved the managed provider was reached and the session returned that same reasonless provider error. Client and session closure succeeded. These were in-process provider/session calls with actual local PE/CLI bytes, not public CLI or MCP cancellation requests. ### Reproduction In a source checkout with locked dependencies and prepared artifacts, save this reduced subset of the executed focused test as `tests/boundary/providers/managed/cancellationReason.repro.test.ts`, then run: ```sh node node_modules/vitest/vitest.mjs run tests/boundary/providers/managed/cancellationReason.repro.test.ts --maxWorkers=1 ``` The original observation run included three failing provider reason-identity assertions, each after a successful non-aborted control. A separate filtered run produced three failing session reason-identity assertions. The assertions below require the intended contract and therefore fail on the affected revision. Spies call through to production methods; no managed parser, file read, or error is replaced. The queued microtask synchronizes the real session admission/dispatch boundary. ```ts import { writeFile } from "node:fs/promises"; import { join } from "node:path"; import { expect, it, vi } from "vitest"; import { createTestTempDirectory } from "../../../fixtures/temporaryDirectory.js"; import { parseBinaryTarget } from "../../../../src/application/BinaryTargetResolver.js"; import { ManagedStaticProvider } from "../../../../src/dotnet/ManagedStaticProvider.js"; import { buildManagedPeFixture } from "../../../../src/dotnet/ManagedPe.fixture.js"; import { createManagedBinarySession } from "../../../../src/composition/binary.js"; it.each(["inspect_managed_artifact", "inspect_managed_members", "inspect_managed_native_boundaries"] as const)( "managed admission preserves reason for %s", async operation => { const root = await createTestTempDirectory("rea-managed-reason-"); const path = join(root, "assembly.dll"); await writeFile(path, buildManagedPeFixture()); const target = await parseBinaryTarget(path); if (!target.ok) throw target.error; const client = new ManagedStaticProvider().createClient(target.value); try { const control = await client.execute(operation, {}); expect(control.ok).toBe(true); const controller = new AbortController(); const reason = Object.freeze({ requested: "managed-stop", operation }); controller.abort(reason); const result = await client.execute(operation, {}, { signal: controller.signal }); if (result.ok) throw new Error("Expected cancellation"); console.log(JSON.stringify({ producer: "managed-static", operation, controlOk: control.ok, tag: result.error._tag, causeAbsent: result.error.cause === undefined, signalRetainsReason: controller.signal.reason === reason })); expect(result.error._tag).toBe("AnalysisCancelledError"); expect(result.error.cause).toBe(reason); } finally { expect((await client.close()).ok).toBe(true); } }); it.each(["inspect_managed_artifact", "inspect_managed_members", "inspect_managed_native_boundaries"] as const)( "managed session reaches provider admission cancellation: %s", async operation => { const root = await createTestTempDirectory("rea-managed-session-reason-"); const path = join(root, "assembly.dll"); await writeFile(path, buildManagedPeFixture()); const create = vi.spyOn(ManagedStaticProvider.prototype, "createClient"); const session = createManagedBinarySession(); try { const opened = await session.open(path); if (!opened.ok) throw opened.error; const client = create.mock.results.at(-1)?.value; if (!client) throw new Error("Provider client not created"); const execute = vi.spyOn(client, "execute"); const controller = new AbortController(); const reason = Object.freeze({ requested: "managed-session-stop", operation }); const pending = session.execute(operation, {}, { signal: controller.signal }); queueMicrotask(() => controller.abort(reason)); const result = await pending; expect(execute).toHaveBeenCalledWith(operation, {}, expect.objectContaining({ signal: controller.signal })); const providerResult = await execute.mock.results[0]?.value; if (result.ok || providerResult?.ok) throw new Error("Expected cancellation"); expect(result.error).toBe(providerResult?.error); console.log(JSON.stringify({ producer: "managed-session", operation, providerReached: true, tag: result.error._tag, causeAbsent: result.error.cause === undefined, signalRetainsReason: controller.signal.reason === reason })); expect(result.error.cause).toBe(reason); } finally { expect((await session.close()).ok).toBe(true); vi.restoreAllMocks(); } }); ``` ### Expected behavior Keep `AnalysisCancelledError` and retain the exact caller reason as its `cause`. The [lifecycle contract](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/tool-design.md#L101-L110) explicitly requires that provenance, and the [lifecycle testing guidance](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/testing.md#L268-L275) requires reason-identity assertions at admission and after provider completion. ### Impact and scope P2: a required diagnostic/provenance value disappears from typed errors for the three managed operations. Cancellation still occurs; the tests do not establish a changed public JSON field, incorrect successful analysis, or damaged artifact. A caller retaining its original signal can still read its reason; this was observed in every case. Each of the three provider cases and three session cases reproduced once. Natural timing frequency, public CLI/MCP delivery, and other platforms were not measured. The tests use synthetic managed bytes as data without executing an assembly. ### REA version rea-agents 6.7.0; source revision `52415975942a4b9c53a817915b313ccc404f280e`. ### Runtime and platform macOS 27.0.1 arm64; Node.js v26.12.0; Vitest 5.0.3. No external analysis engine. ### Target and artifact context Synthetic PE/CLI assembly built with the repository’s buildManagedPeFixture; real regular-file parsing and provider execution. ### Interface Other ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence Observed diagnostics, repeated for each of the three managed operation names: ```json {"producer":"managed-static","operation":"inspect_managed_artifact","controlOk":true,"tag":"AnalysisCancelledError","causeAbsent":true,"signalRetainsReason":true} {"producer":"managed-session","operation":"inspect_managed_artifact","providerReached":true,"tag":"AnalysisCancelledError","causeAbsent":true,"signalRetainsReason":true} ``` ```text AssertionError: expected undefined to be { …(2) } // Object.is equality ``` The session test also asserted identity between the provider’s error and the error returned by the session. Public reachability is source-derived: [managed CLI/MCP workflow](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/application/DirectAnalysis.ts#L180-L200), [MCP signal forwarding](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/server/registerManagedTools.ts#L54-L59), and [session dispatch](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/application/binary/BinarySession.ts#L1102-L1152) feed these provider methods. Those adapters were inspected; their transports were not exercised. ### Typed outcome or recovery expected Preserve the existing cancellation code, operation, and cleanup semantics, with `error.cause === signal.reason` at the point where cancellation is observed. This does not require exposing arbitrary opaque values in public JSON. ### Analysis or suspected cause The [managed entry checkpoint](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/dotnet/ManagedStaticProvider.ts#L96-L109) constructs `AnalysisCancelledError` without error options. The [constructor](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/domain/analysisErrorCore.ts#L151-L161) accepts those options. [Managed read normalization](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/dotnet/ManagedSnapshot.ts#L182-L201) already retains reasons, so the value depends on which stage observes cancellation. The [catch fallback](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/dotnet/ManagedStaticProvider.ts#L134-L139) has another bare constructor; that branch was inspected only. It is a failure-handling branch, and no normal post-read checkpoint or physical-failure reclassification claim is made here. ### Additional context Related cancellation reports identify the same contract at different producers: [#5935](https://github.com/morluto/rea/issues/5935) concerns the JavaScript artifact reader, [#5944](https://github.com/morluto/rea/issues/5944) derived MCP operations, [#5959](https://github.com/morluto/rea/issues/5959) Rizin, [#5960](https://github.com/morluto/rea/issues/5960) call-path traversal, and [#5964](https://github.com/morluto/rea/issues/5964) JavaScript source recovery. None changes ManagedStaticClient's entry checkpoint. [#4381](https://github.com/morluto/rea/issues/4381) and merged PR [#4446](https://github.com/morluto/rea/pull/4446) repair managed hashing/descriptor ownership and ManagedSnapshot normalization, with a different fix. Fetched upstream main `25e1a24a3ed3671a0148757555f18d16c29d936a` and compared the relevant managed source: the entry checkpoint and catch constructor remain unchanged. That newer revision was not executed. No fix or regression range was established.

bsakaguchi closed this issue

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

bsakaguchi commented

2026-10-31 00:42:45 +00:00

Author

Owner

@coygeek commented on GitHub (Oct 24, 2026):

Checked the reported fix on main at d87e73f2db.

Both entry and error-path cancellation attach options.signal.reason as cause. A production-source probe called all three reported operations with pre-aborted synthetic PE targets and asserted exact reason identity; no file was read and client close succeeded. Existing managed regressions also assert identity after completed reads and cleanup.

Implementation: ManagedStaticProvider.ts, ManagedStaticProvider.ts.

The relevant regression checks in managedStaticProvider.test.ts passed in the focused test run on this revision. This closure is based on the current source and those checks; provider fixtures do not establish a real-engine run. Closing as completed on main.

@coygeek commented on GitHub (Oct 24, 2026): Checked the reported fix on `main` at d87e73f2db50be72d33a4b3c7199af35518b441c. Both entry and error-path cancellation attach options.signal.reason as cause. A production-source probe called all three reported operations with pre-aborted synthetic PE targets and asserted exact reason identity; no file was read and client close succeeded. Existing managed regressions also assert identity after completed reads and cleanup. Implementation: [ManagedStaticProvider.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/dotnet/ManagedStaticProvider.ts#L98), [ManagedStaticProvider.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/dotnet/ManagedStaticProvider.ts#L143). The relevant regression checks in [managedStaticProvider.test.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/tests/boundary/providers/managed/managedStaticProvider.test.ts) passed in the focused test run on this revision. This closure is based on the current source and those checks; provider fixtures do not establish a real-engine run. Closing as completed on main.

bsakaguchi referenced this issue

2026-11-01 21:10:57 +00:00

[GH-ISSUE #3796] [Tracker] Open feature requests by area (October 2026) #1471

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#3652

No description provided.

Rows
Columns