From 4ea24f97670bebf8ca053179ca0dc5009e6f6a28 Mon Sep 17 00:00:00 2001 From: jhuang-tw <77732008+jhuang-tw@users.noreply.github.com> Date: Tue, 13 Oct 2026 20:58:51 +0800 Subject: [PATCH] fix(ida): retain private input after ambiguous open (#1845) fix(ida): retain private input after unverified open --- docs/ida-provider.md | 2 + src/ida/IdaProtocolValues.ts | 8 +++ src/ida/IdaSessionClient.test.ts | 98 ++++++++++++++++++++++++++------ src/ida/IdaSessionClient.ts | 13 +++++ 4 files changed, 103 insertions(+), 18 deletions(-) diff --git a/docs/ida-provider.md b/docs/ida-provider.md index 50040425b..fbf67c8a2 100644 --- a/docs/ida-provider.md +++ b/docs/ida-provider.md @@ -88,6 +88,8 @@ REA copies the admitted binary into a private temporary directory, checks its di One REA session owns one database. Stdio registrations default `IDA_MCP_MAX_WORKERS` to `1`; the explicit upstream `--max-workers 1` also bounds a supervisor. Database size and engine memory depend on the target and upstream; REA does not impose an engine memory limit. The original binary is never executed or overwritten. Optional `workspaceRoot` selects an existing absolute temporary-directory parent on the REA host. Ensure it has space for the input copy and IDA working files. +When a completed `idb_open` response reports success or is otherwise ambiguous, but no active owned private worker can be verified, REA retains the private workspace even if `idb_list` appears empty. A later close can retry verification if the original owned worker becomes discoverable. A completed explicit `idb_open` rejection retains the existing no-worker cleanup path. This is a local admission/ownership safeguard and **does not** provide a process-termination receipt or change the legacy reaped-worker protocol. + Headless cleanup uses the upstream database lifecycle. Abruptly terminating REA, its proxy, or the supervisor does not establish worker release. Prefer `close_binary` for MCP sessions; after an interrupted process, inspect upstream session inventory and retained workspace diagnostics before removing working files. This adapter does not claim Windows Job Object containment for the upstream detached workers. ## Connect an agent once diff --git a/src/ida/IdaProtocolValues.ts b/src/ida/IdaProtocolValues.ts index d4afb92db..f18e85ee0 100644 --- a/src/ida/IdaProtocolValues.ts +++ b/src/ida/IdaProtocolValues.ts @@ -218,6 +218,14 @@ export const openDatabaseSchema = z .passthrough(), }) .passthrough(); +/** Only an explicit completed rejection preserves legacy no-worker cleanup. */ +export const rejectedOpenDatabaseSchema = z + .object({ + error: z.string().min(1), + success: z.never().optional(), + }) + .passthrough(); + export const databaseListSchema = z .object({ sessions: z.array( diff --git a/src/ida/IdaSessionClient.test.ts b/src/ida/IdaSessionClient.test.ts index e43f18cf7..b41a3c34a 100644 --- a/src/ida/IdaSessionClient.test.ts +++ b/src/ida/IdaSessionClient.test.ts @@ -515,7 +515,7 @@ describe("headless IDA transient busy health", () => { }); it.each([ - ["free-form text", "token=abc123 /private/home/user secret"], + ["free-form text", "token=abc123 /private secret"], ["an over-long name", "x".repeat(200)], ])( "reports %s from the worker as unknown instead of echoing it", @@ -899,6 +899,85 @@ describe("headless IDA reaped worker cleanup", () => { }); }); +/** 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)( + "retains the private input after %s success with incomplete initial discovery", + async (kind) => { + const { producer, client, root } = await fixture("headless"); + let hidden = true; + let ownedId: string | undefined; + let privateInput: string | undefined; + producer.beforeCall = async (name, args) => { + if (name !== "idb_open") return; + ownedId = z.string().parse(args.preferred_session_id); + privateInput = z.string().parse(args.input_path); + producer.database = ownedId; + producer.inputPath = privateInput; + }; + producer.overrideCall = (name) => { + if (name === "idb_open") { + const originalId = z.string().parse(ownedId); + const originalInput = z.string().parse(privateInput); + if (kind === "foreign-input") + return { + success: true, + session: { + session_id: "foreign-session", + input_path: "/foreign/input", + }, + }; + if (kind === "malformed") + return { success: true, session: { session_id: originalId } }; + return { + success: true, + session: { session_id: originalId, input_path: originalInput }, + }; + } + if (name === "idb_list" && hidden) return { sessions: [] }; + return undefined; + }; + + expect((await client.execute("health", {})).ok).toBe(false); + const incomplete = await client.close(); + expect(incomplete.ok).toBe(false); + if (!incomplete.ok) expect(incomplete.error.cleanupIncomplete).toBe(true); + expect(producer.closes).toBe(0); + expect(producer.database).toBe(ownedId); + expect( + (await readdir(root)).some((name) => name.startsWith("rea-ida-")), + ).toBe(true); + expect(producer.calls.some(({ name }) => name === "idb_close")).toBe( + false, + ); + + // A later positive private ownership record permits ordinary save=false + // cleanup, without ever adopting the foreign open-response identity. + hidden = false; + expect((await client.close()).ok).toBe(true); + expect(producer.calls.filter(({ name }) => name === "idb_close")).toEqual( + [{ name: "idb_close", args: { database: ownedId, save: false } }], + ); + expect(producer.closes).toBe(1); + expect(await readdir(root)).toEqual(["target.elf"]); + }, + ); + + it("preserves cleanup after a completed, explicit idb_open rejection", async () => { + const { producer, client, root } = await fixture("headless"); + producer.overrideCall = (name) => + name === "idb_open" + ? { error: "Input was rejected before creating a session" } + : undefined; + + expect((await client.execute("health", {})).ok).toBe(false); + expect((await client.close()).ok).toBe(true); + expect(producer.calls.some(({ name }) => name === "idb_close")).toBe(false); + expect(producer.closes).toBe(1); + expect(await readdir(root)).toEqual(["target.elf"]); + }); +}); + describe("headless IDA cleanup verification", () => { it("retries failed worker release through the retained connection before removing its workspace", async () => { const { producer, client, root } = await fixture("headless"); @@ -943,23 +1022,6 @@ describe("headless IDA cleanup verification", () => { ).toHaveLength(1); expect(await readdir(root)).toEqual(["target.elf"]); }); - it("never closes an unrelated database when startup returns a different input identity", async () => { - const { producer, client, root } = await fixture("headless"); - producer.overrideCall = (name) => - name === "idb_open" - ? { - success: true, - session: { - session_id: "user-database", - input_path: "/unrelated/input", - }, - } - : undefined; - expect((await client.execute("health", {})).ok).toBe(false); - expect((await client.close()).ok).toBe(true); - expect(producer.calls.some(({ name }) => name === "idb_close")).toBe(false); - expect(await readdir(root)).toEqual(["target.elf"]); - }); it("releases a privately owned worker after malformed open output using verified session inventory", async () => { const { producer, client, root } = await fixture("headless"); producer.beforeCall = async (name, args) => { diff --git a/src/ida/IdaSessionClient.ts b/src/ida/IdaSessionClient.ts index c940e4c6a..14b20b107 100644 --- a/src/ida/IdaSessionClient.ts +++ b/src/ida/IdaSessionClient.ts @@ -43,6 +43,7 @@ import { legacyMetadataSchema, modernHealthSchema, openDatabaseSchema, + rejectedOpenDatabaseSchema, } from "./IdaProtocolValues.js"; import { idaFileDigest, @@ -122,6 +123,7 @@ export class IdaSessionClient implements AnalysisClient { #databaseCloseCompleted = false; #openAttempted = false; #openCompleted = false; + #openRejected = false; #callGraphAdvertised = false; #surveyAdvertised = false; #initialIdentity: string | undefined; @@ -469,6 +471,9 @@ export class IdaSessionClient implements AnalysisClient { preferred_session_id: this.#workspace.sessionId, }); this.#openCompleted = true; + // A completed but malformed/positive open may still own a live Worker. + // Only an explicit producer rejection keeps the old no-worker branch. + this.#openRejected = rejectedOpenDatabaseSchema.safeParse(raw).success; this.#lifecycle.push(raw); const opened = openDatabaseSchema.parse(raw); if ( @@ -633,6 +638,14 @@ export class IdaSessionClient implements AnalysisClient { throw new AnalysisProtocolError( "An unowned worker still uses the private workspace; it was retained.", ); + if ( + this.#database === undefined && + this.#openCompleted && + !this.#openRejected + ) + throw new AnalysisProtocolError( + "IDA open returned without a verified private worker binding. An empty session inventory does not prove that its Worker exited; the private workspace was retained.", + ); } if (this.#database !== undefined) { if (!this.#databaseCloseCompleted) {