fix(ghidra): retain request limits and startup cleanup

Preserve the full request options through the FIFO and settle interrupted or rejected final lineage observation through owned startup cleanup. Keep the startup diagnostic checkpoint until finalization succeeds.

Validate typed expired ping, cancellation, child exit, runtime removal, retry, and MCP recovery without running a native Ghidra instance.

Refs: #4289, #4274
This commit is contained in:
morluto
2026-10-16 08:26:18 +08:00
parent 018ab2c5d9
commit 5f0d18a534
5 changed files with 144 additions and 18 deletions
+13 -1
View File
@@ -731,8 +731,20 @@ export class GhidraClient {
this.#startupOutputCheckpoint = this.#process.checkpointOutput();
const completed = await this.#connectAndHandshake(endpoint, deadline);
if (!completed.ok) return this.#startupFailure(completed.error, deadline);
try {
await this.#lineage.observe(this.#launch);
} catch (cause: unknown) {
return this.#startupFailure(
this.#failure(
"start",
"Ghidra process lineage observation failed",
cause,
),
deadline,
);
}
if (deadline.signal.aborted) return this.#startupInterrupted(deadline);
this.#startupOutputCheckpoint = undefined;
await this.#lineage.observe(this.#launch);
this.#requestQueue.reopen();
return completed;
}
+31
View File
@@ -10,6 +10,37 @@ const failure = bindGhidraSessionFailure(() => ({
}));
describe("Ghidra request queue failure settlement", () => {
it("forwards the request deadline to the executor", async () => {
const optionsSeen: { timeoutMs?: number }[] = [];
const queue = new GhidraRequestQueue(
(_method, _parameters, options) => {
optionsSeen.push(options);
return Promise.resolve(
options.timeoutMs === 0
? err(
failure(
"timeout",
"Ghidra request deadline elapsed",
undefined,
{ timeoutMs: 0 },
),
)
: ok(null),
);
},
failure,
() => Promise.resolve(ok(null)),
);
await expect(
queue.run("ping", {}, { timeoutMs: 0 }),
).resolves.toMatchObject({
ok: false,
error: { kind: "timeout", timeoutMs: 0 },
});
expect(optionsSeen).toEqual([{ timeoutMs: 0 }]);
});
it.each([new TypeError("Decoder rejected"), "decoder rejected", null])(
"preserves a rejected executor cause (%j) and drains the next request",
async (cause) => {
+10 -14
View File
@@ -30,7 +30,7 @@ export type GhidraActiveCancellationHandler = () => Promise<
interface QueuedRequest {
readonly method: string;
readonly parameters: JsonValue;
readonly signal: AbortSignal | undefined;
readonly options: GhidraRequestOptions;
readonly resolve: (result: GhidraQueuedRequestResult) => void;
readonly onAbort: (() => void) | undefined;
}
@@ -72,7 +72,7 @@ export class GhidraRequestQueue {
entry = {
method,
parameters,
signal: options.signal,
options,
resolve,
onAbort,
};
@@ -102,7 +102,7 @@ export class GhidraRequestQueue {
const entry = this.#queue.shift();
if (entry === undefined) return;
this.#release(entry);
if (entry.signal?.aborted === true) {
if (entry.options.signal?.aborted === true) {
entry.resolve(
err(this.failure("cancelled", "Ghidra request was cancelled")),
);
@@ -113,7 +113,7 @@ export class GhidraRequestQueue {
let executionSettled = false;
let activeCancellation: Promise<Result<null, AnalysisError>> | undefined;
const onAbort =
entry.signal === undefined
entry.options.signal === undefined
? undefined
: () => {
if (executionSettled) return;
@@ -126,12 +126,8 @@ export class GhidraRequestQueue {
activeCancellation = this.onActiveCancellation();
};
if (onAbort !== undefined)
entry.signal?.addEventListener("abort", onAbort, { once: true });
void this.execute(
entry.method,
entry.parameters,
entry.signal === undefined ? {} : { signal: entry.signal },
)
entry.options.signal?.addEventListener("abort", onAbort, { once: true });
void this.execute(entry.method, entry.parameters, entry.options)
.then(async (result) => {
if (activeCancellation !== undefined) {
const primary = result.ok
@@ -161,8 +157,8 @@ export class GhidraRequestQueue {
);
})
.finally(() => {
if (entry.signal !== undefined && onAbort !== undefined)
entry.signal.removeEventListener("abort", onAbort);
if (entry.options.signal !== undefined && onAbort !== undefined)
entry.options.signal.removeEventListener("abort", onAbort);
this.#active = false;
if (this.#closedFailure === undefined) this.#drain();
});
@@ -223,7 +219,7 @@ export class GhidraRequestQueue {
}
#release(entry: QueuedRequest): void {
if (entry.signal !== undefined && entry.onAbort !== undefined)
entry.signal.removeEventListener("abort", entry.onAbort);
if (entry.options.signal !== undefined && entry.onAbort !== undefined)
entry.options.signal.removeEventListener("abort", entry.onAbort);
}
}
+2 -3
View File
@@ -248,9 +248,8 @@ const disconnect = async (
child.on("message", received);
child.send("disconnect");
});
await expect
.poll(async () => (await client.ping({ timeoutMs: 200 })).ok)
.toBe(false);
const disconnectedPing = await client.ping({ timeoutMs: 200 });
expect(disconnectedPing.ok).toBe(false);
expect(child.exitCode).toBeNull();
expect(child.signalCode).toBeNull();
};
@@ -14,6 +14,7 @@ import type { ProgressUpdate } from "../../../../src/application/ProgressReporte
import { ok } from "../../../../src/domain/result.js";
import { GhidraClient } from "../../../../src/ghidra/GhidraClient.js";
import { ProviderRunLineage } from "../../../../src/process/ProviderRunLineage.js";
import { OwnedFileHandle } from "../../../../src/filesystem/OwnedFileHandle.js";
import { GHIDRA_PROCESS_DIAGNOSTIC_BYTES } from "../../../../src/ghidra/GhidraDiagnostics.js";
import { GhidraHeadlessLauncher } from "../../../../src/ghidra/GhidraLauncher.js";
@@ -561,6 +562,83 @@ describe("GhidraClient", () => {
});
describe("GhidraClient startup lifecycle", () => {
it("retains startup failure and cleans up when lineage observation rejects", async () => {
const cause = new TypeError("Lineage reader rejected");
const observe = vi.spyOn(ProviderRunLineage.prototype, "observe");
onTestFinished(() => observe.mockRestore());
observe.mockRejectedValueOnce(cause);
const launcher = new FixtureLauncher();
const client = clientFor(launcher);
const result = await client.start();
expect(result).toMatchObject({
ok: false,
error: {
kind: "start",
message: "Ghidra process lineage observation failed",
},
});
if (result.ok) throw new Error("Expected lineage observation failure");
expect(result.error.cause).toBe(cause);
const child = launcher.processes[0];
if (child === undefined) throw new Error("fixture child was not launched");
await expect(waitForChildExit(child, 5_000)).resolves.toBe(true);
await expect(access(launcher.runtimeRoots[0] ?? "")).rejects.toMatchObject({
code: "ENOENT",
});
expect(client.isReady()).toBe(false);
await expect(client.start()).resolves.toMatchObject({ ok: true });
expect(launcher.processes).toHaveLength(2);
});
it("cleans up when the final waiter cancels during lineage observation", async () => {
let markObservationPending: (() => void) | undefined;
const observationPending = new Promise<void>((resolve) => {
markObservationPending = resolve;
});
let releaseObservation: (() => void) | undefined;
const observationGate = new Promise<void>((resolve) => {
releaseObservation = resolve;
});
const originalObserve = ProviderRunLineage.prototype.observe;
const observe = vi.spyOn(ProviderRunLineage.prototype, "observe");
onTestFinished(() => observe.mockRestore());
observe.mockImplementationOnce(async function (
this: ProviderRunLineage,
launch,
) {
markObservationPending?.();
await observationGate;
return await originalObserve.call(this, launch);
});
const launcher = new FixtureLauncher();
const client = clientFor(launcher);
const controller = new AbortController();
const starting = client.start(controller.signal);
await observationPending;
const child = launcher.processes[0];
if (child === undefined) throw new Error("fixture child was not launched");
controller.abort();
releaseObservation?.();
await expect(starting).resolves.toMatchObject({
ok: false,
error: { kind: "cancelled" },
});
await expect(waitForChildExit(child, 5_000)).resolves.toBe(true);
await expect(access(launcher.runtimeRoots[0] ?? "")).rejects.toMatchObject({
code: "ENOENT",
});
await expect(client.callToolIfReady("list_strings", {})).resolves.toBe(
undefined,
);
await expect(client.start()).resolves.toMatchObject({ ok: true });
expect(launcher.processes).toHaveLength(2);
});
it("applies one startup deadline and removes the private runtime", async () => {
vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout", "Date"] });
const launcher = new FixtureLauncher("silent");
@@ -603,6 +681,16 @@ describe("GhidraClient startup lifecycle", () => {
});
describe("GhidraClient established requests", () => {
it("preserves an expired ping deadline through the request queue", async () => {
const client = clientFor(new FixtureLauncher());
await expect(client.start()).resolves.toMatchObject({ ok: true });
await expect(client.ping({ timeoutMs: 0 })).resolves.toMatchObject({
ok: false,
error: { kind: "timeout", timeoutMs: 0 },
});
});
it("fails an established function request when the provider process exits", async () => {
const client = clientFor(new FixtureLauncher("exit_tools"));
await expect(client.start()).resolves.toMatchObject({ ok: true });