From 7abacf97ad7da9de286d37afa00be891b4210d2d Mon Sep 17 00:00:00 2001 From: ptrfariello Date: Fri, 24 Jul 2026 17:57:29 +0200 Subject: [PATCH] feat(workspace): native OS directory picker (#3074) + per-host recents Add an OS-native directory chooser for local workspace selection in the new-session dialog and surface per-host recents as a clickable list. Native dialog: - Host advertises a `native_directory_dialog` capability (macOS + GUI session only); Windows/Linux report unsupported and fall back. - macOS opens an NSOpenPanel-style `choose folder` via osascript and returns the chosen absolute path over the authenticated host tunnel. - Dialog runs as a background task so host keepalives keep flowing; concurrent requests are rejected; disconnect fails pending futures fast (502, not a 300s hang). - Server enforces the capability (422 if absent); client additionally gates on the local Electron desktop host identity (capability alone is insufficient). - Host-switch race: the result is discarded unless the selected host still matches when the promise resolves. - Absolute-path invariant enforced at the frame decode boundary. Recents: - Per-host most-recent-first list in the workspace popover, capped at 8, persisted under `omnigent:recent-workspaces`. Recorded on selection (native pick + recent click) and on session create. Existing schema and ForkSessionDialog usage unchanged. The in-app WorkspacePicker remains the always-available fallback for remote/headless/unsupported/older hosts; no protocol-compatibility break. Co-authored-by: penny (Omnigent) --- omnigent/host/connect.py | 182 ++++++- omnigent/host/frames.py | 124 +++++ omnigent/server/host_registry.py | 51 +- omnigent/server/routes/host_tunnel.py | 12 + omnigent/server/routes/hosts.py | 150 ++++++ tests/host/test_connect.py | 263 ++++++++++ tests/host/test_frames.py | 174 +++++++ .../test_hosts_native_directory_dialog.py | 493 ++++++++++++++++++ tests/server/test_host_registry.py | 36 ++ web/src/hooks/useHostNativeDirectoryDialog.ts | 96 ++++ web/src/hooks/useHosts.ts | 27 + web/src/hooks/useRecentWorkspaces.ts | 4 + web/src/shell/NewChatDialog.test.tsx | 267 ++++++++++ web/src/shell/NewChatDialog.tsx | 175 ++++++- 14 files changed, 2034 insertions(+), 20 deletions(-) create mode 100644 tests/server/integration/test_hosts_native_directory_dialog.py create mode 100644 web/src/hooks/useHostNativeDirectoryDialog.ts diff --git a/omnigent/host/connect.py b/omnigent/host/connect.py index 4b836161ee..14b5641431 100644 --- a/omnigent/host/connect.py +++ b/omnigent/host/connect.py @@ -22,7 +22,7 @@ import websockets.asyncio.client from websockets.exceptions import InvalidStatus, InvalidURI -from omnigent._platform import WINDOWS_ENV_PASSTHROUGH +from omnigent._platform import IS_DARWIN, WINDOWS_ENV_PASSTHROUGH from omnigent.env_credentials import env_names_with_omnigent_prefix from omnigent.harness_aliases import canonicalize_harness from omnigent.harness_availability import HARNESS_BINARY_MISSING, HarnessAvailability @@ -49,6 +49,8 @@ HostListWorktreesResultFrame, HostModelOptionsFrame, HostModelOptionsResultFrame, + HostOpenDirectoryDialogFrame, + HostOpenDirectoryDialogResultFrame, HostRemoveWorktreeFrame, HostRemoveWorktreeResultFrame, HostRunnerExitedFrame, @@ -524,6 +526,99 @@ def _url_is_loopback(url: str) -> bool: _LOGIN_REDIRECT_FATAL_ATTEMPTS = 3 +def _macos_gui_session_available() -> bool: + """Whether this host process appears to run inside a macOS GUI session. + + The native directory dialog only makes sense when the host IS the + machine the user sits at and a window server is reachable — a remote + or headless SSH daemon would pop a dialog nobody can see. + ``SECURITYSESSIONID`` is exported by ``loginwindow`` for Aqua (GUI + login) sessions and absent for SSH/plain-launchd contexts, so it is a + clean, dependency-free signal that the host is interactive. Carefully + conservative: a false negative (not offering the dialog) falls back + to the in-app picker, which always works, while a false positive would + offer a button that hangs. TODO: a display probe robust to launchd + LaunchAgents that strip env would catch more GUI contexts. + """ + return bool(os.environ.get("SECURITYSESSIONID")) + + +def _host_capabilities() -> dict[str, object] | None: + """Advertise host capabilities for the new-session UI to gate against. + + Today the only capability is ``native_directory_dialog``: the host can + show an OS-native folder chooser on its own screen. Offered only on + macOS inside a GUI session (see :func:`_macos_gui_session_available`); + every other host returns ``None`` so the Web UI falls back to the + in-app WorkspacePicker unchanged. Absence is the older-host posture — + the server treats a missing capability as "unsupported". + + This advertises a *capability* (the host CAN show a GUI dialog), NOT + locality. A Mac running ``omnigent host`` from Terminal advertises it + even when a remote user selected that host — correctly so, since the + host genuinely could pop a dialog on ITS screen. Locality (is this host + the machine the user sits at?) is established CLIENT-side in the Web UI + by matching the selected host id against the Electron desktop host + identity, and the server ENFORCES the capability before forwarding. + Together these guarantee the dialog only runs on the local desktop + host the user can actually see and interact with. + """ + if IS_DARWIN and _macos_gui_session_available(): + return {"native_directory_dialog": True} + return None + + +def _open_native_directory_dialog() -> tuple[str, str | None, str | None]: + """Open the host's OS-native directory chooser. + + :returns: A ``(status, path, error)`` triple. ``status`` is one of + ``"ok"`` (``path`` set, ``error`` ``None``), ``"cancelled"``, + ``"unsupported"`` (non-macOS, or no GUI session), or ``"error"`` (the + dialog invocation failed; ``error`` carries a short message). + + Only macOS is implemented (AppleScript ``choose folder`` via + ``osascript`` — dependency-free, ships with macOS, directory-only). + Windows/Linux report ``"unsupported"" so the caller falls back to + the in-app WorkspacePicker; TODO: implement NSOpenPanel-equivalents + there and advertise the capability once a GUI-session probe exists. + """ + if not IS_DARWIN: + return "unsupported", None, "native directory dialog is not supported on this platform" + if not _macos_gui_session_available(): + return "unsupported", None, "no interactive macOS GUI session available" + try: + proc = subprocess.run( + [ + "osascript", + "-e", + 'POSIX path of (choose folder with prompt "Select a workspace directory")', + ], + capture_output=True, + text=True, + check=False, + ) + except FileNotFoundError: + return "error", None, "osascript is not available" + except subprocess.SubprocessError as exc: + return "error", None, f"osascript failed: {exc}" + if proc.returncode != 0: + stderr = (proc.stderr or "").strip() + # osascript exits non-zero with "User canceled." when the user + # dismisses the dialog; any other failure surfaces as an error. + if "cancel" in stderr.lower(): + return "cancelled", None, None + return "error", None, stderr or "osascript failed" + path = (proc.stdout or "").strip() + if not path: + return "error", None, "osascript returned an empty path" + # ``choose folder`` yields a POSIX path with a trailing slash; drop + # it (except at the root) so the result matches the in-app picker's + # shape. Session-create validation canonicalizes symlinks after this. + if len(path) > 1 and path.endswith("/"): + path = path.rstrip("/") + return "ok", path, None + + class HostConnectError(Exception): """A non-retryable failure while opening the host tunnel. @@ -730,6 +825,12 @@ def __init__( # Mutated only via :meth:`_host_subprocess_op`; safe as a plain int # because both the mutation and the reaper run on the event loop. self._owned_subprocess_ops = 0 + # In-flight OS-native directory-dialog task. Only one GUI dialog + # may be open at a time; a second request is rejected with a busy + # status. The dialog runs as a BACKGROUND task so the receive loop + # keeps answering server keepalives (pings) while the user browses + # — otherwise the tunnel would be declared dead after ~90s. + self._directory_dialog_task: asyncio.Task[None] | None = None def _tracked_runner_pids(self) -> set[int]: """PIDs of runners this host spawned and still tracks directly. @@ -1868,6 +1969,60 @@ async def _handle_model_options( models=models, ) + async def _handle_open_directory_dialog( + self, + frame: HostOpenDirectoryDialogFrame, + ) -> HostOpenDirectoryDialogResultFrame: + """Open the host's OS-native directory chooser. + + Runs the GUI dialog OFF the event loop (``osascript`` blocks until + the user picks or cancels); the result carries the absolute POSIX + path on success, or a ``cancelled`` / ``unsupported`` status so the + server's caller falls back to the in-app picker. + + :param frame: The open-directory-dialog request (correlation id). + :returns: Result frame with the chosen path or a status explaining + why none was chosen. + """ + status, path, error = await asyncio.to_thread(_open_native_directory_dialog) + return HostOpenDirectoryDialogResultFrame( + request_id=frame.request_id, + status=status, + path=path, + error=error, + ) + + async def _serve_directory_dialog( + self, + ws: websockets.asyncio.client.ClientConnection, + frame: HostOpenDirectoryDialogFrame, + ) -> None: + """Run one native directory dialog as a BACKGROUND task and reply. + + The OS dialog blocks for as long as the user browses, so awaiting it + inline in :meth:`_dispatch_host_frame` would stall the receive loop + and starve the server's keepalive pings — the tunnel would be + declared dead after ~90s while the endpoint promises 300s. Running + the dialog as a task lets the loop keep answering pings. Only one + dialog is open at a time (serialized in the dispatch branch). If the + tunnel has dropped by the time the dialog completes, the reply send + is swallowed — the server already failed the pending future on + disconnect. + + :param ws: The tunnel connection, used to send the result. + :param frame: The open-directory-dialog request. + """ + try: + result = await self._handle_open_directory_dialog(frame) + await ws.send(encode_host_frame(result)) + except Exception: # noqa: BLE001 + # Tunnel closed mid-dialog (the server failed the pending + # future fast on disconnect) or the send itself dropped — + # nothing useful to do; osascript has already exited. + _logger.debug("directory dialog reply could not be sent", exc_info=True) + finally: + self._directory_dialog_task = None + @staticmethod def _dispatch_fs_op( reader: object, @@ -2310,6 +2465,7 @@ async def _serve_frames(self, ws: websockets.asyncio.client.ClientConnection) -> configured_harnesses=configured_harnesses, telemetry_opt_out=_tel_opt_out, installation_id=_tel_install_id, + capabilities=_host_capabilities(), ) await ws.send(encode_host_frame(hello)) self._ws = ws @@ -2465,6 +2621,30 @@ async def _dispatch_host_frame( await ws.send(encode_host_frame(result)) elif isinstance(frame, HostModelOptionsFrame): await ws.send(encode_host_frame(await self._handle_model_options(frame))) + elif isinstance(frame, HostOpenDirectoryDialogFrame): + # The OS dialog blocks until the user picks/cancels. Run it as a + # BACKGROUND task so the receive loop keeps answering server + # keepalive pings while the user browses — awaiting inline would + # starve the pings and the tunnel would be declared dead after + # ~90s. Serialize: at most one GUI dialog is open at a time; a + # concurrent request is rejected with a busy status so the + # caller falls back to the in-app picker. + in_flight = self._directory_dialog_task + if in_flight is not None and not in_flight.done(): + await ws.send( + encode_host_frame( + HostOpenDirectoryDialogResultFrame( + request_id=frame.request_id, + status="error", + error="a directory dialog is already open on this host", + ), + ), + ) + return + self._directory_dialog_task = asyncio.create_task( + self._serve_directory_dialog(ws, frame), + name=f"host-directory-dialog:{frame.request_id}", + ) def run_host_process( diff --git a/omnigent/host/frames.py b/omnigent/host/frames.py index a560d253ad..885533ccae 100644 --- a/omnigent/host/frames.py +++ b/omnigent/host/frames.py @@ -68,6 +68,8 @@ class HostFrameKind(str, Enum): FS_RESULT = "host.fs_result" MODEL_OPTIONS = "host.model_options" MODEL_OPTIONS_RESULT = "host.model_options_result" + OPEN_DIRECTORY_DIALOG = "host.open_directory_dialog" + OPEN_DIRECTORY_DIALOG_RESULT = "host.open_directory_dialog_result" # ── Frame dataclasses ──────────────────────────────────── @@ -102,6 +104,7 @@ class HostHelloFrame: configured_harnesses: dict[str, HarnessAvailability] | None = None telemetry_opt_out: bool = False installation_id: str | None = None + capabilities: dict[str, Any] | None = None @dataclass @@ -807,6 +810,57 @@ class HostModelOptionsResultFrame: error: str | None = None +@dataclass +class HostOpenDirectoryDialogFrame: + """Server → host: open the host's OS-native directory chooser. + + Backs ``POST /v1/hosts/{id}/native-directory-dialog``, which the Web UI's + new-session workspace picker offers when the host advertises the + ``native_directory_dialog`` capability in its hello frame. The dialog runs + ON THE HOST (never the server) so it opens on the machine whose filesystem + is being selected — only a local, interactive host with a GUI session + advertises support; remote/headless/unsupported hosts fall back to the + in-app WorkspacePicker. + + :param request_id: Correlates the result, e.g. ``"req_od_1"``. + """ + + request_id: str + + +@dataclass +class HostOpenDirectoryDialogResultFrame: + """Host → server: outcome of a native directory-dialog request. + + :param request_id: Correlates to the + :class:`HostOpenDirectoryDialogFrame`, e.g. ``"req_od_1"``. + :param status: One of: + + * ``"ok"`` — the user picked a directory; ``path`` holds its + absolute POSIX path. + * ``"cancelled"`` — the user dismissed the dialog without choosing. + * ``"unsupported"`` — this host cannot show a native dialog + (non-macOS platform, or no GUI/window-server session). The caller + falls back to the in-app WorkspacePicker. + * ``"error"`` — the dialog invocation failed unexpectedly (e.g. + ``osascript`` unavailable, window server unreachable); ``error`` + carries a short message. + :param path: Absolute POSIX path the user selected, e.g. + ``"/Users/corey/projects"``. ``None`` unless ``status`` is ``"ok"``. + The host returns the realpath so symlinks cannot smuggle a workspace + out of an agent's ``os_env.cwd`` boundary — the same posture as + ``host.stat_result.canonical_path``. + :param error: Short failure reason when ``status`` is ``"error"`` (or + ``"unsupported"`` on non-macOS, for diagnostics). ``None`` on + ``"ok"``/``"cancelled"``. + """ + + request_id: str + status: str + path: str | None = None + error: str | None = None + + HostFrame = ( HostHelloFrame | HostHarnessReadinessFrame @@ -833,6 +887,8 @@ class HostModelOptionsResultFrame: | HostFsResultFrame | HostModelOptionsFrame | HostModelOptionsResultFrame + | HostOpenDirectoryDialogFrame + | HostOpenDirectoryDialogResultFrame ) @@ -881,6 +937,7 @@ def encode_host_frame(frame: HostFrame) -> str: "configured_harnesses": frame.configured_harnesses, "telemetry_opt_out": frame.telemetry_opt_out, "installation_id": frame.installation_id, + "capabilities": frame.capabilities, } ) if isinstance(frame, HostHarnessReadinessFrame): @@ -1178,6 +1235,23 @@ def encode_host_frame(frame: HostFrame) -> str: "error": frame.error, } ) + if isinstance(frame, HostOpenDirectoryDialogFrame): + return _encode_payload( + { + "kind": HostFrameKind.OPEN_DIRECTORY_DIALOG.value, + "request_id": frame.request_id, + } + ) + if isinstance(frame, HostOpenDirectoryDialogResultFrame): + return _encode_payload( + { + "kind": HostFrameKind.OPEN_DIRECTORY_DIALOG_RESULT.value, + "request_id": frame.request_id, + "status": frame.status, + "path": frame.path, + "error": frame.error, + } + ) raise TypeError(f"unknown host frame type: {type(frame).__name__}") @@ -1300,6 +1374,10 @@ def _decode_known_host_frame( return _decode_model_options(msg) case HostFrameKind.MODEL_OPTIONS_RESULT: return _decode_model_options_result(msg) + case HostFrameKind.OPEN_DIRECTORY_DIALOG: + return HostOpenDirectoryDialogFrame(request_id=_required_str(msg, "request_id")) + case HostFrameKind.OPEN_DIRECTORY_DIALOG_RESULT: + return _decode_open_directory_dialog_result(msg) raise ValueError(f"unhandled host frame kind: {kind.value!r}") # pragma: no cover @@ -1317,6 +1395,7 @@ def _decode_host_hello(msg: dict[str, Any]) -> HostHelloFrame: configured_harnesses=_optional_str_availability_map(msg, "configured_harnesses"), telemetry_opt_out=bool(msg.get("telemetry_opt_out", False)), installation_id=_optional_nullable_str(msg, "installation_id"), + capabilities=_optional_object_map(msg, "capabilities"), ) @@ -1804,6 +1883,33 @@ def _decode_model_options_result(msg: dict[str, Any]) -> HostModelOptionsResultF ) +def _decode_open_directory_dialog_result( + msg: dict[str, Any], +) -> HostOpenDirectoryDialogResultFrame: + """Decode a host.open_directory_dialog_result frame. + + Enforces the documented invariant at the protocol boundary: an ``ok`` + status MUST carry a non-empty absolute POSIX path (leading ``/``). A + malformed or relative ``ok`` result is rejected so a buggy/malicious + host cannot smuggle a relative path into the workspace field — the + in-app picker only ever yields absolute paths. + """ + status = _required_str(msg, "status") + path = _optional_nullable_str(msg, "path") + if status == "ok": + if not path or not path.startswith("/"): + raise ValueError( + "host.open_directory_dialog_result with status 'ok' must " + "carry a non-empty absolute path" + ) + return HostOpenDirectoryDialogResultFrame( + request_id=_required_str(msg, "request_id"), + status=status, + path=path, + error=_optional_nullable_str(msg, "error"), + ) + + # ── Field validators ───────────────────────────────────── @@ -1884,6 +1990,24 @@ def _optional_str_availability_map( return {k: v for k, v in val.items() if isinstance(k, str) and is_harness_availability(v)} +def _optional_object_map(msg: dict[str, Any], key: str) -> dict[str, Any] | None: + """Return an optional JSON-object field, tolerant of malformed peers. + + Absent/null/non-dict values all decode to ``None`` ("not advertised") + rather than raising, so an older or newer host's hello never breaks the + tunnel handshake — the caller treats ``None`` as "no capabilities + advertised" and falls back to the in-app picker. + + :param msg: Decoded frame object. + :param key: Field name, e.g. ``"capabilities"``. + :returns: A shallow copy of the object when present, else ``None``. + """ + val = msg.get(key) + if not isinstance(val, dict): + return None + return dict(val) + + def _optional_nullable_str(msg: dict[str, Any], key: str) -> str | None: """Return an optional nullable string field. diff --git a/omnigent/server/host_registry.py b/omnigent/server/host_registry.py index 15be5d190d..7786cd95e2 100644 --- a/omnigent/server/host_registry.py +++ b/omnigent/server/host_registry.py @@ -250,6 +250,12 @@ class HostConnection: ``error_code``, and ``error``. :param pending_model_options: Per-``request_id`` futures for pre-launch model catalogs resolved by the selected host. + :param pending_directory_dialogs: Per-``request_id`` futures for + in-flight ``host.open_directory_dialog`` requests (the OS-native + folder chooser). Resolved when the host sends + ``host.open_directory_dialog_result``. Values carry the result fields + (``status``, ``path``, ``error``). Same ``Any`` typing rationale as + ``pending_stats``. """ workspace_id: int @@ -306,6 +312,44 @@ class HostConnection: pending_model_options: dict[str, asyncio.Future[dict[str, Any]]] = field( default_factory=dict, ) + pending_directory_dialogs: dict[str, asyncio.Future[dict[str, Any]]] = field( + default_factory=dict, + ) + + +def _fail_pending_futures( + conn: HostConnection, + reason: str, +) -> None: + """Reject every in-flight host request future on a dropped connection. + + Called from :meth:`HostRegistry.deregister` so a host disconnect fails + pending ``list_dir`` / ``model_options`` / ``open_directory_dialog`` + requests promptly instead of hanging to their per-call timeout — the + host can never reply once its tunnel is gone. ``asyncio.Future`` is + loop-bound, so only futures not yet done are failed. + + :param conn: The host connection being deregistered. + :param reason: Short human-readable cause stamped on the exception. + """ + for store in ( + conn.pending_stats, + conn.pending_list_dirs, + conn.pending_create_worktrees, + conn.pending_remove_worktrees, + conn.pending_list_worktrees, + conn.pending_create_dirs, + conn.pending_installs, + conn.pending_secret_writes, + conn.pending_credential_detects, + conn.pending_fs_requests, + conn.pending_model_options, + conn.pending_directory_dialogs, + ): + for future in list(store.values()): + if not future.done(): + future.set_exception(ConnectionError(reason)) + store.clear() class HostRegistry: @@ -394,7 +438,12 @@ def deregister(self, host_id: str, workspace_id: int | None = None) -> None: """ ws_id = current_workspace_id() if workspace_id is None else workspace_id with self._lock: - self._hosts.pop((ws_id, _canonical_host_id(host_id)), None) + conn = self._hosts.pop((ws_id, _canonical_host_id(host_id)), None) + if conn is not None: + _fail_pending_futures( + conn, + f"host '{_canonical_host_id(host_id)}' disconnected", + ) def get(self, host_id: str, workspace_id: int | None = None) -> HostConnection | None: """Look up a live host connection. diff --git a/omnigent/server/routes/host_tunnel.py b/omnigent/server/routes/host_tunnel.py index f2a63b68cc..db7aabad12 100644 --- a/omnigent/server/routes/host_tunnel.py +++ b/omnigent/server/routes/host_tunnel.py @@ -38,6 +38,7 @@ HostListDirResultFrame, HostListWorktreesResultFrame, HostModelOptionsResultFrame, + HostOpenDirectoryDialogResultFrame, HostRemoveWorktreeResultFrame, HostRunnerExitedFrame, HostRunnerStatusResultFrame, @@ -666,6 +667,17 @@ async def _receive_loop( } ) continue + if isinstance(frame, HostOpenDirectoryDialogResultFrame): + dialog_future = conn.pending_directory_dialogs.pop(frame.request_id, None) + if dialog_future is not None and not dialog_future.done(): + dialog_future.set_result( + { + "status": frame.status, + "path": frame.path, + "error": frame.error, + } + ) + continue _logger.debug( "Host %s sent unexpected frame type: %s", diff --git a/omnigent/server/routes/hosts.py b/omnigent/server/routes/hosts.py index bf568675f3..f4f4bfc2b0 100644 --- a/omnigent/server/routes/hosts.py +++ b/omnigent/server/routes/hosts.py @@ -37,6 +37,7 @@ HostLaunchRunnerFrame, HostListDirFrame, HostModelOptionsFrame, + HostOpenDirectoryDialogFrame, HostStoreSecretFrame, encode_host_frame, ) @@ -71,6 +72,15 @@ # for transient network slowness without making the picker feel hung. _CREATE_DIR_TIMEOUT_S = 5.0 _MODEL_OPTIONS_TIMEOUT_S = 15.0 +# Per-call timeout for host.open_directory_dialog round-trips. The OS +# folder chooser is user-driven — a slow user can take minutes to browse +# and pick — so this is far more generous than the filesystem ops. The +# host runs the dialog as a BACKGROUND task so its receive loop keeps +# answering server keepalive pings while the user browses, so the tunnel +# liveness window (PING_INTERVAL_S * PING_MISS_THRESHOLD ~= 90s) is NOT +# the constraint here. If the host disconnects mid-dialog, deregister +# fails the pending future promptly (well under this timeout). +_NATIVE_DIRECTORY_DIALOG_TIMEOUT_S = 300.0 # Per-call timeout for host.install_harness round-trips. The host runs # `npm install -g ` — install_harness_cli caps that subprocess at 300s — # then recomputes readiness and sends the result back over the tunnel. The @@ -254,6 +264,68 @@ async def _proxy_create_dir( host_conn.pending_create_dirs.pop(request_id, None) +async def _proxy_open_directory_dialog( + *, + host_registry: HostRegistry, + host_conn: HostConnection, +) -> dict[str, Any]: + """ + Send a ``host.open_directory_dialog`` frame and await the result. + + Mirrors :func:`_proxy_list_dir`: enqueue the frame, register a future + on the host connection's ``pending_directory_dialogs`` map, await with a + generous timeout (the user picks at their own pace), and clean up in a + finally block. The host's WS receive loop in ``host_tunnel.py`` resolves + the future when the result frame arrives. + + :param host_registry: Server-side registry; used to enqueue the + outbound frame on the host's send queue. + :param host_conn: Live host connection. + :returns: Dict with the result fields: ``status`` (``"ok"``, + ``"cancelled"``, ``"unsupported"``, or ``"error"``), ``path`` (chosen + absolute path or ``None``), ``error`` (string or ``None``). + :raises HTTPException: 504 on timeout, 502 on connection drop. + """ + request_id = secrets.token_hex(8) + loop = asyncio.get_running_loop() + future: asyncio.Future[dict[str, Any]] = loop.create_future() + host_conn.pending_directory_dialogs[request_id] = future + + frame = encode_host_frame( + HostOpenDirectoryDialogFrame(request_id=request_id), + ) + try: + try: + host_registry.send_text(host_conn, frame) + except ConnectionError as exc: + raise HTTPException( + status_code=502, + detail=f"host '{host_conn.host_id}' connection lost", + ) from exc + try: + return await asyncio.wait_for(future, timeout=_NATIVE_DIRECTORY_DIALOG_TIMEOUT_S) + except asyncio.TimeoutError as exc: + raise HTTPException( + status_code=504, + detail=( + f"host '{host_conn.host_id}' did not respond to the directory " + f"dialog within {_NATIVE_DIRECTORY_DIALOG_TIMEOUT_S:.0f}s" + ), + ) from exc + except ConnectionError as exc: + # The host disconnected mid-dialog (deregister failed this + # future fast); surface as 502 so the Web UI falls back to + # the in-app picker without waiting the full timeout. + raise HTTPException( + status_code=502, + detail=f"host '{host_conn.host_id}' connection lost", + ) from exc + finally: + # Cleanup runs on every path so a cancelled caller doesn't leave an + # orphan in the pending dict. + host_conn.pending_directory_dialogs.pop(request_id, None) + + async def _proxy_install_harness( *, host_registry: HostRegistry, @@ -522,6 +594,22 @@ async def _resolve_agent_harness( return canonicalize_harness(loaded.spec.executor.harness_kind) +def _live_host_capabilities(host_registry: HostRegistry, host_id: str) -> dict[str, Any] | None: + """Capabilities a live host advertised on THIS replica, else ``None``. + + Surfaced from the in-memory registry (not the DB) on purpose: the native + directory dialog only works while the host's tunnel lands on this + replica (the dialog endpoint uses the same per-replica registry), so a + host connected to another replica must read as unsupported and fall back + to the in-app picker. ``None``/absent is the older-host posture — the + Web UI treats it as "no native dialog" and offers the picker unchanged. + """ + conn = host_registry.get(host_id) + if conn is None: + return None + return conn.hello.capabilities + + def create_hosts_router( host_registry: HostRegistry, host_store: HostStore, @@ -601,6 +689,7 @@ async def list_hosts(request: Request) -> dict[str, list[dict[str, Any]]]: # user-connectable machines. "sandbox_provider": host.sandbox_provider, "configured_harnesses": host.configured_harnesses, + "capabilities": _live_host_capabilities(host_registry, host.host_id), } ) return {"hosts": result} @@ -639,6 +728,7 @@ async def get_host(request: Request, host_id: str) -> dict[str, Any]: # server-managed sandbox host (e.g. "modal"). "sandbox_provider": host.sandbox_provider, "configured_harnesses": host.configured_harnesses, + "capabilities": _live_host_capabilities(host_registry, host.host_id), "runners": [], } @@ -1206,6 +1296,66 @@ async def create_host_directory( "path": result.get("path"), } + @router.post("/hosts/{host_id}/native-directory-dialog") + async def open_host_native_directory_dialog( + request: Request, + host_id: str, + ) -> dict[str, Any]: + """Open the host's OS-native directory chooser. + + Backs the Web UI new-session workspace picker's "Use system dialog" + action, shown only when the host advertises the + ``native_directory_dialog`` capability. The dialog runs ON THE HOST + (never the server) over the authenticated host tunnel and returns + the absolute POSIX path the user picked. Owner-scoped exactly like + the filesystem browse endpoints (``GET /v1/hosts/{id}/filesystem``): + only the host owner can request a dialog. The chosen path still + flows through the existing workspace validation at session-create + time, so this never weakens path/security checks. + + :param request: FastAPI request (for auth). + :param host_id: Host identifier, e.g. ``"host_a1b2c3d4..."``. + :returns: ``{"object": "native_directory_dialog", "status": ..., + "path": ..., "error": ...}``. ``status`` is ``"ok"`` (path set), + ``"cancelled"`` (user dismissed), ``"unsupported"`` (this host can't + show a native dialog — fall back to the in-app picker), or + ``"error"`` (the dialog failed — fall back to the picker). + :raises HTTPException: 404 if host not found, 403 if not owned by + caller, 409 if host is offline, 504 on host timeout, 502 on + connection drop. + """ + user_id = require_user(request, auth_provider) + host = await asyncio.to_thread(host_store.get_host, host_id) + if host is None: + raise HTTPException(status_code=404, detail="host not found") + if user_id is not None and host.user_id != user_id: + raise HTTPException(status_code=403, detail="not your host") + conn = host_registry.get(host.host_id) + if conn is None: + raise HTTPException(status_code=409, detail="host is offline") + # Enforce the advertised capability server-side — never forward the + # dialog to a host that did not advertise ``native_directory_dialog`` + # (older hosts, non-macOS, headless, or a GUI host a remote user + # selected). The 422 lets the Web UI fall back to the in-app picker + # without waiting on a host round-trip that would only say + # "unsupported". + caps = conn.hello.capabilities or {} + if not (isinstance(caps, dict) and caps.get("native_directory_dialog") is True): + raise HTTPException( + status_code=422, + detail="host does not support the native directory dialog", + ) + result = await _proxy_open_directory_dialog( + host_registry=host_registry, + host_conn=conn, + ) + return { + "object": "native_directory_dialog", + "status": result.get("status"), + "path": result.get("path"), + "error": result.get("error"), + } + @router.post("/hosts/{host_id}/harnesses/{harness}/install") async def install_host_harness( request: Request, diff --git a/tests/host/test_connect.py b/tests/host/test_connect.py index 9d7fe09118..ebc43f0ada 100644 --- a/tests/host/test_connect.py +++ b/tests/host/test_connect.py @@ -18,6 +18,9 @@ HostConnectError, HostProcess, _build_runner_env, + _host_capabilities, + _macos_gui_session_available, + _open_native_directory_dialog, _RunnerHandle, run_host_process, ) @@ -37,6 +40,8 @@ HostListDirResultFrame, HostModelOptionsFrame, HostModelOptionsResultFrame, + HostOpenDirectoryDialogFrame, + HostOpenDirectoryDialogResultFrame, HostRunnerExitedFrame, HostRunnerStatusFrame, HostRunnerStatusResultFrame, @@ -47,6 +52,7 @@ HostStoreSecretFrame, HostStoreSecretResultFrame, decode_host_frame, + encode_host_frame, ) from omnigent.host.identity import HostIdentity from omnigent.runner.identity import ( @@ -3051,3 +3057,260 @@ def test_run_host_process_announces_session_log_dir_on_start( out = capsys.readouterr().out assert "Session logs: ~/.omnigent/logs/runner/" in out assert "This host's log: ~/.omnigent/logs/host/host-" in out + + +async def test_handle_open_directory_dialog_returns_host_pick() -> None: + """_handle_open_directory_dialog forwards the OS pick as an ok result. + + The handler runs the dialog off the event loop and wraps the status/ + path into the result frame; if the frame wiring is wrong the server + would never resolve the pending future and the UI would time out. + """ + host = _make_host_process() + picked = "/Users/corey/picked-dir" + with patch( + "omnigent.host.connect._open_native_directory_dialog", + return_value=("ok", picked, None), + ): + result = await host._handle_open_directory_dialog( + HostOpenDirectoryDialogFrame(request_id="req_dialog"), + ) + assert result == HostOpenDirectoryDialogResultFrame( + request_id="req_dialog", + status="ok", + path=picked, + error=None, + ) + + +async def test_handle_open_directory_dialog_passes_through_cancelled() -> None: + """A cancelled dialog returns a ``cancelled`` result (not an exception). + + The UI keeps the picker open on cancel; raising here would surface a + 500 instead of the ``cancelled`` status the contract promises. + """ + host = _make_host_process() + with patch( + "omnigent.host.connect._open_native_directory_dialog", + return_value=("cancelled", None, None), + ): + result = await host._handle_open_directory_dialog( + HostOpenDirectoryDialogFrame(request_id="req_dialog"), + ) + assert result.status == "cancelled" + assert result.path is None + + +async def test_handle_open_directory_dialog_passes_through_unsupported() -> None: + """A non-macOS / headless host returns ``unsupported`` (not an error). + + The UI falls back to the in-app picker; the status must pass through + rather than raising, so the endpoint returns 200 + unsupported. + """ + host = _make_host_process() + with patch( + "omnigent.host.connect._open_native_directory_dialog", + return_value=("unsupported", None, "no GUI session"), + ): + result = await host._handle_open_directory_dialog( + HostOpenDirectoryDialogFrame(request_id="req_dialog"), + ) + assert result.status == "unsupported" + assert result.path is None + assert result.error == "no GUI session" + + +def test_host_capabilities_advertises_native_dialog_on_macos_gui() -> None: + """A macOS host with a GUI session advertises the dialog capability. + + This is the gate the Web UI uses to show the "Use system dialog" + button; if the capability map is wrong the button never appears on a + supported host (or appears on an unsupported one). + """ + import omnigent.host.connect as connect_mod + + original_darwin = connect_mod.IS_DARWIN + try: + # macOS + GUI session → capability advertised. + connect_mod.IS_DARWIN = True + with patch.dict("os.environ", {"SECURITYSESSIONID": "fake"}, clear=False): + assert _macos_gui_session_available() is True + assert _host_capabilities() == {"native_directory_dialog": True} + + # macOS but no GUI session (headless / SSH) → no capability. + with patch.dict("os.environ", {}, clear=True): + assert _macos_gui_session_available() is False + assert _host_capabilities() is None + finally: + connect_mod.IS_DARWIN = original_darwin + + +def test_host_capabilities_none_off_macos() -> None: + """A non-macOS host advertises no capabilities. + + Linux/Windows must read as None so the UI never offers a button that + would hang (the OS dialog isn\'t implemented there yet). + """ + import omnigent.host.connect as connect_mod + + original_darwin = connect_mod.IS_DARWIN + try: + connect_mod.IS_DARWIN = False + with patch.dict("os.environ", {"SECURITYSESSIONID": "fake"}, clear=False): + # Even with a GUI-session env, non-macOS is unsupported. + assert _host_capabilities() is None + finally: + connect_mod.IS_DARWIN = original_darwin + + +def test_open_native_directory_dialog_returns_cancelled_on_osascript_cancel( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An osascript non-zero exit with "cancel" in stderr maps to cancelled. + + AppleScript exits non-zero with "User canceled." when the user hits + Cancel; the helper must distinguish that from a genuine osascript error + so the UI keeps the picker open silently rather than surfacing a failure. + """ + import omnigent.host.connect as connect_mod + + monkeypatch.setattr(connect_mod, "IS_DARWIN", True) + monkeypatch.setenv("SECURITYSESSIONID", "fake") + + class _FakeProc: + returncode = 1 + stdout = "" + stderr = "User canceled.\n" + + def _fake_run(*_args: object, **_kwargs: object) -> _FakeProc: + return _FakeProc() + + monkeypatch.setattr(connect_mod.subprocess, "run", _fake_run) + status, path, error = _open_native_directory_dialog() + assert status == "cancelled" + assert path is None + assert error is None + + +# ── Native directory dialog: background task + serialization ────────── + + +class _SendLogTunnel: + """Minimal ws double that records every ``send`` for dispatch tests.""" + + def __init__(self) -> None: + self.sent: list[str] = [] + + async def send(self, data: str) -> None: + self.sent.append(data) + + +async def test_dispatch_directory_dialog_runs_in_background_keeps_loop_alive( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The dialog runs as a BACKGROUND task so pings still get answered. + + The OS dialog blocks for as long as the user browses; if dispatch + awaited it inline the receive loop would stall and the server would + declare the host dead after ~90s. This dispatches a dialog, then a + ping, and asserts the pong is sent WHILE the dialog is still open — + proving the loop is not blocked. + """ + from omnigent.runner.transports.ws_tunnel.frames import ( + PingFrame, + PongFrame, + decode_frame, + encode_frame, + ) + + host = _make_host_process() + ws = _SendLogTunnel() + # Gate the mock dialog on an event so it stays "open" until we release + # it — the handler runs it via asyncio.to_thread, so the blocker must + # be a synchronous function that waits on a threading.Event. + import threading + + ready = threading.Event() + + def _blocking_dialog_sync() -> tuple[str, str | None, str | None]: + ready.wait(5) + return "ok", "/Users/corey/picked", None + + monkeypatch.setattr( + "omnigent.host.connect._open_native_directory_dialog", + _blocking_dialog_sync, + ) + + dialog_frame = HostOpenDirectoryDialogFrame(request_id="req_bg") + # Dispatch the dialog — must return immediately (background task spawned). + await host._handle_raw_message(ws, encode_host_frame(dialog_frame)) + assert ws.sent == [], "dispatch must not block on the dialog" + + # While the dialog is still open, a ping must be answered immediately — + # this is the keepalive the server uses to detect liveness. + ping_ts = 12345 + await host._handle_raw_message(ws, encode_frame(PingFrame(ts=ping_ts))) + assert len(ws.sent) == 1, "pong must be sent while the dialog is open" + pong = decode_frame(ws.sent[0]) + assert isinstance(pong, PongFrame) + assert pong.ts == ping_ts + + # Release the dialog and let the background task send its result. + ready.set() + task = host._directory_dialog_task + assert task is not None + await asyncio.wait_for(task, timeout=2.0) + # The result frame is now the second sent frame. + assert len(ws.sent) == 2 + result = decode_host_frame(ws.sent[1]) + assert isinstance(result, HostOpenDirectoryDialogResultFrame) + assert result.status == "ok" + assert result.path == "/Users/corey/picked" + assert host._directory_dialog_task is None + + +async def test_dispatch_rejects_concurrent_directory_dialog( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A second dialog while one is open is rejected with a busy error. + + Only one GUI dialog may be open at a time; the second request gets an + immediate ``error`` result so the caller falls back to the in-app + picker instead of queuing behind the first. + """ + import threading + + host = _make_host_process() + ws = _SendLogTunnel() + ready = threading.Event() + + def _blocking_dialog_sync() -> tuple[str, str | None, str | None]: + ready.wait(5) + return "ok", "/Users/corey/picked", None + + monkeypatch.setattr( + "omnigent.host.connect._open_native_directory_dialog", + _blocking_dialog_sync, + ) + + await host._handle_raw_message( + ws, + encode_host_frame(HostOpenDirectoryDialogFrame(request_id="req_first")), + ) + # A concurrent request must be rejected immediately, not queued. + await host._handle_raw_message( + ws, + encode_host_frame(HostOpenDirectoryDialogFrame(request_id="req_second")), + ) + assert len(ws.sent) == 1, "only the busy rejection should be sent" + result = decode_host_frame(ws.sent[0]) + assert isinstance(result, HostOpenDirectoryDialogResultFrame) + assert result.request_id == "req_second" + assert result.status == "error" + assert "already open" in (result.error or "") + + # Clean up the first dialog so the test doesn't leak a task. + ready.set() + task = host._directory_dialog_task + assert task is not None + await asyncio.wait_for(task, timeout=2.0) diff --git a/tests/host/test_frames.py b/tests/host/test_frames.py index 3b5ee228e5..992299ea24 100644 --- a/tests/host/test_frames.py +++ b/tests/host/test_frames.py @@ -29,6 +29,8 @@ HostListWorktreesResultFrame, HostModelOptionsFrame, HostModelOptionsResultFrame, + HostOpenDirectoryDialogFrame, + HostOpenDirectoryDialogResultFrame, HostRemoveWorktreeFrame, HostRemoveWorktreeResultFrame, HostRunnerExitedFrame, @@ -1436,3 +1438,175 @@ def test_fs_result_null_payload_round_trip() -> None: assert isinstance(decoded, HostFsResultFrame) assert decoded.payload is None assert decoded.error_status == 500 + + +def test_open_directory_dialog_frames_round_trip() -> None: + """ + Verify the open-directory-dialog frames survive encode → decode. + + If a field is dropped, the server would never correlate a dialog + result back to its request (request_id) or lose the chosen path / + status, and the Web UI would fall back to the picker on a valid pick. + """ + request = decode_host_frame( + encode_host_frame(HostOpenDirectoryDialogFrame(request_id="req_dialog")), + ) + assert request == HostOpenDirectoryDialogFrame(request_id="req_dialog") + + # Success: a chosen path survives the round-trip. + ok = decode_host_frame( + encode_host_frame( + HostOpenDirectoryDialogResultFrame( + request_id="req_dialog", + status="ok", + path="/Users/corey/projects/omnigent", + error=None, + ), + ), + ) + assert ok == HostOpenDirectoryDialogResultFrame( + request_id="req_dialog", + status="ok", + path="/Users/corey/projects/omnigent", + error=None, + ) + + # Cancel + unsupported + error statuses all carry through with path + # left None so the UI can branch on status alone. + for status in ("cancelled", "unsupported", "error"): + decoded = decode_host_frame( + encode_host_frame( + HostOpenDirectoryDialogResultFrame( + request_id="req_dialog", + status=status, + path=None, + error="boom" if status == "error" else None, + ), + ), + ) + assert decoded.status == status + assert decoded.path is None + assert decoded.error == ("boom" if status == "error" else None) + + +def test_hello_frame_capabilities_round_trip() -> None: + """ + Verify HostHelloFrame carries an advertised capabilities map. + + A local macOS host advertises {native_directory_dialog: True}; if the + field is dropped on decode, the Web UI would never offer the native + dialog even on a host that supports it. + """ + original = HostHelloFrame( + version="0.1.0", + frame_protocol_version=1, + name="corey-laptop", + capabilities={"native_directory_dialog": True}, + ) + decoded = decode_host_frame(encode_host_frame(original)) + assert isinstance(decoded, HostHelloFrame) + assert decoded.capabilities == {"native_directory_dialog": True} + + +def test_hello_frame_capabilities_default_none() -> None: + """ + Verify an older host (no capabilities field) decodes as None. + + Backward compatibility: a hello frame without the capabilities field + must decode to None, which the server surfaces as "no native dialog" + so the Web UI falls back to the in-app picker — never a KeyError or + a false "supported" read. + """ + # Mimic the EXACT bytes an older host (pre-capabilities) would have + # sent: a hello payload with NO capabilities key at all. The decoder + # must treat its absence as None, never a KeyError — that is the + # backward-compat contract (a newer server still serves an older host + # and the Web UI falls back to the in-app picker). + legacy_payload = json.dumps( + { + "kind": "host.hello", + "version": "0.1.0", + "frame_protocol_version": 1, + "name": "laptop", + "runners": [], + "configured_harnesses": None, + "telemetry_opt_out": False, + "installation_id": None, + } + ) + assert "capabilities" not in json.loads(legacy_payload) + decoded = decode_host_frame(legacy_payload) + assert isinstance(decoded, HostHelloFrame) + assert decoded.capabilities is None + + # A new host encoding capabilities=None emits an explicit null (mirrors + # the existing configured_harnesses/installation_id posture); that too + # must decode to None so absence and explicit-null are interchangeable. + encoded = encode_host_frame( + HostHelloFrame(version="0.1.0", frame_protocol_version=1, name="laptop"), + ) + assert json.loads(encoded)["capabilities"] is None + assert decode_host_frame(encoded).capabilities is None + + +def test_decode_open_directory_dialog_result_rejects_non_absolute_ok_path() -> None: + """An 'ok' result must carry a non-empty absolute path (protocol boundary). + + A buggy or malicious host could return a relative or empty path on + success; the decoder rejects it so a relative path can never reach the + workspace field (the in-app picker only ever yields absolute paths, and + session-create validation assumes an absolute workspace). Cancelled / + unsupported / error statuses are unaffected (path is None). + """ + # Relative path on ok → rejected. + with pytest.raises(ValueError, match="absolute path"): + decode_host_frame( + json.dumps( + { + "kind": "host.open_directory_dialog_result", + "request_id": "r1", + "status": "ok", + "path": "relative/x", + "error": None, + } + ), + ) + # Empty path on ok → rejected. + with pytest.raises(ValueError, match="absolute path"): + decode_host_frame( + json.dumps( + { + "kind": "host.open_directory_dialog_result", + "request_id": "r1", + "status": "ok", + "path": "", + "error": None, + } + ), + ) + # Missing path on ok → rejected. + with pytest.raises(ValueError, match="absolute path"): + decode_host_frame( + json.dumps( + { + "kind": "host.open_directory_dialog_result", + "request_id": "r1", + "status": "ok", + "error": None, + } + ), + ) + # An absolute ok path is accepted. + ok = decode_host_frame( + json.dumps( + { + "kind": "host.open_directory_dialog_result", + "request_id": "r1", + "status": "ok", + "path": "/Users/corey/picked", + "error": None, + } + ), + ) + assert isinstance(ok, HostOpenDirectoryDialogResultFrame) + assert ok.path == "/Users/corey/picked" diff --git a/tests/server/integration/test_hosts_native_directory_dialog.py b/tests/server/integration/test_hosts_native_directory_dialog.py new file mode 100644 index 0000000000..4b79d918af --- /dev/null +++ b/tests/server/integration/test_hosts_native_directory_dialog.py @@ -0,0 +1,493 @@ +""" +Integration tests for ``POST /v1/hosts/{id}/native-directory-dialog``. + +Wires up a real host tunnel + REST router pair, drives a fake host +that auto-replies to ``host.open_directory_dialog`` frames, and +exercises the endpoint's contract end-to-end. Mirrors the structure of +``test_hosts_create_directory.py`` (the create-folder endpoint) — the +native folder chooser shares the same owner-scoped, host-forwarded +design, differing only in that the host runs an OS dialog instead of a +mkdir. +""" + +from __future__ import annotations + +import asyncio +import contextlib +from collections.abc import AsyncIterator +from typing import Any + +import pytest +from asgiref.testing import ApplicationCommunicator +from fastapi import FastAPI +from httpx import ASGITransport, AsyncClient + +from omnigent.host.frames import ( + HostHelloFrame, + HostOpenDirectoryDialogFrame, + HostOpenDirectoryDialogResultFrame, + decode_host_frame, + encode_host_frame, +) +from omnigent.server.host_registry import HostRegistry +from omnigent.server.routes.host_tunnel import create_host_tunnel_router +from omnigent.server.routes.hosts import create_hosts_router +from omnigent.stores.conversation_store.sqlalchemy_store import ( + SqlAlchemyConversationStore, +) +from omnigent.stores.host_store import HostStore + +# Same liveness-race flake guard as test_hosts_create_directory.py: the +# mock WS host can be starved + deregistered under parallel CI load. +pytestmark = [ + pytest.mark.asyncio, + pytest.mark.flaky(reruns=2, reruns_delay=1), +] + +_HOST_ID = "c1d4e6f90a2b37184d9f0c6b5e8a1d27" +_HOST_NAME = "native-dialog-laptop" + + +def _websocket_scope(path: str) -> dict[str, object]: + """Build a minimal ASGI WebSocket scope. + + :param path: WebSocket path, e.g. ``"/v1/hosts/X/tunnel"``. + :returns: ASGI scope dict. + """ + return { + "type": "websocket", + "asgi": {"version": "3.0"}, + "scheme": "ws", + "path": path, + "raw_path": path.encode("ascii"), + "query_string": b"", + "headers": [], + "client": ("127.0.0.1", 50000), + "server": ("testserver", 80), + "subprotocols": [], + } + + +def _hello_text( + name: str = _HOST_NAME, + capabilities: dict[str, Any] | None = None, +) -> str: + """Encode a hello frame for tests. + + :param name: Host name reported in the hello frame. + :param capabilities: Capabilities map to advertise (e.g. + ``{"native_directory_dialog": True}``); ``None`` mimics an + older host / unsupported host. + :returns: JSON-encoded hello frame. + """ + return encode_host_frame( + HostHelloFrame( + version="0.1.0-test", + frame_protocol_version=1, + name=name, + capabilities=capabilities, + ) + ) + + +@pytest.fixture() +def dialog_app( + db_uri: str, +) -> tuple[FastAPI, HostRegistry, HostStore, SqlAlchemyConversationStore]: + """App with host tunnel + REST routes for native-dialog tests. + + :param db_uri: SQLite URI fixture. + :returns: (app, registry, host_store, conv_store). + """ + registry = HostRegistry() + host_store = HostStore(db_uri) + conv_store = SqlAlchemyConversationStore(db_uri) + app = FastAPI() + app.include_router( + create_host_tunnel_router(registry, host_store), + prefix="/v1", + ) + app.include_router( + create_hosts_router(registry, host_store, conv_store), + prefix="/v1", + ) + return app, registry, host_store, conv_store + + +@pytest.fixture() +async def dialog_setup( + dialog_app: tuple[FastAPI, HostRegistry, HostStore, SqlAlchemyConversationStore], +) -> AsyncIterator[ + tuple[ + FastAPI, + HostRegistry, + ApplicationCommunicator, + dict[str, dict[str, Any]], + dict[str, Any], + asyncio.Task[None], + ] +]: + """Connect a mock host and start an auto-replier for dialog frames. + + Tests configure the host's reply in one of two ways: + + - Set ``default_reply`` (the 5th yielded value, a mutable dict) BEFORE + the call to change the reply used for EVERY request_id. This is the + simple path for cancelled / unsupported / error statuses, since the + endpoint mints its own request_id the test cannot predict. + - Register a per-request_id reply in ``replies`` (the 4th yielded + value) to override the default for one specific id (rarely needed). + + The auto-replier consumes the ``host.open_directory_dialog`` frames the + route pushes, decodes them, and feeds the configured result back — + mirroring what ``host_tunnel.py`` does in production. The default reply + is a successful pick of ``/tmp/picked``. + + :param dialog_app: The fixture above. + :returns: Async iterator yielding + ``(app, registry, comm, replies, default_reply, drain_task)``. + """ + app, registry, _hs, _cs = dialog_app + path = f"/v1/hosts/{_HOST_ID}/tunnel" + comm = ApplicationCommunicator(app, _websocket_scope(path)) + await comm.send_input({"type": "websocket.connect"}) + accepted = await comm.receive_output(timeout=1.0) + assert accepted["type"] == "websocket.accept" + await comm.send_input( + { + "type": "websocket.receive", + "text": _hello_text(capabilities={"native_directory_dialog": True}), + }, + ) + while registry.get(_HOST_ID) is None: + await asyncio.sleep(0.01) + + conn = registry.get(_HOST_ID) + assert conn is not None + # Per-request_id overrides; when none is registered for a frame's id + # the drain falls back to ``default_reply`` (mutated by a test before + # the call to drive a cancelled / unsupported / error status). + replies: dict[str, dict[str, Any]] = {} + default_reply: dict[str, Any] = {"status": "ok", "path": "/tmp/picked"} + stop_drain = asyncio.Event() + + async def _drain() -> None: + """Drain outbound WS frames and reply to dialog frames. + + :returns: None when ``stop_drain`` is set or no events arrive + within the per-iteration timeout. + """ + while not stop_drain.is_set(): + try: + output = await comm.receive_output(timeout=0.5) + except asyncio.TimeoutError: + continue + if output.get("type") != "websocket.send": + continue + text = output.get("text") + if not isinstance(text, str): + continue + frame = decode_host_frame(text) + if not isinstance(frame, HostOpenDirectoryDialogFrame): + continue + reply = replies.get(frame.request_id, default_reply) + reply_frame = HostOpenDirectoryDialogResultFrame( + request_id=frame.request_id, + status=reply.get("status", "ok"), + path=reply.get("path"), + error=reply.get("error"), + ) + await comm.send_input( + { + "type": "websocket.receive", + "text": encode_host_frame(reply_frame), + } + ) + + drain_task = asyncio.create_task(_drain()) + try: + yield app, registry, comm, replies, default_reply, drain_task + finally: + stop_drain.set() + try: + await asyncio.wait_for(drain_task, timeout=1.0) + except asyncio.TimeoutError: + drain_task.cancel() + + +# ── Happy path ────────────────────────────────────────── + + +_Setup = tuple[ + FastAPI, + HostRegistry, + ApplicationCommunicator, + dict[str, dict[str, Any]], + dict[str, Any], + asyncio.Task[None], +] + + +async def test_native_dialog_returns_picked_path(dialog_setup: _Setup) -> None: + """A successful host pick returns the chosen absolute path. + + This is the path the Web UI feeds into ``setWorkspace`` before + session-create validation, so it must round-trip intact. + """ + app, _reg, _comm, _replies, _default, _drain = dialog_setup + # The drain's default reply is ok + /tmp/picked. + async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as client: + resp = await client.post(f"/v1/hosts/{_HOST_ID}/native-directory-dialog") + + assert resp.status_code == 200 + body = resp.json() + assert body["object"] == "native_directory_dialog" + assert body["status"] == "ok" + assert body["path"] == "/tmp/picked" + assert body["error"] is None + + +async def test_native_dialog_cancelled_status_passes_through(dialog_setup: _Setup) -> None: + """A user-cancelled dialog returns status ``cancelled`` (not an error). + + The Web UI uses this to keep the in-app picker open silently rather + than surfacing a failure — so the status must survive intact and the + path must be None. The endpoint mints its own request_id, so the + fixture's ``default_reply`` (applied to every id) drives the status. + """ + app, _reg, _comm, _replies, default_reply, _drain = dialog_setup + default_reply.clear() + default_reply.update({"status": "cancelled", "path": None, "error": None}) + + async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as client: + resp = await client.post(f"/v1/hosts/{_HOST_ID}/native-directory-dialog") + + assert resp.status_code == 200 + body = resp.json() + assert body["status"] == "cancelled" + assert body["path"] is None + assert body["error"] is None + + +async def test_native_dialog_unsupported_status_passes_through(dialog_setup: _Setup) -> None: + """An ``unsupported`` host result passes through as a non-error status. + + A non-macOS / headless host reports ``unsupported``; the Web UI + falls back to the in-app picker, so this must NOT be an HTTP error. + """ + app, _reg, _comm, _replies, default_reply, _drain = dialog_setup + default_reply.clear() + default_reply.update({"status": "unsupported", "path": None, "error": "no GUI session"}) + + async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as client: + resp = await client.post(f"/v1/hosts/{_HOST_ID}/native-directory-dialog") + + assert resp.status_code == 200 + body = resp.json() + assert body["status"] == "unsupported" + assert body["path"] is None + assert body["error"] == "no GUI session" + + +async def test_native_dialog_unknown_host_returns_404( + dialog_app: tuple[FastAPI, HostRegistry, HostStore, SqlAlchemyConversationStore], +) -> None: + """Requesting a dialog on an unknown host returns 404.""" + app, _reg, _hs, _cs = dialog_app + async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as client: + resp = await client.post( + "/v1/hosts/00000000000000000000000000000000/native-directory-dialog", + ) + + assert resp.status_code == 404 + + +async def test_native_dialog_offline_host_returns_409( + dialog_app: tuple[FastAPI, HostRegistry, HostStore, SqlAlchemyConversationStore], +) -> None: + """A registered-but-offline host returns 409 (not 404/502). + + The host row exists in the store (so not 404) but no live tunnel + lands on this replica, so the dialog can't be forwarded → 409 + Conflict, matching the filesystem endpoints' offline posture. + """ + app, _reg, host_store, _cs = dialog_app + # Persist the host row without connecting a tunnel (mirrors the + # filesystem endpoint's offline test): the record exists so the route + # passes the 404 existence check, but registry.get() is None → 409. + host_store.upsert_on_connect( + host_id=_HOST_ID, + name=_HOST_NAME, + user_id="local", + ) + async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as client: + resp = await client.post(f"/v1/hosts/{_HOST_ID}/native-directory-dialog") + + assert resp.status_code == 409 + + +# # ── Capability surfacing ──────────────────────────────── + + +async def test_list_hosts_surfaces_advertised_capabilities( + dialog_app: tuple[FastAPI, HostRegistry, HostStore, SqlAlchemyConversationStore], +) -> None: + """``GET /v1/hosts`` echoes the live capabilities a host advertised. + + The Web UI gates the "Use system dialog" button on + ``capabilities.native_directory_dialog``; if the field is dropped or + read from the DB instead of the live registry, the button would + never appear (or appear for a host on another replica). + """ + app, registry, _hs, _cs = dialog_app + # Connect a host that advertises the native-dialog capability. + path = f"/v1/hosts/{_HOST_ID}/tunnel" + comm = ApplicationCommunicator(app, _websocket_scope(path)) + await comm.send_input({"type": "websocket.connect"}) + accepted = await comm.receive_output(timeout=1.0) + assert accepted["type"] == "websocket.accept" + await comm.send_input( + { + "type": "websocket.receive", + "text": _hello_text(capabilities={"native_directory_dialog": True}), + } + ) + while registry.get(_HOST_ID) is None: + await asyncio.sleep(0.01) + + async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as client: + resp = await client.get("/v1/hosts") + + assert resp.status_code == 200 + hosts = resp.json()["hosts"] + assert len(hosts) == 1 + assert hosts[0]["capabilities"] == {"native_directory_dialog": True} + + # Also verify the single-host endpoint surfaces it. + async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as client: + resp = await client.get(f"/v1/hosts/{_HOST_ID}") + assert resp.status_code == 200 + assert resp.json()["capabilities"] == {"native_directory_dialog": True} + + +async def test_list_hosts_capabilities_none_for_older_host( + dialog_app: tuple[FastAPI, HostRegistry, HostStore, SqlAlchemyConversationStore], +) -> None: + """An older host (no capabilities) reads as ``None`` in the list. + + Backward compat: the Web UI treats ``null``/absent as "no native + dialog" and falls back to the in-app picker — never a crash. + """ + app, registry, _hs, _cs = dialog_app + path = f"/v1/hosts/{_HOST_ID}/tunnel" + comm = ApplicationCommunicator(app, _websocket_scope(path)) + await comm.send_input({"type": "websocket.connect"}) + accepted = await comm.receive_output(timeout=1.0) + assert accepted["type"] == "websocket.accept" + await comm.send_input( + {"type": "websocket.receive", "text": _hello_text()}, # no capabilities + ) + while registry.get(_HOST_ID) is None: + await asyncio.sleep(0.01) + + async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as client: + resp = await client.get("/v1/hosts") + + assert resp.status_code == 200 + hosts = resp.json()["hosts"] + assert len(hosts) == 1 + assert hosts[0]["capabilities"] is None + + +# ── Capability enforcement (BLOCKER 2a) ────────────────── + + +async def test_native_dialog_rejects_when_capability_absent( + dialog_app: tuple[FastAPI, HostRegistry, HostStore, SqlAlchemyConversationStore], +) -> None: + """The route rejects forwarding when the host did NOT advertise support. + + A host connected without ``native_directory_dialog`` (older host, + non-macOS, headless, or a GUI host a remote user selected) must get a + 422 immediately — the server never forwards a dialog to a host that + can't show one. The Web UI falls back to the in-app picker on 422. + """ + app, registry, _hs, _cs = dialog_app + path = f"/v1/hosts/{_HOST_ID}/tunnel" + comm = ApplicationCommunicator(app, _websocket_scope(path)) + await comm.send_input({"type": "websocket.connect"}) + accepted = await comm.receive_output(timeout=1.0) + assert accepted["type"] == "websocket.accept" + # Connect with NO capabilities (mimics an older / unsupported host). + await comm.send_input( + {"type": "websocket.receive", "text": _hello_text()}, + ) + while registry.get(_HOST_ID) is None: + await asyncio.sleep(0.01) + + async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as client: + resp = await client.post(f"/v1/hosts/{_HOST_ID}/native-directory-dialog") + assert resp.status_code == 422 + assert "does not support" in resp.json()["detail"] + + +# ── Disconnect fails the pending future fast (BLOCKER 1) ────────── + + +async def test_native_dialog_endpoint_fails_fast_on_host_disconnect( + dialog_app: tuple[FastAPI, HostRegistry, HostStore, SqlAlchemyConversationStore], +) -> None: + """A host drop mid-dialog fails the endpoint promptly (no 300s hang). + + The drain does NOT reply (the user is still browsing). When the WS + disconnects, deregister fails the pending future with a ConnectionError + that the proxy maps to 502 — the endpoint returns within the test's + short budget instead of waiting the full 300s timeout. + """ + app, registry, _hs, _cs = dialog_app + path = f"/v1/hosts/{_HOST_ID}/tunnel" + comm = ApplicationCommunicator(app, _websocket_scope(path)) + await comm.send_input({"type": "websocket.connect"}) + accepted = await comm.receive_output(timeout=1.0) + assert accepted["type"] == "websocket.accept" + await comm.send_input( + { + "type": "websocket.receive", + "text": _hello_text(capabilities={"native_directory_dialog": True}), + }, + ) + while registry.get(_HOST_ID) is None: + await asyncio.sleep(0.01) + + # Drain outbound frames WITHOUT replying — the dialog is "open" and + # the endpoint's future is pending. + stop_drain = asyncio.Event() + + async def _drain_no_reply() -> None: + while not stop_drain.is_set(): + try: + await comm.receive_output(timeout=0.2) + except asyncio.TimeoutError: + continue + + drain_task = asyncio.create_task(_drain_no_reply()) + try: + async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as client: + request_task = asyncio.create_task( + client.post(f"/v1/hosts/{_HOST_ID}/native-directory-dialog"), + ) + # Let the request reach the pending future. + await asyncio.sleep(0.1) + assert registry.get(_HOST_ID) is not None + # Disconnect the host — deregister fails the pending future. + await comm.send_input({"type": "websocket.disconnect"}) + resp = await asyncio.wait_for(request_task, timeout=5.0) + # The endpoint must return promptly (502 connection-lost or 504), + # NOT hang to the 300s timeout. The disconnect path may surface as + # 502 (ConnectionError from the failed future) — both are a fast + # failure, which is the contract under test. + assert resp.status_code in (502, 504), resp.status_code + finally: + stop_drain.set() + with contextlib.suppress(asyncio.TimeoutError, asyncio.CancelledError): + await asyncio.wait_for(drain_task, timeout=1.0) diff --git a/tests/server/test_host_registry.py b/tests/server/test_host_registry.py index 10d5d867b6..7a00e24293 100644 --- a/tests/server/test_host_registry.py +++ b/tests/server/test_host_registry.py @@ -354,3 +354,39 @@ def test_legacy_prefixed_id_resolves_to_bare_registration() -> None: # Deregistering by any spelling removes the entry. registry.deregister(prefixed) assert registry.get(bare) is None + + +async def test_deregister_fails_pending_directory_dialog_future() -> None: + """Disconnect fails in-flight dialog futures fast (no timeout hang). + + When a host drops mid-dialog, its tunnel can never deliver the result, + so deregister must reject the pending future with a ConnectionError + promptly — otherwise the endpoint hangs to its 300s timeout. The same + guarantee covers list_dir / model_options / stat pending futures. + """ + registry = HostRegistry() + conn = registry.register( + "host_dialog", + FakeWebSocket(), + _make_hello(), + owner="alice", + ) + loop = asyncio.get_running_loop() + future: asyncio.Future[dict[str, object]] = loop.create_future() + conn.pending_directory_dialogs["req_open"] = future + # Also seed a list_dir future to confirm ALL pending stores are drained. + list_future: asyncio.Future[dict[str, object]] = loop.create_future() + conn.pending_list_dirs["req_ls"] = list_future + + registry.deregister("host_dialog") + + assert registry.get("host_dialog") is None + assert future.done() + with pytest.raises(ConnectionError): + await future + assert list_future.done() + with pytest.raises(ConnectionError): + await list_future + # The pending stores are cleared so a late result can't resurrect them. + assert conn.pending_directory_dialogs == {} + assert conn.pending_list_dirs == {} diff --git a/web/src/hooks/useHostNativeDirectoryDialog.ts b/web/src/hooks/useHostNativeDirectoryDialog.ts new file mode 100644 index 0000000000..0ec534ddcc --- /dev/null +++ b/web/src/hooks/useHostNativeDirectoryDialog.ts @@ -0,0 +1,96 @@ +import { useMutation } from "@tanstack/react-query"; + +import { authenticatedFetch } from "@/lib/identity"; + +/** + * Result of a host's OS-native directory chooser. + * + * Mirrors the wire shape from + * ``POST /v1/hosts/{id}/native-directory-dialog``: + * + * - ``"ok"`` — the user picked a folder; ``path`` is its absolute POSIX path. + * - ``"cancelled"`` — the user dismissed the dialog (Esc / Cancel). No path. + * - ``"unsupported"`` — this host can't show a native dialog (non-macOS, no + * GUI session, or an older host build that doesn't advertise the + * capability). The caller falls back to the in-app WorkspacePicker. + * - ``"error"`` — the dialog invocation failed (e.g. osascript error). The + * caller falls back to the in-app WorkspacePicker. + */ +export type NativeDirectoryDialogStatus = "ok" | "cancelled" | "unsupported" | "error"; + +interface NativeDirectoryDialogResponse { + object: "native_directory_dialog"; + status: NativeDirectoryDialogStatus; + /** Absolute POSIX path the user picked (set when ``status === "ok"``). */ + path: string | null; + /** Short message when ``status === "error"``; ``null`` otherwise. */ + error: string | null; +} + +export interface NativeDirectoryDialogResult { + status: NativeDirectoryDialogStatus; + /** Absolute POSIX path, or ``null`` unless ``status === "ok"``. */ + path: string | null; + /** Error message, or ``null`` unless ``status === "error"``. */ + error: string | null; +} + +/** + * Open a host's OS-native directory chooser via + * ``POST /v1/hosts/{id}/native-directory-dialog``. + * + * The dialog runs ON THE HOST over the authenticated host tunnel and blocks + * until the user picks a folder or cancels; the server waits up to 300s (the + * host endpoint's timeout). Only offered for hosts that advertise the + * ``native_directory_dialog`` capability (local + interactive macOS hosts); a + * non-OK HTTP response should be treated as "fall back to the in-app picker". + * + * @param hostId Host identifier, e.g. ``"host_a1b2..."``. + * @returns The dialog result (status + optional path/error). + * @throws Error carrying the HTTP status line on a non-OK response. + */ +export async function openHostNativeDirectoryDialog( + hostId: string, +): Promise { + const res = await authenticatedFetch( + `/v1/hosts/${encodeURIComponent(hostId)}/native-directory-dialog`, + { method: "POST", headers: { "Content-Type": "application/json" }, body: "{}" }, + ); + if (!res.ok) { + // Surface the server's detail (e.g. "host is offline" / 504 timeout) + // so a caller can log a reason, then fall back to the in-app picker. + let detail = `${res.status} ${res.statusText}`; + try { + const err = (await res.json()) as { detail?: string }; + if (typeof err.detail === "string" && err.detail) detail = err.detail; + } catch { + // Non-JSON error body — keep the status-line detail. + } + throw new Error(detail); + } + const body = (await res.json()) as NativeDirectoryDialogResponse; + return { status: body.status, path: body.path, error: body.error }; +} + +/** + * React Query mutation: open the host's native directory chooser. + * + * The OS dialog blocks (the host runs ``osascript`` synchronously), so this + * is a mutation, not a query. Callers gate the trigger on the host's + * ``native_directory_dialog`` capability (see the ``Host`` type) and, on a + * non-``ok`` / thrown result, silently fall back to the in-app + * ``WorkspacePicker``. No cache is invalidated — a chosen path flows straight + * into the dialog's ``setWorkspace`` callback and existing session-create + * validation. + * + * @returns A React Query mutation; call + * ``mutateAsync(hostId)`` and read ``{ status, path, error }``. + */ +export function useOpenHostNativeDirectoryDialog() { + return useMutation({ + mutationFn: (hostId: string) => openHostNativeDirectoryDialog(hostId), + // No retry: the dialog is user-driven; a slow click is not a transient + // failure, and retrying would pop a second dialog on top of the first. + retry: false, + }); +} diff --git a/web/src/hooks/useHosts.ts b/web/src/hooks/useHosts.ts index b538affa95..4e37091f5a 100644 --- a/web/src/hooks/useHosts.ts +++ b/web/src/hooks/useHosts.ts @@ -20,6 +20,15 @@ export interface Host { * "nothing configured". */ configured_harnesses?: Record | null; + /** + * Live capabilities this host advertised on the replica the API request + * landed on (mirrored from the in-memory host registry, not persisted). + * `null`/absent means either the host is offline or connected to a + * different replica (so a capability-driven action like the native + * directory dialog must read as unsupported and fall back to the in-app + * picker). Today the only capability is `native_directory_dialog`. + */ + capabilities?: { native_directory_dialog?: boolean } | null; } interface HostsResponse { @@ -138,3 +147,21 @@ export function useInstallHarness(hostId: string) { }, }); } + +/** + * Whether a host can show its OS-native directory chooser on THIS replica. + * + * True only when the host advertised `native_directory_dialog` over its + * tunnel on the replica serving the current API request (capabilities are + * read from the in-memory registry, not the DB). Older hosts, offline hosts, + * hosts on another replica, and non-macOS / headless hosts all read as + * `false` — the caller falls back to the in-app WorkspacePicker. + * + * @param host Host to check (e.g. the selected host from `useHosts`). + * @returns Whether the native directory dialog should be offered. + */ +export function hostSupportsNativeDirectoryDialog( + host: Pick | null | undefined, +): boolean { + return Boolean(host?.capabilities?.native_directory_dialog); +} diff --git a/web/src/hooks/useRecentWorkspaces.ts b/web/src/hooks/useRecentWorkspaces.ts index 7d924ce925..1ce30d5a4f 100644 --- a/web/src/hooks/useRecentWorkspaces.ts +++ b/web/src/hooks/useRecentWorkspaces.ts @@ -70,6 +70,10 @@ export function useRecentWorkspaces(hostId: string | null): RecentWorkspaces { // previous host's paths right after a host switch (a cross-host leak). const recent = useMemo( () => (hostId === null ? [] : (readAll()[hostId] ?? [])), + // `revision` is an intentional cache-buster: addRecent writes to + // localStorage then bumps revision so this memo recomputes. It is not + // referenced in the body, which exhaustive-deps flags — suppress. + // eslint-disable-next-line react-hooks/exhaustive-deps [hostId, revision], ); diff --git a/web/src/shell/NewChatDialog.test.tsx b/web/src/shell/NewChatDialog.test.tsx index 8bc1a47535..a1a7c04fdf 100644 --- a/web/src/shell/NewChatDialog.test.tsx +++ b/web/src/shell/NewChatDialog.test.tsx @@ -46,6 +46,42 @@ vi.mock("@/hooks/useHosts", () => ({ // The setup dialog mounts these; default to an inert mutation + not-installing // so tests that don't exercise install don't need to wire them up. useInstallHarness: vi.fn(() => ({ mutate: vi.fn(), isPending: false })), + // Capability helper is a pure function — re-export the real impl so tests + // can rely on the host's `capabilities` field directly. + hostSupportsNativeDirectoryDialog: ( + host: { capabilities?: { native_directory_dialog?: boolean } | null } | null | undefined, + ) => Boolean(host?.capabilities?.native_directory_dialog), +})); +// The native directory-dialog mutation is mocked inert by default (no +// host supports it); tests that exercise the button override the mock. +const { useNativeDialogMock } = vi.hoisted(() => ({ + useNativeDialogMock: vi.fn(() => ({ mutateAsync: vi.fn(), isPending: false })), +})); +vi.mock("@/hooks/useHostNativeDirectoryDialog", () => ({ + useOpenHostNativeDirectoryDialog: useNativeDialogMock, +})); +// Desktop-shell identity (Electron). Default: NOT in Electron, so +// `desktopHost` is null and the native-dialog locality gate is closed. +// Tests that exercise the native-dialog button call setDesktopHost("host_X") +// so the selected host matches this machine. +const { desktopHostState, isElectronState } = vi.hoisted(() => ({ + desktopHostState: { current: null as { hostId: string; cliInstalled: boolean } | null }, + isElectronState: { current: false }, +})); +function setDesktopHost(hostId: string | null) { + if (hostId === null) { + desktopHostState.current = null; + isElectronState.current = false; + } else { + desktopHostState.current = { hostId, cliInstalled: true }; + isElectronState.current = true; + } +} +vi.mock("@/lib/nativeBridge", async (importOriginal) => ({ + ...(await importOriginal()), + isElectronShell: () => isElectronState.current, + getHostIdentity: async () => desktopHostState.current, + onHostStatusChanged: () => () => {}, })); // The setup dialog's copyable command rows call copyText; stub it so a click // can be asserted without touching the real clipboard. @@ -1587,6 +1623,237 @@ describe("NewChatLandingScreen", () => { expect(screen.getByTestId("workspace-picker")).toBeTruthy(); }); + it("renders the recents list and sets the workspace on click", async () => { + // Two recents for host_1; the seeded "/Users/corey/repo" plus an older one. + localStorage.setItem( + RECENT_KEY, + JSON.stringify({ host_1: ["/Users/corey/repo", "/Users/corey/other"] }), + ); + renderLanding(); + await waitFor(() => + expect(screen.getByTestId("new-chat-landing-workspace-chip").textContent).toContain("repo"), + ); + fireEvent.click(screen.getByTestId("new-chat-landing-workspace-chip")); + // The recents list renders most-recent-first with a testid per path. + expect(screen.getByTestId("workspace-recents")).toBeTruthy(); + expect(screen.getByTestId("workspace-recent-/Users/corey/repo")).toBeTruthy(); + expect(screen.getByTestId("workspace-recent-/Users/corey/other")).toBeTruthy(); + // Clicking a recent sets the workspace (chip label flips to its basename). + fireEvent.click(screen.getByTestId("workspace-recent-/Users/corey/other")); + await waitFor(() => + expect(screen.getByTestId("new-chat-landing-workspace-chip").textContent).toContain("other"), + ); + }); + + it("hides the recents list when there are no recent paths", async () => { + localStorage.setItem(RECENT_KEY, JSON.stringify({ host_1: [] })); + renderLanding(); + // With no recents the chip shows the generic "Working directory" label; + // open the popover and assert the recents list is absent (empty state + // hidden) while the picker remains as the fallback browser. + fireEvent.click(screen.getByTestId("new-chat-landing-workspace-chip")); + await waitFor(() => expect(screen.getByTestId("workspace-picker")).toBeTruthy()); + expect(screen.queryByTestId("workspace-recents")).toBeNull(); + }); + + it("shows the native-dialog button only when the host advertises support", async () => { + // Default host (no capabilities) → button absent, picker present. + renderLanding(); + await waitFor(() => + expect(screen.getByTestId("new-chat-landing-workspace-chip").textContent).toContain("repo"), + ); + fireEvent.click(screen.getByTestId("new-chat-landing-workspace-chip")); + expect(screen.queryByTestId("workspace-native-dialog")).toBeNull(); + expect(screen.getByTestId("workspace-picker")).toBeTruthy(); + }); + + it("shows the native-dialog button when the host supports it", async () => { + setDesktopHost("host_1"); + mockHosts([ + { + host_id: "host_1", + name: "machine-1", + owner: "me", + status: "online", + capabilities: { native_directory_dialog: true }, + }, + ]); + renderLanding(); + await waitFor(() => + expect(screen.getByTestId("new-chat-landing-workspace-chip").textContent).toContain("repo"), + ); + fireEvent.click(screen.getByTestId("new-chat-landing-workspace-chip")); + expect(screen.getByTestId("workspace-native-dialog")).toBeTruthy(); + // The picker remains available as the secondary browser / fallback. + expect(screen.getByTestId("workspace-picker")).toBeTruthy(); + }); + + it("sets the workspace from the native dialog on a successful pick", async () => { + setDesktopHost("host_1"); + mockHosts([ + { + host_id: "host_1", + name: "machine-1", + owner: "me", + status: "online", + capabilities: { native_directory_dialog: true }, + }, + ]); + const mutateAsync = vi.fn().mockResolvedValue({ + status: "ok", + path: "/Users/corey/picked-native", + error: null, + }); + useNativeDialogMock.mockReturnValue({ mutateAsync, isPending: false }); + renderLanding(); + await waitFor(() => + expect(screen.getByTestId("new-chat-landing-workspace-chip").textContent).toContain("repo"), + ); + fireEvent.click(screen.getByTestId("new-chat-landing-workspace-chip")); + fireEvent.click(screen.getByTestId("workspace-native-dialog")); + await waitFor(() => + expect(screen.getByTestId("new-chat-landing-workspace-chip").textContent).toContain( + "picked-native", + ), + ); + expect(mutateAsync).toHaveBeenCalledWith("host_1"); + }); + + it("falls back to the picker when the native dialog is cancelled", async () => { + setDesktopHost("host_1"); + mockHosts([ + { + host_id: "host_1", + name: "machine-1", + owner: "me", + status: "online", + capabilities: { native_directory_dialog: true }, + }, + ]); + const mutateAsync = vi.fn().mockResolvedValue({ + status: "cancelled", + path: null, + error: null, + }); + useNativeDialogMock.mockReturnValue({ mutateAsync, isPending: false }); + renderLanding(); + await waitFor(() => + expect(screen.getByTestId("new-chat-landing-workspace-chip").textContent).toContain("repo"), + ); + fireEvent.click(screen.getByTestId("new-chat-landing-workspace-chip")); + fireEvent.click(screen.getByTestId("workspace-native-dialog")); + // Cancel keeps the existing workspace; the picker is still there. + await waitFor(() => expect(mutateAsync).toHaveBeenCalled()); + expect(screen.getByTestId("new-chat-landing-workspace-chip").textContent).toContain("repo"); + }); + + it("does NOT show the native-dialog button for a non-local (remote GUI) host", async () => { + // The host advertises the capability (it's a macOS GUI host) but it is + // NOT this desktop machine — a remote user selected it. Locality gate + // (selectedHostId === thisMachineHostId) must keep the button hidden so + // the dialog is never popped on a screen the user can't see. + setDesktopHost("host_local"); + mockHosts([ + { + host_id: "host_remote", + name: "remote-mac", + owner: "me", + status: "online", + capabilities: { native_directory_dialog: true }, + }, + ]); + renderLanding(); + await waitFor(() => + expect(screen.getByTestId("new-chat-landing-workspace-chip").textContent).toContain("repo"), + ); + fireEvent.click(screen.getByTestId("new-chat-landing-workspace-chip")); + expect(screen.queryByTestId("workspace-native-dialog")).toBeNull(); + // The in-app picker is still available as the fallback. + expect(screen.getByTestId("workspace-picker")).toBeTruthy(); + }); + + it("discards the native-dialog result when the host switches mid-dialog", async () => { + // Open the dialog on host_1, then switch the selected host to host_2 + // before the result resolves. Host A's picked path must NOT populate + // host B's workspace field (cross-host contamination). + setDesktopHost("host_1"); + mockHosts([ + { + host_id: "host_1", + name: "machine-1", + owner: "me", + status: "online", + capabilities: { native_directory_dialog: true }, + }, + { + host_id: "host_2", + name: "machine-2", + owner: "me", + status: "online", + capabilities: { native_directory_dialog: true }, + }, + ]); + // Hold the dialog open until the test switches the host. + let resolvePick!: (v: { status: string; path: string | null; error: string | null }) => void; + const pickPromise = new Promise((r) => { + resolvePick = r as typeof resolvePick; + }); + const mutateAsync = vi.fn().mockReturnValue(pickPromise); + useNativeDialogMock.mockReturnValue({ mutateAsync, isPending: false }); + renderLanding(); + await waitFor(() => + expect(screen.getByTestId("new-chat-landing-workspace-chip").textContent).toContain("repo"), + ); + fireEvent.click(screen.getByTestId("new-chat-landing-workspace-chip")); + fireEvent.click(screen.getByTestId("workspace-native-dialog")); + expect(mutateAsync).toHaveBeenCalledWith("host_1"); + // Switch the selected host to host_2 while the dialog is still open. + fireEvent.pointerDown(screen.getByTestId("new-chat-landing-host-chip"), { button: 0 }); + fireEvent.click(screen.getByTestId("new-chat-landing-host-option-host_2")); + // Now resolve host_1's pick — it must be discarded. + resolvePick({ status: "ok", path: "/Users/corey/host-one-pick", error: null }); + await waitFor(() => expect(mutateAsync).toHaveBeenCalled()); + // The chip must NOT show host one's path — the result was discarded. + const chipText = screen.getByTestId("new-chat-landing-workspace-chip").textContent ?? ""; + expect(chipText).not.toContain("host-one-pick"); + }); + + it("moves a clicked older recent to the front of the list", async () => { + // Recents are recorded on selection (not only on session create), so + // clicking an older entry immediately promotes it. Verify the per-host + // localStorage ordering after the click. + setDesktopHost("host_1"); + mockHosts([ + { + host_id: "host_1", + name: "machine-1", + owner: "me", + status: "online", + capabilities: { native_directory_dialog: true }, + }, + ]); + // Seed two recents: newest first. + window.localStorage.setItem( + "omnigent:recent-workspaces", + JSON.stringify({ host_1: ["/Users/corey/repo", "/Users/corey/other"] }), + ); + renderLanding(); + await waitFor(() => + expect(screen.getByTestId("new-chat-landing-workspace-chip").textContent).toContain("repo"), + ); + fireEvent.click(screen.getByTestId("new-chat-landing-workspace-chip")); + await waitFor(() => + expect(screen.getByTestId("workspace-recent-/Users/corey/other")).toBeTruthy(), + ); + // Click the OLDER recent ("other") — it must jump to the front. + fireEvent.click(screen.getByTestId("workspace-recent-/Users/corey/other")); + const stored = JSON.parse( + window.localStorage.getItem("omnigent:recent-workspaces") ?? "{}", + ) as Record; + expect(stored.host_1[0]).toBe("/Users/corey/other"); + expect(stored.host_1).toContain("/Users/corey/repo"); + }); + it("hides the sandbox option when the server doesn't support managed sandboxes", () => { // Default renderLanding: managed_sandboxes_enabled false (the fail-closed // probe sentinel). The dropdown must not advertise a create path the diff --git a/web/src/shell/NewChatDialog.tsx b/web/src/shell/NewChatDialog.tsx index 3ea4c377dd..225d70cd76 100644 --- a/web/src/shell/NewChatDialog.tsx +++ b/web/src/shell/NewChatDialog.tsx @@ -22,6 +22,7 @@ import { Loader2Icon, FileTextIcon, FolderIcon, + ClockIcon, ImageIcon, PaperclipIcon, PlusIcon, @@ -121,7 +122,12 @@ import { nativeCodingAgentForAvailableAgent, nativeWrapperLabelsForAgent, } from "@/lib/nativeCodingAgents"; -import { useHostModelOptions, useHosts, type Host } from "@/hooks/useHosts"; +import { + hostSupportsNativeDirectoryDialog, + useHostModelOptions, + useHosts, + type Host, +} from "@/hooks/useHosts"; import { controlHost, getHostIdentity, @@ -140,6 +146,7 @@ import { useRecentWorkspaces } from "@/hooks/useRecentWorkspaces"; import { useDirectorySessions } from "@/hooks/useDirectorySessions"; import { useRunnerHealthRegistration } from "@/hooks/RunnerHealthProvider"; import { useHostFilesystem, type HostFilesystemEntry } from "@/hooks/useHostFilesystem"; +import { useOpenHostNativeDirectoryDialog } from "@/hooks/useHostNativeDirectoryDialog"; import { useHostWorktrees } from "@/hooks/useHostWorktrees"; import { useNativeServerSwitcherForMainSurface } from "@/hooks/useNativeServerSwitcher"; import type { WorkspaceFile } from "@/hooks/useWorkspaceChangedFiles"; @@ -2079,6 +2086,69 @@ export function NewChatLandingScreen() { }, []); const { recent, addRecent } = useRecentWorkspaces(selectedHostId); + // Live mirror of selectedHostId for the async native-dialog handler: the + // dialog can stay open across a host switch, and the result must NOT be + // applied if the user has since selected a different host (host A's path + // must never populate host B's workspace field). A ref reads the current + // value when the awaited promise resumes. + const selectedHostIdRef = useRef(selectedHostId); + selectedHostIdRef.current = selectedHostId; + // Record a workspace choice immediately (not only on session create) so a + // clicked older recent jumps to the front of the list right away and the + // recents list stays useful mid-flow. addRecent is per-host, de-dups, + // moves-to-front, and caps at 8 (see useRecentWorkspaces). + const commitWorkspace = useCallback( + (path: string) => { + setWorkspace(path); + addRecent(path); + }, + [addRecent], + ); + // OS-native folder chooser; only offered when the selected host is THIS + // desktop machine AND advertises the capability (local + interactive + // macOS host). The desktop-host identity match is the real locality gate + // — a GUI-capable remote host selected from another machine must NOT + // offer a dialog it would pop on a screen the user can't see. On any + // non-``ok`` result or thrown error the popover keeps the in-app + // WorkspacePicker, so the user always has a working browser. + const nativeDialog = useOpenHostNativeDirectoryDialog(); + // `selectedHost` is derived further down; read the capability from the + // hosts list here so this hook group stays above its declaration. The + // helper tolerates null/undefined, so an unset host reads as unsupported. + const selectedHostSupportsNativeDialog = hostSupportsNativeDirectoryDialog( + (hosts ?? []).find((h) => h.host_id === selectedHostId), + ); + + async function handleOpenNativeDirectoryDialog() { + if (selectedHostId === null) return; + // Capture the requesting host so a host switch while the OS dialog is + // open does NOT apply host A's chosen path to host B. The ref holds the + // live selection when the awaited result resumes. + const requestedHostId = selectedHostId; + let result; + try { + result = await nativeDialog.mutateAsync(requestedHostId); + } catch { + // Offline host, timeout, or network error — fall back to the + // in-app picker without bothering the user (the picker is + // already visible below this button). + return; + } + // Discard the result if the user switched hosts while the dialog was + // open — the path belongs to `requestedHostId`, not whatever is now + // selected. Leave the popover open so the user can pick on the new host. + if (selectedHostIdRef.current !== requestedHostId) return; + if (result.status === "ok" && result.path) { + // commitWorkspace sets the path AND records it as a recent + // immediately. setWorkspace runs the SAME path state used for the + // picker; the existing isValidWorkspace + session-create validation + // still gates the launch — the native dialog never weakens checks. + commitWorkspace(result.path); + setWorkspacePopoverOpen(false); + } + // cancelled / unsupported / error → leave the picker open as the + // fallback, silently. + } const allHosts = hosts ?? []; const onlineHosts = allHosts.filter((h) => h.status === "online"); @@ -3797,23 +3867,92 @@ export function NewChatLandingScreen() { narrow screen; desktop still gets the full width. */} {selectedHostId ? ( - occupancyByDir.get(normalizeWorkspacePath(abs) ?? "") ?? 0 - : undefined - } - /> +
+ {/* Per-host recent workspaces — one click sets the + workspace without launching. Most-recent-first, + capped at the hook's 8 entries; empty state is + hidden so a fresh host shows only the picker. */} + {recent.length > 0 && ( +
+ + Recent + + {recent.map((recentPath) => ( + + ))} +
+ )} + {/* OS-native folder chooser — primary action, shown + only when the selected host is THIS desktop + machine (locality) AND advertises the + capability (macOS + GUI). A GUI-capable remote + host selected from another machine is NOT + offered the dialog. The WorkspacePicker below is + always present as the secondary browser / + fallback. */} + {selectedHostSupportsNativeDialog && + thisMachineHostId !== null && + selectedHostId === thisMachineHostId && ( +
+ +
+ )} + occupancyByDir.get(normalizeWorkspacePath(abs) ?? "") ?? 0 + : undefined + } + /> +
) : (

Select a host first.

)}