shifathosn
/
rea
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(ida): retain bounded stderr on upstream transport failures (#2449) 

* fix(ida): retain bounded stderr on upstream transport failures

* fix(ida): preserve stderr through operation and cleanup boundaries

Address review findings at their producing boundaries and preserve directly affected consumers. Source inspection and git diff --check completed. Tests and automated checks were not run at the user’s explicit request.

* fix(ida): preserve cancellation and single-owner stderr diagnostics

---------

Co-authored-by: morluto <76467478+morluto@users.noreply.github.com>
This commit is contained in:
GuoJun Tang 2026-10-13 22:14:39 +08:00 committed by GitHub
parent 5278835b53
commit 30e9b83a68
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 291 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
src/ida/IdaMcpConnection.ts
View File

@ -13,6 +13,7 @@ import { z } from "zod";
import type { IdaConfiguration } from "./IdaConfiguration.js";
import { jsonValueSchema, type JsonValue } from "../domain/jsonValue.js";
import { AnalysisProtocolError } from "../domain/analysisErrorCore.js";
import { ProviderAdapterError } from "../domain/providerAdapterError.js";
/** Injectable MCP boundary used by the provider and transport conformance tests. */
export interface IdaMcpConnection {
@ -142,8 +143,30 @@ export const createIdaMcpConnection = (
environment: Readonly<NodeJS.ProcessEnv>,
): IdaMcpConnection => {
const client = new Client({ name: "rea-ida-provider", version: "1" });
let stderr = Buffer.alloc(0);
let stderrBytes = 0;
const transportFailure = (cause: unknown): never => {
throw redactIdaTransportFailure(cause, config);
const failure = redactIdaTransportFailure(cause, config);
if (stderrBytes === 0) throw failure;
let start = 0;
// The byte tail may start inside a UTF-8 code point. Retain whole units.
if (stderrBytes > stderr.length)
while (start < stderr.length && (stderr[start]! & 0xc0) === 0x80) start++;
const diagnostic = redactIdaTransportFailure(
new Error(stderr.subarray(start).toString("utf8")),
config,
).message;
throw new ProviderAdapterError("ida", "transport", {
cause: failure,
diagnostics: { reason: failure.message },
capturedOutput: {
stdout: "",
stderr: diagnostic,
truncated: stderrBytes > stderr.length - start,
stdout_bytes: 0,
stderr_bytes: stderrBytes,
},
});
};
const transport =
"command" in config
@ -169,9 +192,15 @@ export const createIdaMcpConnection = (
: new StreamableHTTPClientTransport(new URL(config.url), {
requestInit: { headers: config.headers, redirect: "error" },
});
// Drain proxy diagnostics without persisting ambient environment or transport credentials.
// Keep a bounded diagnostic tail for genuine failures; never persist ambient variables.
if (transport instanceof StdioClientTransport)
transport.stderr?.on("data", () => undefined);
transport.stderr?.on("data", (chunk: Buffer) => {
stderrBytes += chunk.length;
const combined = Buffer.concat([stderr, chunk]);
stderr = Buffer.from(
combined.subarray(Math.max(0, combined.length - 16_384)),
);
});
return {
async connect() {
await client

104
src/ida/IdaSessionClient.test.ts
View File

@ -9,6 +9,7 @@ import {
import { IdaSessionClient } from "./IdaSessionClient.js";
import { functionDossierSchema } from "../domain/hopperValues.js";
import { jsonValueSchema, type JsonValue } from "../domain/jsonValue.js";
import { ProviderAdapterError } from "../domain/providerAdapterError.js";
import { projectAnalysisError } from "../domain/analysisErrorProjection.js";
const roots: string[] = [];
@ -1366,3 +1367,106 @@ describe("headless IDA cleanup verification", () => {
expect(await readdir(root)).toEqual(["target.elf"]);
});
});
it.each(["idb_close", "idb_list", "connection_close"])(
"preserves transport stderr through cleanup at %s",
async (stage) => {
const { producer, client } = await fixture("headless");
expect((await client.execute("health", {})).ok).toBe(true);
const failure = new ProviderAdapterError("ida", "transport", {
diagnostics: {
reason: "upstream transport closed",
},
capturedOutput: {
stdout: "",
stderr: "IDA shutdown diagnostic",
truncated: false,
},
});
const originalClose = producer.close.bind(producer);
producer.beforeCall = async (name) => {
if (name === stage) throw failure;
};
if (stage === "connection_close")
producer.close = async () => {
throw failure;
};
try {
const result = await client.close();
if (result.ok) throw new Error("expected cleanup failure");
expect(result.error.capturedOutput?.stderr).toBe(
"IDA shutdown diagnostic",
);
expect(result.error.diagnostics).toMatchObject({
failure_reason: "upstream transport closed",
});
expect(result.error.cleanupIncomplete).toBe(true);
} finally {
producer.beforeCall = undefined;
producer.close = originalClose;
await client.close();
}
},
);
it("binds retained transport diagnostics to the requested operation and recovery", async () => {
const { producer, client } = await fixture("attached");
await client.execute("health", {});
producer.beforeCall = async (name) => {
if (name === "list_funcs")
throw new ProviderAdapterError("ida", "transport", {
diagnostics: { reason: "upstream closed" },
capturedOutput: {
stdout: "",
stderr: "IDA diagnostic",
truncated: false,
},
});
};
const result = await client.execute("list_procedures", {});
if (result.ok) throw new Error("expected upstream failure");
expect(projectAnalysisError(result.error)).toMatchObject({
details: {
operation: "list_procedures",
diagnostics: {
reason: "upstream closed",
recovery: expect.stringContaining("docs/ida-provider.md"),
},
captured_output: { stderr: "IDA diagnostic" },
},
});
await client.close();
});
it("retains transport diagnostics on a cancellation regardless of stderr", async () => {
const { producer, client } = await fixture("attached");
await client.execute("health", {});
const controller = new AbortController();
producer.beforeCall = async (name) => {
if (name === "list_funcs") {
controller.abort();
throw new ProviderAdapterError("ida", "transport", {
diagnostics: { reason: "upstream closed after cancellation" },
capturedOutput: {
stdout: "",
stderr: "shutdown diagnostic",
truncated: false,
},
});
}
};
const result = await client.execute(
"list_procedures",
{},
{ signal: controller.signal },
);
if (result.ok) throw new Error("expected cancellation");
expect(projectAnalysisError(result.error)).toMatchObject({
code: "cancelled",
details: {
diagnostics: { reason: "upstream closed after cancellation" },
captured_output: { stderr: "shutdown diagnostic" },
},
});
await client.close();
});

48
src/ida/IdaSessionClient.ts
View File

@ -111,10 +111,18 @@ class IdaCleanupError extends ProviderAdapterError {
) {
super("ida", "close", {
cause,
capturedOutput:
cause instanceof AnalysisError ? cause.capturedOutput : undefined,
diagnostics: {
...(cause instanceof AnalysisError ? cause.diagnostics : undefined),
reason:
"IDA cleanup could not be verified; the remaining connection or private workspace is retained for retry.",
failure_reason: cause instanceof Error ? cause.message : String(cause),
failure_reason:
cause instanceof AnalysisError
? (cause.diagnostics?.reason ?? cause.message)
: cause instanceof Error
? cause.message
: String(cause),
workspace: path,
...(sessions === undefined
? {}
@ -375,15 +383,47 @@ export class IdaSessionClient implements AnalysisClient {
context: IdaOperationFailureContext,
) {
let failure: AnalysisError;
if (
if (wasCancelled(options))
failure = new AnalysisCancelledError(operation, {
cause,
...(cause instanceof AnalysisError
? {
capturedOutput: cause.capturedOutput,
diagnostics: cause.diagnostics,
partialObservation: cause.partialObservation,
cleanup:
cause.cleanup ??
(cause.cleanupIncomplete
? {
reason: cause.message,
resources: cause.cleanupResources,
}
: undefined),
}
: {}),
});
else if (
cause instanceof ProviderAdapterError &&
cause.operation === "transport"
)
failure = new ProviderAdapterError("ida", operation, {
cause,
capturedOutput: cause.capturedOutput,
partialObservation: cause.partialObservation,
cleanup: cause.cleanup,
diagnostics: {
...cause.diagnostics,
recovery:
"Check the configured upstream MCP registration and selected target; see docs/ida-provider.md.",
},
});
else if (
cause instanceof AnalysisError &&
(cause.partialObservation !== undefined ||
cause.cleanupIncomplete ||
cause.capturedOutput !== undefined)
)
failure = cause;
else if (wasCancelled(options))
failure = new AnalysisCancelledError(operation);
else if (cause instanceof AnalysisError) failure = cause;
else if (cause instanceof z.ZodError)
failure = new AnalysisOutputError(

111
tests/boundary/idaStartupDiagnostics.test.ts Normal file
View File

@ -0,0 +1,111 @@
import { describe, expect, it } from "vitest";
import { createIdaMcpConnection } from "../../src/ida/IdaMcpConnection.js";
describe("IDA startup diagnostics", () => {
it("preserves the failing launcher's stderr instead of reporting only a closed transport", async () => {
const connection = createIdaMcpConnection(
{
command: process.execPath,
args: [
"-e",
'process.stderr.write("FATAL: IDA runtime initialization failed\\n", () => process.exit(2));',
],
env: {},
mode: "headless",
timeoutMs: 1000,
},
{},
);
try {
await expect(connection.connect()).rejects.toMatchObject({
_tag: "ProviderAdapterError",
capturedOutput: {
stderr: expect.stringContaining("IDA runtime initialization failed"),
truncated: false,
},
});
} finally {
await connection.close();
}
});
it("retains only the bounded stderr tail and reports the original byte count", async () => {
const connection = createIdaMcpConnection(
{
command: process.execPath,
args: [
"-e",
'process.stderr.write("x".repeat(20000) + "FATAL-END", () => process.exit(2));',
],
env: {},
mode: "headless",
timeoutMs: 1000,
},
{},
);
try {
await expect(connection.connect()).rejects.toMatchObject({
_tag: "ProviderAdapterError",
capturedOutput: {
stderr: "x".repeat(16384 - "FATAL-END".length) + "FATAL-END",
stderr_bytes: 20000 + "FATAL-END".length,
truncated: true,
},
});
} finally {
await connection.close();
}
});
});
it("drops a leading partial UTF-8 character from the bounded stderr tail", async () => {
const connection = createIdaMcpConnection(
{
command: process.execPath,
args: [
"-e",
'process.stderr.write("€".repeat(6000), () => process.exit(2));',
],
env: {},
mode: "headless",
timeoutMs: 1000,
},
{},
);
try {
await expect(connection.connect()).rejects.toMatchObject({
capturedOutput: {
stderr: "€".repeat(Math.floor(16384 / 3)),
stderr_bytes: 18000,
truncated: true,
},
});
} finally {
await connection.close();
}
});
it("retains an in-budget undecodable leading byte without claiming tail truncation", async () => {
const connection = createIdaMcpConnection(
{
command: process.execPath,
args: [
"-e",
"process.stderr.write(Buffer.from([0x80, 0x61, 0x62]), () => process.exit(2));",
],
env: {},
mode: "headless",
timeoutMs: 1000,
},
{},
);
try {
const failure = await connection.connect().catch((cause: unknown) => cause);
expect(failure).toMatchObject({
capturedOutput: { stderr: "<22>ab", stderr_bytes: 3, truncated: false },
});
expect(failure).not.toHaveProperty("diagnostics.upstream_stderr");
} finally {
await connection.close();
}
});