From c105391db53e2e2ab8b8624d5f3870c14aae3637 Mon Sep 17 00:00:00 2001 From: Ysopwnable <161784452+N0zoM1z0@users.noreply.github.com> Date: Tue, 13 Oct 2026 12:59:22 +0800 Subject: [PATCH] fix(hopper): stop Linux demo children when the REA owner exits (#2499) Bind the owned demo helper to REA's private stdin stream and unwind child cleanup on EOF during startup or analysis. Verify graceful and SIGKILL owner exit through real CLI/MCP workflows and preserve unrelated processes. Assisted-by: Reimu (gensokyo-skills:reimu-incident-triage) --- docs/installation.md | 19 ++- scripts/hopper-demo-x11.py | 56 +++++++- scripts/lib/real-hopper-owner-exit.mjs | 130 ++++++++++++++++++ scripts/verify-real-hopper.mjs | 9 ++ src/hopper/BridgeLauncher.ts | 3 + .../providers/hopper/bridgeLauncher.test.ts | 58 ++++++++ .../hopper/linuxPrivateDisplay.test.ts | 20 +++ tests/fixtures/hopperLifetimeLaunch.py | 16 +++ tests/fixtures/hopperOwnerLifetime.py | 110 +++++++++++++++ 9 files changed, 412 insertions(+), 9 deletions(-) create mode 100644 scripts/lib/real-hopper-owner-exit.mjs create mode 100644 tests/fixtures/hopperLifetimeLaunch.py create mode 100644 tests/fixtures/hopperOwnerLifetime.py diff --git a/docs/installation.md b/docs/installation.md index b9285a7d40..0a8041898c 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -317,8 +317,8 @@ The Hopper launcher action used by REA creates a document from an executable; its supported command-line interface does not attach to an already-open document. While another live REA session owns the same target and loader profile, a second session reports the owning run ID instead of opening a -duplicate document. Closing the owning REA session releases this guard; Hopper -keeps its document open. A later REA session can therefore open another +duplicate document. On macOS, closing the owning REA session releases this guard; +Hopper keeps its document open. A later REA session can therefore open another document for the same target. REA does not currently identify, focus, or reuse that existing GUI document, and the supported launcher exposes no attach or reuse action for it. @@ -341,8 +341,14 @@ Hopper application per user across cooperating REA processes. A competing CLI or MCP session receives the owning session ID before Hopper is launched, for both the same target and a different target. Switch targets through that MCP session, or close it before starting another session. Closing the owner -releases the reservation. This does not attach to or claim ownership of -manually opened Hopper applications. +releases the reservation and stops REA's owned demo application and private +display; the headless Linux demo does not retain documents after MCP shutdown. +Its helper watches a private lifetime pipe, so an owner crash or SIGKILL also +stops those children, without relying on a reusable PID or a process-name scan. +A later session recovers the dead owner's stale lease after child cleanup; +private runtime files can remain when REA itself was killed. This does not attach to +or claim ownership of manually opened Hopper applications. An application left +running by an older REA version must still be closed before starting a new session. Stale lease recovery is serialized. If an interrupted recovery leaves its reservation directory, the diagnostic identifies that exact path. Stop REA @@ -435,8 +441,9 @@ For cursor navigation, string decoding, symbol-name limits, and regex matching, see [Hopper analysis behavior](native-investigation.md#hopper-analysis-behavior). Closing or switching a target closes its bound Hopper document, shuts down REA's -bridge and removes its temporary socket directory while preserving the Hopper -application and unrelated documents. A `cleanup_incomplete` +bridge and removes its temporary socket directory. The macOS Hopper application +and unrelated documents remain open; the owned Linux demo application stops. +A `cleanup_incomplete` result identifies resources whose cleanup could not be verified. REA retains unresolved cleanup ownership and any confirmed shutdown phases. After addressing the reported failure, retry `close_binary` on the same diff --git a/scripts/hopper-demo-x11.py b/scripts/hopper-demo-x11.py index 470c9fd28f..da2fe884f7 100644 --- a/scripts/hopper-demo-x11.py +++ b/scripts/hopper-demo-x11.py @@ -10,6 +10,7 @@ import json import os from pathlib import Path import secrets +import select import stat import subprocess import tempfile @@ -118,6 +119,7 @@ class XWindowAttributes(ctypes.Structure): def parse_args() -> argparse.Namespace: parser = argparse.ArgumentParser() parser.add_argument("--probe", action="store_true") + parser.add_argument("--owner-stdin", action="store_true") parser.add_argument("--hopper") parser.add_argument("--socket") parser.add_argument( @@ -133,6 +135,43 @@ def parse_args() -> argparse.Namespace: return parser.parse_args() +class OwnerLifetime: + """The launcher's private pipe is a lifetime capability, independent of PIDs.""" + + def __init__(self, enabled: bool) -> None: + self._poll = select.poll() if enabled else None + if self._poll is not None: + # Node/libuv can implement piped stdio with a Unix socket pair. + mode = os.fstat(0).st_mode + if not (stat.S_ISFIFO(mode) or stat.S_ISSOCK(mode)): + raise AdapterFailure( + INVALID_LAUNCH_COMMAND, + "invalid_launch_command", + "owner lifetime requires a private stdin pipe", + ) + self._poll.register(0, select.POLLIN | select.POLLHUP | select.POLLERR) + + def alive(self) -> bool: + if self._poll is None: + return True + for _, events in self._poll.poll(0): + if events & (select.POLLHUP | select.POLLERR | select.POLLNVAL): + return False + if events & select.POLLIN and not os.read(0, 4096): + return False + return True + + +def wait_for_hopper(child: subprocess.Popen[bytes], owner: OwnerLifetime) -> int: + while owner.alive(): + try: + return child.wait(timeout=0.1) + except subprocess.TimeoutExpired: + continue + # run_launch's finally owns cleanup of both direct, unreaped children. + return 0 + + def digest(path: Path) -> str: value = hashlib.sha256() with path.open("rb") as file: @@ -572,6 +611,7 @@ def start_xvfb( "-auth", str(authority), ], + stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL, stderr=subprocess.PIPE, ) @@ -627,6 +667,9 @@ def run_launch(args: argparse.Namespace) -> int: "invalid_launch_command", "missing Hopper launch coordinates", ) + owner = OwnerLifetime(args.owner_stdin) + if not owner.alive(): + return 0 hopper = Path(args.hopper) socket = Path(args.socket) try: @@ -654,6 +697,8 @@ def run_launch(args: argparse.Namespace) -> int: child: subprocess.Popen[bytes] | None = None display: int | None = None try: + if not owner.alive(): + return 0 hopper_environment = dict(os.environ) for name in ("WAYLAND_DISPLAY", "WAYLAND_SOCKET"): hopper_environment.pop(name, None) @@ -666,7 +711,10 @@ def run_launch(args: argparse.Namespace) -> int: "XDG_SESSION_TYPE": "x11", } ) - child = subprocess.Popen(command, env=hopper_environment) + # Children must not consume or retain REA's lifetime pipe. + child = subprocess.Popen( + command, env=hopper_environment, stdin=subprocess.DEVNULL + ) leader = os.getpid() os.environ["XAUTHORITY"] = str(authority) @@ -702,10 +750,12 @@ def run_launch(args: argparse.Namespace) -> int: observed: list[tuple[int, int, int, int, int]] = [] dialog: tuple[int, int, int, int, int] | None = None while time.monotonic() < deadline: + if not owner.alive(): + return 0 if socket.exists(): x11.XCloseDisplay(display) display = None - return child.wait() + return wait_for_hopper(child, owner) if not hopper_is_owned(leader, hopper): if child.poll() is None: time.sleep(0.1) @@ -749,7 +799,7 @@ def run_launch(args: argparse.Namespace) -> int: x11.XFlush(display) x11.XCloseDisplay(display) display = None - return child.wait() + return wait_for_hopper(child, owner) except AdapterFailure as error: raise error.with_stderr(capture) finally: diff --git a/scripts/lib/real-hopper-owner-exit.mjs b/scripts/lib/real-hopper-owner-exit.mjs new file mode 100644 index 0000000000..300b35ada3 --- /dev/null +++ b/scripts/lib/real-hopper-owner-exit.mjs @@ -0,0 +1,130 @@ +import assert from "node:assert/strict"; +import { execFile } from "node:child_process"; +import { setTimeout as delay } from "node:timers/promises"; +import { promisify } from "node:util"; + +import { Client } from "@modelcontextprotocol/client"; +import { StdioClientTransport } from "@modelcontextprotocol/client/stdio"; + +import { verifyNoTokenOwnedProcesses } from "../../dist/process/ProcessOwnership.js"; +import { + observeProcessStartIdentity, + signalProcessWithStartIdentity, +} from "../../dist/process/ProcessOwnershipObservation.js"; +import { startUnrelatedHopperSentinel } from "./unrelated-hopper-sentinel.mjs"; +import { + requireMcpEvidenceResult, + requireMcpLifecycleResult, +} from "./mcp-verifier-results.mjs"; + +const execFileAsync = promisify(execFile); + +/** Prove actual MCP owner exit releases the demo singleton for subsequent CLI/MCP sessions. */ +export async function verifyLinuxHopperOwnerExit({ + primary, + secondary, + serverCommand, + serverArgs, + environment, +}) { + const sentinel = await startUnrelatedHopperSentinel(); + const outcomes = []; + try { + for (const mode of ["graceful", "SIGKILL"]) { + const transport = new StdioClientTransport({ + command: serverCommand, + args: serverArgs, + env: environment, + stderr: "pipe", + }); + // Drain launcher diagnostics even on the deliberately interrupted owner. + transport.stderr?.resume(); + const client = new Client({ name: "hopper-owner-exit", version: "1" }); + const options = { timeout: 180_000 }; + try { + await client.connect(transport); + const ownerPid = transport.pid; + assert.ok(ownerPid !== null); + const owner = await observeProcessStartIdentity(ownerPid); + assert.equal(owner?.state, "readable"); + requireMcpLifecycleResult( + await client.callTool( + { + name: "open_binary", + arguments: { path: primary }, + }, + options, + ), + "open_binary", + ); + const procedures = requireMcpEvidenceResult( + await client.callTool( + { name: "list_procedures", arguments: {} }, + options, + ), + "list_procedures", + ); + assert.ok(procedures.length > 0); + const status = requireMcpLifecycleResult( + await client.callTool( + { name: "binary_session", arguments: {} }, + options, + ), + "binary_session", + ); + const runId = status.analysis_run?.run_id; + assert.equal(typeof runId, "string"); + assert.equal(status.analysis_provider_binding?.provider.id, "hopper"); + assert.equal((await verifyNoTokenOwnedProcesses(runId)).cleaned, false); + if (mode === "graceful") await client.close(); + else { + assert.equal( + await signalProcessWithStartIdentity( + ownerPid, + owner.identity, + "SIGKILL", + ), + "signaled", + ); + } + const deadline = Date.now() + 10_000; + let cleanup; + do { + cleanup = await verifyNoTokenOwnedProcesses(runId); + if (cleanup.cleaned) break; + await delay(100); + } while (Date.now() < deadline); + assert.equal(cleanup.cleaned, true, cleanup.reason); + // The SIGKILL case must recover the stale lease for both target identities. + for (const target of [primary, secondary]) { + const result = await execFileAsync( + process.execPath, + [ + "scripts/rea.mjs", + "inspect", + target, + "--provider", + "hopper", + "--format", + "json", + ], + { env: environment, timeout: 180_000, maxBuffer: 16 * 1024 * 1024 }, + ); + assert.ok(JSON.parse(result.stdout).normalized_result); + } + outcomes.push({ + mode, + ownedProcessesStopped: true, + sameTargetReopened: true, + differentTargetReopened: true, + }); + } finally { + await client.close(); + await transport.close(); + } + } + } finally { + assert.equal(await sentinel.verifyAndClose(), true); + } + return { outcomes, unrelatedHopperSurvived: true }; +} diff --git a/scripts/verify-real-hopper.mjs b/scripts/verify-real-hopper.mjs index 12400cb948..8cad9a2565 100644 --- a/scripts/verify-real-hopper.mjs +++ b/scripts/verify-real-hopper.mjs @@ -47,6 +47,7 @@ import { } from "./lib/real-hopper-boundaries.mjs"; import { startUnrelatedHopperSentinel } from "./lib/unrelated-hopper-sentinel.mjs"; import { snapshotHopperRuntime } from "./lib/real-hopper-cleanup.mjs"; +import { verifyLinuxHopperOwnerExit } from "./lib/real-hopper-owner-exit.mjs"; const execFileAsync = promisify(execFile); const verifierRun = createVerifierRun(); const timeout = 180_000; @@ -577,6 +578,14 @@ if (sessionsAfter.length > 0) { } if (summary === undefined) throw new Error("Real-Hopper verification did not produce a summary"); +if (process.platform === "linux") + summary.ownerExit = await verifyLinuxHopperOwnerExit({ + primary: targetA, + secondary: targetB, + serverCommand, + serverArgs, + environment: serverEnvironment, + }); const completedVerifierRun = await completeVerifierRun(verifierRun); if ( completedVerifierRun.process_lineage.status !== "verified" || diff --git a/src/hopper/BridgeLauncher.ts b/src/hopper/BridgeLauncher.ts index 8201ee2dfb..043d394a4b 100644 --- a/src/hopper/BridgeLauncher.ts +++ b/src/hopper/BridgeLauncher.ts @@ -413,6 +413,8 @@ const launchHopperProcess = async (input: { runId: input.session.runId, expectedCommand: ownershipCommand, hostEnvironment: input.options.environment, + // The helper observes EOF when this REA process exits, including SIGKILL. + stdin: "pipe", ...(input.signal === undefined ? {} : { signal: input.signal }), }); return spawn(input.argumentsForTarget); @@ -434,6 +436,7 @@ export const linuxDemoLaunch = ( return undefined; const helperArguments = [ options.demoHelperPath, + "--owner-stdin", "--strategy", strategy ?? "direct", ...(strategy === "user-mount-namespace" ? ["--mount-private-x11"] : []), diff --git a/tests/boundary/providers/hopper/bridgeLauncher.test.ts b/tests/boundary/providers/hopper/bridgeLauncher.test.ts index 722a25beda..542858a0df 100644 --- a/tests/boundary/providers/hopper/bridgeLauncher.test.ts +++ b/tests/boundary/providers/hopper/bridgeLauncher.test.ts @@ -84,6 +84,7 @@ describe("Hopper bridge launcher selection", () => { "private", "/usr/bin/python3", "/rea/hopper-demo-x11.py", + "--owner-stdin", "--strategy", "user-mount-namespace", "--mount-private-x11", @@ -117,6 +118,7 @@ describe("Hopper bridge launcher selection", () => { command: "/usr/bin/python3", args: [ "/rea/hopper-demo-x11.py", + "--owner-stdin", "--strategy", "direct", "--hopper", @@ -202,3 +204,59 @@ it("launches Hopper helpers with the caller-selected environment without restori await launched.value.releaseLease?.(); } }); + +it.skipIf(process.platform !== "linux")( + "gives the demo helper a live owner stream through Node's actual stdio transport", + async () => { + const directory = await createTestTempDirectory("rea-hopper-owner-stdio-"); + const launcher = new HopperApplicationLauncher( + { + ...options(process.execPath), + launchMode: "verified_linux_demo", + demoHelperPath: fileURLToPath( + new URL("../../../fixtures/hopperLifetimeLaunch.py", import.meta.url), + ), + }, + { + selectPrivateDisplay: async () => ({ + ok: true, + strategy: "direct", + diagnostic: { + component: "hopper_private_display", + operation: "probe", + status: "ready", + failure_code: null, + reason: "ready", + socket_directory: "/tmp/.X11-unix", + socket_directory_mode: "1777", + mount_read_only: false, + effective_socket_directory_mode: "1777", + effective_mount_read_only: false, + wsl: false, + strategy: "direct", + fallback_reason: null, + xvfb_stderr_bytes: 0, + }, + }), + }, + ); + const launched = await launcher.launch({ + directory, + socketPath: join(directory, "bridge.sock"), + token: "fixture-token", + runId: randomUUID(), + }); + if (!launched.ok) throw launched.error; + const supervisor = new ProviderProcessSupervisor(launched.value); + try { + expect(await supervisor.waitForOutputClose(5_000)).toBe(true); + expect(supervisor.snapshot().exitCode).toBe(0); + expect(JSON.parse(supervisor.snapshot().stdout.text)).toEqual({ + owner_alive: true, + }); + } finally { + await supervisor.stop(); + await launched.value.releaseLease?.(); + } + }, +); diff --git a/tests/boundary/providers/hopper/linuxPrivateDisplay.test.ts b/tests/boundary/providers/hopper/linuxPrivateDisplay.test.ts index f2c99980f6..c5fd45c965 100644 --- a/tests/boundary/providers/hopper/linuxPrivateDisplay.test.ts +++ b/tests/boundary/providers/hopper/linuxPrivateDisplay.test.ts @@ -31,6 +31,9 @@ const dialogDetectionFixturePath = fileURLToPath( const cleanupTimeoutFixturePath = fileURLToPath( new URL("../../../fixtures/hopperCleanupTimeout.py", import.meta.url), ); +const ownerLifetimeFixturePath = fileURLToPath( + new URL("../../../fixtures/hopperOwnerLifetime.py", import.meta.url), +); type DiagnosticOverrides = Partial< Omit @@ -107,6 +110,23 @@ describe("Hopper demo dialog detection", () => { }); describe("Hopper process cleanup", () => { + it.skipIf(process.platform !== "linux").each(["startup", "ready"])( + "reaps only its own children when the owner pipe closes during %s", + (phase) => { + const result = spawnSync( + "/usr/bin/python3", + [ownerLifetimeFixturePath, helperPath, phase], + { encoding: "utf8", timeout: 10_000 }, + ); + expect(result.status, result.stderr).toBe(0); + expect(JSON.parse(result.stdout)).toEqual({ + phase, + children_cleaned: true, + sentinel_alive: true, + }); + }, + ); + it.skipIf(process.platform !== "linux")( "kills a child that ignores SIGTERM after the bounded timeout", () => { diff --git a/tests/fixtures/hopperLifetimeLaunch.py b/tests/fixtures/hopperLifetimeLaunch.py new file mode 100644 index 0000000000..68d65c9ffa --- /dev/null +++ b/tests/fixtures/hopperLifetimeLaunch.py @@ -0,0 +1,16 @@ +#!/usr/bin/env python3 +"""Inspect the real Node launcher stdio with the production lifetime parser.""" + +import importlib.util +import json +from pathlib import Path +import sys + +spec = importlib.util.spec_from_file_location( + "hopper_demo_x11", Path(__file__).resolve().parents[2] / "scripts/hopper-demo-x11.py" +) +assert spec is not None and spec.loader is not None +helper = importlib.util.module_from_spec(spec) +spec.loader.exec_module(helper) +assert "--owner-stdin" in sys.argv +print(json.dumps({"owner_alive": helper.OwnerLifetime(True).alive()})) diff --git a/tests/fixtures/hopperOwnerLifetime.py b/tests/fixtures/hopperOwnerLifetime.py new file mode 100644 index 0000000000..741bffbd9d --- /dev/null +++ b/tests/fixtures/hopperOwnerLifetime.py @@ -0,0 +1,110 @@ +#!/usr/bin/env python3 +"""Exercise the production runner with real children and a simulated X11 surface.""" + +import argparse +import importlib.util +import json +from pathlib import Path +import signal +import subprocess +import sys +import tempfile +import time + + +spec = importlib.util.spec_from_file_location("hopper_demo_x11", sys.argv[1]) +assert spec is not None and spec.loader is not None +helper = importlib.util.module_from_spec(spec) +spec.loader.exec_module(helper) + + +def child_command(receipt): + return [ + sys.executable, + "-c", + "import os,pathlib,time;" + f"receipt=pathlib.Path({str(receipt)!r});" + "pending=receipt.with_suffix('.pending');" + "pending.write_text(str(os.getpid()));pending.replace(receipt);" + "time.sleep(100)", + ] + + +if len(sys.argv) > 3: + phase, directory = sys.argv[2], Path(sys.argv[3]) + + def terminate_worker(*_): + raise SystemExit(143) + + # Failed assertions in the parent must still unwind run_launch's finally. + signal.signal(signal.SIGTERM, terminate_worker) + + class X11: + def XOpenDisplay(self, *_): + return 1 + + def XDefaultRootWindow(self, *_): + return 1 + + def XDisplayWidth(self, *_): + return 1280 + + def XDisplayHeight(self, *_): + return 1024 + + def XCloseDisplay(self, *_): + return 1 + + def start_xvfb(*_): + child = subprocess.Popen( + child_command(directory / "xvfb.pid"), + stdin=subprocess.DEVNULL, + stdout=subprocess.DEVNULL, + stderr=subprocess.PIPE, + ) + return child, ":123", directory / "Xauthority", helper.CapturedStderr(child.stderr) + + helper.digest = lambda _: next(iter(helper.SUPPORTED_HOPPER_SHA256)) + helper.start_xvfb = start_xvfb + helper.ctypes.CDLL = lambda _: X11() + helper.configure_x11 = lambda *_: None + helper.windows = lambda *_: [] + socket = directory / "bridge.sock" + if phase == "ready": + socket.touch() + raise SystemExit(helper.run_launch(argparse.Namespace( + owner_stdin=True, + hopper=sys.executable, + socket=str(socket), + command=child_command(directory / "hopper.pid"), + ))) + + +with tempfile.TemporaryDirectory(prefix="rea-owner-lifetime-") as root: + directory = Path(root) + sentinel = subprocess.Popen([sys.executable, "-c", "import time;time.sleep(100)"]) + worker = subprocess.Popen( + [sys.executable, __file__, sys.argv[1], sys.argv[2], root], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + try: + deadline = time.monotonic() + 5 + receipts = [directory / "hopper.pid", directory / "xvfb.pid"] + while not all(receipt.exists() for receipt in receipts): + assert worker.poll() is None, worker.communicate() + assert time.monotonic() < deadline, "owned children did not start" + time.sleep(0.01) + pids = [int(receipt.read_text()) for receipt in receipts] + assert worker.poll() is None, "live owner did not retain the helper" + assert worker.stdin is not None + worker.stdin.close() + assert worker.wait(timeout=5) == 0, worker.stderr.read() + for pid in pids: + assert not Path(f"/proc/{pid}").exists(), f"owned child {pid} survived EOF" + assert sentinel.poll() is None, "unrelated process was stopped" + print(json.dumps({"phase": sys.argv[2], "children_cleaned": True, "sentinel_alive": True})) + finally: + helper.stop_process(worker) + helper.stop_process(sentinel)