fix(ida): let one-shot commands exit after retaining cleanup (#1841)
* fix(ida): let one-shot commands exit after retaining cleanup When a headless IDA request outlives timeoutMs (for example idb_open on a large binary still running auto-analysis), REA cannot prove the worker stopped and correctly retains the upstream connection and private workspace. The retained stdio child and its pipes stayed referenced, so a one-shot CLI printed its cleanup_incomplete result and then never exited, keeping the supervisor, worker and database alive. Detach the retained connection's child process and pipes from the event loop when cleanup fails. A long-lived MCP server can still retry through the retained connection; a one-shot CLI exits with status 1, and the supervisor observes end of input. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(ida): report an unverified upstream worker after failed cleanup Upstream ida-pro-mcp spawns headless workers in their own session and deliberately leaves them running when its supervisor exits, so neither a retained connection nor REA exiting proves that a timed-out worker stopped. When cleanup fails before the owned worker's release is confirmed, the cleanup diagnostics now name its private session and state that it may still be running until it finishes and reaches its idle timeout. Failures after a confirmed release, or before any open was attempted, do not report a worker. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(ida): name the inventory sessions that block worker release When idb_close confirmed the requested worker gone but the follow-up inventory still found a different peer on the private input, the upstream_worker diagnostic named the released session and could misdirect manual cleanup. Record the session ids of the inventory entries that actually blocked release and report those; fall back to the requested private session only when no inventory evidence exists (an incomplete open or an unanswered request). The field becomes session_ids, since more than one entry can block release. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(ida): keep blocker evidence and backend in cleanup diagnostics The release diagnostic reset its observed blockers on every close, so a retry whose inventory was unavailable fell back to the already-released session; and it kept only session ids, describing a blocking GUI session with detached-worker guidance. Blocker evidence is now replaced only by a successful inventory, and the diagnostic becomes upstream_sessions with each session's id and backend, with guidance that distinguishes workers from GUI sessions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(ida): stop naming a worker after its close was confirmed When idb_close confirmed the requested worker's release but the following inventory failed, no blocker evidence existed yet and the diagnostic fell back to the confirmed-absent worker. After a confirmed close the diagnostic now reports an empty session list, meaning any remaining same-input sessions are unknown. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(ida): group cleanup diagnostic cases in their own suite The added cases pushed the reaped-worker describe block past the 150-line limit enforced by lint:built. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(ida): keep ownership and pre-close peers in cleanup diagnostics Blocking sessions dropped the parsed ownership fact, so an external unowned worker looked the same as REA's own; and recovery inventory recorded same-input peers only when the requested worker was missing, so a peer seen beside it was lost if the following close failed. Each reported session now carries owned (null when no inventory has reported it), and recovery records every same-input session until a later successful inventory replaces that evidence. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(ida): drop a confirmed worker from retained cleanup blockers Recovery inventory now records the requested worker beside any peers, so once idb_close confirmed its release a failing follow-up inventory still reported it as unreleased. Remove the confirmed session from the retained blockers at close confirmation and keep the remaining peers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(ida): retry cleanup through a detached stdio connection The retry coverage used a recording connection whose detach() only counts calls, so it could not show that detaching keeps a retained connection usable. Run a real stdio supervisor in a long-lived host, fail the first post-close inventory, and require the retry through the same detached connection to confirm release and remove the workspace. The test fails if detach() destroys the pipes instead of unref-ing them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(ida): close the stdio supervisor in failed retry tests Merge main and give its new IdaNativeProtocol connection double the required detach() method. The real-stdio retry test now closes its connection in an unconditional teardown, so a failing assertion cannot leave the supervisor waiting on stdin. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(ida): share the cleanup diagnostic expectation in session tests Repeated upstream_sessions assertions and the unrelated live peer literal pushed repository duplication over the jscpd threshold. Build the expected diagnostic with one helper and reuse one peer constant. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(ida): distinguish observed ownership from unreported input bindings * fix(ida): keep requested-ID conflicts in recovery cleanup Recovery inventory kept only same-input entries, unlike the startup and post-close filters. After an explicitly rejected open, an entry under the requested session ID on another input was dropped, so cleanup could close the connection, remove the private workspace and report success while that session stayed registered. Match the requested ID as well so the entry is retained as a blocker and cleanup stays incomplete. Also split the cleanup diagnostics suite under the lint:built function length limit, and make two startup-inventory fixtures pass a defined session id so the test file typechecks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Xingchuan Ma <xingchuan.ma@msn.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: morluto <morluto@users.noreply.github.com>
This commit is contained in:
Max
GitHub
parent
811bd9f635
commit
5dd2ce3125
|
|
@ -18,6 +18,7 @@ const scripted = (answers: readonly JsonValue[]) => {
|
|||
connect: async () => [],
|
||||
serverInfo: () => null,
|
||||
close: async () => undefined,
|
||||
detach: () => undefined,
|
||||
call: async () => {
|
||||
reads.push(reads.length);
|
||||
const answer = answers[Math.min(reads.length - 1, answers.length - 1)];
|
||||
|
|
|
|||
|
|
@ -1,3 +1,5 @@
|
|||
import { ChildProcess } from "node:child_process";
|
||||
import { Socket } from "node:net";
|
||||
import { snapshotEnvironment } from "../process/snapshotEnvironment.js";
|
||||
import {
|
||||
Client,
|
||||
|
|
@ -22,6 +24,12 @@ export interface IdaMcpConnection {
|
|||
readResource?(uri: string): Promise<JsonValue>;
|
||||
serverInfo(): JsonValue;
|
||||
close(): Promise<void>;
|
||||
/**
|
||||
* Keep a connection retained after failed cleanup from holding the host
|
||||
* process open. A long-lived MCP server can still retry through it; a
|
||||
* one-shot CLI exits, and the upstream proxy observes end of input.
|
||||
*/
|
||||
detach(): void;
|
||||
}
|
||||
|
||||
const toolResultSchema = z.object({
|
||||
|
|
@ -205,5 +213,14 @@ export const createIdaMcpConnection = (
|
|||
},
|
||||
serverInfo: () => jsonValueSchema.parse(client.getServerVersion() ?? null),
|
||||
close: () => client.close().catch(transportFailure),
|
||||
detach() {
|
||||
if (!(transport instanceof StdioClientTransport)) return;
|
||||
// The SDK exposes only the child's pid, not the handles it holds.
|
||||
const child: unknown = Reflect.get(transport, "_process");
|
||||
if (!(child instanceof ChildProcess)) return;
|
||||
child.unref();
|
||||
for (const stream of [child.stdin, child.stdout, child.stderr])
|
||||
if (stream instanceof Socket) stream.unref();
|
||||
},
|
||||
};
|
||||
};
|
||||
|
|
|
|||
|
|
@ -90,6 +90,7 @@ const fixture = async () => {
|
|||
},
|
||||
serverInfo: () => producer.serverInfo(),
|
||||
close: () => producer.close(),
|
||||
detach: () => producer.detach(),
|
||||
};
|
||||
const client = new IdaSessionClient(
|
||||
{
|
||||
|
|
|
|||
|
|
@ -16,6 +16,18 @@ afterEach(async () => {
|
|||
for (const root of roots.splice(0))
|
||||
await rm(root, { recursive: true, force: true });
|
||||
});
|
||||
/** A failed close whose diagnostic names exactly these unverified sessions. */
|
||||
const reportsSessions = (...sessions: readonly object[]) => ({
|
||||
error: {
|
||||
diagnostics: { upstream_sessions: { state: "unverified", sessions } },
|
||||
},
|
||||
});
|
||||
const LIVE_PEER = {
|
||||
session_id: "unrelated-live-peer",
|
||||
backend: "worker",
|
||||
owned: false,
|
||||
} as const;
|
||||
|
||||
const fixture = async (
|
||||
mode: "attached" | "headless" = "attached",
|
||||
timeoutMs = 1000,
|
||||
|
|
@ -792,10 +804,19 @@ describe("headless IDA session ownership and cleanup", () => {
|
|||
it("retains the private workspace and reports cleanup failure when worker release is unverified", async () => {
|
||||
const { producer, client, root } = await fixture("headless");
|
||||
await client.execute("health", {});
|
||||
const sessionId = producer.database;
|
||||
producer.failClose = true;
|
||||
const result = await client.close();
|
||||
expect(result.ok).toBe(false);
|
||||
if (!result.ok) expect(result.error.cleanupIncomplete).toBe(true);
|
||||
expect(result).toMatchObject(
|
||||
reportsSessions({
|
||||
session_id: sessionId,
|
||||
backend: "worker",
|
||||
owned: true,
|
||||
input_path: producer.inputPath,
|
||||
}),
|
||||
);
|
||||
expect(
|
||||
(await readdir(root)).some((name) => name.startsWith("rea-ida-")),
|
||||
).toBe(true);
|
||||
|
|
@ -843,10 +864,20 @@ describe("headless IDA reaped worker cleanup", () => {
|
|||
const result = await client.close();
|
||||
expect(result.ok).toBe(false);
|
||||
if (!result.ok) expect(result.error.cleanupIncomplete).toBe(true);
|
||||
// The released session must not be named as the remaining worker.
|
||||
expect(result).toMatchObject(reportsSessions(LIVE_PEER));
|
||||
expect(producer.closes).toBe(0);
|
||||
expect(
|
||||
(await readdir(root)).some((name) => name.startsWith("rea-ida-")),
|
||||
).toBe(true);
|
||||
|
||||
// A retry whose inventory is unavailable keeps the observed blocker.
|
||||
producer.beforeCall = async (name) => {
|
||||
if (name === "idb_list") throw new Error("inventory unavailable");
|
||||
};
|
||||
const retried = await client.close();
|
||||
expect(retried.ok).toBe(false);
|
||||
expect(retried).toMatchObject(reportsSessions(LIVE_PEER));
|
||||
});
|
||||
|
||||
it("retries independent inventory verification after an absent-session response", async () => {
|
||||
|
|
@ -899,6 +930,203 @@ describe("headless IDA reaped worker cleanup", () => {
|
|||
});
|
||||
});
|
||||
|
||||
/** A rejected open response, with helpers for separately inventoried workers. */
|
||||
const rejectedOpenFixture = async () => {
|
||||
const { producer, client } = await fixture("headless");
|
||||
producer.overrideCall = (name) =>
|
||||
name === "idb_open" ? { error: "open did not complete" } : undefined;
|
||||
expect((await client.execute("health", {})).ok).toBe(false);
|
||||
const open = producer.calls.find(({ name }) => name === "idb_open")?.args;
|
||||
const entry = (session_id: string, owned: boolean) => ({
|
||||
session_id,
|
||||
input_path: String(open?.input_path),
|
||||
owned,
|
||||
backend: "worker",
|
||||
is_active: true,
|
||||
});
|
||||
return {
|
||||
producer,
|
||||
client,
|
||||
requested: String(open?.preferred_session_id),
|
||||
entry,
|
||||
};
|
||||
};
|
||||
|
||||
describe("headless IDA cleanup diagnostics", () => {
|
||||
it("does not name a worker whose close was confirmed when inventory then fails", async () => {
|
||||
const { producer, client } = await fixture("headless");
|
||||
expect((await client.execute("health", {})).ok).toBe(true);
|
||||
producer.beforeCall = async (name) => {
|
||||
if (name === "idb_list") throw new Error("inventory unavailable");
|
||||
};
|
||||
|
||||
const result = await client.close();
|
||||
expect(result.ok).toBe(false);
|
||||
expect(producer.calls.map(({ name }) => name)).toContain("idb_close");
|
||||
expect(result).toMatchObject(reportsSessions());
|
||||
});
|
||||
|
||||
it("retains peers and confirmed ownership from successful startup when close fails", async () => {
|
||||
const { producer, client } = await fixture("headless");
|
||||
producer.overrideCall = (name) => {
|
||||
if (name === "idb_list")
|
||||
return {
|
||||
sessions: [
|
||||
{
|
||||
session_id: String(producer.database),
|
||||
input_path: producer.inputPath,
|
||||
owned: true,
|
||||
backend: "worker",
|
||||
is_active: true,
|
||||
},
|
||||
{
|
||||
session_id: "startup-peer",
|
||||
input_path: producer.inputPath,
|
||||
owned: false,
|
||||
backend: "worker",
|
||||
is_active: false,
|
||||
},
|
||||
],
|
||||
};
|
||||
if (name === "idb_close") return { error: "close rejected" };
|
||||
return undefined;
|
||||
};
|
||||
expect((await client.execute("health", {})).ok).toBe(true);
|
||||
const result = await client.close();
|
||||
expect(result).toMatchObject(
|
||||
reportsSessions(
|
||||
{
|
||||
session_id: producer.database,
|
||||
backend: "worker",
|
||||
owned: true,
|
||||
input_path: producer.inputPath,
|
||||
},
|
||||
{
|
||||
session_id: "startup-peer",
|
||||
backend: "worker",
|
||||
owned: false,
|
||||
input_path: producer.inputPath,
|
||||
},
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
it("preserves an observed conflicting input binding after close", async () => {
|
||||
const { producer, client } = await fixture("headless");
|
||||
expect((await client.execute("health", {})).ok).toBe(true);
|
||||
const requested = String(producer.database);
|
||||
producer.overrideCall = (name) =>
|
||||
name === "idb_list"
|
||||
? {
|
||||
sessions: [
|
||||
{
|
||||
session_id: requested,
|
||||
input_path: "/different/input",
|
||||
owned: false,
|
||||
backend: "worker",
|
||||
is_active: true,
|
||||
},
|
||||
],
|
||||
}
|
||||
: undefined;
|
||||
const result = await client.close();
|
||||
expect(result).toMatchObject(
|
||||
reportsSessions({
|
||||
session_id: requested,
|
||||
backend: "worker",
|
||||
owned: false,
|
||||
input_path: "/different/input",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps peers seen beside the requested worker when its close fails", async () => {
|
||||
const { producer, client, requested, entry } = await rejectedOpenFixture();
|
||||
producer.overrideCall = (name) => {
|
||||
if (name === "idb_list")
|
||||
return {
|
||||
sessions: [
|
||||
entry(requested, true),
|
||||
entry("unrelated-live-peer", false),
|
||||
],
|
||||
};
|
||||
if (name === "idb_close") return { error: "close rejected" };
|
||||
return undefined;
|
||||
};
|
||||
|
||||
const result = await client.close();
|
||||
expect(result.ok).toBe(false);
|
||||
expect(result).toMatchObject(
|
||||
reportsSessions(
|
||||
{ session_id: requested, backend: "worker", owned: true },
|
||||
LIVE_PEER,
|
||||
),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("headless IDA cleanup diagnostics for observed peers", () => {
|
||||
it("drops the requested worker from blockers once its close is confirmed", async () => {
|
||||
const { producer, client, requested, entry } = await rejectedOpenFixture();
|
||||
let inventories = 0;
|
||||
producer.beforeCall = async (name) => {
|
||||
if (name === "idb_list" && ++inventories > 1)
|
||||
throw new Error("inventory unavailable");
|
||||
};
|
||||
producer.overrideCall = (name) => {
|
||||
if (name === "idb_list")
|
||||
return {
|
||||
sessions: [
|
||||
entry(requested, true),
|
||||
entry("unrelated-live-peer", false),
|
||||
],
|
||||
};
|
||||
if (name === "idb_close")
|
||||
return {
|
||||
success: true,
|
||||
session_id: requested,
|
||||
saved: false,
|
||||
owned: true,
|
||||
backend: "worker",
|
||||
};
|
||||
return undefined;
|
||||
};
|
||||
|
||||
const result = await client.close();
|
||||
expect(result.ok).toBe(false);
|
||||
expect(result).toMatchObject(reportsSessions(LIVE_PEER));
|
||||
});
|
||||
|
||||
it("reports a blocking GUI session by its backend", async () => {
|
||||
const { producer, client } = await fixture("headless");
|
||||
expect((await client.execute("health", {})).ok).toBe(true);
|
||||
const sessionId = producer.database;
|
||||
producer.overrideCall = (name) => {
|
||||
if (name === "idb_close")
|
||||
return { error: `Session not found: ${sessionId}` };
|
||||
if (name === "idb_list")
|
||||
return {
|
||||
sessions: [
|
||||
{
|
||||
session_id: "",
|
||||
input_path: producer.inputPath,
|
||||
owned: false,
|
||||
backend: "gui",
|
||||
is_active: true,
|
||||
},
|
||||
],
|
||||
};
|
||||
return undefined;
|
||||
};
|
||||
|
||||
const result = await client.close();
|
||||
expect(result.ok).toBe(false);
|
||||
expect(result).toMatchObject(
|
||||
reportsSessions({ session_id: "", backend: "gui", owned: false }),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
/** A completed open response is not proof that its private Worker was ever released. */
|
||||
describe("headless IDA unresolved successful-open ownership", () => {
|
||||
it.each(["matching", "foreign-input", "malformed"] as const)(
|
||||
|
|
@ -976,6 +1204,29 @@ describe("headless IDA unresolved successful-open ownership", () => {
|
|||
expect(producer.closes).toBe(1);
|
||||
expect(await readdir(root)).toEqual(["target.elf"]);
|
||||
});
|
||||
|
||||
it("retains cleanup when a rejected open leaves the requested ID on another input", async () => {
|
||||
const { producer, client, requested } = await rejectedOpenFixture();
|
||||
producer.overrideCall = (name) =>
|
||||
name === "idb_list"
|
||||
? {
|
||||
sessions: [
|
||||
{
|
||||
session_id: requested,
|
||||
input_path: "/different/input",
|
||||
owned: false,
|
||||
backend: "worker",
|
||||
is_active: true,
|
||||
},
|
||||
],
|
||||
}
|
||||
: undefined;
|
||||
|
||||
const result = await client.close();
|
||||
expect(result.ok).toBe(false);
|
||||
if (!result.ok) expect(result.error.cleanupIncomplete).toBe(true);
|
||||
expect(producer.closes).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe("headless IDA cleanup verification", () => {
|
||||
|
|
|
|||
|
|
@ -93,11 +93,25 @@ interface IdaOperationFailureContext {
|
|||
parameters: Readonly<Record<string, JsonValue>>;
|
||||
}
|
||||
|
||||
/** Upstream session ids of inventory entries, omitting unreported ones. */
|
||||
interface UpstreamSession {
|
||||
readonly session_id: string;
|
||||
readonly backend: "worker" | "gui";
|
||||
/** Upstream ownership; null when no inventory has reported it. */
|
||||
readonly owned: boolean | null;
|
||||
/** Last inventoried input binding; null until upstream reports it. */
|
||||
readonly input_path: string | null;
|
||||
}
|
||||
|
||||
/** A failed cleanup retains its private workspace and reports the remaining resource. */
|
||||
class IdaCleanupError extends ProviderAdapterError {
|
||||
override readonly cleanupIncomplete = true;
|
||||
override readonly cleanupResources: readonly string[];
|
||||
constructor(path: string, cause: unknown) {
|
||||
constructor(
|
||||
path: string,
|
||||
cause: unknown,
|
||||
sessions?: readonly UpstreamSession[],
|
||||
) {
|
||||
super("ida", "close", {
|
||||
cause,
|
||||
diagnostics: {
|
||||
|
|
@ -105,6 +119,23 @@ class IdaCleanupError extends ProviderAdapterError {
|
|||
"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),
|
||||
workspace: path,
|
||||
...(sessions === undefined
|
||||
? {}
|
||||
: {
|
||||
upstream_sessions: {
|
||||
state: "unverified",
|
||||
sessions: sessions.map(
|
||||
({ session_id, backend, owned, input_path }) => ({
|
||||
session_id,
|
||||
backend,
|
||||
owned,
|
||||
input_path,
|
||||
}),
|
||||
),
|
||||
reason:
|
||||
"Release of upstream sessions matching the requested session ID or private input was not confirmed; input_path preserves the last observed binding and is null when no inventory has reported it. A worker outlives its supervisor and may keep running until it finishes and reaches its idle timeout; exiting REA does not stop it. A gui session belongs to an IDA instance REA does not control. An empty session_id means upstream reported none; owned is null when no inventory has reported ownership; an empty list means the requested worker's close was confirmed but no inventory has identified any remaining session.",
|
||||
},
|
||||
}),
|
||||
},
|
||||
});
|
||||
this.cleanupResources = [path];
|
||||
|
|
@ -123,6 +154,7 @@ export class IdaSessionClient implements AnalysisClient {
|
|||
#databaseCloseCompleted = false;
|
||||
#openAttempted = false;
|
||||
#openCompleted = false;
|
||||
#blockingSessions: readonly UpstreamSession[] | undefined;
|
||||
#openRejected = false;
|
||||
#callGraphAdvertised = false;
|
||||
#surveyAdvertised = false;
|
||||
|
|
@ -490,6 +522,14 @@ export class IdaSessionClient implements AnalysisClient {
|
|||
await this.connection.call("idb_list", {}),
|
||||
);
|
||||
this.#lifecycle.push(jsonValueSchema.parse(inventory));
|
||||
const workspace = this.#workspace;
|
||||
this.#recordBlockingSessions(
|
||||
inventory.sessions.filter(
|
||||
(entry) =>
|
||||
entry.session_id === workspace.sessionId ||
|
||||
sameIdaHostPath(entry.input_path, workspace.inputPath),
|
||||
),
|
||||
);
|
||||
const entry = inventory.sessions.find(
|
||||
({ session_id }) => session_id === opened.session.session_id,
|
||||
);
|
||||
|
|
@ -589,6 +629,52 @@ export class IdaSessionClient implements AnalysisClient {
|
|||
return shared;
|
||||
}
|
||||
|
||||
/**
|
||||
* Upstream sessions whose release cleanup has not confirmed: the entries
|
||||
* the latest successful inventory found blocking release; else the
|
||||
* requested private worker until its close is confirmed, and then none
|
||||
* known.
|
||||
*/
|
||||
#unreleasedSessions(): readonly UpstreamSession[] | undefined {
|
||||
if (this.#blockingSessions !== undefined) return this.#blockingSessions;
|
||||
if (!this.#openAttempted || this.#workspace === undefined) return undefined;
|
||||
return this.#databaseCloseCompleted
|
||||
? []
|
||||
: [
|
||||
{
|
||||
session_id: this.#workspace.sessionId,
|
||||
backend: "worker",
|
||||
owned: null,
|
||||
input_path: null,
|
||||
},
|
||||
];
|
||||
}
|
||||
|
||||
/** Drop a session whose release upstream confirmed, keeping other blockers. */
|
||||
#forgetReleasedSession(sessionId: string): void {
|
||||
const remaining = this.#blockingSessions?.filter(
|
||||
({ session_id }) => session_id !== sessionId,
|
||||
);
|
||||
this.#blockingSessions =
|
||||
remaining !== undefined && remaining.length > 0 ? remaining : undefined;
|
||||
}
|
||||
|
||||
/** Replace blocker evidence from a successful inventory; true when blocked. */
|
||||
#recordBlockingSessions(
|
||||
entries: readonly (UpstreamSession & { readonly owned: boolean })[],
|
||||
): boolean {
|
||||
this.#blockingSessions =
|
||||
entries.length > 0
|
||||
? entries.map(({ session_id, backend, owned, input_path }) => ({
|
||||
session_id,
|
||||
backend,
|
||||
owned,
|
||||
input_path,
|
||||
}))
|
||||
: undefined;
|
||||
return entries.length > 0;
|
||||
}
|
||||
|
||||
async #closeDatabase(database: string): Promise<void> {
|
||||
const response = await this.connection.call("idb_close", {
|
||||
database,
|
||||
|
|
@ -629,12 +715,17 @@ export class IdaSessionClient implements AnalysisClient {
|
|||
entry.session_id === workspace.sessionId &&
|
||||
sameIdaHostPath(entry.input_path, workspace.inputPath),
|
||||
)?.session_id;
|
||||
if (
|
||||
this.#database === undefined &&
|
||||
inventory.sessions.some((entry) =>
|
||||
sameIdaHostPath(entry.input_path, workspace.inputPath),
|
||||
)
|
||||
)
|
||||
// Every same-input session and any entry under the requested ID,
|
||||
// whatever its input, stays reported until a later successful
|
||||
// inventory confirms release.
|
||||
const blocked = this.#recordBlockingSessions(
|
||||
inventory.sessions.filter(
|
||||
(entry) =>
|
||||
entry.session_id === workspace.sessionId ||
|
||||
sameIdaHostPath(entry.input_path, workspace.inputPath),
|
||||
),
|
||||
);
|
||||
if (this.#database === undefined && blocked)
|
||||
throw new AnalysisProtocolError(
|
||||
"An unowned worker still uses the private workspace; it was retained.",
|
||||
);
|
||||
|
|
@ -651,20 +742,20 @@ export class IdaSessionClient implements AnalysisClient {
|
|||
if (!this.#databaseCloseCompleted) {
|
||||
await this.#closeDatabase(this.#database);
|
||||
this.#databaseCloseCompleted = true;
|
||||
this.#forgetReleasedSession(this.#database);
|
||||
}
|
||||
const inventory = databaseListSchema.parse(
|
||||
await this.connection.call("idb_list", {}),
|
||||
);
|
||||
if (
|
||||
inventory.sessions.some(
|
||||
(entry) =>
|
||||
entry.session_id === this.#database ||
|
||||
sameIdaHostPath(
|
||||
entry.input_path,
|
||||
this.#workspace?.inputPath ?? "",
|
||||
),
|
||||
)
|
||||
)
|
||||
const remaining = inventory.sessions.filter(
|
||||
(entry) =>
|
||||
entry.session_id === this.#database ||
|
||||
sameIdaHostPath(
|
||||
entry.input_path,
|
||||
this.#workspace?.inputPath ?? "",
|
||||
),
|
||||
);
|
||||
if (this.#recordBlockingSessions(remaining))
|
||||
throw new AnalysisProtocolError(
|
||||
"IDA worker remains registered after close.",
|
||||
);
|
||||
|
|
@ -681,10 +772,12 @@ export class IdaSessionClient implements AnalysisClient {
|
|||
this.#closed = true;
|
||||
return ok(null);
|
||||
} catch (cause: unknown) {
|
||||
this.connection.detach();
|
||||
return err(
|
||||
new IdaCleanupError(
|
||||
this.#workspace?.directory ?? "IDA MCP connection",
|
||||
cause,
|
||||
this.#unreleasedSessions(),
|
||||
),
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -98,11 +98,17 @@ describe("IDA workspace ownership before input preparation", () => {
|
|||
expect(closed.error.cleanupResources).toEqual([
|
||||
expect.stringContaining(workspace ?? "missing workspace"),
|
||||
]);
|
||||
// No open was attempted, so there is no worker to report.
|
||||
expect(closed.error).not.toHaveProperty(
|
||||
"diagnostics.upstream_sessions",
|
||||
);
|
||||
}
|
||||
expect(producer.detaches).toBe(1);
|
||||
} finally {
|
||||
await chmod(root, 0o700);
|
||||
}
|
||||
expect((await client.close()).ok).toBe(true);
|
||||
expect(producer.detaches).toBe(1);
|
||||
expect(await readdir(root)).toEqual(["target.elf"]);
|
||||
},
|
||||
);
|
||||
|
|
|
|||
|
|
@ -0,0 +1,92 @@
|
|||
import { describe, expect } from "vitest";
|
||||
|
||||
import { workspaceCliTest } from "../../support/cli/workspaceCliFixture.js";
|
||||
|
||||
import { elf } from "../../../src/domain/binaryTarget.fixture.js";
|
||||
|
||||
/**
|
||||
* A headless IDA supervisor that completes the MCP handshake and then leaves
|
||||
* every tool call unanswered, as an `idb_open` still running auto-analysis
|
||||
* would. It exits when REA closes its input.
|
||||
*/
|
||||
const UNRESPONSIVE_SUPERVISOR = `import { createInterface } from "node:readline";
|
||||
const tools = ["idb_open", "idb_list", "idb_close", "server_health", "list_funcs",
|
||||
"lookup_funcs", "find_regex", "decompile", "disasm", "callees", "xrefs_to"];
|
||||
const send = (message) =>
|
||||
process.stdout.write(JSON.stringify({ jsonrpc: "2.0", ...message }) + "\\n");
|
||||
for await (const line of createInterface({ input: process.stdin })) {
|
||||
const request = JSON.parse(line);
|
||||
if (request.method === "initialize")
|
||||
send({ id: request.id, result: {
|
||||
protocolVersion: request.params.protocolVersion,
|
||||
capabilities: { tools: {} },
|
||||
serverInfo: { name: "unresponsive-ida-fixture", version: "1" },
|
||||
} });
|
||||
else if (request.method === "tools/list")
|
||||
send({ id: request.id, result: {
|
||||
tools: tools.map((name) => ({ name, inputSchema: { type: "object" } })),
|
||||
} });
|
||||
}
|
||||
`;
|
||||
|
||||
describe("headless IDA timeout through the one-shot CLI", () => {
|
||||
workspaceCliTest(
|
||||
"exits after a retained-cleanup failure instead of waiting on the supervisor",
|
||||
async ({ cli, workspace }) => {
|
||||
const supervisor = await workspace.write(
|
||||
"supervisor.mjs",
|
||||
UNRESPONSIVE_SUPERVISOR,
|
||||
);
|
||||
const workspaceRoot = await workspace.mkdir("ida-workspaces");
|
||||
const registration = await workspace.write(
|
||||
"ida-mcp.json",
|
||||
JSON.stringify({
|
||||
command: process.execPath,
|
||||
args: [supervisor],
|
||||
mode: "headless",
|
||||
timeoutMs: 500,
|
||||
workspaceRoot,
|
||||
}),
|
||||
);
|
||||
const target = await workspace.write("target.elf", elf(2, 1, 62));
|
||||
|
||||
const result = await cli.run({
|
||||
arguments: ["decompile", target, "main", "--json"],
|
||||
environment: {
|
||||
...workspace.environment,
|
||||
REA_ANALYSIS_PROVIDER: "ida",
|
||||
REA_IDA_MCP_CONFIG: registration,
|
||||
REA_LOG_LEVEL: "silent",
|
||||
},
|
||||
timeoutMs: 30_000,
|
||||
});
|
||||
|
||||
expect(result.exitCode).toBe(1);
|
||||
expect(result.json).toMatchObject({
|
||||
code: "cleanup_incomplete",
|
||||
details: {
|
||||
provider_id: "ida",
|
||||
cleanup: "incomplete",
|
||||
resources: [expect.stringContaining(workspaceRoot)],
|
||||
cleanup_error: {
|
||||
details: {
|
||||
diagnostics: {
|
||||
upstream_sessions: {
|
||||
state: "unverified",
|
||||
sessions: [
|
||||
{
|
||||
session_id: expect.stringMatching(/^rea-/),
|
||||
backend: "worker",
|
||||
owned: null,
|
||||
input_path: null,
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
},
|
||||
);
|
||||
});
|
||||
|
|
@ -0,0 +1,114 @@
|
|||
import { createHash } from "node:crypto";
|
||||
import { mkdir, readdir, writeFile } from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
|
||||
import { describe, expect, it, onTestFinished } from "vitest";
|
||||
|
||||
import { createTestTempDirectory } from "../../../fixtures/temporaryDirectory.js";
|
||||
|
||||
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";
|
||||
|
||||
/**
|
||||
* A headless IDA supervisor over real stdio that answers the session
|
||||
* lifecycle, but rejects the first inventory after a close so cleanup fails
|
||||
* once and must be retried through the retained connection.
|
||||
*/
|
||||
const FLAKY_INVENTORY_SUPERVISOR = `import { createInterface } from "node:readline";
|
||||
const tools = ["idb_open", "idb_list", "idb_close", "server_health", "list_funcs",
|
||||
"lookup_funcs", "find_regex", "decompile", "disasm", "callees", "xrefs_to"];
|
||||
let session;
|
||||
let inputPath;
|
||||
let closed = false;
|
||||
let inventoryFailed = false;
|
||||
const send = (message) =>
|
||||
process.stdout.write(JSON.stringify({ jsonrpc: "2.0", ...message }) + "\\n");
|
||||
const result = (value) => ({
|
||||
content: [{ type: "text", text: JSON.stringify(value) }],
|
||||
structuredContent: value,
|
||||
});
|
||||
const call = ({ name, arguments: args }) => {
|
||||
if (name === "idb_open") {
|
||||
session = args.preferred_session_id;
|
||||
inputPath = args.input_path;
|
||||
return result({ success: true, session: { session_id: session, input_path: inputPath } });
|
||||
}
|
||||
if (name === "server_health")
|
||||
return result({ status: "ok", input_path: inputPath, idb_path: inputPath + ".i64",
|
||||
module: "target.elf", imagebase: "0x1000", auto_analysis_ready: true });
|
||||
if (name === "idb_close") {
|
||||
closed = true;
|
||||
return result({ success: true, session_id: session, saved: null, owned: true, backend: "worker" });
|
||||
}
|
||||
if (name === "idb_list") {
|
||||
if (closed && !inventoryFailed) {
|
||||
inventoryFailed = true;
|
||||
return { isError: true, content: [{ type: "text", text: "inventory unavailable" }] };
|
||||
}
|
||||
return result({ sessions: closed ? [] : [{ session_id: session, input_path: inputPath,
|
||||
owned: true, backend: "worker", is_active: true }] });
|
||||
}
|
||||
return { isError: true, content: [{ type: "text", text: "unsupported " + name }] };
|
||||
};
|
||||
for await (const line of createInterface({ input: process.stdin })) {
|
||||
const request = JSON.parse(line);
|
||||
if (request.method === "initialize")
|
||||
send({ id: request.id, result: {
|
||||
protocolVersion: request.params.protocolVersion,
|
||||
capabilities: { tools: {} },
|
||||
serverInfo: { name: "flaky-inventory-ida-fixture", version: "1" },
|
||||
} });
|
||||
else if (request.method === "tools/list")
|
||||
send({ id: request.id, result: {
|
||||
tools: tools.map((name) => ({ name, inputSchema: { type: "object" } })),
|
||||
} });
|
||||
else if (request.method === "tools/call")
|
||||
send({ id: request.id, result: call(request.params) });
|
||||
}
|
||||
`;
|
||||
|
||||
describe("retained headless IDA connection in a long-lived host", () => {
|
||||
it("retries cleanup through the detached stdio connection", async () => {
|
||||
const root = await createTestTempDirectory("rea-ida-retry-");
|
||||
const supervisor = join(root, "supervisor.mjs");
|
||||
await writeFile(supervisor, FLAKY_INVENTORY_SUPERVISOR);
|
||||
const workspaceRoot = join(root, "workspaces");
|
||||
await mkdir(workspaceRoot);
|
||||
const bytes = elf(2, 1, 62);
|
||||
const targetPath = join(root, "target.elf");
|
||||
await writeFile(targetPath, bytes);
|
||||
const target: BinaryTarget = {
|
||||
path: targetPath,
|
||||
sha256: createHash("sha256").update(bytes).digest("hex"),
|
||||
kind: "executable",
|
||||
format: "elf",
|
||||
architecture: "x86_64",
|
||||
availableArchitectures: ["x86_64"],
|
||||
};
|
||||
const config = {
|
||||
command: process.execPath,
|
||||
args: [supervisor],
|
||||
env: {},
|
||||
mode: "headless" as const,
|
||||
timeoutMs: 5000,
|
||||
workspaceRoot,
|
||||
};
|
||||
const connection = createIdaMcpConnection(config, process.env);
|
||||
// Close the supervisor even when an assertion fails before cleanup does.
|
||||
onTestFinished(() => connection.close().catch(() => undefined));
|
||||
const client = new IdaSessionClient(config, target, connection);
|
||||
|
||||
expect((await client.execute("health", {})).ok).toBe(true);
|
||||
const failed = await client.close();
|
||||
expect(failed.ok).toBe(false);
|
||||
if (!failed.ok) expect(failed.error.cleanupIncomplete).toBe(true);
|
||||
expect(await readdir(workspaceRoot)).toHaveLength(1);
|
||||
|
||||
// The connection was detached when cleanup failed; it must still serve
|
||||
// the retry that confirms release and removes the private workspace.
|
||||
expect((await client.close()).ok).toBe(true);
|
||||
expect(await readdir(workspaceRoot)).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
|
@ -68,6 +68,7 @@ export class RecordingIdaMcp implements IdaMcpConnection {
|
|||
}[] = [];
|
||||
connects = 0;
|
||||
closes = 0;
|
||||
detaches = 0;
|
||||
inputSha256: string;
|
||||
inputPath: string;
|
||||
imageBase = "0x1000";
|
||||
|
|
@ -104,6 +105,9 @@ export class RecordingIdaMcp implements IdaMcpConnection {
|
|||
async close() {
|
||||
this.closes += 1;
|
||||
}
|
||||
detach() {
|
||||
this.detaches += 1;
|
||||
}
|
||||
async call(
|
||||
name: string,
|
||||
args: Readonly<Record<string, JsonValue>>,
|
||||
|
|
|
|||
Loading…
Reference in New Issue