morluto/rea

Fork 0

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

[GH-ISSUE #5822] [Bug] IDA session cancellation loses the caller's abort reason #3521

New issue

Closed

opened 2026-10-31 00:41:27 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-31 00:41:27 +00:00

Owner

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

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

An executed provider-session test called IdaSessionClient.execute("list_procedures", {}, { signal }) with an opaque object passed to AbortController.abort(reason).

With the signal aborted before the attached session starts, the returned AnalysisCancelledError.cause was undefined; the connection was never started.
With cancellation triggered during a headless list_funcs call, the adapter recorded the completed reply and returned AnalysisCancelledError. Its cause was another AnalysisCancelledError with no cause, rather than the caller's object. Partial observations retained the reply.

Both exact-object cause assertions failed. The non-aborted control succeeded, and client.close() succeeded in all three cases. These are executed adapter-boundary observations using the existing RecordingIdaMcp fake. No IDA engine or public CLI/MCP request was run.

Reproduction

From the source revision below, with locked dependencies installed, place the following temporary test beside the adapter as src/ida/repro.test.ts. Run npm run test:local -- src/ida/repro.test.ts, then delete it. Run in an isolated HOME and XDG_CONFIG_HOME, with TMPDIR set to an existing test-owned directory. One run of this three-case test produced two failed cause assertions and one passing control.

import { rm } from "node:fs/promises";
import { expect, it } from "vitest";
import { createIdaTarget, RecordingIdaMcp } from "../../tests/fixtures/idaMcp.js";
import { IdaSessionClient } from "./IdaSessionClient.js";

it.each(["before-start", "after-reply", "control"] as const)(
  "preserves cancellation reason: %s",
  async (phase) => {
    const { root, target } = await createIdaTarget();
    const mode = phase === "before-start" ? "attached" : "headless";
    const producer = new RecordingIdaMcp(target, mode);
    const controller = new AbortController();
    const reason = Object.freeze({ requested: "stop", marker: 7 });
    if (phase === "before-start") controller.abort(reason);
    if (phase === "after-reply") {
      producer.beforeCall = async (name) => {
        if (name === "list_funcs") controller.abort(reason);
      };
    }
    const client = new IdaSessionClient(
      { command: "fixture", args: [], env: {}, mode,
        timeoutMs: 1000, workspaceRoot: root },
      target,
      producer,
    );
    try {
      const result = await client.execute(
        "list_procedures", {}, { signal: controller.signal },
      );
      if (phase === "control") {
        expect(result.ok).toBe(true);
        console.log(JSON.stringify({ phase, ok: result.ok }));
      } else {
        if (result.ok) throw new Error("Expected cancellation");
        const cause = result.error.cause;
        console.log(JSON.stringify({
          phase,
          tag: result.error._tag,
          same_reason: cause === reason,
          cause_absent: cause === undefined,
          cause_is_error: cause instanceof Error,
          nested_cause_absent: cause instanceof Error && cause.cause === undefined,
          partial_observation: result.error.partialObservation !== undefined,
          calls: producer.calls.map(({ name }) => name),
        }));
        expect(result.error._tag).toBe("AnalysisCancelledError");
        if (phase === "before-start") expect(producer.connects).toBe(0);
        if (phase === "after-reply") {
          expect(result.error.partialObservation).toMatchObject({
            normalized_result: {
              observations: [expect.objectContaining({ tool: "list_funcs" })],
            },
          });
        }
        expect(result.error.cause).toBe(reason);
      }
    } finally {
      try {
        const closed = await client.close();
        console.log(JSON.stringify({ phase, close_ok: closed.ok }));
        expect(closed.ok).toBe(true);
      } finally {
        await rm(root, { recursive: true, force: true });
      }
    }
  },
);

The after-reply case aborts inside the fake's beforeCall hook. The fake then returns its normal reply, which production code records before observing cancellation.

Expected behavior

The typed cancellation's cause should equal the exact object supplied to abort(reason). The lifecycle contract explicitly requires preservation of the caller's abort reason, including cancellation observed after provider completion. The test guidance requires assertions of that identity at admission and after provider completion.

Impact and scope

P2. Callers of the typed provider-session result lose a required diagnostic value and cannot identify their supplied cancellation reason through the returned error. A caller that retains its own signal can still read signal.reason.

Both cancellation cases reproduced once in the focused run. Cancellation classification remained correct, the post-reply case retained partial observations, and all three closes succeeded. No data loss or incorrect analysis result was observed. This does not establish real-engine frequency, native-profile behavior, queued/final-observation races, or public transport delivery of opaque causes.

Source-derived reachability exists through supported MCP tools: the official inventory contract declares list_procedures, the tool handler passes the request signal after admission, and the session dispatcher forwards accepted execution options to the active provider client. Cancellation during the awaited provider call occurs after those admission checks. This is not a claim that the public JSON schema must serialize the opaque cause.

REA version

rea-agents 6.7.0; source baseline a5f982de8d.

Runtime and platform

Node.js v26.12.0; macOS, darwin arm64; Vitest 5.0.3; no IDA or other native analysis engine launched.

Target and artifact context

Synthetic ELF x86_64 fixture from createIdaTarget; inert attached/headless connection fake; no target execution.

Interface

Other

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

Trimmed diagnostics from the executed focused source test:

{"phase":"before-start","tag":"AnalysisCancelledError","same_reason":false,"cause_absent":true,"cause_is_error":false,"nested_cause_absent":false,"partial_observation":false,"calls":[]}
{"phase":"before-start","close_ok":true}
{"phase":"after-reply","tag":"AnalysisCancelledError","same_reason":false,"cause_absent":false,"cause_is_error":true,"nested_cause_absent":true,"partial_observation":true,"calls":["idb_open","idb_list","server_health","list_funcs"]}
{"phase":"after-reply","close_ok":true}
{"phase":"control","ok":true}
{"phase":"control","close_ok":true}

Tests  2 failed | 1 passed (3)
AssertionError: expected undefined to be { requested: 'stop', marker: 7 } // Object.is equality

The second failure received AnalysisCancelledError for operation list_funcs instead of the same object. The temporary test was removed, and git status --porcelain was empty afterward.

Typed outcome or recovery expected

Keep AnalysisCancelledError and set its cause to the caller's exact abort reason while retaining completed observations and the actual cleanup state.

Analysis or suspected cause

Source-derived: the session pre-start guard constructs cancellation without options. The operation runner does the same after retaining a reply. The session failure wrapper then uses that internal error as the outer cause. These paths do not read signal.reason.

This report concerns those guards. It does not claim every IDA cancellation loses its reason; profile-resolution admission already preserves it.

Additional context

Read-only searches of open/closed issues and PRs found no same-failure report. Closed reports #5670 and #5714 cover similar reason loss in other adapters. Closed IDA report #4633 covers cleanup-owner binding rather than abort-reason retention.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5822 ### 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 An executed provider-session test called `IdaSessionClient.execute("list_procedures", {}, { signal })` with an opaque object passed to `AbortController.abort(reason)`. - With the signal aborted before the attached session starts, the returned `AnalysisCancelledError.cause` was `undefined`; the connection was never started. - With cancellation triggered during a headless `list_funcs` call, the adapter recorded the completed reply and returned `AnalysisCancelledError`. Its cause was another `AnalysisCancelledError` with no cause, rather than the caller's object. Partial observations retained the reply. Both exact-object cause assertions failed. The non-aborted control succeeded, and `client.close()` succeeded in all three cases. These are executed adapter-boundary observations using the existing `RecordingIdaMcp` fake. No IDA engine or public CLI/MCP request was run. ### Reproduction From the source revision below, with locked dependencies installed, place the following temporary test beside the adapter as `src/ida/repro.test.ts`. Run `npm run test:local -- src/ida/repro.test.ts`, then delete it. Run in an isolated HOME and XDG_CONFIG_HOME, with TMPDIR set to an existing test-owned directory. One run of this three-case test produced two failed cause assertions and one passing control. ```ts import { rm } from "node:fs/promises"; import { expect, it } from "vitest"; import { createIdaTarget, RecordingIdaMcp } from "../../tests/fixtures/idaMcp.js"; import { IdaSessionClient } from "./IdaSessionClient.js"; it.each(["before-start", "after-reply", "control"] as const)( "preserves cancellation reason: %s", async (phase) => { const { root, target } = await createIdaTarget(); const mode = phase === "before-start" ? "attached" : "headless"; const producer = new RecordingIdaMcp(target, mode); const controller = new AbortController(); const reason = Object.freeze({ requested: "stop", marker: 7 }); if (phase === "before-start") controller.abort(reason); if (phase === "after-reply") { producer.beforeCall = async (name) => { if (name === "list_funcs") controller.abort(reason); }; } const client = new IdaSessionClient( { command: "fixture", args: [], env: {}, mode, timeoutMs: 1000, workspaceRoot: root }, target, producer, ); try { const result = await client.execute( "list_procedures", {}, { signal: controller.signal }, ); if (phase === "control") { expect(result.ok).toBe(true); console.log(JSON.stringify({ phase, ok: result.ok })); } else { if (result.ok) throw new Error("Expected cancellation"); const cause = result.error.cause; console.log(JSON.stringify({ phase, tag: result.error._tag, same_reason: cause === reason, cause_absent: cause === undefined, cause_is_error: cause instanceof Error, nested_cause_absent: cause instanceof Error && cause.cause === undefined, partial_observation: result.error.partialObservation !== undefined, calls: producer.calls.map(({ name }) => name), })); expect(result.error._tag).toBe("AnalysisCancelledError"); if (phase === "before-start") expect(producer.connects).toBe(0); if (phase === "after-reply") { expect(result.error.partialObservation).toMatchObject({ normalized_result: { observations: [expect.objectContaining({ tool: "list_funcs" })], }, }); } expect(result.error.cause).toBe(reason); } } finally { try { const closed = await client.close(); console.log(JSON.stringify({ phase, close_ok: closed.ok })); expect(closed.ok).toBe(true); } finally { await rm(root, { recursive: true, force: true }); } } }, ); ``` The `after-reply` case aborts inside the fake's `beforeCall` hook. The fake then returns its normal reply, which production code records before observing cancellation. ### Expected behavior The typed cancellation's `cause` should equal the exact object supplied to `abort(reason)`. The [lifecycle contract](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/docs/tool-design.md#L101-L110) explicitly requires preservation of the caller's abort reason, including cancellation observed after provider completion. The [test guidance](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/docs/testing.md#L268-L275) requires assertions of that identity at admission and after provider completion. ### Impact and scope P2. Callers of the typed provider-session result lose a required diagnostic value and cannot identify their supplied cancellation reason through the returned error. A caller that retains its own signal can still read `signal.reason`. Both cancellation cases reproduced once in the focused run. Cancellation classification remained correct, the post-reply case retained partial observations, and all three closes succeeded. No data loss or incorrect analysis result was observed. This does not establish real-engine frequency, native-profile behavior, queued/final-observation races, or public transport delivery of opaque causes. Source-derived reachability exists through supported MCP tools: the [official inventory contract](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/contracts/officialToolContracts.ts#L120-L124) declares `list_procedures`, the [tool handler](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/server/registerEvidenceTools.ts#L63-L105) passes the request signal after admission, and the [session dispatcher](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/application/binary/BinarySession.ts#L1129-L1156) forwards accepted execution options to the active provider client. Cancellation during the awaited provider call occurs after those admission checks. This is not a claim that the public JSON schema must serialize the opaque cause. ### REA version rea-agents 6.7.0; source baseline a5f982de8dbe4b092cc20ab89d492b7e7028b758. ### Runtime and platform Node.js v26.12.0; macOS, darwin arm64; Vitest 5.0.3; no IDA or other native analysis engine launched. ### Target and artifact context Synthetic ELF x86_64 fixture from createIdaTarget; inert attached/headless connection fake; no target execution. ### Interface Other ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence Trimmed diagnostics from the executed focused source test: ```json {"phase":"before-start","tag":"AnalysisCancelledError","same_reason":false,"cause_absent":true,"cause_is_error":false,"nested_cause_absent":false,"partial_observation":false,"calls":[]} {"phase":"before-start","close_ok":true} {"phase":"after-reply","tag":"AnalysisCancelledError","same_reason":false,"cause_absent":false,"cause_is_error":true,"nested_cause_absent":true,"partial_observation":true,"calls":["idb_open","idb_list","server_health","list_funcs"]} {"phase":"after-reply","close_ok":true} {"phase":"control","ok":true} {"phase":"control","close_ok":true} ``` ```text Tests 2 failed | 1 passed (3) AssertionError: expected undefined to be { requested: 'stop', marker: 7 } // Object.is equality ``` The second failure received `AnalysisCancelledError` for operation `list_funcs` instead of the same object. The temporary test was removed, and `git status --porcelain` was empty afterward. ### Typed outcome or recovery expected Keep `AnalysisCancelledError` and set its cause to the caller's exact abort reason while retaining completed observations and the actual cleanup state. ### Analysis or suspected cause Source-derived: the [session pre-start guard](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/ida/IdaSessionClient.ts#L220-L233) constructs cancellation without options. The [operation runner](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/ida/IdaOperationRunner.ts#L114-L124) does the same after retaining a reply. The [session failure wrapper](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/ida/IdaSessionClient.ts#L383-L412) then uses that internal error as the outer cause. These paths do not read `signal.reason`. This report concerns those guards. It does not claim every IDA cancellation loses its reason; profile-resolution admission already preserves it. ### Additional context Read-only searches of open/closed issues and PRs found no same-failure report. Closed reports [#5670](https://github.com/morluto/rea/issues/5670) and [#5714](https://github.com/morluto/rea/issues/5714) cover similar reason loss in other adapters. Closed IDA report [#4633](https://github.com/morluto/rea/issues/4633) covers cleanup-owner binding rather than abort-reason retention.

bsakaguchi closed this issue

2026-10-31 00:41:27 +00:00

bsakaguchi commented

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

Author

Owner

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

At d87e73f2db50be72d33a4b3c7199af35518b441c, IDA pre-start admission preserves signal.reason, the operation runner retains the reply before reason-preserving cancellation, and the session wrapper preserves the direct cause. The two-phase regression matches the attached pre-start/headless list_funcs triggers and asserts exact reason identity and clean close.

I ran the cited regression file on this revision; all passed as part of the focused 28-file, 603-test run. Provider-facing checks here use controlled fixtures; this does not claim a run with the real analysis engine. Closing as completed on main.

@coygeek commented on GitHub (Oct 24, 2026): At `d87e73f2db50be72d33a4b3c7199af35518b441c`, IDA [pre-start admission preserves `signal.reason`](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/ida/IdaSessionClient.ts#L275-L279), the [operation runner retains the reply before reason-preserving cancellation](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/ida/IdaOperationRunner.ts#L124-L137), and the [session wrapper preserves the direct cause](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/ida/IdaSessionClient.ts#L453-L467). The [two-phase regression](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/ida/IdaSessionClient.test.ts#L2015-L2043) matches the attached pre-start/headless `list_funcs` triggers and asserts exact reason identity and clean close. I ran the cited regression file on this revision; all passed as part of the focused 28-file, 603-test run. Provider-facing checks here use controlled fixtures; this does not claim a run with the real analysis engine. Closing as completed on main.

bsakaguchi referenced this issue

2026-10-31 01:09:32 +00:00

[PR #3521] feat(reverse): add radare2 one-shot commands and analysis sessions #5800

bsakaguchi referenced this issue

2026-10-31 01:11:46 +00:00

[PR #4148] feat(radare2): prototype transports and analyst capabilities #6088

bsakaguchi referenced this issue

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

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

bsakaguchi referenced this issue

2026-11-01 21:11:07 +00:00

[GH-ISSUE #5654] [Tracker] Open PRs that can't merge yet: conflicts and red CI (October 2026) #3425