From bae5d74eb034668bfbd81a9af8fb400ad7e4942c Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sun, 15 Nov 2026 19:51:33 +0800 Subject: [PATCH] refactor(ida): require the host platform for IDA MCP connections and snapshot the child environment with it (#11073) --- src/ida/IdaMcpConnection.test.ts | 9 ++++++++ src/ida/IdaMcpConnection.ts | 16 +++++++------- src/ida/IdaMcpEnvironment.test.ts | 4 +++- src/ida/IdaMcpWindowsLaunch.test.ts | 21 +++++++++++++++++++ src/ida/IdaProvider.ts | 5 ++++- src/ida/IdaSessionClient.test.ts | 3 ++- tests/boundary/idaStartupDiagnostics.test.ts | 5 +++++ tests/boundary/mcp/idaTransport.test.ts | 6 ++++-- .../ida/retainedConnectionRetry.test.ts | 7 ++++++- tests/boundary/providers/mcpHttpUtf8.test.ts | 2 ++ tests/support/idaConnectionDependencies.ts | 13 ++++++++++++ 11 files changed, 77 insertions(+), 14 deletions(-) create mode 100644 tests/support/idaConnectionDependencies.ts diff --git a/src/ida/IdaMcpConnection.test.ts b/src/ida/IdaMcpConnection.test.ts index a9b2e06bc5..455c105b09 100644 --- a/src/ida/IdaMcpConnection.test.ts +++ b/src/ida/IdaMcpConnection.test.ts @@ -16,6 +16,7 @@ import { AnalysisTimeoutError, } from "../domain/analysisErrorCore.js"; import { readSettledHealth } from "./IdaBusyHealth.js"; +import { idaConnectionDependencies } from "../../tests/support/idaConnectionDependencies.js"; describe("upstream IDA tool result decoding", () => { it("leaves successful legacy, structured, and native evidence unchanged", () => { @@ -187,6 +188,7 @@ const failingHttpConnection = ( timeoutMs: 1000, }, {}, + idaConnectionDependencies(), ); }; @@ -289,6 +291,7 @@ it("preserves the SDK request timeout as a typed IDA timeout", async () => { timeoutMs, }, {}, + idaConnectionDependencies(), ); try { await expect(connection.connect()).resolves.toEqual([]); @@ -356,6 +359,7 @@ it("bounds a stalled health poll by the remaining readiness deadline", async () timeoutMs, }, {}, + idaConnectionDependencies(), ); try { await connection.connect(); @@ -510,6 +514,7 @@ const withIdaResource = async ( timeoutMs: 1000, }, {}, + idaConnectionDependencies(), ); try { await connection.connect(); @@ -677,6 +682,7 @@ it("uses the actual SDK abort outcome to preserve the caller reason", async () = timeoutMs: 1000, }, {}, + idaConnectionDependencies(), ); const controller = new AbortController(); const reason = { caller: "exact opaque reason" }; @@ -734,6 +740,7 @@ it("keeps a real SDK physical failure when abort arrives during its outer unwind timeoutMs: 1000, }, {}, + idaConnectionDependencies(), ); try { await connection.connect(); @@ -879,6 +886,7 @@ const cleanupConnection = () => timeoutMs: 1000, }, {}, + idaConnectionDependencies(), ); it("refuses an oversized HTTP error promptly and retains its exact pending cancellation for retry", async () => { @@ -1126,6 +1134,7 @@ it.each(["completed", "pending", "rejected"] as const)( timeoutMs: 30, }, {}, + idaConnectionDependencies(), ); try { const failure = await connection diff --git a/src/ida/IdaMcpConnection.ts b/src/ida/IdaMcpConnection.ts index 051c96df66..b775f770e3 100644 --- a/src/ida/IdaMcpConnection.ts +++ b/src/ida/IdaMcpConnection.ts @@ -2,7 +2,6 @@ import { ArtifactResourceScope, type ArtifactResourceOwner, } from "../artifacts/ArtifactResourceScope.js"; -import { HOST_PLATFORM } from "../platform/HostPlatform.js"; import { ResponseBodyCleanup } from "../providers/ResponseBodyCleanup.js"; import { bindArtifactFileCleanup } from "../artifacts/ArtifactFileOperation.js"; import { ProviderCleanupError } from "../domain/providerCleanupError.js"; @@ -97,10 +96,11 @@ export interface IdaMcpConnection { export type IdaMcpConnectionDependencies = Readonly<{ createDiagnosticRedactor?: typeof createIdaDiagnosticRedactor; /** - * Host platform that selects the Windows launch resolution. Composition - * passes the running host; omitted, stdio registrations launch directly. + * Host platform that selects the Windows launch resolution and the + * environment-name semantics of the child's environment snapshot. + * Composition passes the running host. */ - platform?: NodeJS.Platform; + platform: NodeJS.Platform; /** Owned process acquisition for a stdio registration. */ launcher?: OwnedMcpLauncher; /** Largest stdio JSON-RPC message accepted from the upstream server. */ @@ -133,7 +133,7 @@ const IDA_STDERR_TAIL_BYTES = 16_384; */ const idaStdioLauncher = ( - platform: NodeJS.Platform | undefined, + platform: NodeJS.Platform, launch: OwnedMcpLauncher, timeoutMs: number, ): OwnedMcpLauncher => @@ -511,7 +511,7 @@ const retainedTransportCause = (cause: unknown, message: string): Error => { export const createIdaMcpConnection = ( config: IdaConfiguration, environment: Readonly, - dependencies: IdaMcpConnectionDependencies = {}, + dependencies: IdaMcpConnectionDependencies, ): IdaMcpConnection => { const client = new StructuredContentMcpClient({ name: "rea-ida-provider", @@ -675,11 +675,11 @@ export const createIdaMcpConnection = ( // Only the selected snapshot reaches the child, never REA's ambient environment. hostEnvironment: snapshotEnvironment( environment, - HOST_PLATFORM.platform, + dependencies.platform, ), env: { IDA_MCP_MAX_WORKERS: "1", - ...snapshotEnvironment(config.env, HOST_PLATFORM.platform), + ...snapshotEnvironment(config.env, dependencies.platform), }, // Registrations commonly launch interpreter-driven scripts. expectedCommand: null, diff --git a/src/ida/IdaMcpEnvironment.test.ts b/src/ida/IdaMcpEnvironment.test.ts index 630220c980..2e6b42f061 100644 --- a/src/ida/IdaMcpEnvironment.test.ts +++ b/src/ida/IdaMcpEnvironment.test.ts @@ -3,6 +3,7 @@ import { join } from "node:path"; import { afterEach, expect, it, vi } from "vitest"; import { createIdaMcpConnection } from "./IdaMcpConnection.js"; import { createTestTempDirectory } from "../../tests/fixtures/temporaryDirectory.js"; +import { idaConnectionDependencies } from "../../tests/support/idaConnectionDependencies.js"; afterEach(() => vi.unstubAllEnvs()); @@ -58,6 +59,7 @@ input.on('line', (line) => { timeoutMs: 5000, }, selected, + idaConnectionDependencies(), ); environment.REA_SELECTED_CONTEXT = "mutated-after-construction"; try { @@ -105,7 +107,7 @@ input.on('line', (line) => { timeoutMs: 5000, }, {}, - { maxStdioMessageBytes: 4096 }, + idaConnectionDependencies({ maxStdioMessageBytes: 4096 }), ); try { expect(await connection.connect()).toEqual(["large"]); diff --git a/src/ida/IdaMcpWindowsLaunch.test.ts b/src/ida/IdaMcpWindowsLaunch.test.ts index 26c7f4adb3..f7f2086fb7 100644 --- a/src/ida/IdaMcpWindowsLaunch.test.ts +++ b/src/ida/IdaMcpWindowsLaunch.test.ts @@ -118,6 +118,27 @@ it("reports an uninspectable Windows command candidate with its path and code, n expect(String(failure)).not.toContain("was not found"); }); +it("snapshots the child's environment with the injected platform's name semantics", async () => { + files.present.add("C:\\Tools\\ida-mcp.EXE"); + const { launches } = await observeLaunch("ida-mcp", { + SystemRoot: "C:\\Windows", + Path: "C:\\Tools", + PathExt: ".EXE", + }); + // Windows names are case-insensitive: the snapshot folds them to one + // spelling on whatever host runs the connection. + expect(launches).toEqual([ + expect.objectContaining({ + command: "C:\\Tools\\ida-mcp.EXE", + hostEnvironment: expect.objectContaining({ + SYSTEMROOT: "C:\\Windows", + PATH: "C:\\Tools", + PATHEXT: ".EXE", + }), + }), + ]); +}); + it("closes while a Windows command lookup is stalled on an unreachable PATH directory", async () => { files.stalled.add("\\\\offline\\tools\\ida-mcp.EXE"); const launches: unknown[] = []; diff --git a/src/ida/IdaProvider.ts b/src/ida/IdaProvider.ts index 068306439a..7f1791ab28 100644 --- a/src/ida/IdaProvider.ts +++ b/src/ida/IdaProvider.ts @@ -90,7 +90,10 @@ export class IdaProvider implements AnalysisProviderCandidate { private readonly connectionFactory: ( config: IdaConfiguration, environment: Readonly, - ) => IdaMcpConnection = createIdaMcpConnection, + ) => IdaMcpConnection = (config, environment) => + createIdaMcpConnection(config, environment, { + platform: HOST_PLATFORM.platform, + }), ) { this.environment = snapshotEnvironment(environment, HOST_PLATFORM.platform); } diff --git a/src/ida/IdaSessionClient.test.ts b/src/ida/IdaSessionClient.test.ts index 86ef28d0c7..5a5bfeb77a 100644 --- a/src/ida/IdaSessionClient.test.ts +++ b/src/ida/IdaSessionClient.test.ts @@ -33,6 +33,7 @@ import { import { ProviderAdapterError } from "../domain/providerAdapterError.js"; import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import { WindowsPrivateRuntime } from "../windows/WindowsPrivateRuntime.js"; +import { idaConnectionDependencies } from "../../tests/support/idaConnectionDependencies.js"; const roots: string[] = []; const windowsRuntimes = new Set(); @@ -2331,7 +2332,7 @@ it("keeps the session cleanup owner until an ignored-abort HTTP acquisition and const client = new IdaSessionClient( config, target, - createIdaMcpConnection(config, {}), + createIdaMcpConnection(config, {}, idaConnectionDependencies()), ); const executing = client.execute("health", {}); try { diff --git a/tests/boundary/idaStartupDiagnostics.test.ts b/tests/boundary/idaStartupDiagnostics.test.ts index cd69873abd..fe01cebb24 100644 --- a/tests/boundary/idaStartupDiagnostics.test.ts +++ b/tests/boundary/idaStartupDiagnostics.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; import { createIdaMcpConnection } from "../../src/ida/IdaMcpConnection.js"; +import { idaConnectionDependencies } from "../support/idaConnectionDependencies.js"; const launcherEnvironment = (): Readonly> => { if (process.platform !== "win32") return {}; @@ -24,6 +25,7 @@ describe("IDA startup diagnostics", () => { timeoutMs: 1000, }, {}, + idaConnectionDependencies(), ); try { await expect(connection.connect()).rejects.toMatchObject({ @@ -52,6 +54,7 @@ describe("IDA startup diagnostics", () => { timeoutMs: 1000, }, {}, + idaConnectionDependencies(), ); try { await expect(connection.connect()).rejects.toMatchObject({ @@ -82,6 +85,7 @@ it("drops a leading partial UTF-8 character from the bounded stderr tail", async timeoutMs: 1000, }, {}, + idaConnectionDependencies(), ); try { await expect(connection.connect()).rejects.toMatchObject({ @@ -110,6 +114,7 @@ it("retains an in-budget undecodable leading byte without claiming tail truncati timeoutMs: 1000, }, {}, + idaConnectionDependencies(), ); try { const failure = await connection.connect().catch((cause: unknown) => cause); diff --git a/tests/boundary/mcp/idaTransport.test.ts b/tests/boundary/mcp/idaTransport.test.ts index 08c2830434..834ad80168 100644 --- a/tests/boundary/mcp/idaTransport.test.ts +++ b/tests/boundary/mcp/idaTransport.test.ts @@ -16,6 +16,7 @@ import type { IdaMcpConnection, IdaMcpConnectionDependencies, } from "../../../src/ida/IdaMcpConnection.js"; +import { idaConnectionDependencies } from "../../support/idaConnectionDependencies.js"; const cleanups: (() => Promise)[] = []; afterEach(async () => { @@ -48,7 +49,7 @@ type IdaTransportFixtureOptions = { readonly toolContentType?: string; readonly toolResponseBody?: string | Buffer; readonly configurationHeaders?: Readonly>; - readonly connectionDependencies?: IdaMcpConnectionDependencies; + readonly connectionDependencies?: Partial; }; const fixtureResponse = ( input: z.infer, @@ -188,7 +189,7 @@ const fixture = async (options: IdaTransportFixtureOptions = {}) => { timeoutMs: 2000, }, {}, - connectionDependencies, + idaConnectionDependencies(connectionDependencies), ); cleanups.push(() => connection.close()); return { @@ -304,6 +305,7 @@ process.stdin.on("data", (chunk) => { timeoutMs: 10_000, }, {}, + idaConnectionDependencies(), ); cleanups.push(() => connection.close()); expect(await connection.connect()).toEqual(["list_strings"]); diff --git a/tests/boundary/providers/ida/retainedConnectionRetry.test.ts b/tests/boundary/providers/ida/retainedConnectionRetry.test.ts index 034657701d..0ea8d84bab 100644 --- a/tests/boundary/providers/ida/retainedConnectionRetry.test.ts +++ b/tests/boundary/providers/ida/retainedConnectionRetry.test.ts @@ -10,6 +10,7 @@ import { elf } from "../../../../src/domain/binaryTarget.fixture.js"; import type { BinaryTarget } from "../../../../src/domain/binaryTargetTypes.js"; import { createIdaMcpConnection } from "../../../../src/ida/IdaMcpConnection.js"; import { IdaSessionClient } from "../../../../src/ida/IdaSessionClient.js"; +import { idaConnectionDependencies } from "../../../support/idaConnectionDependencies.js"; /** * A headless IDA supervisor over real stdio that answers the session @@ -96,7 +97,11 @@ describe("retained headless IDA connection in a long-lived host", () => { timeoutMs: 5000, workspaceRoot, }; - const connection = createIdaMcpConnection(config, process.env); + const connection = createIdaMcpConnection( + config, + process.env, + idaConnectionDependencies(), + ); // Close the supervisor even when an assertion fails before cleanup does. onTestFinished(() => connection.close().catch(() => undefined)); const client = new IdaSessionClient(config, target, connection); diff --git a/tests/boundary/providers/mcpHttpUtf8.test.ts b/tests/boundary/providers/mcpHttpUtf8.test.ts index dc60357c1f..4ef7fcaf98 100644 --- a/tests/boundary/providers/mcpHttpUtf8.test.ts +++ b/tests/boundary/providers/mcpHttpUtf8.test.ts @@ -6,6 +6,7 @@ import { createVerifierHttpServer } from "../../../scripts/lib/verifier-http-ser import { createIdaMcpConnection } from "../../../src/ida/IdaMcpConnection.js"; import { createStreamableHttpJebMcpConnection } from "../../../src/jeb/JebMcpConnection.js"; import { AnalysisCancelledError } from "../../../src/domain/analysisErrorCore.js"; +import { idaConnectionDependencies } from "../../support/idaConnectionDependencies.js"; const requestSchema = z.object({ id: z.union([z.string(), z.number()]).optional(), @@ -143,6 +144,7 @@ const createConnection = (provider: "ida" | "jeb", endpoint: string) => { timeoutMs: 1_000, }, {}, + idaConnectionDependencies(), ); return { connect: () => connection.connect(), diff --git a/tests/support/idaConnectionDependencies.ts b/tests/support/idaConnectionDependencies.ts new file mode 100644 index 0000000000..9bceafbd78 --- /dev/null +++ b/tests/support/idaConnectionDependencies.ts @@ -0,0 +1,13 @@ +import type { IdaMcpConnectionDependencies } from "../../src/ida/IdaMcpConnection.js"; +import { HOST_PLATFORM } from "../../src/platform/HostPlatform.js"; + +/** + * IDA connection dependencies for tests: the running host's platform, as + * composition passes it, plus any test overrides (including `platform`). + */ +export const idaConnectionDependencies = ( + overrides: Partial = {}, +): IdaMcpConnectionDependencies => ({ + platform: HOST_PLATFORM.platform, + ...overrides, +});