From b1ff7cdacc03110d51bcac33da3cbb7850ac27fc Mon Sep 17 00:00:00 2001 From: Henglie <90338155+Henglie@users.noreply.github.com> Date: Fri, 30 Oct 2026 18:29:18 +0800 Subject: [PATCH] fix(hopper): distinguish an absent demo dialog from publisher rejection (#8074) * fix(hopper): distinguish an absent demo dialog from publisher rejection The Linux demo watcher collapsed an admitted launcher with no first-run dialog into unsupported_demo_dialog. Report that post-authentication timeout as demo_dialog_absent, keep the unsupported dialog code for an observed but unrecognized Hopper window, and allow REA_HOPPER_DEMO_DIALOG_TIMEOUT_MS to widen the fixed wait. Closes #8008. * fix(hopper): tolerate callers without the dialog timeout argument * fix(hopper): report an invalid demo dialog timeout as its own failure An invalid REA_HOPPER_DEMO_DIALOG_TIMEOUT_MS exited 73 and was shown as invalid_launch_command. Exit 82 is invalid_dialog_timeout. The public error names the setting, shows the rejected value and source, lists the allowed range, and says to restart REA. Drive the demo watcher with controlled windows and time. No window exits 81, a Hopper window without a supported dialog exits 76, and a dialog that arrives after the old 10 second limit is still accepted. * fix(hopper): keep startup stderr capture on AdapterFailure DemoWatchStop was inserted above with_stderr, so launch failures called a method they no longer had and became unexpected_failure. Restore with_stderr on AdapterFailure and reject a watcher fixture that cannot capture stderr. --------- Co-authored-by: Cursor Agent --- docs/installation.md | 8 + scripts/hopper-demo-x11.py | 220 +++++++++++++++--- src/domain/analysisErrorProjection.test.ts | 70 ++++++ src/domain/hopperErrors.ts | 9 +- src/domain/hopperStartupFailure.ts | 85 +++++++ src/hopper/LinuxPrivateDisplayDiagnostic.ts | 28 ++- tests/boundary/mcp/toolResult.test.ts | 17 ++ .../hopperClient.startupFailures.test.ts | 2 +- .../hopper/linuxPrivateDisplay.test.ts | 108 ++++++++- tests/fixtures/hopperDemoDialogOutcome.py | 173 ++++++++++++++ 10 files changed, 675 insertions(+), 45 deletions(-) create mode 100644 tests/fixtures/hopperDemoDialogOutcome.py diff --git a/docs/installation.md b/docs/installation.md index 2b0ab76286..dcd6cbfbdf 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -993,6 +993,14 @@ When Hopper cannot start in CI, run `rea doctor --provider hopper --json` in the failing runner. Structured failures distinguish private-display dependencies, an unsupported demo dialog or build, process-ownership conflicts, and an early lifecycle exit. +Once launcher authentication succeeds, an absent first-run dialog is reported as +`demo_dialog_absent` rather than a build or publisher rejection. The watcher +waits 10 seconds by default. Set `REA_HOPPER_DEMO_DIALOG_TIMEOUT_MS` in the +environment that starts REA to a positive decimal integer of milliseconds from +1 to 2147483647 when a slower host needs a longer window. An invalid value is +`invalid_dialog_timeout`: the error names that setting, shows the rejected +value and its source, lists the allowed range, and asks you to correct the +setting and restart REA. If a private-display probe times out or is cancelled and its process cleanup fails, REA retains that exact probe owner and reports `cleanup_incomplete`. Session close retries the probe before releasing its cleanup marker; doctor diff --git a/scripts/hopper-demo-x11.py b/scripts/hopper-demo-x11.py index fdb91ec15b..a19c22a356 100644 --- a/scripts/hopper-demo-x11.py +++ b/scripts/hopper-demo-x11.py @@ -4,6 +4,7 @@ from __future__ import annotations import argparse +from collections.abc import Callable import ctypes import hashlib import json @@ -35,6 +36,14 @@ UNEXPECTED_DISPLAY_GEOMETRY = 77 X11_INPUT_FAILED = 78 RUNTIME_DEPENDENCY_UNAVAILABLE = 79 X11_SOCKET_DIRECTORY_UNUSABLE = 80 +DEMO_DIALOG_ABSENT = 81 +INVALID_DIALOG_TIMEOUT = 82 +DEFAULT_DIALOG_TIMEOUT_MS = 10_000 +DIALOG_TIMEOUT_MINIMUM_MS = 1 +# Node schedules a delay above this signed 32-bit bound as 1 ms. Keep the +# watcher on the same ceiling as REA's other millisecond settings. +DIALOG_TIMEOUT_MAXIMUM_MS = 2_147_483_647 +DIALOG_TIMEOUT_ENV = "REA_HOPPER_DEMO_DIALOG_TIMEOUT_MS" DIAGNOSTIC_PREFIX = "REA_X11_DIAGNOSTIC=" X11_SOCKET_DIRECTORY = Path("/tmp/.X11-unix") class AdapterFailure(Exception): @@ -44,17 +53,25 @@ class AdapterFailure(Exception): reason: str, message: str, stderr_bytes: int = 0, + dialog_timeout: dict[str, object] | None = None, ) -> None: super().__init__(message) self.code = code self.reason = reason self.stderr_bytes = stderr_bytes + self.dialog_timeout = dialog_timeout def with_stderr(self, capture: CapturedStderr) -> AdapterFailure: self.stderr_bytes, _ = capture.snapshot() return self +class DemoWatchStop(Exception): + def __init__(self, reason: str) -> None: + super().__init__(reason) + self.reason = reason + + class CapturedStderr: def __init__(self, stream: object) -> None: self._stream = stream @@ -111,6 +128,80 @@ class XWindowAttributes(ctypes.Structure): ] +def rejected_timeout_value(value: str) -> str: + visible = "".join( + character if character.isprintable() else "\ufffd" for character in value + ) + if len(visible) <= 128: + return visible + return f"{visible[:127]}…" + + +def dialog_timeout_message(value: str | None) -> str: + current = ( + "was rejected" + if value is None + else f"is {json.dumps(value, ensure_ascii=False)} ms" + ) + return ( + f"The effective {DIALOG_TIMEOUT_ENV} value {current} " + f"(source setting; default {DEFAULT_DIALOG_TIMEOUT_MS} ms, " + f"floor {DIALOG_TIMEOUT_MINIMUM_MS} ms, " + f"maximum {DIALOG_TIMEOUT_MAXIMUM_MS} ms). " + f"Set {DIALOG_TIMEOUT_ENV} in the environment that starts REA to a " + f"positive decimal integer from {DIALOG_TIMEOUT_MINIMUM_MS} to " + f"{DIALOG_TIMEOUT_MAXIMUM_MS}, then restart REA." + ) + + +def dialog_timeout_decimal(value: str) -> int | None: + if re.fullmatch(r"[1-9]\d{0,9}", value) is None: + return None + parsed = int(value) + if parsed > DIALOG_TIMEOUT_MAXIMUM_MS: + return None + return parsed + + +def dialog_timeout_rejection(value: str) -> AdapterFailure: + shown = rejected_timeout_value(value) + return AdapterFailure( + INVALID_DIALOG_TIMEOUT, + "invalid_dialog_timeout", + dialog_timeout_message(shown), + dialog_timeout={ + "setting": DIALOG_TIMEOUT_ENV, + "value": shown, + "source": "setting", + "minimum_ms": DIALOG_TIMEOUT_MINIMUM_MS, + "maximum_ms": DIALOG_TIMEOUT_MAXIMUM_MS, + "default_ms": DEFAULT_DIALOG_TIMEOUT_MS, + }, + ) + + +def positive_timeout(value: str) -> int: + parsed = dialog_timeout_decimal(value) + if parsed is None: + raise argparse.ArgumentTypeError( + "dialog timeout must be a positive decimal integer from " + f"{DIALOG_TIMEOUT_MINIMUM_MS} to {DIALOG_TIMEOUT_MAXIMUM_MS}" + ) + return parsed + + +def configured_dialog_timeout_ms(requested: int | None) -> int: + if requested is not None: + return requested + configured = os.environ.get(DIALOG_TIMEOUT_ENV) + if configured is None: + return DEFAULT_DIALOG_TIMEOUT_MS + parsed = dialog_timeout_decimal(configured) + if parsed is None: + raise dialog_timeout_rejection(configured) + return parsed + + def parse_args() -> argparse.Namespace: parser = argparse.ArgumentParser() parser.add_argument("--probe", action="store_true") @@ -127,6 +218,7 @@ def parse_args() -> argparse.Namespace: parser.add_argument("--xvfb", default="/usr/bin/Xvfb") parser.add_argument("--xauth", default="/usr/bin/xauth") parser.add_argument("--display-number", type=int) + parser.add_argument("--dialog-timeout-ms", type=positive_timeout) parser.add_argument("command", nargs=argparse.REMAINDER) return parser.parse_args() @@ -275,6 +367,70 @@ def demo_click(dialog: tuple[int, int, int, int, int]) -> tuple[int, int]: return x + DEMO_CLICK_LEFT, y + height - DEMO_CLICK_BOTTOM +def watch_for_demo_dialog( + x11: ctypes.CDLL, + display: int, + timeout_ms: int, + *, + owner_alive: Callable[[], bool], + socket_ready: Callable[[], bool], + process_owned: Callable[[], bool], + process_exited: Callable[[], bool], + monotonic: Callable[[], float], + sleep: Callable[[float], None], + list_windows: Callable[[], list[tuple[int, int, int, int, int]]], +) -> tuple[int, int, int, int, int]: + deadline = monotonic() + timeout_ms / 1000 + hopper_window_observed = False + while monotonic() < deadline: + if not owner_alive(): + raise DemoWatchStop("owner_closed") + if socket_ready(): + raise DemoWatchStop("socket_ready") + if not process_owned(): + if not process_exited(): + sleep(0.1) + continue + raise AdapterFailure( + HOPPER_EXITED_DURING_STARTUP, + "hopper_exited_during_startup", + "owned Hopper process exited", + ) + observed = list_windows() + hopper_window_observed = hopper_window_observed or any( + window_title(x11, display, candidate[0]) == HOPPER_WINDOW_TITLE + for candidate in observed + ) + dialog = find_demo_dialog(x11, display, observed) + if dialog is not None: + return dialog + sleep(0.1) + raise classify_watch_timeout(process_owned(), hopper_window_observed) + + +def classify_watch_timeout( + owned: bool, + hopper_window_observed: bool, +) -> AdapterFailure: + if not owned: + return AdapterFailure( + PROCESS_OWNERSHIP_MISMATCH, + "process_ownership_mismatch", + "Hopper process ownership mismatch", + ) + if hopper_window_observed: + return AdapterFailure( + UNSUPPORTED_DEMO_DIALOG, + "unsupported_demo_dialog", + "Hopper did not present a supported registration dialog", + ) + return AdapterFailure( + DEMO_DIALOG_ABSENT, + "demo_dialog_absent", + "Hopper started without presenting a registration dialog", + ) + + def configure_x11(x11: ctypes.CDLL, xtst: ctypes.CDLL) -> None: window_pointer = ctypes.POINTER(ctypes.c_ulong) x11.XOpenDisplay.argtypes = [ctypes.c_char_p] @@ -372,7 +528,7 @@ def diagnostic( ) -> dict[str, object]: mode, read_only, _ = socket_directory_state() captured_bytes, _ = (0, "") if capture is None else capture.snapshot() - return { + record = { "component": "hopper_private_display", "operation": operation, "status": status, @@ -390,6 +546,9 @@ def diagnostic( if failure is None else failure.stderr_bytes, } + if failure is not None and failure.dialog_timeout is not None: + record["dialog_timeout"] = failure.dialog_timeout + return record def failure_name(code: int) -> str: @@ -405,6 +564,8 @@ def failure_name(code: int) -> str: X11_INPUT_FAILED: "x11_input_failed", RUNTIME_DEPENDENCY_UNAVAILABLE: "runtime_dependency_unavailable", X11_SOCKET_DIRECTORY_UNUSABLE: "x11_socket_directory_unusable", + DEMO_DIALOG_ABSENT: "demo_dialog_absent", + INVALID_DIALOG_TIMEOUT: "invalid_dialog_timeout", }[code] @@ -699,6 +860,9 @@ def run_launch(args: argparse.Namespace) -> int: "invalid_launch_command", "invalid Hopper launch command", ) + timeout_ms = configured_dialog_timeout_ms( + getattr(args, "dialog_timeout_ms", None) + ) xvfb, display_name, authority, capture = start_xvfb(socket.parent, args) child: subprocess.Popen[bytes] | None = None display: int | None = None @@ -752,44 +916,26 @@ def run_launch(args: argparse.Namespace) -> int: "unexpected private display geometry", ) - deadline = time.monotonic() + 10 - 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 wait_for_hopper(child, owner) - if not hopper_is_owned(leader, hopper): - if child.poll() is None: - time.sleep(0.1) - continue - raise AdapterFailure( - HOPPER_EXITED_DURING_STARTUP, - "hopper_exited_during_startup", - "owned Hopper process exited", - ) - observed = windows(x11, display, root) - dialog = find_demo_dialog(x11, display, observed) - if dialog is not None: - break - time.sleep(0.1) - else: - if not hopper_is_owned(leader, hopper): - raise AdapterFailure( - PROCESS_OWNERSHIP_MISMATCH, - "process_ownership_mismatch", - "Hopper process ownership mismatch", - ) - raise AdapterFailure( - UNSUPPORTED_DEMO_DIALOG, - "unsupported_demo_dialog", - "expected Hopper demo dialog not found", + try: + dialog = watch_for_demo_dialog( + x11, + display, + timeout_ms, + owner_alive=owner.alive, + socket_ready=socket.exists, + process_owned=lambda: hopper_is_owned(leader, hopper), + process_exited=lambda: child is not None and child.poll() is not None, + monotonic=time.monotonic, + sleep=time.sleep, + list_windows=lambda: windows(x11, display, root), ) + except DemoWatchStop as stopped: + if stopped.reason == "owner_closed": + return 0 + x11.XCloseDisplay(display) + display = None + return wait_for_hopper(child, owner) - assert dialog is not None click_x, click_y = demo_click(dialog) input_results = ( xtst.XTestFakeMotionEvent(display, -1, click_x, click_y, 0), diff --git a/src/domain/analysisErrorProjection.test.ts b/src/domain/analysisErrorProjection.test.ts index e4cb29d829..6fdd2b5b3e 100644 --- a/src/domain/analysisErrorProjection.test.ts +++ b/src/domain/analysisErrorProjection.test.ts @@ -21,6 +21,14 @@ import { HopperStartError, HopperTimeoutError, } from "./hopperErrors.js"; +import { + DEFAULT_HOPPER_DEMO_DIALOG_TIMEOUT_MS, + HOPPER_DEMO_DIALOG_TIMEOUT_SETTING, + MAXIMUM_HOPPER_DEMO_DIALOG_TIMEOUT_MS, + MINIMUM_HOPPER_DEMO_DIALOG_TIMEOUT_MS, + type DialogTimeoutRejection, + type HopperStartupFailureDiagnostic, +} from "./hopperStartupFailure.js"; import { ProviderAdapterError } from "./providerAdapterError.js"; import { UnknownRegistryError } from "./unknownRegistryError.js"; import { projectAnalysisError } from "./analysisErrorProjection.js"; @@ -786,6 +794,8 @@ describe("analysis error projection: Hopper startup failures", () => { [78, "x11_input_failed"], [79, "runtime_dependency_unavailable"], [80, "x11_socket_directory_unusable"], + [81, "demo_dialog_absent"], + [82, "invalid_dialog_timeout"], ] as const; for (const [exitCode, code] of expected) { @@ -794,10 +804,70 @@ describe("analysis error projection: Hopper startup failures", () => { code: "provider_unavailable", details: { failure_code: code, exit_code: exitCode }, }); + expect(projected.details).not.toHaveProperty("launch_rejection"); expect(projected.message).toMatch(/\S/u); expect(JSON.stringify(projected)).not.toContain("/proc/"); } }); + + it("does not attribute an absent post-authentication dialog to publisher verification", () => { + const projected = projectAnalysisError(new HopperProcessError(81)); + expect(projected).toMatchObject({ + code: "provider_unavailable", + details: { + stage: "launch", + failure_code: "demo_dialog_absent", + exit_code: 81, + }, + }); + expect(projected.details).not.toHaveProperty("launch_rejection"); + expect(projected.message).toContain("REA_HOPPER_DEMO_DIALOG_TIMEOUT_MS"); + }); + + it("reports a rejected demo dialog timeout through its setting fields", () => { + const dialogTimeout = { + setting: HOPPER_DEMO_DIALOG_TIMEOUT_SETTING, + value: "-5", + source: "setting", + minimum_ms: MINIMUM_HOPPER_DEMO_DIALOG_TIMEOUT_MS, + maximum_ms: MAXIMUM_HOPPER_DEMO_DIALOG_TIMEOUT_MS, + default_ms: DEFAULT_HOPPER_DEMO_DIALOG_TIMEOUT_MS, + } satisfies DialogTimeoutRejection; + const projected = projectAnalysisError( + new HopperProcessError(82, { + diagnostic: { + component: "hopper_private_display", + operation: "launch", + reason: "invalid_dialog_timeout", + socket_directory: "/tmp/.X11-unix", + socket_directory_mode: null, + mount_read_only: null, + effective_socket_directory_mode: null, + effective_mount_read_only: null, + wsl: false, + strategy: "direct", + fallback_reason: null, + xvfb_stderr_bytes: 0, + status: "error", + failure_code: "invalid_dialog_timeout", + dialog_timeout: dialogTimeout, + } as HopperStartupFailureDiagnostic, + }), + ); + expect(projected).toMatchObject({ + code: "provider_unavailable", + details: { + stage: "launch", + failure_code: "invalid_dialog_timeout", + exit_code: 82, + diagnostics: { dialog_timeout: dialogTimeout }, + }, + }); + expect(projected.details).not.toHaveProperty("launch_rejection"); + expect(analysisErrorProjectionSchema.safeParse(projected).success).toBe( + true, + ); + }); }); describe("analysis error projection: retry guidance", () => { diff --git a/src/domain/hopperErrors.ts b/src/domain/hopperErrors.ts index 2bc37b03d4..88a577da9f 100644 --- a/src/domain/hopperErrors.ts +++ b/src/domain/hopperErrors.ts @@ -7,7 +7,9 @@ import type { ProviderFailureStage, } from "./providerOperationHealth.js"; import { + dialogTimeoutFromDiagnostic, hopperStartupFailure, + invalidDialogTimeoutMessage, type HopperStartupFailureCode, type HopperStartupFailureDiagnostic, } from "./hopperStartupFailure.js"; @@ -163,6 +165,12 @@ export class HopperProcessError extends HopperError { this.requestId = context.requestId; this.launcherFailure = context.launcherFailure; const failure = hopperStartupFailure(exitCode); + this.userMessage = + failure?.code === "invalid_dialog_timeout" + ? invalidDialogTimeoutMessage( + dialogTimeoutFromDiagnostic(context.diagnostic), + ) + : failure?.message; this.stage = context.stage ?? (failure === undefined @@ -171,7 +179,6 @@ export class HopperProcessError extends HopperError { : "analysis" : "launch"); this.failureCode = failure?.code; - this.userMessage = failure?.message; this.providerState = context.providerState ?? (exitCode === null ? "unknown" : "exited"); } diff --git a/src/domain/hopperStartupFailure.ts b/src/domain/hopperStartupFailure.ts index 26a9f75fad..d85ae9fdff 100644 --- a/src/domain/hopperStartupFailure.ts +++ b/src/domain/hopperStartupFailure.ts @@ -1,8 +1,54 @@ +import { MAX_PROVIDER_STARTUP_TIMEOUT_MS } from "./analysisResourceLimits.js"; + export type HopperPrivateDisplayStrategy = | "direct" | "user-mount-namespace" | "unavailable"; +/** Environment setting that widens the Linux demo-dialog watch. */ +export const HOPPER_DEMO_DIALOG_TIMEOUT_SETTING = + "REA_HOPPER_DEMO_DIALOG_TIMEOUT_MS"; + +/** Watch used when the setting is unset. */ +export const DEFAULT_HOPPER_DEMO_DIALOG_TIMEOUT_MS = 10_000; + +/** Smallest accepted dialog watch. Zero is not a deadline. */ +export const MINIMUM_HOPPER_DEMO_DIALOG_TIMEOUT_MS = 1; + +/** + * Largest accepted dialog watch. Delays above this signed 32-bit bound are + * the same ceiling REA uses for its other millisecond settings. + */ +export const MAXIMUM_HOPPER_DEMO_DIALOG_TIMEOUT_MS = + MAX_PROVIDER_STARTUP_TIMEOUT_MS; + +/** Rejected dialog-timeout setting retained with the adapter diagnostic. */ +export type DialogTimeoutRejection = { + readonly setting: typeof HOPPER_DEMO_DIALOG_TIMEOUT_SETTING; + readonly value: string; + readonly source: "setting"; + readonly minimum_ms: typeof MINIMUM_HOPPER_DEMO_DIALOG_TIMEOUT_MS; + readonly maximum_ms: typeof MAXIMUM_HOPPER_DEMO_DIALOG_TIMEOUT_MS; + readonly default_ms: typeof DEFAULT_HOPPER_DEMO_DIALOG_TIMEOUT_MS; +}; + +/** Public recovery for a rejected REA_HOPPER_DEMO_DIALOG_TIMEOUT_MS value. */ +export const invalidDialogTimeoutMessage = ( + setting?: DialogTimeoutRejection, +): string => { + const current = + setting === undefined + ? "was rejected" + : `is ${JSON.stringify(setting.value)} ms`; + const floor = String(MINIMUM_HOPPER_DEMO_DIALOG_TIMEOUT_MS); + const maximum = String(MAXIMUM_HOPPER_DEMO_DIALOG_TIMEOUT_MS); + const source = setting?.source ?? "setting"; + return [ + `The effective ${HOPPER_DEMO_DIALOG_TIMEOUT_SETTING} value ${current} (source ${source}; default ${String(DEFAULT_HOPPER_DEMO_DIALOG_TIMEOUT_MS)} ms, floor ${floor} ms, maximum ${maximum} ms).`, + `Set ${HOPPER_DEMO_DIALOG_TIMEOUT_SETTING} in the environment that starts REA to a positive decimal integer from ${floor} to ${maximum}, then restart REA.`, + ].join(" "); +}; + const failures = [ [ 70, @@ -59,6 +105,12 @@ const failures = [ "x11_socket_directory_unusable", "REA cannot use the host X11 socket directory for a private display. Run rea doctor --provider hopper for exact mount and namespace diagnostics.", ], + [ + 81, + "demo_dialog_absent", + "Hopper passed launcher authentication but did not present a registration dialog within the wait window. If this host is slow, raise REA_HOPPER_DEMO_DIALOG_TIMEOUT_MS and retry; otherwise complete Hopper setup or close stale sessions and retry.", + ], + [82, "invalid_dialog_timeout", invalidDialogTimeoutMessage()], ] as const; export type HopperStartupFailureCode = (typeof failures)[number][1]; @@ -118,6 +170,39 @@ export const hopperStartupExitCode = ( ): number | undefined => failures.find(([, candidate]) => candidate === code)?.[0]; +const isDialogTimeoutRejection = ( + value: unknown, +): value is DialogTimeoutRejection => { + if (typeof value !== "object" || value === null) return false; + const setting = Reflect.get(value, "setting"); + const shown = Reflect.get(value, "value"); + const source = Reflect.get(value, "source"); + return ( + setting === HOPPER_DEMO_DIALOG_TIMEOUT_SETTING && + typeof shown === "string" && + shown.length <= 128 && + source === "setting" && + Reflect.get(value, "minimum_ms") === + MINIMUM_HOPPER_DEMO_DIALOG_TIMEOUT_MS && + Reflect.get(value, "maximum_ms") === + MAXIMUM_HOPPER_DEMO_DIALOG_TIMEOUT_MS && + Reflect.get(value, "default_ms") === DEFAULT_HOPPER_DEMO_DIALOG_TIMEOUT_MS + ); +}; + +/** + * Read a validated dialog-timeout rejection carried beside the diagnostic. + * The field stays off the static diagnostic type so an absent rejection is + * not an undefined JSON value. + */ +export const dialogTimeoutFromDiagnostic = ( + diagnostic: HopperStartupDiagnostic | undefined, +): DialogTimeoutRejection | undefined => { + if (diagnostic?.status !== "error") return undefined; + const value: unknown = Reflect.get(diagnostic, "dialog_timeout"); + return isDialogTimeoutRejection(value) ? value : undefined; +}; + /** Narrow an untrusted diagnostic field to the public startup code union. */ export const isHopperStartupFailureCode = ( value: string, diff --git a/src/hopper/LinuxPrivateDisplayDiagnostic.ts b/src/hopper/LinuxPrivateDisplayDiagnostic.ts index e5404699e6..e8f39b0328 100644 --- a/src/hopper/LinuxPrivateDisplayDiagnostic.ts +++ b/src/hopper/LinuxPrivateDisplayDiagnostic.ts @@ -1,7 +1,11 @@ import { z } from "zod"; import { + DEFAULT_HOPPER_DEMO_DIALOG_TIMEOUT_MS, + HOPPER_DEMO_DIALOG_TIMEOUT_SETTING, isHopperStartupFailureCode, + MAXIMUM_HOPPER_DEMO_DIALOG_TIMEOUT_MS, + MINIMUM_HOPPER_DEMO_DIALOG_TIMEOUT_MS, type HopperStartupDiagnostic, } from "../domain/hopperStartupFailure.js"; import { safeParseJson } from "../domain/safeJson.js"; @@ -32,6 +36,17 @@ const diagnosticContextSchema = z.object({ xvfb_stderr_bytes: z.int().nonnegative().safe(), }); +const dialogTimeoutSchema = z + .object({ + setting: z.literal(HOPPER_DEMO_DIALOG_TIMEOUT_SETTING), + value: z.string().max(128), + source: z.literal("setting"), + minimum_ms: z.literal(MINIMUM_HOPPER_DEMO_DIALOG_TIMEOUT_MS), + maximum_ms: z.literal(MAXIMUM_HOPPER_DEMO_DIALOG_TIMEOUT_MS), + default_ms: z.literal(DEFAULT_HOPPER_DEMO_DIALOG_TIMEOUT_MS), + }) + .strict(); + const failureCodeSchema = z.string().transform((value, context) => { if (isHopperStartupFailureCode(value)) return value; context.addIssue({ @@ -46,7 +61,11 @@ const diagnosticSchema = z.discriminatedUnion("status", [ .extend({ status: z.literal("ready"), failure_code: z.null() }) .strict(), diagnosticContextSchema - .extend({ status: z.literal("error"), failure_code: failureCodeSchema }) + .extend({ + status: z.literal("error"), + failure_code: failureCodeSchema, + dialog_timeout: dialogTimeoutSchema.optional(), + }) .strict(), ]); @@ -77,5 +96,10 @@ export const parseLinuxPrivateDisplayDiagnostic = ( if (!decoded.ok) return { ok: false, reason: "diagnostic_malformed" }; const parsed = diagnosticSchema.safeParse(decoded.value); if (!parsed.success) return { ok: false, reason: "diagnostic_malformed" }; - return { ok: true, value: parsed.data }; + const value = parsed.data; + if (value.status === "error" && value.dialog_timeout === undefined) { + const { dialog_timeout: _omitted, ...diagnostic } = value; + return { ok: true, value: diagnostic }; + } + return { ok: true, value }; }; diff --git a/tests/boundary/mcp/toolResult.test.ts b/tests/boundary/mcp/toolResult.test.ts index fe5d207d75..1ee63a03aa 100644 --- a/tests/boundary/mcp/toolResult.test.ts +++ b/tests/boundary/mcp/toolResult.test.ts @@ -355,6 +355,23 @@ describe("tool result projection", () => { }, }); }); + it("keeps an absent post-authentication dialog out of publisher verification", () => { + const result = delivery.toCallToolResult( + err(new HopperProcessError(81)), + contract, + ); + expect(result.structuredContent).toMatchObject({ + error: { + code: "provider_unavailable", + details: { failure_code: "demo_dialog_absent", exit_code: 81 }, + category: "unavailable", + }, + }); + expect( + (result.structuredContent as { error: { details: unknown } }).error + .details, + ).not.toHaveProperty("launch_rejection"); + }); it("projects bounded private-display coordinates without raw stderr", () => { const result = delivery.toCallToolResult( err( diff --git a/tests/boundary/providers/hopper/hopperClient.startupFailures.test.ts b/tests/boundary/providers/hopper/hopperClient.startupFailures.test.ts index 6d23a1a86d..03b64fbd32 100644 --- a/tests/boundary/providers/hopper/hopperClient.startupFailures.test.ts +++ b/tests/boundary/providers/hopper/hopperClient.startupFailures.test.ts @@ -327,7 +327,7 @@ describe("HopperClient startup failures", () => { expect(client.operationHealth()).toMatchObject({ state: "unknown" }); }); - it.each([70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80])( + it.each([70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82])( "reports Linux adapter exit %i during bridge startup", async (exitCode) => { const client = new HopperClient({ diff --git a/tests/boundary/providers/hopper/linuxPrivateDisplay.test.ts b/tests/boundary/providers/hopper/linuxPrivateDisplay.test.ts index 0f18c5b271..608c90d5f1 100644 --- a/tests/boundary/providers/hopper/linuxPrivateDisplay.test.ts +++ b/tests/boundary/providers/hopper/linuxPrivateDisplay.test.ts @@ -7,10 +7,17 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import * as ownership from "../../../../src/process/ProcessOwnership.js"; -import type { - HopperStartupDiagnostic, - HopperStartupFailureCode, +import { + DEFAULT_HOPPER_DEMO_DIALOG_TIMEOUT_MS, + HOPPER_DEMO_DIALOG_TIMEOUT_SETTING, + MAXIMUM_HOPPER_DEMO_DIALOG_TIMEOUT_MS, + MINIMUM_HOPPER_DEMO_DIALOG_TIMEOUT_MS, + type HopperStartupDiagnostic, + type HopperStartupFailureCode, } from "../../../../src/domain/hopperStartupFailure.js"; +import { projectAnalysisError } from "../../../../src/domain/analysisErrorProjection.js"; +import { HopperProcessError } from "../../../../src/domain/hopperErrors.js"; +import { analysisErrorProjectionSchema } from "../../../../src/contracts/errorSchemas.js"; import { runLinuxPrivateDisplayProbe, selectLinuxPrivateDisplayStrategy, @@ -32,6 +39,13 @@ const hangingHelperPath = fileURLToPath( const dialogDetectionFixturePath = fileURLToPath( new URL("../../../fixtures/hopperDialogDetection.py", import.meta.url), ); +const dialogOutcomeFixturePath = fileURLToPath( + new URL("../../../fixtures/hopperDemoDialogOutcome.py", import.meta.url), +); +const pythonCommand = ["python3", "python"].find( + (candidate) => + spawnSync(candidate, ["--version"], { stdio: "ignore" }).status === 0, +); const cleanupTimeoutFixturePath = fileURLToPath( new URL("../../../fixtures/hopperCleanupTimeout.py", import.meta.url), ); @@ -95,7 +109,9 @@ const failureExit = (code: HopperStartupFailureCode): number => ? 80 : code === "runtime_dependency_unavailable" ? 79 - : 70; + : code === "demo_dialog_absent" + ? 81 + : 70; describe("Hopper demo dialog detection", () => { it.skipIf(process.platform !== "linux")( @@ -116,6 +132,90 @@ describe("Hopper demo dialog detection", () => { ); }); +describe("Hopper demo dialog timeout outcomes", () => { + it.skipIf(pythonCommand === undefined)( + "distinguishes an unowned launcher from an admitted launcher without a dialog", + () => { + const result = spawnSync( + pythonCommand ?? "python3", + [dialogOutcomeFixturePath], + { encoding: "utf8" }, + ); + expect(result.status, result.stderr).toBe(0); + expect(JSON.parse(result.stdout)).toEqual({ + not_owned: { code: 74, reason: "process_ownership_mismatch" }, + admitted_without_window: { + code: 81, + reason: "demo_dialog_absent", + }, + admitted_with_window: { + code: 76, + reason: "unsupported_demo_dialog", + }, + default_timeout: { ms: 10_000 }, + configured_timeout: { ms: 25_000 }, + requested_timeout: { ms: 500 }, + invalid_timeout: { + code: 82, + reason: "invalid_dialog_timeout", + dialog_timeout: { + setting: HOPPER_DEMO_DIALOG_TIMEOUT_SETTING, + value: "-5", + source: "setting", + minimum_ms: MINIMUM_HOPPER_DEMO_DIALOG_TIMEOUT_MS, + maximum_ms: MAXIMUM_HOPPER_DEMO_DIALOG_TIMEOUT_MS, + default_ms: DEFAULT_HOPPER_DEMO_DIALOG_TIMEOUT_MS, + }, + }, + watcher_no_window: { + code: 81, + reason: "demo_dialog_absent", + elapsed_ms: 1_000, + }, + watcher_hopper_window_without_dialog: { + code: 76, + reason: "unsupported_demo_dialog", + elapsed_ms: 1_000, + }, + watcher_dialog_after_legacy_limit: { + dialog: 10, + elapsed_ms: DEFAULT_HOPPER_DEMO_DIALOG_TIMEOUT_MS + 100, + legacy_limit_ms: DEFAULT_HOPPER_DEMO_DIALOG_TIMEOUT_MS, + }, + }); + const timeout = JSON.parse(result.stdout).invalid_timeout.dialog_timeout; + const parsed = parseLinuxPrivateDisplayDiagnostic( + `${LINUX_PRIVATE_DISPLAY_DIAGNOSTIC_PREFIX}${JSON.stringify({ + ...diagnostic({ + status: "error", + operation: "launch", + reason: "invalid_dialog_timeout", + failure_code: "invalid_dialog_timeout", + }), + dialog_timeout: timeout, + })}\n`, + ); + expect(parsed.ok).toBe(true); + if (!parsed.ok || parsed.value.status !== "error") return; + const projected = projectAnalysisError( + new HopperProcessError(82, { diagnostic: parsed.value }), + ); + expect(projected).toMatchObject({ + code: "provider_unavailable", + details: { + failure_code: "invalid_dialog_timeout", + exit_code: 82, + diagnostics: { dialog_timeout: timeout }, + }, + }); + expect(projected.details).not.toHaveProperty("launch_rejection"); + expect(analysisErrorProjectionSchema.safeParse(projected).success).toBe( + true, + ); + }, + ); +}); + describe("Hopper process cleanup", () => { it.skipIf(process.platform !== "linux").each(["startup", "ready"])( "reaps only its own children when the owner pipe closes during %s", diff --git a/tests/fixtures/hopperDemoDialogOutcome.py b/tests/fixtures/hopperDemoDialogOutcome.py new file mode 100644 index 0000000000..ab978150cc --- /dev/null +++ b/tests/fixtures/hopperDemoDialogOutcome.py @@ -0,0 +1,173 @@ +#!/usr/bin/env python3 +"""Exercise demo-dialog timeout parsing and the watcher itself.""" + +import importlib.util +import json +import os +from pathlib import Path + +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) + +class _Stderr: + def snapshot(self): + return (4, "") + + +_failure = helper.AdapterFailure(81, "demo_dialog_absent", "missing dialog") +if _failure.with_stderr(_Stderr()) is not _failure or _failure.stderr_bytes != 4: + raise SystemExit("AdapterFailure.with_stderr did not keep the startup failure") + +DIALOG = 10 +HOPPER = 11 +DIALOG_TITLES = { + DIALOG: "Registration", + HOPPER: helper.HOPPER_WINDOW_TITLE, +} +DIALOG_TRANSIENTS = {DIALOG: HOPPER} +DIALOG_WINDOW = (DIALOG, 20, 100, 400, 280) +HOPPER_WINDOW = (HOPPER, 0, 0, 1280, 1024) + + +class Clock: + def __init__(self) -> None: + self.ms = 0 + + def monotonic(self) -> float: + return self.ms / 1000 + + def sleep(self, seconds: float) -> None: + self.ms += int(round(seconds * 1000)) + + +class FakeX11: + def __init__(self, titles, transients) -> None: + self.titles = titles + self.transients = transients + + def XFetchName(self, _display, window, target): + value = self.titles.get(window) + if value is None: + return 0 + target._obj.value = value.encode("utf-8") + return 1 + + def XGetTransientForHint(self, _display, window, target): + value = self.transients.get(window) + if value is None: + return 0 + target._obj.value = value + return 1 + + def XFree(self, _value): + return 1 + + +def timeout_outcome(configured, requested=None): + if configured is None: + os.environ.pop(helper.DIALOG_TIMEOUT_ENV, None) + else: + os.environ[helper.DIALOG_TIMEOUT_ENV] = configured + try: + return {"ms": helper.configured_dialog_timeout_ms(requested)} + except helper.AdapterFailure as failure: + return { + "code": failure.code, + "reason": failure.reason, + "dialog_timeout": failure.dialog_timeout, + } + + +def failure_outcome(owned, window_observed): + failure = helper.classify_watch_timeout(owned, window_observed) + return {"code": failure.code, "reason": failure.reason} + + +def watch_outcome(list_windows, timeout_ms, titles=None, transients=None): + clock = Clock() + try: + dialog = helper.watch_for_demo_dialog( + FakeX11(titles or {}, transients or {}), + 0, + timeout_ms, + owner_alive=lambda: True, + socket_ready=lambda: False, + process_owned=lambda: True, + process_exited=lambda: False, + monotonic=clock.monotonic, + sleep=clock.sleep, + list_windows=list_windows, + ) + except helper.AdapterFailure as failure: + return { + "code": failure.code, + "reason": failure.reason, + "elapsed_ms": clock.ms, + } + return {"dialog": dialog[0], "elapsed_ms": clock.ms} + + +def late_dialog(timeout_ms, legacy_limit_ms): + clock = Clock() + arrival_ms = legacy_limit_ms + 100 + + def list_windows(): + if clock.ms < arrival_ms: + return [] + return [DIALOG_WINDOW] + + try: + dialog = helper.watch_for_demo_dialog( + FakeX11(DIALOG_TITLES, DIALOG_TRANSIENTS), + 0, + timeout_ms, + owner_alive=lambda: True, + socket_ready=lambda: False, + process_owned=lambda: True, + process_exited=lambda: False, + monotonic=clock.monotonic, + sleep=clock.sleep, + list_windows=list_windows, + ) + except helper.AdapterFailure as failure: + return { + "code": failure.code, + "reason": failure.reason, + "elapsed_ms": clock.ms, + "legacy_limit_ms": legacy_limit_ms, + } + return { + "dialog": dialog[0], + "elapsed_ms": clock.ms, + "legacy_limit_ms": legacy_limit_ms, + } + + +print( + json.dumps( + { + "not_owned": failure_outcome(False, False), + "admitted_without_window": failure_outcome(True, False), + "admitted_with_window": failure_outcome(True, True), + "default_timeout": timeout_outcome(None), + "configured_timeout": timeout_outcome("25000"), + "requested_timeout": timeout_outcome("25000", 500), + "invalid_timeout": timeout_outcome("-5"), + "watcher_no_window": watch_outcome(lambda: [], 1_000), + "watcher_hopper_window_without_dialog": watch_outcome( + lambda: [HOPPER_WINDOW], + 1_000, + {HOPPER: helper.HOPPER_WINDOW_TITLE}, + ), + "watcher_dialog_after_legacy_limit": late_dialog( + helper.DEFAULT_DIALOG_TIMEOUT_MS + 10_000, + helper.DEFAULT_DIALOG_TIMEOUT_MS, + ), + }, + separators=(",", ":"), + ) +)