fix(ida): retain cleanup facts and let one-shot commands exit (#2814)

* 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>

* fix(ida): preserve retained session inventory facts

---------

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:
morluto 2026-10-13 22:27:26 +08:00 committed by GitHub
parent c85abacdb2
commit 7369b5ee63
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 80 additions and 10 deletions

View File

@ -1077,6 +1077,7 @@ describe("headless IDA cleanup diagnostics", () => {
backend: "worker",
owned: false,
input_path: producer.inputPath,
is_active: false,
},
),
);
@ -1368,6 +1369,62 @@ describe("headless IDA cleanup verification", () => {
});
});
it("retains a conflicting requested session ID during pre-close recovery", async () => {
const { producer, client, root } = await fixture("headless");
producer.overrideCall = (name) =>
name === "idb_list"
? {
sessions: [
{
session_id: String(producer.database),
input_path: "/conflicting/input",
owned: true,
backend: "worker",
is_active: false,
},
],
}
: undefined;
expect((await client.execute("health", {})).ok).toBe(false);
const requested = String(producer.database);
const result = await client.close();
expect(result).toMatchObject(
reportsSessions({
session_id: requested,
input_path: "/conflicting/input",
owned: true,
backend: "worker",
is_active: false,
}),
);
expect(producer.calls.some(({ name }) => name === "idb_close")).toBe(false);
expect(
(await readdir(root)).some((name) => name.startsWith("rea-ida-")),
).toBe(true);
});
it.each(["unavailable", "malformed"] as const)(
"does not invent a worker after explicit rejection and %s inventory",
async (kind) => {
const { producer, client } = await fixture("headless");
producer.overrideCall = (name) =>
name === "idb_open"
? { error: "rejected before worker creation" }
: name === "idb_list" && kind === "malformed"
? {}
: undefined;
expect((await client.execute("health", {})).ok).toBe(false);
producer.beforeCall = async (name) => {
if (name === "idb_list" && kind === "unavailable")
throw new Error("inventory unavailable");
};
const result = await client.close();
expect(result.ok).toBe(false);
if (result.ok) throw new Error("Expected retained cleanup");
expect(result.error).not.toHaveProperty("diagnostics.upstream_sessions");
},
);
it.each(["idb_close", "idb_list", "connection_close"])(
"preserves transport stderr through cleanup at %s",
async (stage) => {

View File

@ -98,6 +98,8 @@ interface UpstreamSession {
readonly owned: boolean | null;
/** Last inventoried input binding; null until upstream reports it. */
readonly input_path: string | null;
/** Last inventoried activity; null before upstream reports it. */
readonly is_active: boolean | null;
}
/** A failed cleanup retains its private workspace and reports the remaining resource. */
@ -130,15 +132,16 @@ class IdaCleanupError extends ProviderAdapterError {
upstream_sessions: {
state: "unverified",
sessions: sessions.map(
({ session_id, backend, owned, input_path }) => ({
({ session_id, backend, owned, input_path, is_active }) => ({
session_id,
backend,
owned,
input_path,
is_active,
}),
),
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.",
"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. is_active preserves registry activity and is null when unknown; it does not establish process liveness. 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.",
},
}),
},
@ -691,7 +694,12 @@ export class IdaSessionClient implements AnalysisClient {
*/
#unreleasedSessions(): readonly UpstreamSession[] | undefined {
if (this.#blockingSessions !== undefined) return this.#blockingSessions;
if (!this.#openAttempted || this.#workspace === undefined) return undefined;
if (
!this.#openAttempted ||
this.#workspace === undefined ||
this.#openRejected
)
return undefined;
return this.#databaseCloseCompleted
? []
: [
@ -700,6 +708,7 @@ export class IdaSessionClient implements AnalysisClient {
backend: "worker",
owned: null,
input_path: null,
is_active: null,
},
];
}
@ -719,12 +728,15 @@ export class IdaSessionClient implements AnalysisClient {
): boolean {
this.#blockingSessions =
entries.length > 0
? entries.map(({ session_id, backend, owned, input_path }) => ({
session_id,
backend,
owned,
input_path,
}))
? entries.map(
({ session_id, backend, owned, input_path, is_active }) => ({
session_id,
backend,
owned,
input_path,
is_active,
}),
)
: undefined;
return entries.length > 0;
}
@ -781,7 +793,7 @@ export class IdaSessionClient implements AnalysisClient {
);
if (this.#database === undefined && blocked)
throw new AnalysisProtocolError(
"An unowned worker still uses the private workspace; it was retained.",
"A registered session still matches the requested ID or private input without a verified private worker binding; the workspace was retained.",
);
if (
this.#database === undefined &&

View File

@ -79,6 +79,7 @@ describe("headless IDA timeout through the one-shot CLI", () => {
backend: "worker",
owned: null,
input_path: null,
is_active: null,
},
],
},