From 4d2aff2ec6eadb0e8dd475748a410ca0f3a37687 Mon Sep 17 00:00:00 2001 From: Akshad Jogi <107343579+akshad-exe@users.noreply.github.com> Date: Wed, 4 Nov 2026 17:19:40 +0530 Subject: [PATCH] fix(mcp): report session tool target requirements accurately (#7340) * fix(mcp): report session tool target requirements accurately - Declare requiresActiveTarget on inspect_address_context in sessionToolContracts.ts so closed-session discovery stops advertising an address query that fails with target_unavailable. - Route session-kind tools through targetAvailability in CapabilityInventory.ts before the available fallback, and report target_unsupported with mode-level remediation for get_navigation_context and inspect_address_context on artifact and archive targets. - Keep each navigation mode's missing_operations and remediation aligned with the target restriction so recommendations name the target instead of a provider. * test(mcp): cover closed-session and artifact target requirements - Add CapabilityInventory controls for closed-session address and explicit-path APK availability, artifact and archive restrictions with per-mode navigation flags, and structured reasons instead of pinned remediation sentences. - Add a production stdio MCP regression in sessionTargetRequirements.test.ts that observes both binary_session entries before and after the calls, the target_unavailable address error, the successful unsigned-ZIP APK call, and the sibling navigation prerequisite. --- src/application/CapabilityInventory.test.ts | 73 +++++++++ src/application/CapabilityInventory.ts | 38 +++-- src/contracts/sessionToolContracts.ts | 13 +- .../mcp/sessionTargetRequirements.test.ts | 150 ++++++++++++++++++ 4 files changed, 260 insertions(+), 14 deletions(-) create mode 100644 tests/boundary/mcp/sessionTargetRequirements.test.ts diff --git a/src/application/CapabilityInventory.test.ts b/src/application/CapabilityInventory.test.ts index 8425bd1ab9..074dd92d23 100644 --- a/src/application/CapabilityInventory.test.ts +++ b/src/application/CapabilityInventory.test.ts @@ -196,6 +196,54 @@ it("reports the active resource session's dispatch policy without limiting indep ); }); +it("derives closed-session address and explicit-path APK availability from their contracts", () => { + expect(entry("inspect_address_context", status())).toMatchObject({ + available: false, + reason: "target_required", + remediation: expect.any(String), + }); + expect(entry("inspect_apk_signature", status())).toMatchObject({ + available: true, + reason: "available", + remediation: null, + }); + expect( + entry( + "inspect_address_context", + status({ + open: true, + kind: "executable", + capabilities: [ + { + operation: "inspect_address_context", + available: true, + reason: null, + }, + ], + }), + ), + ).toMatchObject({ available: true, reason: "available" }); + expect(entry("inspect_apk_signature", status({ open: true }))).toMatchObject({ + available: true, + reason: "available", + }); + for (const kind of ["artifact", "archive"] as const) { + expect( + entry("inspect_address_context", status({ open: true, kind })), + ).toMatchObject({ + available: false, + reason: "target_unsupported", + remediation: expect.stringContaining("explicit document"), + }); + expect( + entry("inspect_apk_signature", status({ open: true, kind })), + ).toMatchObject({ + available: true, + reason: "available", + }); + } +}); + describe("ADB and Apktool host availability", () => { it("uses only their selected owner probes", () => { const ready = { @@ -513,6 +561,31 @@ describe("compact tool availability", () => { }); }); +it("reports target restrictions for navigation modes even when providers are declared", () => { + const capabilities = [ + "current_document", + "current_address", + "resolve_containing_procedure", + ].map((operation) => ({ operation, available: true, reason: null })); + for (const kind of ["artifact", "archive"] as const) { + const navigation = entry( + "get_navigation_context", + status({ open: true, kind, capabilities }), + ); + expect(navigation).toMatchObject({ + available: false, + reason: "target_unsupported", + default_mode_available: false, + modes: [ + { name: "current_selection", available: false }, + { name: "explicit_document", available: false }, + ], + }); + for (const mode of navigation.modes ?? []) + expect(mode.remediation).toBe(navigation.remediation); + } +}); + describe("partial navigation availability", () => { it("requires a target before diagnosing navigation provider modes", () => { expect(entry("get_navigation_context", status())).toMatchObject({ diff --git a/src/application/CapabilityInventory.ts b/src/application/CapabilityInventory.ts index b1316b5d6f..2d7d023645 100644 --- a/src/application/CapabilityInventory.ts +++ b/src/application/CapabilityInventory.ts @@ -416,12 +416,25 @@ const availabilityFor = (context: AvailabilityContext): Availability => { ) return { reason: "available", remediation: null }; if (context.name === "get_navigation_context") { + const target = targetAvailability(context); + if (target !== null) { + return { + ...target, + defaultModeAvailable: false, + modes: NAVIGATION_CONTEXT_MODES.map((mode) => ({ + name: mode.name, + available: false, + missing_operations: [...mode.requirements], + remediation: target.remediation, + })), + }; + } const modes = availabilityForModes( NAVIGATION_CONTEXT_MODES, "current_selection", context.descriptors, ); - return { ...modes, ...targetAvailability(context) }; + return modes; } const javascriptApplication = javascriptApplicationAvailability(context); if (javascriptApplication !== null) return javascriptApplication; @@ -467,11 +480,10 @@ const javascriptApplicationAvailability = ({ : null; }; -const workflowAvailabilityFor = ({ - name, - kind, - policy, -}: AvailabilityContext): Availability | null => { +const workflowAvailabilityFor = ( + context: AvailabilityContext, +): Availability | null => { + const { name, kind, policy } = context; if (name === "start_gdb_session" || name === "start_rizin_debug_session") return hostRestrictedAvailability( policy, @@ -681,7 +693,10 @@ const workflowAvailabilityFor = ({ remediation: "Start REA with its JavaScript runtime provider available.", }; - if (kind === "session") return { reason: "available", remediation: null }; + if (kind === "session") + return ( + targetAvailability(context) ?? { reason: "available", remediation: null } + ); return null; }; @@ -822,12 +837,17 @@ const targetAvailability = ({ targetKind !== undefined && targetKind !== "executable" && targetKind !== "database" && - (kind === "official-proxy" || kind === "enhanced") + (kind === "official-proxy" || + kind === "enhanced" || + name === "get_navigation_context" || + name === "inspect_address_context") ) return { reason: "target_unsupported", remediation: - "Inventory or extract a native executable, then call open_binary on that executable.", + name === "inspect_address_context" + ? "Inventory or extract a native executable, then call open_binary on that executable. An explicit document can still be queried, but its facets may be unavailable." + : "Inventory or extract a native executable, then call open_binary on that executable.", }; if ( targetOpen && diff --git a/src/contracts/sessionToolContracts.ts b/src/contracts/sessionToolContracts.ts index 44d5655bf6..d74c15b768 100644 --- a/src/contracts/sessionToolContracts.ts +++ b/src/contracts/sessionToolContracts.ts @@ -184,9 +184,12 @@ export const SESSION_TOOL_CONTRACTS = [ ), requiresActiveTarget: true, }, - session( - "inspect_address_context", - "Inspect one explicit reproducible address for its analyzed name, containing procedure, regular and inline comments, and matching bookmarks. Each unsupported facet returns a typed unavailable outcome. A facet rejected for invalid input retains its canonical error and issues alongside any successful facets. Use xrefs, assembly, or pseudocode for those deeper views.", - addressContextInputSchema, - ), + { + ...session( + "inspect_address_context", + "Inspect one explicit reproducible address for its analyzed name, containing procedure, regular and inline comments, and matching bookmarks. Each unsupported facet returns a typed unavailable outcome. A facet rejected for invalid input retains its canonical error and issues alongside any successful facets. Use xrefs, assembly, or pseudocode for those deeper views.", + addressContextInputSchema, + ), + requiresActiveTarget: true, + }, ] as const satisfies readonly ToolContract[]; diff --git a/tests/boundary/mcp/sessionTargetRequirements.test.ts b/tests/boundary/mcp/sessionTargetRequirements.test.ts new file mode 100644 index 0000000000..5dc9879b28 --- /dev/null +++ b/tests/boundary/mcp/sessionTargetRequirements.test.ts @@ -0,0 +1,150 @@ +import { writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { expect, it } from "vitest"; + +import { buildCapabilityInventory } from "../../../src/application/CapabilityInventory.js"; +import { toolContract } from "../../../src/contracts/toolContracts.js"; +import { expandToolAvailability } from "../../../src/contracts/toolOutputSchemaPrimitives.js"; +import { connectLocalToolsMcp } from "../../fixtures/localToolsMcp.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; +import { zipWith } from "../../fixtures/apkSignature/v1SignatureBlock.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; + +it("advertises closed-session address and explicit-path APK tools according to their actual prerequisites", async () => { + const root = await createTestTempDirectory( + "rea-session-target-requirements-", + ); + const path = join(root, "unsigned.zip"); + await writeFile(path, await zipWith([{ name: "sample.txt", data: "hello" }])); + const { call } = await connectLocalToolsMcp(); + const status = async () => { + const reply = await call("binary_session", {}); + expect(reply.isError).not.toBe(true); + const result = toolContract("binary_session").outputSchema.parse( + reply.structuredContent, + ).result; + expect(result).toMatchObject({ open: false, cleanup_pending: false }); + return expandToolAvailability(result.tool_availability); + }; + const availability = ( + tools: Awaited>, + name: string, + ) => { + const entry = tools.find((tool) => tool.name === name); + expect(entry, `missing ${name} in binary_session`).toBeDefined(); + return entry; + }; + + const before = await status(); + expect(availability(before, "get_navigation_context")).toMatchObject({ + available: false, + reason: "target_required", + }); + expect.soft(availability(before, "inspect_address_context")).toMatchObject({ + available: false, + reason: "target_required", + }); + const owningAvailability = buildCapabilityInventory( + { open: false, capabilities: [] }, + { processCaptureEnabled: false }, + ); + expect(availability(before, "inspect_address_context")?.remediation).toBe( + owningAvailability.find((tool) => tool.name === "inspect_address_context") + ?.remediation, + ); + expect.soft(availability(before, "inspect_apk_signature")).toMatchObject({ + available: true, + reason: "available", + remediation: null, + }); + + const address = await call("inspect_address_context", { address: "0x1000" }); + expect(address.isError).toBe(true); + expect(parseMcpToolError(address)).toMatchObject({ + error: { code: "target_unavailable" }, + }); + const apk = await call("inspect_apk_signature", { path }); + expect(apk.isError, JSON.stringify(apk)).not.toBe(true); + expect(apk.structuredContent).toMatchObject({ + operation: "inspect_analysis_view", + }); + + const after = await status(); + expect(availability(after, "inspect_address_context")).toMatchObject({ + available: false, + reason: "target_required", + }); + expect(availability(after, "inspect_apk_signature")).toMatchObject({ + available: true, + reason: "available", + }); +}); + +it("reports default address resolution unavailable for an artifact while retaining explicit-document results", async () => { + const root = await createTestTempDirectory("rea-address-artifact-status-"); + const path = join(root, "sample.plist"); + await writeFile( + path, + 'CFBundleNameExample', + ); + const { call } = await connectLocalToolsMcp(); + const opened = await call("open_binary", { path }); + expect(opened.isError, JSON.stringify(opened)).not.toBe(true); + for (const detail of ["summary", "complete"] as const) { + const reply = await call("binary_session", { detail }); + expect(reply.isError).not.toBe(true); + const result = toolContract("binary_session").outputSchema.parse( + reply.structuredContent, + ).result; + const entry = expandToolAvailability(result.tool_availability).find( + (tool) => tool.name === "inspect_address_context", + ); + expect(entry).toMatchObject({ + available: false, + reason: "target_unsupported", + remediation: expect.stringContaining("explicit document"), + }); + const navigation = expandToolAvailability(result.tool_availability).find( + (tool) => tool.name === "get_navigation_context", + ); + expect(navigation).toMatchObject({ + available: false, + reason: "target_unsupported", + default_mode_available: false, + modes: [ + { name: "current_selection", available: false }, + { name: "explicit_document", available: false }, + ], + }); + } + for (const input of [{}, { document: "sample.plist" }]) { + const navigation = await call("get_navigation_context", input); + expect(parseMcpToolError(navigation)).toMatchObject({ + error: { + code: "capability_unavailable", + details: { selection_reason: "target_unsupported" }, + }, + }); + } + const implicit = await call("inspect_address_context", { address: "0x1000" }); + expect(parseMcpToolError(implicit)).toMatchObject({ + error: { + code: "capability_unavailable", + details: { selection_reason: "target_unsupported" }, + }, + }); + const explicit = await call("inspect_address_context", { + address: "0x1000", + document: "sample.plist", + }); + expect(explicit.isError, JSON.stringify(explicit)).not.toBe(true); + expect(explicit.structuredContent).toMatchObject({ + result: { + name: { state: "unavailable" }, + procedure: { state: "unavailable" }, + comment: { state: "unavailable" }, + inline_comment: { state: "unavailable" }, + bookmarks: { state: "unavailable" }, + }, + }); +});