morluto/rea

Fork 0

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

[GH-ISSUE #5839] [Bug] Inventory fallbacks return MCP success while a capability error still reports incomplete cleanup #3537

New issue

Closed

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

bsakaguchi commented

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

Owner

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

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

Executed three SDK MCP calls through the repository's in-memory server harness with controlled provider responses. Each provider refusal was an AnalysisCapabilityUnavailableError with unresolved cleanup, but each tool returned successful Evidence (isError was not true):

binary_overview: after a clean refusal of the native summary, list_segments failed with incomplete cleanup. The result contained only the document and procedure/string counts. Its normalized result omitted the segment facet and cleanup diagnostics.
get_objc_classes: one class candidate triggered inspect_native_dispatch_metadata, which failed with incomplete cleanup. The result contained the class and unknown identity coverage, with no cleanup diagnostics in its normalized result.
analyze_swift_types: list_names failed with incomplete cleanup after a successful procedure read. The tool returned success. Unlike the other two paths, it retained symbol_inventory_error.code: "cleanup_incomplete", resource names and remediation inside the successful normalized result.

The source errors still had cleanupIncomplete: true after all three responses. A fourth control returned the same kind of error from current_document; binary_overview correctly returned an MCP error with code: "cleanup_incomplete" and did not fall back.

These are controlled provider-boundary reproductions. No native engine or real resource leak was exercised.

Reproduction

Run from the source revision below with its locked dependencies installed. The following is the essential body of the temporary test executed beside EnhancedTools.ts, using the existing MCP harness. Its filename is normalized to src/application/repro.test.ts here. No production source change or rebuild is needed.

import { afterEach, expect, it } from "vitest";
import { AnalysisCapabilityUnavailableError } from "../domain/analysisErrorCore.js";
import { projectAnalysisError } from "../domain/analysisErrorProjection.js";
import { err } from "../domain/result.js";
import { observed as ok } from "../../tests/fixtures/analysisExecution.js";
import {
  closeEnhancedToolResources,
  connect,
  jsonResult,
} from "../../tests/boundary/mcp/enhancedToolsHarness.js";

afterEach(closeEnhancedToolResources);

it.each([
  { tool: "binary_overview", operation: "list_segments" },
  { tool: "get_objc_classes", operation: "inspect_native_dispatch_metadata" },
  { tool: "analyze_swift_types", operation: "list_names" },
])("reproduces success despite retained cleanup: $tool", async ({ tool, operation }) => {
  const failure = new AnalysisCapabilityUnavailableError(
    "fixture", operation, "unsupported", {
      cleanup: {
        reason: "controlled unresolved owner",
        resources: ["fixture owner"],
      },
    },
  );
  const client = await connect({ execute: async (name) => {
    if (name === operation) return err(failure);
    if (name === "read_binary_inventory_summary")
      return err(new AnalysisCapabilityUnavailableError(
        "fixture", name, "no direct summary",
      ));
    if (name === "current_document") return ok("fixture-document");
    if (name === "list_names")
      return ok([{ address: "0x1", value: "_OBJC_CLASS_$_Fixture" }]);
    if (name === "list_procedures" || name === "list_strings") return ok([]);
    throw new Error(`Unexpected operation: ${name}`);
  }});
  const response = await client.callTool({ name: tool, arguments: {} });
  // These assertions establish the observed defective success classification.
  expect(response.isError).not.toBe(true);
  expect(failure.cleanupIncomplete).toBe(true);
  expect(projectAnalysisError(failure)).toMatchObject({
    code: "cleanup_incomplete",
    details: { resources: ["fixture owner"] },
  });
  console.log(JSON.stringify({
    tool,
    isError: response.isError === true,
    cleanupStillIncomplete: failure.cleanupIncomplete,
    normalized_result: jsonResult(response),
  }));
});

npm run test:local -- src/application/repro.test.ts

Each of the three cases reproduced once. The executed file also contained the current_document control described above; all four assertions of observed behavior passed. The temporary test was deleted afterward. Use a source-owned test location: this runner rejects the root tests location as unowned and refuses MCP boundary tests under test:local.

Expected behavior

Return a typed cleanup_incomplete failure while the provider's cleanup remains unresolved, preserving the failed operation and remaining resource kinds. A capability refusal should permit ordinary inventory fallback only when it does not carry unresolved cleanup.

This expectation follows the MCP cleanup contract, which requires cleanup failures to use cleanup_incomplete until the owning resources are confirmed released. The lifecycle design contract requires preserved primary diagnostics and exact unresolved ownership. The error projection already encodes these fields.

The native-summary fallback in the same class explicitly checks cleanupIncomplete. The executed current-document control confirms that ordinary MCP error delivery can preserve this outcome.

Impact and scope

P2: these calls misclassify an unresolved lifecycle failure as successful analysis. For the overview and Objective-C cases, the normalized result also loses the information needed to distinguish ordinary unsupported capability from incomplete cleanup. Swift retains that information as a nested error, but still exposes a successful tool result.

The trigger is bounded to capability errors accompanied by incomplete cleanup. Naturally occurring provider frequency, actual resource retention and CLI behavior were not tested. No data loss or security-boundary crossing was observed. No real-provider workaround was verified. These fixtures establish the shared MCP workflow's response semantics, not native engine behavior.

REA version

rea-agents 6.7.0; source revision a5f982de8d

Runtime and platform

Node.js v24.22.0; macOS Darwin arm64; Vitest 5.0.3; no Hopper or Ghidra process used

Target and artifact context

Controlled typed provider fixtures with one Objective-C class label and empty procedure/string inventories; no target binary or operating-system resource

Interface

MCP

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

The focused test command above completed with exit 0. The following lines are copied from the test output; runner paths and timing lines are omitted:

{"tool":"binary_overview","isError":false,"calls":["read_binary_inventory_summary","current_document","list_segments","list_procedures","list_strings"],"cleanupStillIncomplete":true,"normalized_result":{"document":"fixture-document","procedure_count":0,"string_count":0}}
{"tool":"get_objc_classes","isError":false,"calls":["list_names","inspect_native_dispatch_metadata"],"cleanupStillIncomplete":true,"normalized_result":{"count":1,"classes":[{"address":"0x1","name":"_OBJC_CLASS_$_Fixture"}],"identity_coverage":{"status":"unknown","reason":"Native class-import binding evidence is unavailable (AnalysisCapabilityUnavailableError)."}}}
{"tool":"analyze_swift_types","isError":false,"calls":["list_procedures","list_names"],"cleanupStillIncomplete":true,"normalized_result":{"total":0,"categories":{"classes":{"count":0,"items":[]},"structs":{"count":0,"items":[]},"enums":{"count":0,"items":[]},"protocols":{"count":0,"items":[]},"extensions":{"count":0,"items":[]},"other":{"count":0,"items":[]}},"unclassified":[],"limitations":["Identification uses Swift mangled procedure names or same-address symbol aliases. Demangled names without retained Swift manglings cannot be identified. Counts deduplicate procedure names, not source declarations.","The provider's symbol inventory is unavailable; demangled procedures could not be joined to their Swift aliases."],"symbol_inventory_error":{"code":"cleanup_incomplete","category":"unsupported_provider","message":"This analysis is unavailable for the current target. Choose another analysis or target.","retryable":false,"remediation":{"action":"This analysis is unavailable for the current target. Choose another analysis or target."},"details":{"provider_id":"fixture","operation":"list_names","reason":"unsupported","cleanup":"incomplete","resources":["fixture owner"],"cleanup_reason":"controlled unresolved owner","execution_failure":"capability_unavailable"}}}}
{"control":"current_document","isError":true,"code":"cleanup_incomplete"}

 Test Files  1 passed (1)
      Tests  4 passed (4)

The observed operation sequences were:

binary_overview: read_binary_inventory_summary, current_document,
                 list_segments, list_procedures, list_strings
get_objc_classes: list_names, inspect_native_dispatch_metadata
analyze_swift_types: list_procedures, list_names

The production MCP handler invokes the composed workflow. The three fallback decisions are at Objective-C binding projection, Swift symbol projection, and overview segment projection.

Typed outcome or recovery expected

The outer MCP result should report isError: true with error.code: "cleanup_incomplete", preserving details.operation, details.resources, details.cleanup_reason and the original projected recovery information. Preserve available observations without turning unresolved cleanup into ordinary successful fallback.

Analysis or suspected cause

Source-derived: these three branches decide whether to fall back by capability-error class or tag without inspecting cleanupIncomplete. Swift explicitly projects the error into symbol_inventory_error, explaining why that case retains diagnostics despite returning success.

This error combination is supported by the production algebra: Hopper normalization can preserve a remote capability error's cleanup dependency. The reproduction directly supplies the corresponding typed provider result; it does not establish a naturally occurring Hopper trigger.

Additional context

Read-only searches of open and closed issues and PRs found no report for these three fallback sites. The closed document-selection report concerns AnalysisDocument, whose cleanup-bearing fallback was the passing control here. The runtime used was Node 24.22.0; the source checkout's Node pin is 24.18.0, which was not exercised.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5839 ### 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 Executed three SDK MCP calls through the repository's in-memory server harness with controlled provider responses. Each provider refusal was an `AnalysisCapabilityUnavailableError` with unresolved cleanup, but each tool returned successful Evidence (`isError` was not true): - `binary_overview`: after a clean refusal of the native summary, `list_segments` failed with incomplete cleanup. The result contained only the document and procedure/string counts. Its normalized result omitted the segment facet and cleanup diagnostics. - `get_objc_classes`: one class candidate triggered `inspect_native_dispatch_metadata`, which failed with incomplete cleanup. The result contained the class and unknown identity coverage, with no cleanup diagnostics in its normalized result. - `analyze_swift_types`: `list_names` failed with incomplete cleanup after a successful procedure read. The tool returned success. Unlike the other two paths, it retained `symbol_inventory_error.code: "cleanup_incomplete"`, resource names and remediation inside the successful normalized result. The source errors still had `cleanupIncomplete: true` after all three responses. A fourth control returned the same kind of error from `current_document`; `binary_overview` correctly returned an MCP error with `code: "cleanup_incomplete"` and did not fall back. These are controlled provider-boundary reproductions. No native engine or real resource leak was exercised. ### Reproduction Run from the source revision below with its locked dependencies installed. The following is the essential body of the temporary test executed beside `EnhancedTools.ts`, using the existing MCP harness. Its filename is normalized to `src/application/repro.test.ts` here. No production source change or rebuild is needed. ```ts import { afterEach, expect, it } from "vitest"; import { AnalysisCapabilityUnavailableError } from "../domain/analysisErrorCore.js"; import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import { err } from "../domain/result.js"; import { observed as ok } from "../../tests/fixtures/analysisExecution.js"; import { closeEnhancedToolResources, connect, jsonResult, } from "../../tests/boundary/mcp/enhancedToolsHarness.js"; afterEach(closeEnhancedToolResources); it.each([ { tool: "binary_overview", operation: "list_segments" }, { tool: "get_objc_classes", operation: "inspect_native_dispatch_metadata" }, { tool: "analyze_swift_types", operation: "list_names" }, ])("reproduces success despite retained cleanup: $tool", async ({ tool, operation }) => { const failure = new AnalysisCapabilityUnavailableError( "fixture", operation, "unsupported", { cleanup: { reason: "controlled unresolved owner", resources: ["fixture owner"], }, }, ); const client = await connect({ execute: async (name) => { if (name === operation) return err(failure); if (name === "read_binary_inventory_summary") return err(new AnalysisCapabilityUnavailableError( "fixture", name, "no direct summary", )); if (name === "current_document") return ok("fixture-document"); if (name === "list_names") return ok([{ address: "0x1", value: "_OBJC_CLASS_$_Fixture" }]); if (name === "list_procedures" || name === "list_strings") return ok([]); throw new Error(`Unexpected operation: ${name}`); }}); const response = await client.callTool({ name: tool, arguments: {} }); // These assertions establish the observed defective success classification. expect(response.isError).not.toBe(true); expect(failure.cleanupIncomplete).toBe(true); expect(projectAnalysisError(failure)).toMatchObject({ code: "cleanup_incomplete", details: { resources: ["fixture owner"] }, }); console.log(JSON.stringify({ tool, isError: response.isError === true, cleanupStillIncomplete: failure.cleanupIncomplete, normalized_result: jsonResult(response), })); }); ``` ```sh npm run test:local -- src/application/repro.test.ts ``` Each of the three cases reproduced once. The executed file also contained the `current_document` control described above; all four assertions of observed behavior passed. The temporary test was deleted afterward. Use a source-owned test location: this runner rejects the root `tests` location as unowned and refuses MCP boundary tests under `test:local`. ### Expected behavior Return a typed `cleanup_incomplete` failure while the provider's cleanup remains unresolved, preserving the failed operation and remaining resource kinds. A capability refusal should permit ordinary inventory fallback only when it does not carry unresolved cleanup. This expectation follows the [MCP cleanup contract](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/docs/mcp-contracts.md#L670-L676), which requires cleanup failures to use `cleanup_incomplete` until the owning resources are confirmed released. The [lifecycle design contract](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/docs/tool-design.md#L123-L147) requires preserved primary diagnostics and exact unresolved ownership. The [error projection](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/domain/analysisErrorProjection.ts#L69-L88) already encodes these fields. The native-summary fallback in the same class [explicitly checks `cleanupIncomplete`](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/application/EnhancedTools.ts#L756-L767). The executed current-document control confirms that ordinary MCP error delivery can preserve this outcome. ### Impact and scope P2: these calls misclassify an unresolved lifecycle failure as successful analysis. For the overview and Objective-C cases, the normalized result also loses the information needed to distinguish ordinary unsupported capability from incomplete cleanup. Swift retains that information as a nested error, but still exposes a successful tool result. The trigger is bounded to capability errors accompanied by incomplete cleanup. Naturally occurring provider frequency, actual resource retention and CLI behavior were not tested. No data loss or security-boundary crossing was observed. No real-provider workaround was verified. These fixtures establish the shared MCP workflow's response semantics, not native engine behavior. ### REA version rea-agents 6.7.0; source revision a5f982de8dbe4b092cc20ab89d492b7e7028b758 ### Runtime and platform Node.js v24.22.0; macOS Darwin arm64; Vitest 5.0.3; no Hopper or Ghidra process used ### Target and artifact context Controlled typed provider fixtures with one Objective-C class label and empty procedure/string inventories; no target binary or operating-system resource ### Interface MCP ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence The focused test command above completed with exit 0. The following lines are copied from the test output; runner paths and timing lines are omitted: ```text {"tool":"binary_overview","isError":false,"calls":["read_binary_inventory_summary","current_document","list_segments","list_procedures","list_strings"],"cleanupStillIncomplete":true,"normalized_result":{"document":"fixture-document","procedure_count":0,"string_count":0}} {"tool":"get_objc_classes","isError":false,"calls":["list_names","inspect_native_dispatch_metadata"],"cleanupStillIncomplete":true,"normalized_result":{"count":1,"classes":[{"address":"0x1","name":"_OBJC_CLASS_$_Fixture"}],"identity_coverage":{"status":"unknown","reason":"Native class-import binding evidence is unavailable (AnalysisCapabilityUnavailableError)."}}} {"tool":"analyze_swift_types","isError":false,"calls":["list_procedures","list_names"],"cleanupStillIncomplete":true,"normalized_result":{"total":0,"categories":{"classes":{"count":0,"items":[]},"structs":{"count":0,"items":[]},"enums":{"count":0,"items":[]},"protocols":{"count":0,"items":[]},"extensions":{"count":0,"items":[]},"other":{"count":0,"items":[]}},"unclassified":[],"limitations":["Identification uses Swift mangled procedure names or same-address symbol aliases. Demangled names without retained Swift manglings cannot be identified. Counts deduplicate procedure names, not source declarations.","The provider's symbol inventory is unavailable; demangled procedures could not be joined to their Swift aliases."],"symbol_inventory_error":{"code":"cleanup_incomplete","category":"unsupported_provider","message":"This analysis is unavailable for the current target. Choose another analysis or target.","retryable":false,"remediation":{"action":"This analysis is unavailable for the current target. Choose another analysis or target."},"details":{"provider_id":"fixture","operation":"list_names","reason":"unsupported","cleanup":"incomplete","resources":["fixture owner"],"cleanup_reason":"controlled unresolved owner","execution_failure":"capability_unavailable"}}}} {"control":"current_document","isError":true,"code":"cleanup_incomplete"} Test Files 1 passed (1) Tests 4 passed (4) ``` The observed operation sequences were: ```text binary_overview: read_binary_inventory_summary, current_document, list_segments, list_procedures, list_strings get_objc_classes: list_names, inspect_native_dispatch_metadata analyze_swift_types: list_procedures, list_names ``` The production MCP handler [invokes the composed workflow](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/server/registerEnhancedTools.ts#L217-L267). The three fallback decisions are at [Objective-C binding projection](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/application/EnhancedTools.ts#L329-L342), [Swift symbol projection](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/application/EnhancedTools.ts#L698-L722), and [overview segment projection](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/application/EnhancedTools.ts#L774-L805). ### Typed outcome or recovery expected The outer MCP result should report `isError: true` with `error.code: "cleanup_incomplete"`, preserving `details.operation`, `details.resources`, `details.cleanup_reason` and the original projected recovery information. Preserve available observations without turning unresolved cleanup into ordinary successful fallback. ### Analysis or suspected cause Source-derived: these three branches decide whether to fall back by capability-error class or tag without inspecting `cleanupIncomplete`. Swift explicitly projects the error into `symbol_inventory_error`, explaining why that case retains diagnostics despite returning success. This error combination is supported by the production algebra: [Hopper normalization](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/hopper/HopperClient.ts#L609-L630) can preserve a remote capability error's cleanup dependency. The reproduction directly supplies the corresponding typed provider result; it does not establish a naturally occurring Hopper trigger. ### Additional context Read-only searches of open and closed issues and PRs found no report for these three fallback sites. [The closed document-selection report](https://github.com/morluto/rea/issues/4934) concerns `AnalysisDocument`, whose cleanup-bearing fallback was the passing control here. The runtime used was Node 24.22.0; the source checkout's Node pin is 24.18.0, which was not exercised.

bsakaguchi closed this issue

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

bsakaguchi commented

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

Author

Owner

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

Confirmed — the fallback gating is the right place to fix this: inventory fallback should only be permitted when the capability error carries no unresolved cleanup, otherwise the caller gets a false "success" while resources are still retained (and in the overview/ObjC paths even the cleanup diagnostic disappears). The native-summary fallback already checks cleanupIncomplete, so extending that same check to the other inventory fallbacks keeps behavior consistent with the documented MCP cleanup contract.

@XinwangTao commented on GitHub (Oct 21, 2026): Confirmed — the fallback gating is the right place to fix this: inventory fallback should only be permitted when the capability error carries no unresolved cleanup, otherwise the caller gets a false "success" while resources are still retained (and in the overview/ObjC paths even the cleanup diagnostic disappears). The native-summary fallback already checks cleanupIncomplete, so extending that same check to the other inventory fallbacks keeps behavior consistent with the documented MCP cleanup contract.

bsakaguchi commented

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

Author

Owner

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

Resolved on main in 27e927f70. One capability-fallback predicate excludes unresolved cleanup. Six executed SDK MCP cases cover all three reported inventory tools with cleanup_incomplete versus ordinary clean fallback controls. Validation: tests/boundary/mcp/enhancedTools.inventoryCleanup.test.ts — binary_overview/get_objc_classes/analyze_swift_types preserves unresolved cleanup and permits a clean capability fallback; recorded execution at 9cd46d619. Actual in-memory SDK MCP/shared tool code with controlled provider errors; no real provider execution. This matches the issue reproduction boundary.

@morluto commented on GitHub (Oct 23, 2026): Resolved on main in [27e927f70](https://github.com/morluto/rea/commit/27e927f70fd710ce0097d4549cd94d7d692d1ad8). One capability-fallback predicate excludes unresolved cleanup. Six executed SDK MCP cases cover all three reported inventory tools with cleanup_incomplete versus ordinary clean fallback controls. Validation: `tests/boundary/mcp/enhancedTools.inventoryCleanup.test.ts` — binary_overview/get_objc_classes/analyze_swift_types preserves unresolved cleanup and permits a clean capability fallback; recorded execution at `9cd46d619`. Actual in-memory SDK MCP/shared tool code with controlled provider errors; no real provider execution. This matches the issue reproduction boundary.

bsakaguchi referenced this issue

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

[GH-ISSUE #3582] [Tracker] Ready to close or merge: issues and pull requests (October 2026) #1330

bsakaguchi referenced this issue

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

[GH-ISSUE #3795] [Tracker] Open documentation issues and pull requests (October 2026) #1468

bsakaguchi referenced this issue

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

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

bsakaguchi referenced this issue

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

[GH-ISSUE #5653] [Tracker] New issue intake by area (October 2026) #3423