diff --git a/src/ghidra/GhidraClient.ts b/src/ghidra/GhidraClient.ts index 66b09caae9..214ed73a8b 100644 --- a/src/ghidra/GhidraClient.ts +++ b/src/ghidra/GhidraClient.ts @@ -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; } diff --git a/src/ghidra/GhidraRequestQueue.test.ts b/src/ghidra/GhidraRequestQueue.test.ts index c529295e7b..1be5007e23 100644 --- a/src/ghidra/GhidraRequestQueue.test.ts +++ b/src/ghidra/GhidraRequestQueue.test.ts @@ -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) => { diff --git a/src/ghidra/GhidraRequestQueue.ts b/src/ghidra/GhidraRequestQueue.ts index e3072dab27..0b03d448ae 100644 --- a/src/ghidra/GhidraRequestQueue.ts +++ b/src/ghidra/GhidraRequestQueue.ts @@ -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> | 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); } } diff --git a/tests/boundary/mcp/ghidraReopen.test.ts b/tests/boundary/mcp/ghidraReopen.test.ts index 7af55924af..a69b41d65a 100644 --- a/tests/boundary/mcp/ghidraReopen.test.ts +++ b/tests/boundary/mcp/ghidraReopen.test.ts @@ -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(); }; diff --git a/tests/boundary/providers/ghidra/ghidraClient.test.ts b/tests/boundary/providers/ghidra/ghidraClient.test.ts index 452c122f7a..5cc3d20756 100644 --- a/tests/boundary/providers/ghidra/ghidraClient.test.ts +++ b/tests/boundary/providers/ghidra/ghidraClient.test.ts @@ -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((resolve) => { + markObservationPending = resolve; + }); + let releaseObservation: (() => void) | undefined; + const observationGate = new Promise((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 });