From 7346397106544c5aa262b0daabfd8ef84079f95e Mon Sep 17 00:00:00 2001 From: Kaoru0822 <208281322+kaoru0822-kitauji@users.noreply.github.com> Date: Wed, 14 Oct 2026 02:40:48 +0000 Subject: [PATCH] fix(ghidra): keep earlier LaunchSupport warnings when a later helper fails d4bd65c53 moved a failed LaunchSupport helper's output into the nested `command_outcome`, but the cause projection also returned early for those failures and dropped `launch_support`. A `-vmargs` failure therefore lost the warning that the successful `-envvars` helper had already reported. Keep the retained helper outputs and skip only the launcher outcome that the nested cause now owns. Update the Ghidra launch and Hopper startup boundary expectations to current main: failed helpers report through `cause.command_outcome`, and bridge startup results carry the launched `processId` (599a8ee11), asserted against the exact retried process. Co-Authored-By: Claude Opus 5.5 --- src/ghidra/GhidraSessionError.ts | 4 ++- .../providers/ghidra/ghidraJavaLaunch.test.ts | 20 +++++++++----- .../hopperClient.startupFailures.test.ts | 27 ++++++++++++++----- 3 files changed, 38 insertions(+), 13 deletions(-) diff --git a/src/ghidra/GhidraSessionError.ts b/src/ghidra/GhidraSessionError.ts index dedf27bd4..9f4abfb52 100644 --- a/src/ghidra/GhidraSessionError.ts +++ b/src/ghidra/GhidraSessionError.ts @@ -263,7 +263,6 @@ const causeLaunchOutcome = ( redact: (value: string, incomplete?: boolean) => string, ): Record => { if (!(cause instanceof GhidraLaunchError)) return {}; - if (cause.cause instanceof GhidraLaunchSupportError) return {}; const launchSupport = cause.launchSupport.length === 0 ? {} @@ -273,6 +272,9 @@ const causeLaunchOutcome = ( stderr: redact(output.stderr), })), }; + // A failed LaunchSupport helper reports its own command outcome in the + // nested cause; earlier helper warnings still belong to this launch. + if (cause.cause instanceof GhidraLaunchSupportError) return launchSupport; const output = cause.capturedOutput; if (output === undefined) return launchSupport; const stdout = boundedUtf8Text( diff --git a/tests/boundary/providers/ghidra/ghidraJavaLaunch.test.ts b/tests/boundary/providers/ghidra/ghidraJavaLaunch.test.ts index 9b80722e7..09ad518ab 100644 --- a/tests/boundary/providers/ghidra/ghidraJavaLaunch.test.ts +++ b/tests/boundary/providers/ghidra/ghidraJavaLaunch.test.ts @@ -188,9 +188,13 @@ describe.skipIf(process.platform === "win32")( const cause = failure.diagnostics.failure_cause; if (mode === "env-failure") { expect(cause).toMatchObject({ - launch_outcome: { - exit_code: 3, - stderr: expect.stringContaining(`-envvars: ${warning}`), + cause: { + name: "GhidraLaunchSupportError", + command_outcome: { + killed: false, + stderr: expect.stringContaining(`-envvars: ${warning}`), + }, + cause: { code: 3 }, }, }); } else { @@ -205,9 +209,13 @@ describe.skipIf(process.platform === "win32")( } if (mode === "vm-failure") expect(cause).toMatchObject({ - launch_outcome: { - exit_code: 3, - stderr: expect.stringContaining(`-vmargs: ${warning}`), + cause: { + name: "GhidraLaunchSupportError", + command_outcome: { + killed: false, + stderr: expect.stringContaining(`-vmargs: ${warning}`), + }, + cause: { code: 3 }, }, }); if (mode === "vm-malformed") diff --git a/tests/boundary/providers/hopper/hopperClient.startupFailures.test.ts b/tests/boundary/providers/hopper/hopperClient.startupFailures.test.ts index 3d96ddb67..391a7a47b 100644 --- a/tests/boundary/providers/hopper/hopperClient.startupFailures.test.ts +++ b/tests/boundary/providers/hopper/hopperClient.startupFailures.test.ts @@ -41,6 +41,7 @@ class CancelThenFixtureLauncher implements BridgeLauncher { readonly firstLaunch = new Promise((resolve) => { this.#firstLaunchReady = resolve; }); + retriedProcess: ChildProcess | undefined; async launch(session: BridgeSession) { this.#launches += 1; @@ -49,6 +50,7 @@ class CancelThenFixtureLauncher implements BridgeLauncher { : new FixtureLauncher().launch(session)); if (this.#launches === 1 && launched.ok) this.#firstLaunchReady?.(launched.value.process); + else if (launched.ok) this.retriedProcess = launched.value.process; return launched; } } @@ -256,9 +258,17 @@ describe("HopperClient startup failures", () => { expect( firstProcess.exitCode !== null || firstProcess.signalCode !== null, ).toBe(true); - await expect(client.start()).resolves.toEqual({ + const restarted = await client.start(); + const retriedProcess = launcher.retriedProcess; + expect(retriedProcess).toBeDefined(); + expect(retriedProcess).not.toBe(firstProcess); + expect(restarted).toEqual({ ok: true, - value: { name: "REA Hopper bridge", version: "1.0.0" }, + value: { + name: "REA Hopper bridge", + version: "1.0.0", + processId: retriedProcess?.pid, + }, }); await expect( client.callTool("echo", { value: "retried" }), @@ -282,11 +292,16 @@ describe("HopperClient startup failures", () => { error: { _tag: "HopperCancelledError" }, }); expect(launcher.processes).toHaveLength(0); - await expect(client.start()).resolves.toEqual({ - ok: true, - value: { name: "REA Hopper bridge", version: "1.0.0" }, - }); + const started = await client.start(); expect(launcher.processes).toHaveLength(1); + expect(started).toEqual({ + ok: true, + value: { + name: "REA Hopper bridge", + version: "1.0.0", + processId: launcher.processes[0]?.pid, + }, + }); await expect( client.callTool("echo", { value: "retried" }), ).resolves.toEqual({