From e8528cedcdfb1ca1d769304afdc353936e8e0d70 Mon Sep 17 00:00:00 2001 From: Kaoru0822 <208281322+kaoru0822-kitauji@users.noreply.github.com> Date: Sun, 11 Oct 2026 01:38:34 -0700 Subject: [PATCH] fix(hopper): let failed one-shot commands exit while retaining a lease (#1763) * fix(hopper): let failed one-shot commands exit while retaining a lease A Hopper target lease retained after unconfirmed cleanup kept its listening socket referenced, so a one-shot CLI emitted its final cleanup_incomplete result but never exited, and its live lease rejected every retry for the same target as already open. Unreference the lease listener. A live MCP process still holds the lease for as long as it runs; a completed CLI exits with status 1, and the stale socket it leaves is recovered by the next acquirer. Fixes #1751 Co-Authored-By: Claude Opus 5.5 * test(hopper): remove only this test's stale lease sockets Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- src/hopper/HopperTargetLease.ts | 4 + .../cli/hopperLaunchFailureCli.test.ts | 128 ++++++++++++++++++ 2 files changed, 132 insertions(+) create mode 100644 tests/boundary/cli/hopperLaunchFailureCli.test.ts diff --git a/src/hopper/HopperTargetLease.ts b/src/hopper/HopperTargetLease.ts index cfa35124b6..af65b729fc 100644 --- a/src/hopper/HopperTargetLease.ts +++ b/src/hopper/HopperTargetLease.ts @@ -131,6 +131,10 @@ const tryAcquireLease = async ( owner: HopperTargetLeaseOwner, ): Promise => { const server = createServer((socket) => sendOwner(socket, owner)); + // The lease lives exactly as long as its owning process. A lease retained + // after unconfirmed cleanup must not keep a completed one-shot command + // alive; process exit leaves a stale socket that later acquirers recover. + server.unref(); if (!(await bind(server, socketPath))) { const existingOwner = await readOwner(socketPath); return typeof existingOwner === "string" diff --git a/tests/boundary/cli/hopperLaunchFailureCli.test.ts b/tests/boundary/cli/hopperLaunchFailureCli.test.ts new file mode 100644 index 0000000000..74f4b16613 --- /dev/null +++ b/tests/boundary/cli/hopperLaunchFailureCli.test.ts @@ -0,0 +1,128 @@ +import { chmod, copyFile, mkdir, readdir, rm, rmdir } from "node:fs/promises"; +import { createConnection } from "node:net"; +import { join } from "node:path"; + +import { describe, expect, onTestFinished } from "vitest"; +import { z } from "zod"; + +import { workspaceCliTest } from "../../support/cli/workspaceCliFixture.js"; + +import { HOPPER_TARGET_LEASE_DIRECTORY } from "../../../src/hopper/HopperTargetLease.js"; + +const FAILED_LAUNCHER = `#!/bin/sh +printf 'Cannot launch Hopper!\\nNot authorized to send Apple events to Hopper Disassembler.\\n' >&2 +exit 2 +`; + +/** The private runtime directory a failed launch reports as retained. */ +const RetainedRuntime = z.object({ + details: z.object({ + diagnostics: z.object({ + cleanup_error: z.object({ + details: z.object({ + diagnostics: z.object({ runtime_root: z.string() }), + }), + }), + }), + }), +}); + +const listLeases = async (): Promise> => + new Set(await readdir(HOPPER_TARGET_LEASE_DIRECTORY).catch(() => [])); + +const refusesConnections = (socketPath: string): Promise => + new Promise((resolve) => { + const socket = createConnection(socketPath); + socket.once("connect", () => { + socket.destroy(); + resolve(false); + }); + socket.once("error", (error: NodeJS.ErrnoException) => + resolve(error.code === "ECONNREFUSED"), + ); + }); + +/** + * Remove a lease socket only when its owner has exited, holding the same + * recovery reservation as production so a concurrent acquirer is never + * unlinked. Live or busy leases are left for their owners. + */ +const removeStaleLease = async (socketPath: string): Promise => { + const recoveryPath = `${socketPath}.recovery`; + try { + await mkdir(recoveryPath, { mode: 0o700 }); + } catch { + return; + } + try { + if (await refusesConnections(socketPath)) + await rm(socketPath, { force: true }); + } finally { + await rmdir(recoveryPath); + } +}; + +describe("Hopper launcher failure through the one-shot CLI", () => { + // Hopper target leases are acquired only for the macOS launcher route. + workspaceCliTest.skipIf(process.platform !== "darwin")( + "exits after a retained-cleanup failure and releases the target to a retry", + async ({ cli, workspace }) => { + const launcherPath = await workspace.write( + "failure-helper", + FAILED_LAUNCHER, + ); + await chmod(launcherPath, 0o700); + const targetPath = workspace.path("probe"); + await copyFile("/usr/bin/true", targetPath); + const leasesBefore = await listLeases(); + onTestFinished(async () => { + for (const name of await listLeases()) + if (!leasesBefore.has(name) && name.endsWith(".sock")) + await removeStaleLease(join(HOPPER_TARGET_LEASE_DIRECTORY, name)); + }); + + for (const attempt of ["first", "retry"]) { + const result = await cli.run({ + arguments: ["analyze", targetPath, "--provider", "hopper", "--json"], + environment: { + ...workspace.environment, + HOPPER_LAUNCHER_PATH: launcherPath, + REA_LOG_LEVEL: "silent", + }, + timeoutMs: 30_000, + }); + const retained = RetainedRuntime.safeParse(result.json); + if (retained.success) + await rm( + retained.data.details.diagnostics.cleanup_error.details.diagnostics + .runtime_root, + { recursive: true, force: true }, + ); + + expect(result.exitCode, attempt).toBe(1); + expect(result.json, attempt).toMatchObject({ + code: "cleanup_incomplete", + details: { + cleanup: "incomplete", + diagnostics: { + primary_error: { + code: "provider_unavailable", + details: { + stage: "launch", + launcher: { + exit_code: 2, + stderr: { + text: expect.stringContaining( + "Not authorized to send Apple events", + ), + }, + }, + }, + }, + }, + }, + }); + } + }, + ); +});