morluto/rea

Fork 0

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

[GH-ISSUE #5996] [Bug] IDA replaces an earlier provider failure with cancellation during error unwinding #3664

New issue

Closed

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

bsakaguchi commented

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

Owner

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

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 IDA provider-session test threw a transport failure from list_functions, then aborted the caller signal in the next microtask. The event log recorded provider-failure before abort. IdaSessionClient.execute("list_procedures", {}, { signal }) nevertheless returned AnalysisCancelledError, projected as cancelled, retryable: true, with restart advice.

The same producer failure without the abort returned ProviderAdapterError, projected as execution_failure, retryable: false, with provider-registration recovery details. The original error and diagnostic survive under the cancellation cause; the primary classification and recovery are what change. This was a production-session boundary test with the repository’s simulated upstream connection, not an IDA engine or public CLI/MCP invocation.

Reproduction

From the reported source revision with locked dependencies installed, save these executed cases as src/ida/lateCancellationRepro.test.ts and run:

node node_modules/vitest/vitest.mjs run src/ida/lateCancellationRepro.test.ts --maxWorkers=1

The two cases were executed together in a focused temporary test: the non-aborted control passed and the late-abort expected-contract assertion failed. Each session closed successfully.

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

it.each([false, true])("IDA provider failure with late abort=%s", async (lateAbort) => {
  const { root, target } = await createIdaTarget();
  const producer = new RecordingIdaMcp(target, "attached");
  const client = new IdaSessionClient({ command: "fixture", args: [], env: {}, mode: "attached", timeoutMs: 1000, workspaceRoot: root }, target, producer);
  const controller = new AbortController();
  const events: string[] = [];
  const failure = new ProviderAdapterError("ida", "transport", { diagnostics: { reason: "upstream closed before cancellation" } });
  try {
    expect((await client.execute("health", {})).ok).toBe(true);
    producer.beforeCall = async (name) => {
      if (name !== "list_functions") return;
      if (lateAbort) queueMicrotask(() => { events.push("abort"); controller.abort("late stop"); });
      events.push("provider-failure");
      throw failure;
    };
    const result = await client.execute("list_procedures", {}, { signal: controller.signal });
    if (result.ok) throw new Error("expected provider failure");
    const projected = projectAnalysisError(result.error);
    console.log(JSON.stringify({ lateAbort, events, tag: result.error._tag, projection: projected, causeIsProviderFailure: result.error.cause === failure }));
    expect(projected.code).toBe("execution_failure");
  } finally { expect((await client.close()).ok).toBe(true); await rm(root, { recursive: true, force: true }); }
});

Expected behavior

An already-produced primary provider failure should keep its typed classification and recovery when a later cancellation arrives during error handling. The lifecycle contract requires preserving an existing primary failure. The lifecycle regression row requires existing primary/cleanup failures to keep their precedence. The Hopper counterpart already tests that distinction.

Impact and scope

P2: callers receive a retryable cancellation and restart advice for a provider failure that is non-retryable in the control. This can conceal which recovery is needed when an upstream request fails immediately before cancellation. The diagnostic remains available for manual inspection, so the failure is not entirely erased. The same controlled interleaving reproduced in the initial run and in an independent rerun of the exact issue snippet; native frequency and real-client occurrence are unknown. No target mutation or data loss was observed.

REA version

rea-agents 6.7.0, source revision 52415975942a4b9c53a817915b313ccc404f280e. Fetched origin/main at c538a843cb49f7156f86beb3b21501e03255af9a leaves the producing code and cited lifecycle contract unchanged; that newer revision was source-compared, not executed.

Runtime and platform

macOS 27.0.1 (26A434), arm64; Node.js v26.12.0; locked Vitest 5.0.3. No IDA engine was launched.

Target and artifact context

A synthetic ELF fixture and the existing RecordingIdaMcp attached-mode connection. The fixture bytes were inspected as data and never executed. Other IDA profiles were not run.

Interface

Other

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

Observed results:

control: events=["provider-failure"]
  ProviderAdapterError; code=execution_failure; retryable=false
late abort: events=["provider-failure","abort"]
  AnalysisCancelledError; code=cancelled; retryable=true
  cause === originalProviderFailure: true
  diagnostics.reason: "upstream closed before cancellation"

AssertionError: expected 'cancelled' to be 'execution_failure'

The control and late-abort cases differ only in the queued abort. The session cleanup assertion passed in both.

Typed outcome or recovery expected

Keep the original provider failure as the primary typed result and retain the later cancellation as a separate lifecycle fact. Preserve actual cleanup state and diagnostics. In this reproduction, keep execution_failure, retryable: false, and the upstream-registration recovery details.

Analysis or suspected cause

Source-derived: the operation call awaits the upstream connection; the session catch sends the rejection to the failure mapper. That mapper checks current signal.aborted before inspecting the provider error. It cannot distinguish cancellation before the failure from cancellation during unwinding. The existing cancellation-first test is a separate contract that should remain covered; an unconditional cause-first rule would need to account for that case. Record producing-boundary ordering rather than reconstructing it from a later signal sample.

Additional context

Related cancellation reports: #5822, #5935, #5944, #5959, #5960, #5964, #5968, #5985, #5987. In particular, #5822 concerns IDA losing the exact abort reason; forwarding that reason alone would not restore primary-failure precedence here. #4633 concerns IDA cleanup-owner binding. PR #2695 preserves upstream diagnostics, which this reproduction shows are still present. PR #4645 concerns the analysis profile in partial Evidence. Reports #5670, #5279 and #4598 address similar precedence defects in other producers; they do not change this IDA mapper. Design #4227 requests focused lifecycle defects. No software fix or regression range was established.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5996 ### 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 IDA provider-session test threw a transport failure from `list_functions`, then aborted the caller signal in the next microtask. The event log recorded `provider-failure` before `abort`. `IdaSessionClient.execute("list_procedures", {}, { signal })` nevertheless returned `AnalysisCancelledError`, projected as `cancelled`, `retryable: true`, with restart advice. The same producer failure without the abort returned `ProviderAdapterError`, projected as `execution_failure`, `retryable: false`, with provider-registration recovery details. The original error and diagnostic survive under the cancellation cause; the primary classification and recovery are what change. This was a production-session boundary test with the repository’s simulated upstream connection, not an IDA engine or public CLI/MCP invocation. ### Reproduction From the reported source revision with locked dependencies installed, save these executed cases as `src/ida/lateCancellationRepro.test.ts` and run: ```sh node node_modules/vitest/vitest.mjs run src/ida/lateCancellationRepro.test.ts --maxWorkers=1 ``` The two cases were executed together in a focused temporary test: the non-aborted control passed and the late-abort expected-contract assertion failed. Each session closed successfully. ```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"; import { ProviderAdapterError } from "../domain/providerAdapterError.js"; import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; it.each([false, true])("IDA provider failure with late abort=%s", async (lateAbort) => { const { root, target } = await createIdaTarget(); const producer = new RecordingIdaMcp(target, "attached"); const client = new IdaSessionClient({ command: "fixture", args: [], env: {}, mode: "attached", timeoutMs: 1000, workspaceRoot: root }, target, producer); const controller = new AbortController(); const events: string[] = []; const failure = new ProviderAdapterError("ida", "transport", { diagnostics: { reason: "upstream closed before cancellation" } }); try { expect((await client.execute("health", {})).ok).toBe(true); producer.beforeCall = async (name) => { if (name !== "list_functions") return; if (lateAbort) queueMicrotask(() => { events.push("abort"); controller.abort("late stop"); }); events.push("provider-failure"); throw failure; }; const result = await client.execute("list_procedures", {}, { signal: controller.signal }); if (result.ok) throw new Error("expected provider failure"); const projected = projectAnalysisError(result.error); console.log(JSON.stringify({ lateAbort, events, tag: result.error._tag, projection: projected, causeIsProviderFailure: result.error.cause === failure })); expect(projected.code).toBe("execution_failure"); } finally { expect((await client.close()).ok).toBe(true); await rm(root, { recursive: true, force: true }); } }); ``` ### Expected behavior An already-produced primary provider failure should keep its typed classification and recovery when a later cancellation arrives during error handling. [The lifecycle contract](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/tool-design.md#L101-L110) requires preserving an existing primary failure. [The lifecycle regression row](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/testing.md#L249-L260) requires existing primary/cleanup failures to keep their precedence. [The Hopper counterpart](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/hopper/HopperRegexSearch.test.ts#L168-L197) already tests that distinction. ### Impact and scope P2: callers receive a retryable cancellation and restart advice for a provider failure that is non-retryable in the control. This can conceal which recovery is needed when an upstream request fails immediately before cancellation. The diagnostic remains available for manual inspection, so the failure is not entirely erased. The same controlled interleaving reproduced in the initial run and in an independent rerun of the exact issue snippet; native frequency and real-client occurrence are unknown. No target mutation or data loss was observed. ### REA version `rea-agents` 6.7.0, source revision `52415975942a4b9c53a817915b313ccc404f280e`. Fetched `origin/main` at `c538a843cb49f7156f86beb3b21501e03255af9a` leaves the producing code and cited lifecycle contract unchanged; that newer revision was source-compared, not executed. ### Runtime and platform macOS 27.0.1 (26A434), arm64; Node.js v26.12.0; locked Vitest 5.0.3. No IDA engine was launched. ### Target and artifact context A synthetic ELF fixture and the existing `RecordingIdaMcp` attached-mode connection. The fixture bytes were inspected as data and never executed. Other IDA profiles were not run. ### Interface Other ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence Observed results: ```text control: events=["provider-failure"] ProviderAdapterError; code=execution_failure; retryable=false late abort: events=["provider-failure","abort"] AnalysisCancelledError; code=cancelled; retryable=true cause === originalProviderFailure: true diagnostics.reason: "upstream closed before cancellation" AssertionError: expected 'cancelled' to be 'execution_failure' ``` The control and late-abort cases differ only in the queued abort. The session cleanup assertion passed in both. ### Typed outcome or recovery expected Keep the original provider failure as the primary typed result and retain the later cancellation as a separate lifecycle fact. Preserve actual cleanup state and diagnostics. In this reproduction, keep `execution_failure`, `retryable: false`, and the upstream-registration recovery details. ### Analysis or suspected cause Source-derived: [the operation call](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/ida/IdaOperationRunner.ts#L114-L125) awaits the upstream connection; [the session catch](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/ida/IdaSessionClient.ts#L338-L343) sends the rejection to [the failure mapper](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/ida/IdaSessionClient.ts#L383-L440). That mapper checks current `signal.aborted` before inspecting the provider error. It cannot distinguish cancellation before the failure from cancellation during unwinding. The [existing cancellation-first test](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/ida/IdaSessionClient.test.ts#L1647-L1677) is a separate contract that should remain covered; an unconditional cause-first rule would need to account for that case. Record producing-boundary ordering rather than reconstructing it from a later signal sample. ### Additional context Related cancellation reports: [#5822](https://github.com/morluto/rea/issues/5822), [#5935](https://github.com/morluto/rea/issues/5935), [#5944](https://github.com/morluto/rea/issues/5944), [#5959](https://github.com/morluto/rea/issues/5959), [#5960](https://github.com/morluto/rea/issues/5960), [#5964](https://github.com/morluto/rea/issues/5964), [#5968](https://github.com/morluto/rea/issues/5968), [#5985](https://github.com/morluto/rea/issues/5985), [#5987](https://github.com/morluto/rea/issues/5987). In particular, #5822 concerns IDA losing the exact abort reason; forwarding that reason alone would not restore primary-failure precedence here. #4633 concerns IDA cleanup-owner binding. PR #2695 preserves upstream diagnostics, which this reproduction shows are still present. PR #4645 concerns the analysis profile in partial Evidence. Reports #5670, #5279 and #4598 address similar precedence defects in other producers; they do not change this IDA mapper. Design #4227 requests focused lifecycle defects. No software fix or regression range was established.

bsakaguchi closed this issue

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

bsakaguchi commented

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

Author

Owner

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

Checked the reported fix on main at d87e73f2db.

IdaSessionClient no longer samples signal.aborted before classifying a produced provider error. It handles explicit AnalysisCancelledError separately and preserves transport/provider failures with their cause and recovery. The regression throws a ProviderAdapterError from list_functions and optionally queues a later abort; both branches must project execution_failure and retain exactly that cause.

Implementation: IdaSessionClient.ts, IdaSessionClient.ts.

The relevant regression checks in IdaSessionClient.test.ts, IdaMcpConnection.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. IdaSessionClient no longer samples signal.aborted before classifying a produced provider error. It handles explicit AnalysisCancelledError separately and preserves transport/provider failures with their cause and recovery. The regression throws a ProviderAdapterError from list_functions and optionally queues a later abort; both branches must project execution_failure and retain exactly that cause. Implementation: [IdaSessionClient.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/ida/IdaSessionClient.ts#L453), [IdaSessionClient.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/ida/IdaSessionClient.ts#L488). The relevant regression checks in [IdaSessionClient.test.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/ida/IdaSessionClient.test.ts), [IdaMcpConnection.test.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/ida/IdaMcpConnection.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-10-31 01:10:05 +00:00

[PR #3728] [MERGED] fix(ghidra): invalidate synthetic entry reference snapshots #5872

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

No description provided.

Rows
Columns