morluto/rea

Fork 0

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

[GH-ISSUE #5825] [Bug] Headless x86 callee reads make one lookup request per internal callee #3525

New issue

Closed

opened 2026-10-31 00:41:29 +00:00 by bsakaguchi · 2 comments

bsakaguchi commented

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

Owner

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

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 adapter-boundary reproduction of procedure_callees with { procedure: "main" } returned all twenty expected callee addresses, but the modern headless x86_64 session issued twenty separate lookup_funcs requests to resolve them. Each request contained one query. A separate selector lookup makes 21 lookup requests for the operation.

With the same twenty function destinations and lookup replies, an arm64 control used one relationship lookup containing twenty queries, plus the selector lookup. Both reads and session closes succeeded.

This used the production IdaSessionClient and the repository's fake supervisor at the connection boundary. It did not launch IDA or execute a public CLI/MCP request. The serial ordering is source-confirmed by the awaited loop cited below.

Reproduction

I ran the following temporary adapter test once; both architecture cases passed their assertions of the current behavior. To repeat from the reported source revision with installed dependencies, save this body as src/ida/repro.test.ts and run npm run test:local -- src/ida/repro.test.ts with the pinned Node.js. Remove the test afterward. The first placement under tests/ was not owned by a test project, so the executed test was placed beside the adapter.

import { afterEach, expect, it } from "vitest";
import { callGraphSession, edge, graphRow, removeCallGraphSessions } from "../../tests/fixtures/idaCallGraph.js";

afterEach(removeCallGraphSessions);

it.each(["x86_64", "arm64"] as const)("records callee lookup batches on %s", async (architecture) => {
  const addresses = Array.from({ length: 20 }, (_, i) => `0x${(0x2000 + i * 0x100).toString(16)}`);
  const { client, producer } = await callGraphSession({
    callees: addresses.map(addr => ({ addr, name: `callee_${addr}`, type: "internal" })),
    callgraph: graphRow(addresses.map(addr => edge("0x1000", addr))),
  }, architecture);
  const original = producer.overrideCall;
  producer.overrideCall = (name, args) => {
    if (name === "lookup_funcs" && Array.isArray(args.queries)) {
      return args.queries.map(value => {
        const query = String(value);
        return { query, fn: { addr: query === "main" ? "0x1000" : query, name: query === "main" ? "main" : `callee_${query}`, size: "0x20" }, error: null };
      });
    }
    return original?.(name, args);
  };
  try {
    const execution = await client.execute("procedure_callees", { procedure: "main" });
    if (!execution.ok) throw execution.error;
    expect(execution.value.result).toEqual(addresses);
    const lookups = producer.calls.filter(call => call.name === "lookup_funcs");
    const queriesPerRequest = lookups.map(call => Array.isArray(call.args.queries) ? call.args.queries.length : -1);
    const relationLookups = lookups.slice(1);
    expect(relationLookups.length).toBe(architecture === "x86_64" ? 20 : 1);
    expect(queriesPerRequest).toEqual(architecture === "x86_64" ? Array(21).fill(1) : [1, 20]);
    console.log(JSON.stringify({ architecture, resultCount: addresses.length, lookupRequestsIncludingSelector: lookups.length, relationshipLookupRequests: relationLookups.length, queriesPerRequest }));
  } finally {
    expect((await client.close()).ok).toBe(true);
  }
});

The fixture provides the advertised supervisor tools, a test-owned executable input, owned database identity, healthy readiness replies, and successful release. Its override supports both singleton and batched lookup arguments. This reproduces REA's dispatched request count; real IDA performance remains untested.

Expected behavior

Resolve the already-collected unique internal callee addresses in one batched lookup, preserving query identity validation, per-item failures, returned addresses and Evidence. Keep the caller's selector lookup separate.

The applicable contract is the tool-design guide's instruction to batch homogeneous reads when that removes repeated work while preserving per-item outcomes. These addresses are known before any lookup begins. The same modern profile already submits a query array in its non-x86 graph reader, corroborated by the executed control.

For twenty internal callees, the relationship portion should use two analysis requests: one callees read and one batched lookup. Including the selector lookup gives three, rather than the current 22. Lifecycle and identity requests are separate from these counts.

Impact and scope

P2: bounded request amplification in a supported analysis path. Twenty internal callees produced twenty relationship lookups instead of one, while preserving the correct result in the fixture. The additional requests are awaited sequentially. Real wall time, CPU, peak memory, agent tokens and timeout frequency were not measured; no wrong result or workflow failure was observed.

Source-derived scope is modern headless analysis of targets REA proves x86-only, including procedure_callees and the callee facet of analyze_function. The dossier path was not separately executed. External callee rows bypass lookup; repeated internal query addresses are deduplicated. Native attached mode already batches, and non-x86 modern targets use the graph path. The upstream callee request asks for a limit of 500; this test used twenty rows. No caller-facing control to batch the internal resolution was verified.

REA version

rea-agents 6.7.0; source commit a5f982de8d.

Runtime and platform

Node.js 24.18.0; macOS 27.0.1 arm64; Vitest 5.0.3; simulated IDA supervisor, no IDA engine used.

Target and artifact context

Test-owned executable fixtures declaring x86_64 or arm64, each with twenty internal callee addresses and deterministic function lookup replies.

Interface

MCP

Earliest failure stage

Native analysis or reconstruction

Relevant output or evidence

Trimmed output from the focused adapter test; architecture cases differ only in target architecture and the corresponding callee source:

> rea-agents@6.7.0 test:local
> node scripts/run-development-tests.mjs local <temporary-adapter-test>
Source-test feedback; compiled boundaries and real providers need explicit verification.

{"architecture":"x86_64","resultCount":20,"lookupRequestsIncludingSelector":21,"relationshipLookupRequests":20,"queriesPerRequest":[1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1]}
{"architecture":"arm64","resultCount":20,"lookupRequestsIncludingSelector":2,"relationshipLookupRequests":1,"queriesPerRequest":[1,20]}

Test Files  1 passed (1)
     Tests  2 passed (2)

The fixture assertions checked every returned address and successful close. The temporary test was removed and git status --porcelain was empty.

Checked source explains the observed requests:

The x86-only predicate and graph-path predicate send this target through the upstream callee reader.
The reader collects internal addresses, batches only for native protocol, then awaits one resolve per query otherwise.
Modern resolve sends a singleton query array.
The public operation's selector resolution and relationship dispatch explain the extra selector request. The dossier uses the same relationship reader.
The session client validates and dispatches the operation, then checks target identity again. The test exercised this production boundary, including successful startup and close.

Typed outcome or recovery expected

No response

Analysis or suspected cause

Source-derived: the batch branch is restricted to protocol === "native"; modern x86 reads fall into the legacy-compatible serial loop despite having a batch-capable lookup tool. Extending shared batch resolution to the modern branch appears sufficient, provided it preserves the current x86 query/error semantics. The graph reader has different recovery behavior for unresolved names, so copying its failure handling would require separate justification.

Additional context

No response

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5825 ### 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 adapter-boundary reproduction of `procedure_callees` with `{ procedure: "main" }` returned all twenty expected callee addresses, but the modern headless x86_64 session issued twenty separate `lookup_funcs` requests to resolve them. Each request contained one query. A separate selector lookup makes 21 lookup requests for the operation. With the same twenty function destinations and lookup replies, an arm64 control used one relationship lookup containing twenty queries, plus the selector lookup. Both reads and session closes succeeded. This used the production `IdaSessionClient` and the repository's fake supervisor at the connection boundary. It did not launch IDA or execute a public CLI/MCP request. The serial ordering is source-confirmed by the awaited loop cited below. ### Reproduction I ran the following temporary adapter test once; both architecture cases passed their assertions of the current behavior. To repeat from the reported source revision with installed dependencies, save this body as `src/ida/repro.test.ts` and run `npm run test:local -- src/ida/repro.test.ts` with the pinned Node.js. Remove the test afterward. The first placement under `tests/` was not owned by a test project, so the executed test was placed beside the adapter. ```ts import { afterEach, expect, it } from "vitest"; import { callGraphSession, edge, graphRow, removeCallGraphSessions } from "../../tests/fixtures/idaCallGraph.js"; afterEach(removeCallGraphSessions); it.each(["x86_64", "arm64"] as const)("records callee lookup batches on %s", async (architecture) => { const addresses = Array.from({ length: 20 }, (_, i) => `0x${(0x2000 + i * 0x100).toString(16)}`); const { client, producer } = await callGraphSession({ callees: addresses.map(addr => ({ addr, name: `callee_${addr}`, type: "internal" })), callgraph: graphRow(addresses.map(addr => edge("0x1000", addr))), }, architecture); const original = producer.overrideCall; producer.overrideCall = (name, args) => { if (name === "lookup_funcs" && Array.isArray(args.queries)) { return args.queries.map(value => { const query = String(value); return { query, fn: { addr: query === "main" ? "0x1000" : query, name: query === "main" ? "main" : `callee_${query}`, size: "0x20" }, error: null }; }); } return original?.(name, args); }; try { const execution = await client.execute("procedure_callees", { procedure: "main" }); if (!execution.ok) throw execution.error; expect(execution.value.result).toEqual(addresses); const lookups = producer.calls.filter(call => call.name === "lookup_funcs"); const queriesPerRequest = lookups.map(call => Array.isArray(call.args.queries) ? call.args.queries.length : -1); const relationLookups = lookups.slice(1); expect(relationLookups.length).toBe(architecture === "x86_64" ? 20 : 1); expect(queriesPerRequest).toEqual(architecture === "x86_64" ? Array(21).fill(1) : [1, 20]); console.log(JSON.stringify({ architecture, resultCount: addresses.length, lookupRequestsIncludingSelector: lookups.length, relationshipLookupRequests: relationLookups.length, queriesPerRequest })); } finally { expect((await client.close()).ok).toBe(true); } }); ``` The fixture provides the advertised supervisor tools, a test-owned executable input, owned database identity, healthy readiness replies, and successful release. Its override supports both singleton and batched lookup arguments. This reproduces REA's dispatched request count; real IDA performance remains untested. ### Expected behavior Resolve the already-collected unique internal callee addresses in one batched lookup, preserving query identity validation, per-item failures, returned addresses and Evidence. Keep the caller's selector lookup separate. The applicable contract is the tool-design guide's instruction to [batch homogeneous reads when that removes repeated work while preserving per-item outcomes](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/docs/tool-design.md#L36-L39). These addresses are known before any lookup begins. The same modern profile already submits a query array in its [non-x86 graph reader](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/ida/IdaCallGraph.ts#L123-L154), corroborated by the executed control. For twenty internal callees, the relationship portion should use two analysis requests: one `callees` read and one batched lookup. Including the selector lookup gives three, rather than the current 22. Lifecycle and identity requests are separate from these counts. ### Impact and scope P2: bounded request amplification in a supported analysis path. Twenty internal callees produced twenty relationship lookups instead of one, while preserving the correct result in the fixture. The additional requests are awaited sequentially. Real wall time, CPU, peak memory, agent tokens and timeout frequency were not measured; no wrong result or workflow failure was observed. Source-derived scope is modern headless analysis of targets REA proves x86-only, including `procedure_callees` and the callee facet of `analyze_function`. The dossier path was not separately executed. External callee rows bypass lookup; repeated internal query addresses are deduplicated. Native attached mode already batches, and non-x86 modern targets use the graph path. The upstream callee request asks for a limit of 500; this test used twenty rows. No caller-facing control to batch the internal resolution was verified. ### REA version rea-agents 6.7.0; source commit a5f982de8dbe4b092cc20ab89d492b7e7028b758. ### Runtime and platform Node.js 24.18.0; macOS 27.0.1 arm64; Vitest 5.0.3; simulated IDA supervisor, no IDA engine used. ### Target and artifact context Test-owned executable fixtures declaring x86_64 or arm64, each with twenty internal callee addresses and deterministic function lookup replies. ### Interface MCP ### Earliest failure stage Native analysis or reconstruction ### Relevant output or evidence Trimmed output from the focused adapter test; architecture cases differ only in target architecture and the corresponding callee source: ```text > rea-agents@6.7.0 test:local > node scripts/run-development-tests.mjs local <temporary-adapter-test> Source-test feedback; compiled boundaries and real providers need explicit verification. {"architecture":"x86_64","resultCount":20,"lookupRequestsIncludingSelector":21,"relationshipLookupRequests":20,"queriesPerRequest":[1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1]} {"architecture":"arm64","resultCount":20,"lookupRequestsIncludingSelector":2,"relationshipLookupRequests":1,"queriesPerRequest":[1,20]} Test Files 1 passed (1) Tests 2 passed (2) ``` The fixture assertions checked every returned address and successful close. The temporary test was removed and `git status --porcelain` was empty. Checked source explains the observed requests: - The [x86-only predicate](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/ida/IdaCallGraph.ts#L32-L43) and [graph-path predicate](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/ida/IdaOperationRunner.ts#L278-L284) send this target through the upstream callee reader. - The reader [collects internal addresses, batches only for native protocol, then awaits one resolve per query otherwise](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/ida/IdaOperationRunner.ts#L539-L573). - Modern [resolve sends a singleton query array](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/ida/IdaOperationRunner.ts#L163-L169). - The public operation's [selector resolution and relationship dispatch](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/ida/IdaOperationRunner.ts#L630-L653) explain the extra selector request. The [dossier uses the same relationship reader](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/ida/IdaOperationRunner.ts#L666-L670). - The [session client validates and dispatches the operation, then checks target identity again](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/ida/IdaSessionClient.ts#L220-L315). The test exercised this production boundary, including successful startup and close. ### Typed outcome or recovery expected _No response_ ### Analysis or suspected cause Source-derived: the batch branch is restricted to `protocol === "native"`; modern x86 reads fall into the legacy-compatible serial loop despite having a batch-capable lookup tool. Extending shared batch resolution to the modern branch appears sufficient, provided it preserves the current x86 query/error semantics. The graph reader has different recovery behavior for unresolved names, so copying its failure handling would require separate justification. ### Additional context _No response_

bsakaguchi closed this issue

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

bsakaguchi commented

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

Author

Owner

@XinwangTao commented on GitHub (Oct 21, 2026):

Confirmed — for headless x86_64, procedure_callees issues 21 lookup_funcs requests (one query each) to resolve the same 20 internal callees that the arm64 path resolves in a single 20-query relationship lookup. The batch-homogeneous-reads contract already exists and the reader clearly supports multi-query payloads; making the x86 path collect internal addresses into one lookup (while keeping per-address attribution in results) would cut N+1 round trips and preserve correctness for large call graphs.

@XinwangTao commented on GitHub (Oct 21, 2026): Confirmed — for headless x86_64, procedure_callees issues 21 lookup_funcs requests (one query each) to resolve the same 20 internal callees that the arm64 path resolves in a single 20-query relationship lookup. The batch-homogeneous-reads contract already exists and the reader clearly supports multi-query payloads; making the x86 path collect internal addresses into one lookup (while keeping per-address attribution in results) would cut N+1 round trips and preserve correctness for large call graphs.

bsakaguchi commented

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

Author

Owner

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

Checked the reported fix on main at d87e73f2db.

The relationship lookup batch condition is now protocol !== legacy, which includes modern headless x86. The reader deduplicates known queries, validates batch cardinality/query identity, correlates reordered results and preserves faults. The modern/native regression asserts exactly one 20-query relationship lookup plus the selector, raw observations, duplicate handling and cancellation; a session-boundary case checks Evidence.

Implementation: IdaOperationRunner.ts, calleeLookupBatch.test.ts.

The relevant regression checks in calleeLookupBatch.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. The relationship lookup batch condition is now protocol !== legacy, which includes modern headless x86. The reader deduplicates known queries, validates batch cardinality/query identity, correlates reordered results and preserves faults. The modern/native regression asserts exactly one 20-query relationship lookup plus the selector, raw observations, duplicate handling and cancellation; a session-boundary case checks Evidence. Implementation: [IdaOperationRunner.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/ida/IdaOperationRunner.ts#L582), [calleeLookupBatch.test.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/tests/boundary/providers/ida/calleeLookupBatch.test.ts#L57). The relevant regression checks in [calleeLookupBatch.test.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/tests/boundary/providers/ida/calleeLookupBatch.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:09:33 +00:00

[PR #3525] fix(mcp): recover structured tool arguments clients serialize as JSON text #5802

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

bsakaguchi referenced this issue

2026-11-03 13:39:25 +00:00

[PR #7119] [MERGED] fix(mcp): fit every advertised input schema within the 64 KiB client limit #7653