morluto/rea

Fork 0

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

[GH-ISSUE #5908] [Bug] find_xrefs_to_name silently selects the first address for ambiguous names #3584

New issue

Closed

opened 2026-10-31 00:42:07 +00:00 by bsakaguchi · 2 comments

bsakaguchi commented

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

Owner

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

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

MCP find_xrefs_to_name silently selects the first inventory row when the requested exact name appears at two distinct addresses. It returns status: "resolved" and the xrefs for that one address. Reversing the inventory changes the selected address and reference list without any ambiguity signal.

This was executed through REA's in-memory MCP client/server harness with a simulated analysis provider. No real analysis engine or binary was used.

Reproduction

The focused test below supplies two valid list_names rows, both named shared_symbol, at 0x1000 and 0x2000. It calls the public MCP tool with {"name":"shared_symbol"}, reverses the rows, and repeats. It also checks the shared name resolver, explicit-address MCP xrefs, and unique/missing-name controls.

Both inventory orderings reproduced. Four test cases passed; the main case asserts the incorrect current behavior to make the observation repeatable.

To rerun, save the following beside enhancedToolsHarness.ts as tests/boundary/mcp/xrefsAmbiguity.test.ts, then run npx vitest run tests/boundary/mcp/xrefsAmbiguity.test.ts --maxWorkers=1 using the pinned Node version:

import { afterEach, expect, it } from "vitest";
import { closeEnhancedToolResources, connect, jsonResult } from "./enhancedToolsHarness.js";
import { observed } from "../../fixtures/analysisExecution.js";
import type { AnalysisOperationPort } from "../../../src/application/AnalysisProvider.js";
import { resolveXrefsAddress } from "../../../src/application/XrefsAddressResolution.js";

afterEach(closeEnhancedToolResources);

it("shows first-row selection for an ambiguous exact name through MCP", async () => {
  for (const addresses of [["0x1000", "0x2000"], ["0x2000", "0x1000"]]) {
    const xrefCalls: unknown[] = [];
    const analysis: AnalysisOperationPort = {
      execute: async (operation, parameters) => {
        if (operation === "list_names")
          return observed(addresses.map(address => ({ address, value: "shared_symbol" })));
        if (operation === "xrefs") {
          xrefCalls.push(parameters.address);
          return observed(parameters.address === "0x1000" ? ["0x3000"] : ["0x4000"]);
        }
        throw new Error(`Unexpected operation: ${operation}`);
      },
    };
    const direct = await resolveXrefsAddress(analysis, "shared_symbol", new AbortController().signal);
    expect(direct.ok).toBe(false);
    if (!direct.ok) {
      expect(direct.error._tag).toBe("AnalysisInputError");
      expect(JSON.stringify(direct.error)).toContain("ambiguous");
      expect(JSON.stringify(direct.error)).toContain("0x1000");
      expect(JSON.stringify(direct.error)).toContain("0x2000");
    }
    const client = await connect(analysis);
    const reply = await client.callTool({ name: "find_xrefs_to_name", arguments: { name: "shared_symbol" } });
    expect(reply.isError).not.toBe(true);
    const actual = jsonResult(reply);
    console.log(JSON.stringify({ inventory: addresses, result: actual, xrefCalls }));
    expect(actual).toEqual({ status: "resolved", name: "shared_symbol", address: addresses[0], xrefs: addresses[0] === "0x1000" ? ["0x3000"] : ["0x4000"] });
    expect(xrefCalls).toEqual([addresses[0]]);
    const explicit = jsonResult(await client.callTool({ name: "xrefs", arguments: { address: "0x2000" } }));
    expect(explicit).toEqual(["0x4000"]);
  }
});

it.each([
  { addresses: ["0x1000"], expected: { status: "resolved", name: "shared_symbol", address: "0x1000", xrefs: [] } },
  { addresses: ["0x1000", "0x1000"], expected: { status: "resolved", name: "shared_symbol", address: "0x1000", xrefs: [] } },
  { addresses: [], expected: { status: "unresolved", name: "shared_symbol", reason: "name_not_found" } },
])("retains unique, same-address duplicate and missing-name behavior: $addresses", async ({addresses, expected}) => {
  const client = await connect({execute: async (operation) => observed(operation === "list_names" ? addresses.map(address => ({address, value: "shared_symbol"})) : [])});
  expect(jsonResult(await client.callTool({name: "find_xrefs_to_name", arguments: {name: "shared_symbol"}}))).toEqual(expected);
});

Expected behavior

An exact name with multiple distinct candidate addresses should preserve that ambiguity, rather than select an address according to inventory order. A typed error listing the candidates would be sufficient. Repeated rows for the same address should remain resolvable.

The tool contract promises exact-name resolution. The existing shared xrefs resolver requires exactly one distinct address and reports ambiguity otherwise. Its behavior was checked against the same simulated inventories.

Impact and scope

P2: callers can receive references for the wrong definition, and the answer depends on inventory ordering. The demonstrated scope is a valid provider inventory with the same exact name at two distinct addresses. The frequency of that input in real engines was not measured.

A safe workaround is to inspect list_names, select the desired address explicitly, and call xrefs with that address. The explicit-address MCP call was exercised in the test and returned the expected reference list. Unique names, duplicate rows at one address, and missing names passed the controls.

REA version

6.7.0; source revision 52415975942a4b9c53a817915b313ccc404f280e.

Runtime and platform

macOS 27.0.1 arm64; Node v24.18.0; Vitest 5.0.3. In-memory MCP transport with a simulated provider; no Hopper, Ghidra, or IDA session.

Target and artifact context

Synthetic symbol inventory with two distinct addresses sharing one exact symbol name; no target artifact.

Interface

MCP

Earliest failure stage

Native analysis or reconstruction

Relevant output or evidence

The executed MCP requests returned these normalized results:

{"inventory":["0x1000","0x2000"],"result":{"status":"resolved","name":"shared_symbol","address":"0x1000","xrefs":["0x3000"]},"xrefCalls":["0x1000"]}
{"inventory":["0x2000","0x1000"],"result":{"status":"resolved","name":"shared_symbol","address":"0x2000","xrefs":["0x4000"]},"xrefCalls":["0x2000"]}

Only the first matching address was queried in each case. The shared resolver returned AnalysisInputError with an ambiguity message containing both candidate addresses. The focused file completed with 4 passed.

Typed outcome or recovery expected

Return a typed ambiguity error with the candidate addresses, or another explicit multiple-match result. Do not emit an unqualified singular resolved result for distinct matching addresses. Callers should be able to choose an exact address before requesting its references.

Analysis or suspected cause

Source-derived: EnhancedTools.#findXrefs uses Array.find and immediately queries the selected address. It never counts distinct matching addresses. The public registration reaches this workflow through normal MCP dispatch.

The direct CLI uses the shared resolver only as a fallback after provider address-format rejection; this report does not claim that every provider rejects ambiguous CLI names.

Additional context

The relevant implementation and contract files were unchanged at fetched upstream revision c324d31b45fcab77e4ed7320ed60f0bf5a4ba188.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5908 ### 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 MCP `find_xrefs_to_name` silently selects the first inventory row when the requested exact name appears at two distinct addresses. It returns `status: "resolved"` and the xrefs for that one address. Reversing the inventory changes the selected address and reference list without any ambiguity signal. This was executed through REA's in-memory MCP client/server harness with a simulated analysis provider. No real analysis engine or binary was used. ### Reproduction The focused test below supplies two valid `list_names` rows, both named `shared_symbol`, at `0x1000` and `0x2000`. It calls the public MCP tool with `{"name":"shared_symbol"}`, reverses the rows, and repeats. It also checks the shared name resolver, explicit-address MCP xrefs, and unique/missing-name controls. Both inventory orderings reproduced. Four test cases passed; the main case asserts the incorrect current behavior to make the observation repeatable. To rerun, save the following beside `enhancedToolsHarness.ts` as `tests/boundary/mcp/xrefsAmbiguity.test.ts`, then run `npx vitest run tests/boundary/mcp/xrefsAmbiguity.test.ts --maxWorkers=1` using the pinned Node version: ```ts import { afterEach, expect, it } from "vitest"; import { closeEnhancedToolResources, connect, jsonResult } from "./enhancedToolsHarness.js"; import { observed } from "../../fixtures/analysisExecution.js"; import type { AnalysisOperationPort } from "../../../src/application/AnalysisProvider.js"; import { resolveXrefsAddress } from "../../../src/application/XrefsAddressResolution.js"; afterEach(closeEnhancedToolResources); it("shows first-row selection for an ambiguous exact name through MCP", async () => { for (const addresses of [["0x1000", "0x2000"], ["0x2000", "0x1000"]]) { const xrefCalls: unknown[] = []; const analysis: AnalysisOperationPort = { execute: async (operation, parameters) => { if (operation === "list_names") return observed(addresses.map(address => ({ address, value: "shared_symbol" }))); if (operation === "xrefs") { xrefCalls.push(parameters.address); return observed(parameters.address === "0x1000" ? ["0x3000"] : ["0x4000"]); } throw new Error(`Unexpected operation: ${operation}`); }, }; const direct = await resolveXrefsAddress(analysis, "shared_symbol", new AbortController().signal); expect(direct.ok).toBe(false); if (!direct.ok) { expect(direct.error._tag).toBe("AnalysisInputError"); expect(JSON.stringify(direct.error)).toContain("ambiguous"); expect(JSON.stringify(direct.error)).toContain("0x1000"); expect(JSON.stringify(direct.error)).toContain("0x2000"); } const client = await connect(analysis); const reply = await client.callTool({ name: "find_xrefs_to_name", arguments: { name: "shared_symbol" } }); expect(reply.isError).not.toBe(true); const actual = jsonResult(reply); console.log(JSON.stringify({ inventory: addresses, result: actual, xrefCalls })); expect(actual).toEqual({ status: "resolved", name: "shared_symbol", address: addresses[0], xrefs: addresses[0] === "0x1000" ? ["0x3000"] : ["0x4000"] }); expect(xrefCalls).toEqual([addresses[0]]); const explicit = jsonResult(await client.callTool({ name: "xrefs", arguments: { address: "0x2000" } })); expect(explicit).toEqual(["0x4000"]); } }); it.each([ { addresses: ["0x1000"], expected: { status: "resolved", name: "shared_symbol", address: "0x1000", xrefs: [] } }, { addresses: ["0x1000", "0x1000"], expected: { status: "resolved", name: "shared_symbol", address: "0x1000", xrefs: [] } }, { addresses: [], expected: { status: "unresolved", name: "shared_symbol", reason: "name_not_found" } }, ])("retains unique, same-address duplicate and missing-name behavior: $addresses", async ({addresses, expected}) => { const client = await connect({execute: async (operation) => observed(operation === "list_names" ? addresses.map(address => ({address, value: "shared_symbol"})) : [])}); expect(jsonResult(await client.callTool({name: "find_xrefs_to_name", arguments: {name: "shared_symbol"}}))).toEqual(expected); }); ``` ### Expected behavior An exact name with multiple distinct candidate addresses should preserve that ambiguity, rather than select an address according to inventory order. A typed error listing the candidates would be sufficient. Repeated rows for the same address should remain resolvable. The [tool contract](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/contracts/enhancedToolContracts.ts#L58-L62) promises exact-name resolution. The existing [shared xrefs resolver](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/application/XrefsAddressResolution.ts#L55-L80) requires exactly one distinct address and reports ambiguity otherwise. Its behavior was checked against the same simulated inventories. ### Impact and scope P2: callers can receive references for the wrong definition, and the answer depends on inventory ordering. The demonstrated scope is a valid provider inventory with the same exact name at two distinct addresses. The frequency of that input in real engines was not measured. A safe workaround is to inspect `list_names`, select the desired address explicitly, and call `xrefs` with that address. The explicit-address MCP call was exercised in the test and returned the expected reference list. Unique names, duplicate rows at one address, and missing names passed the controls. ### REA version 6.7.0; source revision `52415975942a4b9c53a817915b313ccc404f280e`. ### Runtime and platform macOS 27.0.1 arm64; Node v24.18.0; Vitest 5.0.3. In-memory MCP transport with a simulated provider; no Hopper, Ghidra, or IDA session. ### Target and artifact context Synthetic symbol inventory with two distinct addresses sharing one exact symbol name; no target artifact. ### Interface MCP ### Earliest failure stage Native analysis or reconstruction ### Relevant output or evidence The executed MCP requests returned these normalized results: ```json {"inventory":["0x1000","0x2000"],"result":{"status":"resolved","name":"shared_symbol","address":"0x1000","xrefs":["0x3000"]},"xrefCalls":["0x1000"]} {"inventory":["0x2000","0x1000"],"result":{"status":"resolved","name":"shared_symbol","address":"0x2000","xrefs":["0x4000"]},"xrefCalls":["0x2000"]} ``` Only the first matching address was queried in each case. The shared resolver returned `AnalysisInputError` with an ambiguity message containing both candidate addresses. The focused file completed with `4 passed`. ### Typed outcome or recovery expected Return a typed ambiguity error with the candidate addresses, or another explicit multiple-match result. Do not emit an unqualified singular `resolved` result for distinct matching addresses. Callers should be able to choose an exact address before requesting its references. ### Analysis or suspected cause Source-derived: [`EnhancedTools.#findXrefs`](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/application/EnhancedTools.ts#L725-L754) uses `Array.find` and immediately queries the selected address. It never counts distinct matching addresses. The [public registration](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/server/registerEnhancedTools.ts#L141-L150) reaches this workflow through normal MCP dispatch. The direct CLI uses the shared resolver only as a [fallback after provider address-format rejection](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/application/DirectAnalysis.ts#L489-L520); this report does not claim that every provider rejects ambiguous CLI names. ### Additional context The relevant implementation and contract files were unchanged at fetched upstream revision `c324d31b45fcab77e4ed7320ed60f0bf5a4ba188`.

bsakaguchi closed this issue

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

bsakaguchi commented

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

Author

Owner

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

Checked the reported fix on main at d87e73f2db.

find_xrefs_to_name delegates exact-name lookup to resolveXrefsName and returns its ambiguity failure before xrefs dispatch. The shared resolver deduplicates addresses, refuses multiple distinct candidates with an address list, and allows repeated rows for one address. The MCP regression reverses inventory order, asserts both candidates and no arbitrary xrefs call, and tests duplicate rows.

Implementation: EnhancedTools.ts, XrefsAddressResolution.ts.

The relevant regression checks in enhancedTools.03-returns-typed-graph-failures-and.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. find_xrefs_to_name delegates exact-name lookup to resolveXrefsName and returns its ambiguity failure before xrefs dispatch. The shared resolver deduplicates addresses, refuses multiple distinct candidates with an address list, and allows repeated rows for one address. The MCP regression reverses inventory order, asserts both candidates and no arbitrary xrefs call, and tests duplicate rows. Implementation: [EnhancedTools.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/application/EnhancedTools.ts#L699), [XrefsAddressResolution.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/application/XrefsAddressResolution.ts#L95). The relevant regression checks in [enhancedTools.03-returns-typed-graph-failures-and.test.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/tests/boundary/mcp/enhancedTools.03-returns-typed-graph-failures-and.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:48 +00:00

[PR #3584] [MERGED] test(perf): measure genuine SAZ selection and retained recovery #5836

bsakaguchi commented

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

Author

Owner

@morluto commented on GitHub (Oct 31, 2026):

Real-provider confirmation of the ambiguity fix

I exercised this fixed workflow through REA stdio MCP at b43c329df779708e43ae297f498a844e45d9c923, using Ghidra 12.1.4 on a locally compiled arm64 Mach-O containing two C++ overloads, audit_over(int) and audit_over(long), and a caller invoking both. The target was not executed, and its SHA-256 was unchanged.

search_procedures({pattern:"audit_over"}) returned two audit_over entries, at 0x1000003f8 and 0x100000408. find_xrefs_to_name({name:"audit_over"}) returned invalid_request; its structured details.issues named both addresses and asked the caller to select one. Explicit xrefs at those addresses returned the respective call sites 0x100000428 and 0x100000434 (Ghidra also reported other references). This complements the original simulated-inventory check with one real engine and fixture; it does not establish coverage across all providers.

As a separate control, batch_decompile rejected the ambiguous name per item and successfully returned both distinct bodies by address. An exact, unique _audit_caller name also succeeded. Kuna 1.986 likewise handled this batch correctly, but its find_xrefs_to_name still reports the existing missing-name-inventory capability gap in #8183. Its explicit-address xrefs returned the two respective call sites; Kuna does not promise exhaustive xrefs.

@morluto commented on GitHub (Oct 31, 2026): ### Real-provider confirmation of the ambiguity fix I exercised this fixed workflow through REA stdio MCP at `b43c329df779708e43ae297f498a844e45d9c923`, using Ghidra 12.1.4 on a locally compiled arm64 Mach-O containing two C++ overloads, `audit_over(int)` and `audit_over(long)`, and a caller invoking both. The target was not executed, and its SHA-256 was unchanged. `search_procedures({pattern:"audit_over"})` returned two `audit_over` entries, at `0x1000003f8` and `0x100000408`. `find_xrefs_to_name({name:"audit_over"})` returned `invalid_request`; its structured `details.issues` named both addresses and asked the caller to select one. Explicit `xrefs` at those addresses returned the respective call sites `0x100000428` and `0x100000434` (Ghidra also reported other references). This complements the original simulated-inventory check with one real engine and fixture; it does not establish coverage across all providers. As a separate control, `batch_decompile` rejected the ambiguous name per item and successfully returned both distinct bodies by address. An exact, unique `_audit_caller` name also succeeded. Kuna 1.986 likewise handled this batch correctly, but its `find_xrefs_to_name` still reports the existing missing-name-inventory capability gap in #8183. Its explicit-address xrefs returned the two respective call sites; Kuna does not promise exhaustive xrefs.

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

No description provided.

Rows
Columns