From 8656a4ef4f7fa926ec3519eada462c461f9fe579 Mon Sep 17 00:00:00 2001 From: Jack Lau <72348727+jackulau@users.noreply.github.com> Date: Sat, 8 Aug 2026 21:01:35 -0500 Subject: [PATCH 01/25] feat(gateway): add session-scoped Desktop connection mode Adds the authoritative runtime value behind #82140: a task-local contextvar holding the Desktop shell's resolved 'local'/'remote' connection mode, plus normalize/set/read helpers. Deliberately kept out of _VAR_MAP so get_session_env's os.environ fallback never applies -- the value must not be user-configurable, or a skill can be told a gateway-side file is sitting on the Desktop machine when it isn't. Reset alongside the other session vars so a concurrent turn's mode can't be inherited. Goal: 001-desktop-connection-mode (deliverable 1) --- gateway/session_context.py | 95 ++++++++++++ tests/gateway/test_desktop_connection_mode.py | 143 ++++++++++++++++++ 2 files changed, 238 insertions(+) create mode 100644 tests/gateway/test_desktop_connection_mode.py diff --git a/gateway/session_context.py b/gateway/session_context.py index 7a2c53ab3a95..026f1e6a02b5 100644 --- a/gateway/session_context.py +++ b/gateway/session_context.py @@ -135,6 +135,34 @@ def session_context_engaged() -> bool: _CRON_AUTO_DELIVER_CHAT_ID: ContextVar = ContextVar("HERMES_CRON_AUTO_DELIVER_CHAT_ID", default=_UNSET) _CRON_AUTO_DELIVER_THREAD_ID: ContextVar = ContextVar("HERMES_CRON_AUTO_DELIVER_THREAD_ID", default=_UNSET) +# The resolved Desktop connection mode for this turn: 'local' when the Desktop +# app drives its own local backend, 'remote' when it drives an SSH/URL/cloud +# backend on another machine. ``None`` for every non-Desktop surface (CLI, TUI, +# messaging, cron, API server) and for a Desktop client too old to announce it. +# +# Deliberately NOT a member of ``_VAR_MAP``: mapped vars are readable through +# ``get_session_env``, which falls back to ``os.environ``. That fallback is what +# makes a var user-configurable, and this value must be authoritative — a user +# who exports HERMES_DESKTOP_CONNECTION_MODE=local must not be able to convince +# a skill that a gateway-side file is sitting on their Desktop machine. The only +# writer is :func:`set_desktop_connection_mode`, driven by the connection +# descriptor the Desktop shell already resolves (``getConnection().mode``). +# +# Read it with :func:`desktop_connection_mode`. The subprocess bridge in +# ``tools/environments/local.py`` stamps it onto child environments write-only +# (always overwritten, stripped when unset) so skills and their helper scripts +# can branch on it without it ever becoming an input. +_DESKTOP_CONNECTION_MODE: ContextVar = ContextVar("HERMES_DESKTOP_CONNECTION_MODE", default=_UNSET) + +# The env var name used for the write-only subprocess stamp. Not read anywhere. +DESKTOP_CONNECTION_MODE_ENV = "HERMES_DESKTOP_CONNECTION_MODE" + +# Saved-config connection modes that resolve to a backend on another machine. +# The Desktop descriptor already collapses these to 'remote', but the RPC edge +# accepts them so a client that forwards its raw saved mode still lands on a +# correct answer rather than an unavailable one. +_REMOTE_LIKE_CONNECTION_MODES = frozenset({"cloud", "remote", "ssh", "url"}) + _VAR_MAP = { "HERMES_SESSION_PLATFORM": _SESSION_PLATFORM, "HERMES_SESSION_SOURCE": _SESSION_SOURCE, @@ -158,6 +186,56 @@ def session_context_engaged() -> bool: } +def normalize_desktop_connection_mode(value: Any) -> str | None: + """Coerce a client-announced connection mode to ``'local'``/``'remote'``/``None``. + + ``'local'`` stays local; every remote-shaped saved mode (``remote``, + ``cloud``, ``ssh``, ``url``) resolves to ``'remote'``. Anything else — + empty, ``None``, a typo, a hostile string — resolves to ``None`` (mode + unknown), because a wrong answer here is worse than no answer: an extension + that believes a gateway-side path is Desktop-local will hand the user a link + to a file that isn't on their machine. + """ + text = str(value or "").strip().lower() + if text == "local": + return "local" + if text in _REMOTE_LIKE_CONNECTION_MODES: + return "remote" + return None + + +def set_desktop_connection_mode(value: Any) -> None: + """Bind the resolved Desktop connection mode for this task. + + Called by the Desktop-facing RPC edge with the mode the Desktop shell + resolved via ``window.hermesDesktop.getConnection()``. Non-Desktop surfaces + never call this, so :func:`desktop_connection_mode` keeps returning ``None`` + for them. + """ + _DESKTOP_CONNECTION_MODE.set(normalize_desktop_connection_mode(value)) + + +def desktop_connection_mode() -> str | None: + """The resolved Desktop connection mode, or ``None`` when not applicable. + + ``'local'`` — the Desktop app is driving its own local backend, so a + gateway-side path is already a path on the user's machine. + ``'remote'`` — the Desktop app is driving an SSH/URL/cloud backend, so a + gateway-side path must be transferred before the Desktop can + open it. + ``None`` — not a Desktop session (CLI, TUI, messaging, cron, API + server), or a Desktop client that didn't announce a mode. + + This is the only supported read path on the Python side. It reports the + connection *shape* and nothing else — no base URL, host, token, SSH key, or + auth mode ever passes through here. + """ + value = _DESKTOP_CONNECTION_MODE.get() + if value is _UNSET: + return None + return value + + def set_current_session_id(session_id: str) -> None: """Synchronize ``HERMES_SESSION_ID`` across ContextVar and ``os.environ``. @@ -232,6 +310,7 @@ def set_session_vars( async_delivery: bool = True, ui_session_id: str = "", cron_session: Any = _UNSET, + desktop_connection_mode: Any = None, ) -> list: """Set all session context variables and return reset tokens. @@ -251,6 +330,11 @@ def set_session_vars( ``cron_session`` is tri-state: ``_UNSET`` preserves legacy ``os.environ["HERMES_CRON_SESSION"]`` fallback, ``"1"`` marks a cron job, and ``""`` explicitly marks a non-cron session while masking leaked env. + + ``desktop_connection_mode`` is the Desktop shell's resolved connection mode + (see :func:`desktop_connection_mode`). Every caller that isn't the + Desktop-facing RPC edge leaves it ``None``, which is exactly the "not a + Desktop session" answer non-Desktop surfaces should report. """ # Mark the session-context machinery engaged for this process. The # subprocess-env bridge uses this to switch from "os.environ fallback" to @@ -275,6 +359,7 @@ def set_session_vars( _SESSION_PROFILE.set(profile), _CRON_SESSION.set(cron_session), _SESSION_ASYNC_DELIVERY.set(bool(async_delivery)), + _DESKTOP_CONNECTION_MODE.set(normalize_desktop_connection_mode(desktop_connection_mode)), ] try: from agent.runtime_cwd import set_session_cwd @@ -320,6 +405,11 @@ def clear_session_vars(tokens: list) -> None: # behavior (CLI / unaware paths), not be mistaken for an opted-out # stateless adapter. _SESSION_ASYNC_DELIVERY.set(_UNSET) + # A finished handler is no longer a Desktop turn. Setting None (rather than + # _UNSET) is the same "explicitly cleared" posture the mapped vars take — + # both read as "no Desktop connection", and there is no os.environ fallback + # behind this var for the distinction to matter. + _DESKTOP_CONNECTION_MODE.set(None) try: from agent.runtime_cwd import clear_session_cwd @@ -368,6 +458,11 @@ def reset_session_vars() -> None: # same inheritance-leak reason as the mapped vars above — see clear_session_vars, # which resets this var on the handler-exit path for the symmetric concern. _SESSION_ASYNC_DELIVERY.set(_UNSET) + # Same leak concern, sharper consequence: a task spawned from a context where + # a concurrent Desktop turn had bound 'local' would otherwise inherit it, and + # a skill running for a *remote* Desktop client (or for the CLI) would be told + # its gateway-side files are already on the user's machine. + _DESKTOP_CONNECTION_MODE.set(_UNSET) try: from agent.runtime_cwd import clear_session_cwd diff --git a/tests/gateway/test_desktop_connection_mode.py b/tests/gateway/test_desktop_connection_mode.py new file mode 100644 index 000000000000..90d9a0671cb8 --- /dev/null +++ b/tests/gateway/test_desktop_connection_mode.py @@ -0,0 +1,143 @@ +"""The resolved Desktop connection mode exposed to skills, MCP, and plugins. + +See NousResearch/hermes-agent#82140. The value answers one question — is the +gateway's filesystem the same machine the user is looking at? — and must answer +it authoritatively, so the tests below pin three properties: + +1. Only ``'local'``, ``'remote'``, or ``None`` ever come out. +2. A user-set ``HERMES_DESKTOP_CONNECTION_MODE`` in the environment is NOT a + source of truth (issue acceptance criterion: no user-configurable env var). +3. The value is task-local, so a concurrent local-Desktop turn can't convince a + remote-Desktop turn (or the CLI) that gateway files are already local. +""" + +import asyncio + +import pytest + +from gateway.session_context import ( + _DESKTOP_CONNECTION_MODE, + _UNSET, + _VAR_MAP, + DESKTOP_CONNECTION_MODE_ENV, + clear_session_vars, + desktop_connection_mode, + get_session_env, + normalize_desktop_connection_mode, + reset_session_vars, + set_desktop_connection_mode, + set_session_vars, +) + + +@pytest.fixture(autouse=True) +def _reset_contextvars(): + """Tests share one thread context; restore the "never bound" sentinel.""" + yield + for var in _VAR_MAP.values(): + var.set(_UNSET) + _DESKTOP_CONNECTION_MODE.set(_UNSET) + + +class TestNormalize: + @pytest.mark.parametrize("value", ["local", "LOCAL", " Local "]) + def test_local_variants_resolve_local(self, value): + assert normalize_desktop_connection_mode(value) == "local" + + @pytest.mark.parametrize("value", ["remote", "cloud", "ssh", "url", "SSH"]) + def test_remote_like_saved_modes_resolve_remote(self, value): + """A client forwarding its raw saved mode still gets a usable answer.""" + assert normalize_desktop_connection_mode(value) == "remote" + + @pytest.mark.parametrize("value", ["", None, " ", "lokal", "true", 0, [], {"mode": "local"}]) + def test_unknown_values_resolve_none_not_a_guess(self, value): + """Unknown must be None: a wrong 'local' sends the user to a missing file.""" + assert normalize_desktop_connection_mode(value) is None + + +class TestAccessor: + def test_unbound_session_reports_none(self): + assert desktop_connection_mode() is None + + def test_bound_mode_is_readable(self): + set_desktop_connection_mode("remote") + assert desktop_connection_mode() == "remote" + + def test_bound_garbage_reports_none(self): + set_desktop_connection_mode("something-else") + assert desktop_connection_mode() is None + + +class TestNotUserConfigurable: + """The acceptance criterion: no user-configurable HERMES_* env var.""" + + def test_env_var_is_not_a_source_of_truth(self, monkeypatch): + monkeypatch.setenv(DESKTOP_CONNECTION_MODE_ENV, "local") + assert desktop_connection_mode() is None + + def test_env_var_cannot_override_a_bound_remote_session(self, monkeypatch): + monkeypatch.setenv(DESKTOP_CONNECTION_MODE_ENV, "local") + set_desktop_connection_mode("remote") + assert desktop_connection_mode() == "remote" + + def test_not_reachable_through_get_session_env(self, monkeypatch): + """Mapped vars fall back to os.environ; this one must not be mapped.""" + assert DESKTOP_CONNECTION_MODE_ENV not in _VAR_MAP + monkeypatch.setenv(DESKTOP_CONNECTION_MODE_ENV, "local") + assert get_session_env(DESKTOP_CONNECTION_MODE_ENV, "") == "local" # raw env read + assert desktop_connection_mode() is None # the supported API is unmoved + + +class TestSessionLifecycle: + def test_set_session_vars_binds_the_mode(self): + set_session_vars(source="desktop", desktop_connection_mode="remote") + assert desktop_connection_mode() == "remote" + + def test_set_session_vars_defaults_to_none_for_non_desktop_surfaces(self): + set_session_vars(platform="telegram", chat_id="-100") + assert desktop_connection_mode() is None + + def test_clear_session_vars_drops_the_mode(self): + tokens = set_session_vars(source="desktop", desktop_connection_mode="local") + clear_session_vars(tokens) + assert desktop_connection_mode() is None + + def test_reset_session_vars_drops_an_inherited_mode(self): + """A freshly-spawned task must not inherit a sibling turn's mode.""" + set_desktop_connection_mode("local") + reset_session_vars() + assert desktop_connection_mode() is None + + def test_rebinding_reflects_a_connection_switch(self): + """Switching the active Desktop connection re-announces; last write wins.""" + set_session_vars(source="desktop", desktop_connection_mode="local") + assert desktop_connection_mode() == "local" + set_session_vars(source="desktop", desktop_connection_mode="remote") + assert desktop_connection_mode() == "remote" + + +def test_mode_is_task_local_across_concurrent_sessions(): + """Two concurrent Desktop clients on one gateway keep their own answers.""" + + async def scenario(): + seen: dict[str, str | None] = {} + started = asyncio.Event() + + async def turn(name: str, mode: str, wait_for_sibling: bool) -> None: + set_session_vars(source="desktop", desktop_connection_mode=mode) + if wait_for_sibling: + started.set() + else: + await started.wait() + # Yield so the sibling task definitely interleaves before we read. + await asyncio.sleep(0) + seen[name] = desktop_connection_mode() + + await asyncio.gather( + turn("local-client", "local", wait_for_sibling=True), + turn("remote-client", "remote", wait_for_sibling=False), + ) + return seen + + seen = asyncio.run(scenario()) + assert seen == {"local-client": "local", "remote-client": "remote"} From 981bd108f98e6030a4b563088de34fab1086f18e Mon Sep 17 00:00:00 2001 From: Jack Lau <72348727+jackulau@users.noreply.github.com> Date: Sat, 8 Aug 2026 21:05:01 -0500 Subject: [PATCH 02/25] feat(gateway): bind the Desktop-announced connection mode per turn The Desktop shell resolves local/remote via getConnection(); the TUI gateway now accepts it as a connection_mode param on session.create, session.resume and prompt.submit, stores it on the live session, and binds it into session context on every turn. Refreshing on prompt.submit (next to the existing client_surface rewrite, for the same reason) is what makes a mid-session connection or profile switch land on the next turn instead of being pinned to whatever was true when the chat was opened. An omitted param leaves the stored value alone so an older client can't erase a newer one's announcement. Gated on source == 'desktop': the mode describes the Desktop shell's relationship to this backend, so a stray param from the TUI or a messaging platform is ignored. Goal: 001-desktop-connection-mode (deliverable 2) --- .../test_desktop_connection_mode_rpc.py | 122 ++++++++++++++++++ tui_gateway/methods_prompt.py | 6 + tui_gateway/methods_session.py | 11 ++ tui_gateway/server.py | 69 ++++++++++ 4 files changed, 208 insertions(+) create mode 100644 tests/tui_gateway/test_desktop_connection_mode_rpc.py diff --git a/tests/tui_gateway/test_desktop_connection_mode_rpc.py b/tests/tui_gateway/test_desktop_connection_mode_rpc.py new file mode 100644 index 000000000000..9a2b457b00be --- /dev/null +++ b/tests/tui_gateway/test_desktop_connection_mode_rpc.py @@ -0,0 +1,122 @@ +"""The TUI gateway's Desktop connection-mode plumbing (#82140). + +The Desktop shell already resolves ``local``/``remote`` via +``window.hermesDesktop.getConnection()``. These tests pin the server side of +that announcement: where it is stored, when it is refreshed, and which sessions +are allowed to have one at all. + +The helpers under test are pure dict/param transforms, so they run without +standing up a gateway. +""" + +import pytest + +from gateway.session_context import _DESKTOP_CONNECTION_MODE, _UNSET, _VAR_MAP + + +def _srv(): + import tui_gateway.server as srv + + return srv + + +@pytest.fixture(autouse=True) +def _reset_contextvars(): + yield + for var in _VAR_MAP.values(): + var.set(_UNSET) + _DESKTOP_CONNECTION_MODE.set(_UNSET) + + +def _desktop_session(**extra) -> dict: + return {"session_key": "k", "source": "desktop", **extra} + + +class TestNormalizeParam: + def test_reads_and_normalizes_the_param(self): + assert _srv()._normalize_connection_mode_param({"connection_mode": "cloud"}) == "remote" + + @pytest.mark.parametrize("params", [None, {}, {"connection_mode": ""}, {"connection_mode": "nope"}]) + def test_missing_or_unknown_is_none(self, params): + assert _srv()._normalize_connection_mode_param(params) is None + + +class TestSessionConnectionMode: + def test_desktop_session_reports_its_mode(self): + session = _desktop_session(connection_mode="remote") + assert _srv()._session_connection_mode(session) == "remote" + + @pytest.mark.parametrize("source", ["tui", "telegram", "cli", "kanban"]) + def test_non_desktop_sources_never_report_a_mode(self, source): + """A stray connection_mode from a non-Desktop client must not be honored.""" + session = {"session_key": "k", "source": source, "connection_mode": "local"} + assert _srv()._session_connection_mode(session) is None + + def test_missing_session_is_none(self): + assert _srv()._session_connection_mode(None) is None + + def test_desktop_session_without_an_announcement_is_none(self): + assert _srv()._session_connection_mode(_desktop_session()) is None + + +class TestRememberConnectionMode: + def test_refreshes_the_stored_mode(self): + """This is what makes a mid-session connection switch land.""" + session = _desktop_session(connection_mode="local") + _srv()._remember_connection_mode(session, {"connection_mode": "remote"}) + assert session["connection_mode"] == "remote" + + def test_omitted_param_leaves_the_stored_mode_alone(self): + """An older Desktop build must not erase a mode a newer one announced.""" + session = _desktop_session(connection_mode="remote") + _srv()._remember_connection_mode(session, {"text": "hello"}) + assert session["connection_mode"] == "remote" + + def test_explicit_unknown_value_clears_to_none(self): + """Explicitly unknown is 'I don't know', not 'keep believing local'.""" + session = _desktop_session(connection_mode="local") + _srv()._remember_connection_mode(session, {"connection_mode": "banana"}) + assert session["connection_mode"] is None + + def test_no_session_is_a_noop(self): + _srv()._remember_connection_mode(None, {"connection_mode": "remote"}) + + +class TestBindSessionContext: + """``_set_session_context`` is what every turn runs through.""" + + def test_binds_a_desktop_session_mode(self, monkeypatch): + from gateway.session_context import desktop_connection_mode + + srv = _srv() + session = _desktop_session(connection_mode="remote") + monkeypatch.setattr(srv, "_sessions", {"s1": session}, raising=False) + srv._set_session_context("k") + assert desktop_connection_mode() == "remote" + + def test_non_desktop_session_binds_none(self, monkeypatch): + from gateway.session_context import desktop_connection_mode + + srv = _srv() + session = {"session_key": "k", "source": "tui", "connection_mode": "local"} + monkeypatch.setattr(srv, "_sessions", {"s1": session}, raising=False) + srv._set_session_context("k") + assert desktop_connection_mode() is None + + def test_unknown_session_key_binds_none(self, monkeypatch): + from gateway.session_context import desktop_connection_mode + + srv = _srv() + monkeypatch.setattr(srv, "_sessions", {}, raising=False) + srv._set_session_context("no-such-key") + assert desktop_connection_mode() is None + + +def test_new_session_records_carry_a_connection_mode_slot(): + """Both live-session record shapes must have the field _set_session_context reads.""" + srv = _srv() + record = srv._deferred_session_record( + "key", cols=80, cwd="", history=[], lease=None, source="desktop", + connection_mode="remote", + ) + assert record["connection_mode"] == "remote" diff --git a/tui_gateway/methods_prompt.py b/tui_gateway/methods_prompt.py index 745a855e3d6b..23fe73a11227 100644 --- a/tui_gateway/methods_prompt.py +++ b/tui_gateway/methods_prompt.py @@ -319,6 +319,12 @@ def _(rid, params: dict) -> dict: # in turn: a stale "hud" would tell the model the user is still floating # over another app when they are back in Hermes. session["client_surface"] = "hud" if params.get("surface") == "hud" else "" + # Same reasoning for the Desktop connection mode (#82140): the user can + # switch the active connection or profile between turns, and an extension + # that acts on a stale "local" hands them a link to a file that lives on the + # gateway machine. The client re-announces on every submit; an omitted + # ``connection_mode`` (older client) leaves the stored value alone. + _remember_connection_mode(session, params) has_truncation = ( truncate_user_ordinal is not None or params.get("truncate_before_row_id") is not None diff --git a/tui_gateway/methods_session.py b/tui_gateway/methods_session.py index 281a18b53fe1..0dd4598b13e1 100644 --- a/tui_gateway/methods_session.py +++ b/tui_gateway/methods_session.py @@ -83,6 +83,11 @@ def _(rid, params: dict) -> dict: "close_on_disconnect": is_truthy_value(params.get("close_on_disconnect", False)), "active_session_lease": lease, "cols": cols, + # The Desktop shell's resolved 'local'/'remote' connection mode for + # THIS backend (#82140). Refreshed on every resume/prompt so a + # connection or profile switch lands on the next turn. None for + # every non-Desktop client. + "connection_mode": _normalize_connection_mode_param(params), "created_at": now, "edit_snapshots": {}, "explicit_cwd": explicit_cwd, @@ -482,6 +487,7 @@ def _reuse_live_payload(sid: str, session: dict) -> dict: close_on_disconnect=is_truthy_value(params.get("close_on_disconnect", False)), profile_home=profile_home, lazy=True, + connection_mode=_normalize_connection_mode_param(params), ) if (live := _claim_or_reuse_live(sid, target, record, lease)) is not None: return _ok(rid, _reuse_live_payload(*live)) @@ -646,6 +652,7 @@ def _reuse_live_payload(sid: str, session: dict) -> dict: profile_home=profile_home, model_override=overrides.get("model_override"), resume_runtime_overrides=overrides or None, + connection_mode=_normalize_connection_mode_param(params), ) if (live := _claim_or_reuse_live(sid, target, record, lease)) is not None: return _ok(rid, _reuse_live_payload(*live)) @@ -790,6 +797,7 @@ def _reuse_live_payload(sid: str, session: dict) -> dict: cwd=profile_resume_cwd, session_db=db, source=source, + connection_mode=_normalize_connection_mode_param(params), ) # Ownership TRANSFER — the registered session's agent now # holds this handle for its whole life, and _init_session @@ -3061,6 +3069,9 @@ def _visible_branch_history(messages): session_db=branch_db, source=source, profile_home=parent_home, + # A branch inherits the parent chat's connection mode: it is the + # same Desktop client talking to the same backend. + connection_mode=_session_connection_mode(session), ) # Ownership TRANSFER — the branched session's agent holds this # handle for its whole life and closes it on teardown. Drop is diff --git a/tui_gateway/server.py b/tui_gateway/server.py index bd3c38ffde28..83c9b0cf072b 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -2878,6 +2878,63 @@ def _session_source(session: dict | None) -> str: return _resolve_session_platform() +def _session_connection_mode(session: dict | None) -> str | None: + """The Desktop connection mode this session's client announced, if any. + + Gated on ``source == 'desktop'``: the mode describes the Desktop shell's + relationship to this backend, so it is meaningless coming from the TUI, a + messaging platform, or a plugin-opened session — and a stray + ``connection_mode`` param from one of those must not be honored. Every + non-Desktop surface therefore reports ``None``, which is what + ``desktop_connection_mode()`` promises them. See #82140. + """ + if not session or _session_source(session) != "desktop": + return None + try: + from gateway.session_context import normalize_desktop_connection_mode + + return normalize_desktop_connection_mode(session.get("connection_mode")) + except Exception: + return None + + +def _normalize_connection_mode_param(params: dict | None) -> str | None: + """Read ``connection_mode`` out of RPC *params* for a brand-new session.""" + try: + from gateway.session_context import normalize_desktop_connection_mode + + return normalize_desktop_connection_mode((params or {}).get("connection_mode")) + except Exception: + return None + + +def _remember_connection_mode(session: dict | None, params: dict | None) -> None: + """Refresh a session's announced Desktop connection mode from RPC *params*. + + The Desktop re-announces on every session.create/resume and every + prompt.submit, so switching the active connection or profile mid-session is + reflected on the very next turn rather than being pinned to whatever was + true when the chat was opened. + + An OMITTED ``connection_mode`` leaves the stored value alone — an older + Desktop build (or an internal caller that reuses these handlers) must not + silently erase a mode a newer client already announced. An explicitly + unrecognized value stores ``None`` ("mode unknown"), which is the safe + answer: extensions fall back to treating the location as unknown instead of + assuming local. + """ + if session is None or not params or "connection_mode" not in params: + return + try: + from gateway.session_context import normalize_desktop_connection_mode + + session["connection_mode"] = normalize_desktop_connection_mode( + params.get("connection_mode") + ) + except Exception: + pass + + def _register_session_cwd(session: dict | None) -> None: if not session: return @@ -3424,6 +3481,7 @@ def _set_session_context( # fall back to the session_key (matching the id derivation used at # session-finalize), so an identified session is never left blank. session_id = session_key + connection_mode = None with _sessions_lock: for sess in list(_sessions.values()): if sess.get("session_key") == session_key: @@ -3431,6 +3489,7 @@ def _set_session_context( session_id = ( getattr(sess.get("agent"), "session_id", None) or session_key ) + connection_mode = _session_connection_mode(sess) break return set_session_vars( session_key=session_key, @@ -3439,6 +3498,7 @@ def _set_session_context( cwd=resolved, ui_session_id=ui_session_id, cron_session="", + desktop_connection_mode=connection_mode, ) except Exception: return [] @@ -7027,6 +7087,7 @@ def _init_session( session_db=None, source: str | None = None, profile_home: str | None = None, + connection_mode: str | None = None, ): now = time.time() with _sessions_lock: @@ -7047,6 +7108,10 @@ def _init_session( "slash_worker": None, "show_reasoning": _load_show_reasoning(), "source": _resolve_session_source(source), + # Desktop shell's resolved 'local'/'remote' connection mode (#82140); + # None for every non-Desktop client. Refreshed per turn from + # prompt.submit so a connection/profile switch lands immediately. + "connection_mode": connection_mode, "tool_progress_mode": _load_tool_progress_mode(), "edit_snapshots": {}, "tool_started_at": {}, @@ -8388,6 +8453,7 @@ def _deferred_session_record( lazy: bool = False, model_override=None, resume_runtime_overrides: dict | None = None, + connection_mode: str | None = None, ) -> dict: """A live-session record whose AIAgent is built later (lazy watch / cold resume) — _init_session's shape minus the agent.""" @@ -8400,6 +8466,9 @@ def _deferred_session_record( "close_on_disconnect": close_on_disconnect, "active_session_lease": lease, "cols": cols, + # Desktop shell's resolved 'local'/'remote' connection mode (#82140); + # None for every non-Desktop client. + "connection_mode": connection_mode, "created_at": now, "cwd": cwd, "display_history_prefix": display_history_prefix or [], From 1fbf8e47fc98bdf8d76275624c85a93731415acc Mon Sep 17 00:00:00 2001 From: Jack Lau <72348727+jackulau@users.noreply.github.com> Date: Sat, 8 Aug 2026 21:06:12 -0500 Subject: [PATCH 03/25] feat(tools): expose the Desktop connection mode to skill subprocesses The subprocess bridge now stamps HERMES_DESKTOP_CONNECTION_MODE from the live session so a skill's helper script can tell whether a gateway-side artifact is already on the user's machine. The stamp is strictly write-only and unconditional: set when a mode is bound, popped otherwise, on every spawn. An inherited value from the user's shell -- or a stale one from a previous turn -- is removed rather than passed through, which is what keeps the contextvar the only source of truth and satisfies #82140's "no user-configurable HERMES_* env var". Goal: 001-desktop-connection-mode (deliverable 3) --- .../tools/test_desktop_connection_mode_env.py | 115 ++++++++++++++++++ tools/environments/local.py | 20 +++ 2 files changed, 135 insertions(+) create mode 100644 tests/tools/test_desktop_connection_mode_env.py diff --git a/tests/tools/test_desktop_connection_mode_env.py b/tests/tools/test_desktop_connection_mode_env.py new file mode 100644 index 000000000000..fedfd3151ce1 --- /dev/null +++ b/tests/tools/test_desktop_connection_mode_env.py @@ -0,0 +1,115 @@ +"""The skill-facing read path for the Desktop connection mode (#82140). + +Skills branch on ``HERMES_DESKTOP_CONNECTION_MODE`` from their helper scripts to +decide whether a gateway-side artifact is already on the user's machine or has +to be transferred first. The subprocess bridge stamps it — **write-only**, on +every spawn — which is precisely what keeps the issue's "no user-configurable +``HERMES_*`` env var" criterion true: a value inherited from the user's shell is +stripped rather than honored, and the contextvar remains the only source. +""" + +import os + +import pytest + +import gateway.session_context as sc +from gateway.session_context import ( + DESKTOP_CONNECTION_MODE_ENV as MODE_ENV, + _VAR_MAP, + clear_session_vars, + set_desktop_connection_mode, + set_session_vars, +) +from tools.environments.local import _make_run_env + +SESSION_VARS = list(_VAR_MAP.keys()) + + +@pytest.fixture(autouse=True) +def _isolate_session_context(): + """Clean ContextVar + os.environ + engaged-latch slate per test, restored.""" + tracked = SESSION_VARS + [MODE_ENV] + saved_env = {k: os.environ.get(k) for k in tracked} + saved_ctx = {name: var.get() for name, var in _VAR_MAP.items()} + saved_mode = sc._DESKTOP_CONNECTION_MODE.get() + saved_engaged = sc._session_context_engaged + for var in _VAR_MAP.values(): + var.set(sc._UNSET) + sc._DESKTOP_CONNECTION_MODE.set(sc._UNSET) + sc._session_context_engaged = False + try: + yield + finally: + for var, val in zip(_VAR_MAP.values(), saved_ctx.values()): + var.set(val) + sc._DESKTOP_CONNECTION_MODE.set(saved_mode) + sc._session_context_engaged = saved_engaged + for k, v in saved_env.items(): + if v is None: + os.environ.pop(k, None) + else: + os.environ[k] = v + + +@pytest.mark.parametrize("mode", ["local", "remote"]) +def test_bound_mode_is_stamped_for_the_child(mode): + set_desktop_connection_mode(mode) + assert _make_run_env({})[MODE_ENV] == mode + + +def test_remote_like_saved_mode_is_stamped_normalized(): + set_desktop_connection_mode("ssh") + assert _make_run_env({})[MODE_ENV] == "remote" + + +def test_unbound_session_stamps_nothing(): + """CLI, TUI, messaging, cron: the var is simply absent.""" + assert MODE_ENV not in _make_run_env({}) + + +def test_inherited_env_value_is_stripped_when_no_mode_is_bound(monkeypatch): + """The criterion: a user-set value is NOT a source of truth. + + A user who exports HERMES_DESKTOP_CONNECTION_MODE=local in their shell must + not be able to convince a CLI-session skill that gateway files are sitting + on a Desktop machine. + """ + monkeypatch.setenv(MODE_ENV, "local") + assert MODE_ENV not in _make_run_env({}) + + +def test_inherited_env_value_cannot_override_the_live_mode(monkeypatch): + """A remote Desktop session stays remote no matter what the shell says.""" + monkeypatch.setenv(MODE_ENV, "local") + set_desktop_connection_mode("remote") + assert _make_run_env({})[MODE_ENV] == "remote" + + +def test_stale_value_from_a_previous_turn_does_not_survive(monkeypatch): + """Each spawn re-derives; a cleared session strips rather than lingers.""" + tokens = set_session_vars(source="desktop", desktop_connection_mode="local") + assert _make_run_env({})[MODE_ENV] == "local" + monkeypatch.setenv(MODE_ENV, "local") # simulate a leaked process-global + clear_session_vars(tokens) + assert MODE_ENV not in _make_run_env({}) + + +def test_set_session_vars_carries_the_mode_through_to_the_child(): + tokens = set_session_vars(source="desktop", desktop_connection_mode="remote") + try: + assert _make_run_env({})[MODE_ENV] == "remote" + finally: + clear_session_vars(tokens) + + +def test_no_connection_details_are_ever_stamped(): + """Only the mode crosses the boundary — never a URL, host, or token.""" + set_desktop_connection_mode("remote") + env = _make_run_env({}) + leaked = [ + key + for key in env + if key.startswith("HERMES_DESKTOP") and key != MODE_ENV + ] + assert leaked == [] + assert env[MODE_ENV] in {"local", "remote"} diff --git a/tools/environments/local.py b/tools/environments/local.py index de2a6e034670..5c89b96a5ed3 100644 --- a/tools/environments/local.py +++ b/tools/environments/local.py @@ -436,6 +436,8 @@ def _inject_session_context_env(env: dict) -> None: from gateway.session_context import ( _UNSET, _VAR_MAP, + DESKTOP_CONNECTION_MODE_ENV, + desktop_connection_mode, session_context_engaged, ) except Exception: @@ -452,6 +454,24 @@ def _inject_session_context_env(env: dict) -> None: # inherited global so a sibling session's value can't leak in. env.pop(var_name, None) + # The Desktop connection mode (#82140) is the skill-facing read path for + # "is the gateway's filesystem the machine the user is looking at?" — + # skills and their helper scripts branch on it to decide whether a file has + # to be transferred before it can be presented for local viewing/editing. + # + # STRICTLY WRITE-ONLY, unconditionally: stamped when a mode is bound and + # POPPED otherwise, on every spawn. That is deliberate and is what keeps the + # value from becoming a user-configurable env var — an inherited + # HERMES_DESKTOP_CONNECTION_MODE from the user's shell (or a stale one from + # a previous turn) is removed rather than passed through, so a child can + # only ever see what the live session actually resolved. Nothing in Hermes + # reads this name back; the source of truth is the contextvar. + mode = desktop_connection_mode() + if mode: + env[DESKTOP_CONNECTION_MODE_ENV] = mode + else: + env.pop(DESKTOP_CONNECTION_MODE_ENV, None) + def _sanitize_subprocess_env(base_env: dict | None, extra_env: dict | None = None) -> dict: """Filter Hermes-managed secrets from a subprocess environment.""" From 3cfb72410b35c7720c2d94005a1663723eec4fe6 Mon Sep 17 00:00:00 2001 From: Jack Lau <72348727+jackulau@users.noreply.github.com> Date: Sat, 8 Aug 2026 21:07:28 -0500 Subject: [PATCH 04/25] feat(mcp): carry the Desktop connection mode in per-call _meta MCP servers can't read gateway contextvars, and a stdio server's env is fixed at spawn while the mode is per-session -- one gateway can serve a local Desktop client and a remote one at the same time. Per-call `_meta` is the only vehicle that is both live and session-correct. Sends exactly one key (the third-party-namespaced hermes-agent.nousresearch.com/desktop-connection-mode) holding local/remote, and only when a Desktop session is bound. The SDK's support for per-call meta is probed rather than assumed, so an older mcp package keeps today's request shape instead of raising. Goal: 001-desktop-connection-mode (deliverable 4) --- tests/tools/test_mcp_connection_mode_meta.py | 101 +++++++++++++++++++ tools/mcp_tool.py | 60 ++++++++++- 2 files changed, 160 insertions(+), 1 deletion(-) create mode 100644 tests/tools/test_mcp_connection_mode_meta.py diff --git a/tests/tools/test_mcp_connection_mode_meta.py b/tests/tools/test_mcp_connection_mode_meta.py new file mode 100644 index 000000000000..71cb7e2585a6 --- /dev/null +++ b/tests/tools/test_mcp_connection_mode_meta.py @@ -0,0 +1,101 @@ +"""MCP servers read the Desktop connection mode from per-call ``_meta`` (#82140). + +An MCP server can't read the gateway's contextvars, and its stdio env is fixed +at spawn time while the mode is per-session — one gateway can serve a local +Desktop client and a remote one at once. Per-call ``_meta`` is the only vehicle +that is both live and session-correct. +""" + +import pytest + +import gateway.session_context as sc +from gateway.session_context import set_desktop_connection_mode +from tools.mcp_tool import ( + MCP_DESKTOP_CONNECTION_MODE_META_KEY as META_KEY, + _call_tool_meta, + _call_tool_supports_meta, +) + + +@pytest.fixture(autouse=True) +def _reset_mode(): + saved = sc._DESKTOP_CONNECTION_MODE.get() + sc._DESKTOP_CONNECTION_MODE.set(sc._UNSET) + try: + yield + finally: + sc._DESKTOP_CONNECTION_MODE.set(saved) + + +@pytest.mark.parametrize("mode", ["local", "remote"]) +def test_bound_mode_becomes_call_meta(mode): + set_desktop_connection_mode(mode) + assert _call_tool_meta() == {META_KEY: mode} + + +def test_remote_like_saved_mode_is_normalized_before_it_leaves(): + set_desktop_connection_mode("cloud") + assert _call_tool_meta() == {META_KEY: "remote"} + + +def test_non_desktop_session_sends_no_meta(): + """CLI/TUI/messaging requests keep exactly today's shape.""" + assert _call_tool_meta() is None + + +def test_meta_carries_the_mode_and_nothing_else(): + """No base URL, host, token, SSH key, or auth mode may ride along.""" + set_desktop_connection_mode("remote") + meta = _call_tool_meta() + assert list(meta) == [META_KEY] + assert meta[META_KEY] in {"local", "remote"} + + +def test_meta_key_does_not_squat_the_reserved_spec_prefix(): + """MCP reserves `modelcontextprotocol.io/` for the spec itself.""" + assert not META_KEY.startswith("modelcontextprotocol.io/") + assert "/" in META_KEY + + +class TestSdkCapabilityProbe: + def test_probe_is_boolean_and_never_raises_without_the_sdk(self): + _call_tool_supports_meta.cache_clear() + try: + assert isinstance(_call_tool_supports_meta(), bool) + finally: + _call_tool_supports_meta.cache_clear() + + def test_probe_reports_false_when_the_sdk_lacks_meta(self, monkeypatch): + """An older SDK degrades to today's request shape instead of raising.""" + import sys + import types + + class _Session: + async def call_tool(self, name, arguments=None): # no `meta` param + ... + + module = types.ModuleType("mcp") + module.ClientSession = _Session + monkeypatch.setitem(sys.modules, "mcp", module) + _call_tool_supports_meta.cache_clear() + try: + assert _call_tool_supports_meta() is False + finally: + _call_tool_supports_meta.cache_clear() + + def test_probe_reports_true_when_the_sdk_accepts_meta(self, monkeypatch): + import sys + import types + + class _Session: + async def call_tool(self, name, arguments=None, meta=None): + ... + + module = types.ModuleType("mcp") + module.ClientSession = _Session + monkeypatch.setitem(sys.modules, "mcp", module) + _call_tool_supports_meta.cache_clear() + try: + assert _call_tool_supports_meta() is True + finally: + _call_tool_supports_meta.cache_clear() diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py index 49fbf6f7eea4..e9e4e84de31f 100644 --- a/tools/mcp_tool.py +++ b/tools/mcp_tool.py @@ -99,6 +99,7 @@ import concurrent.futures import errno import fnmatch +import functools import inspect import json import logging @@ -571,6 +572,52 @@ def _context_var_value(ref: str) -> Optional[str]: return None +# --------------------------------------------------------------------------- +# Per-call request metadata +# --------------------------------------------------------------------------- + +# The `_meta` key carrying the resolved Desktop connection mode to MCP servers +# (#82140). MCP reserves the `modelcontextprotocol.io/` prefix for the spec, so +# this uses the documented third-party form: a domain we own plus a path. +MCP_DESKTOP_CONNECTION_MODE_META_KEY = "hermes-agent.nousresearch.com/desktop-connection-mode" + + +@functools.lru_cache(maxsize=1) +def _call_tool_supports_meta() -> bool: + """Whether the installed MCP SDK's ``call_tool`` accepts per-call ``meta``. + + Per-call metadata landed after the transport API stabilized, so probe rather + than pin behavior to a version: on an SDK without it we simply omit the + field and MCP servers see exactly today's request shape. + """ + try: + from mcp import ClientSession + + return "meta" in inspect.signature(ClientSession.call_tool).parameters + except Exception: + return False + + +def _call_tool_meta() -> Optional[dict]: + """Per-call ``_meta`` for the current turn, or ``None`` when there's nothing to say. + + Carries the resolved Desktop connection mode so an MCP server can tell + whether a path it returns will be openable on the machine the user is + looking at. Deliberately narrow: the mode and nothing else — no base URL, + host, token, SSH key, or auth mode. Non-Desktop sessions (CLI, TUI, + messaging, cron) contribute no key at all, so their requests are unchanged. + """ + try: + from gateway.session_context import desktop_connection_mode + + mode = desktop_connection_mode() + except Exception: + return None + if not mode: + return None + return {MCP_DESKTOP_CONNECTION_MODE_META_KEY: mode} + + # --------------------------------------------------------------------------- # Security helpers # --------------------------------------------------------------------------- @@ -5428,8 +5475,19 @@ async def _call(): # task, which doesn't inherit our contextvars) can replay # it and detect the gateway platform / session for routing. server._pending_call_context = contextvars.copy_context() + # Per-call `_meta` rides the request so a server can see the + # resolved Desktop connection mode (#82140). Read here, inside + # the agent's context, and only sent when the SDK supports it + # and there is a mode to report — otherwise the request shape + # is byte-identical to before. + call_meta = _call_tool_meta() if _call_tool_supports_meta() else None try: - result = await server.session.call_tool(tool_name, arguments=args) + if call_meta: + result = await server.session.call_tool( + tool_name, arguments=args, meta=call_meta + ) + else: + result = await server.session.call_tool(tool_name, arguments=args) finally: server._pending_call_context = None # The RPC round-trip completed — the session is demonstrably From 1743f0e96402c5f908b6b005a2bf37a4ce864993 Mon Sep 17 00:00:00 2001 From: Jack Lau <72348727+jackulau@users.noreply.github.com> Date: Sat, 8 Aug 2026 21:15:27 -0500 Subject: [PATCH 05/25] feat(desktop): expose the resolved connection mode to plugins Adds ctx.connection to the plugin contract -- mode() for a point-in-time read and onModeChange() for switches -- so a plugin can tell whether a gateway-side path is openable on this machine without reaching through the raw Electron bridge. Mode only: base URL, host, tokens, SSH keys and auth mode stay behind the bridge. It reads the live $connection atom rather than calling getConnection() directly, because the atom is what stays in lockstep with the ACTIVE profile; a raw bridge call describes the primary window backend, which is the wrong answer whenever a background profile is active. Only real transitions are forwarded, so a reconnect that re-mints the descriptor on the same mode doesn't wake every listener. The renderer also announces its mode to the backend on session.create/resume and every prompt.submit. That is stamped at the single requestGateway choke point rather than at the ~10 call sites, so a new session or prompt path announces by construction. Goal: 001-desktop-connection-mode (deliverable 5) --- .../app/gateway/hooks/use-gateway-request.ts | 11 +++- apps/desktop/src/contrib/plugin.test.ts | 61 ++++++++++++++++- apps/desktop/src/contrib/plugin.ts | 56 ++++++++++++++++ apps/desktop/src/lib/connection-mode.test.ts | 64 ++++++++++++++++++ apps/desktop/src/lib/connection-mode.ts | 66 +++++++++++++++++++ apps/desktop/src/sdk/index.ts | 2 + 6 files changed, 257 insertions(+), 3 deletions(-) create mode 100644 apps/desktop/src/lib/connection-mode.test.ts create mode 100644 apps/desktop/src/lib/connection-mode.ts diff --git a/apps/desktop/src/app/gateway/hooks/use-gateway-request.ts b/apps/desktop/src/app/gateway/hooks/use-gateway-request.ts index 04f53f785b48..4a3e698da805 100644 --- a/apps/desktop/src/app/gateway/hooks/use-gateway-request.ts +++ b/apps/desktop/src/app/gateway/hooks/use-gateway-request.ts @@ -3,9 +3,10 @@ import { useStore } from '@nanostores/react' import { useCallback, useEffect, useRef } from 'react' import type { HermesGateway } from '@/hermes' +import { resolveConnectionMode, withConnectionMode } from '@/lib/connection-mode' import { $gateway, ensureActiveGatewayOpen, isActivePrimary } from '@/store/gateway' import { $activeGatewayProfile } from '@/store/profile' -import { $gatewayState, setConnection } from '@/store/session' +import { $connection, $gatewayState, setConnection } from '@/store/session' export function useGatewayRequest() { const gatewayState = useStore($gatewayState) @@ -104,13 +105,19 @@ export function useGatewayRequest() { }, []) const requestGateway = useCallback( - async (method: string, params: Record = {}, timeoutMs?: number, signal?: AbortSignal) => { + async (method: string, rawParams: Record = {}, timeoutMs?: number, signal?: AbortSignal) => { const gateway = gatewayRef.current if (!gateway) { throw new Error('Hermes gateway unavailable') } + // Announce the live connection mode on session/prompt RPCs (#82140). + // Read here, per request, so a connection or profile switch reaches the + // backend on the very next turn — $connection is kept in lockstep with + // the active profile by syncConnectionToActiveProfile. + const params = withConnectionMode(method, rawParams, resolveConnectionMode($connection.get())) + try { return await gateway.request(method, params, timeoutMs, signal) } catch (error) { diff --git a/apps/desktop/src/contrib/plugin.test.ts b/apps/desktop/src/contrib/plugin.test.ts index 9f522fe7038b..2417043e1fd6 100644 --- a/apps/desktop/src/contrib/plugin.test.ts +++ b/apps/desktop/src/contrib/plugin.test.ts @@ -1,6 +1,8 @@ -import { describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { HermesConnection } from '@/global' import { dispatchPluginNativeNotification } from '@/store/native-notifications' +import { $connection, setConnection } from '@/store/session' import { createPluginContext } from './plugin' @@ -60,3 +62,60 @@ describe('createPluginContext.os', () => { } }) }) + +describe('createPluginContext.connection', () => { + afterEach(() => { + setConnection(null) + }) + + const conn = (mode?: 'local' | 'remote') => + ({ baseUrl: 'http://127.0.0.1:8787', mode, token: 'secret' }) as unknown as HermesConnection + + it('reports the live mode without exposing the descriptor', () => { + setConnection(conn('remote')) + const ctx = createPluginContext('demo') + + expect(ctx.connection.mode()).toBe('remote') + // The whole door is two functions — there is no descriptor to reach past. + expect(Object.keys(ctx.connection).sort()).toEqual(['mode', 'onModeChange']) + }) + + it('reports null before a connection resolves', () => { + expect(createPluginContext('demo').connection.mode()).toBeNull() + }) + + it('fires immediately and on every real transition', () => { + setConnection(conn('local')) + const seen: Array<'local' | 'remote' | null> = [] + createPluginContext('demo').connection.onModeChange(mode => seen.push(mode)) + + setConnection(conn('remote')) + setConnection(null) + + expect(seen).toEqual(['local', 'remote', null]) + }) + + it('stays quiet when a descriptor refresh does not move the mode', () => { + setConnection(conn('remote')) + const listener = vi.fn() + createPluginContext('demo').connection.onModeChange(listener) + + // A reconnect re-mints the descriptor (new token/wsUrl) on the same mode. + setConnection({ ...conn('remote'), token: 'rotated' } as unknown as HermesConnection) + + expect(listener).toHaveBeenCalledTimes(1) + }) + + it('stops listening when the plugin unloads, even if the disposer is ignored', () => { + setConnection(conn('local')) + const disposers: Array<() => void> = [] + const listener = vi.fn() + createPluginContext('demo', dispose => disposers.push(dispose)).connection.onModeChange(listener) + + disposers.forEach(dispose => dispose()) + setConnection(conn('remote')) + + expect(listener).toHaveBeenCalledTimes(1) + expect($connection.get()?.mode).toBe('remote') + }) +}) diff --git a/apps/desktop/src/contrib/plugin.ts b/apps/desktop/src/contrib/plugin.ts index aa5d0f107ad7..58ee9c5d1fba 100644 --- a/apps/desktop/src/contrib/plugin.ts +++ b/apps/desktop/src/contrib/plugin.ts @@ -14,13 +14,16 @@ import { pluginRest, type PluginRestOptions, pluginSocket } from '@/hermes' import { createPluginI18n, type PluginI18n } from '@/i18n' +import { type HermesConnectionMode, resolveConnectionMode } from '@/lib/connection-mode' import { readKey, writeKey } from '@/lib/storage' import { dispatchPluginNativeNotification, type PluginNativeNotificationInput } from '@/store/native-notifications' +import { $connection } from '@/store/session' import { registry } from './registry' import type { Contribution } from './types' export type { PluginRestOptions } from '@/hermes' +export type { HermesConnectionMode } from '@/lib/connection-mode' export type { PluginNativeNotificationInput } from '@/store/native-notifications' /** A contribution as a plugin author writes it — provenance + id scoping are @@ -56,6 +59,25 @@ export interface PluginOs { writeClipboard: (text: string) => Promise } +/** The supported read of the resolved backend connection — the connection's + * *shape*, never its credentials. `mode` is `'local'` when this Desktop drives + * its own local backend (a path the agent reports is already openable here), + * `'remote'` when it drives an SSH/URL/cloud backend (a gateway-side path must + * be transferred first), and `null` when it isn't resolved yet. + * + * Base URL, host, tokens, SSH keys, and auth mode stay behind the Electron + * bridge on purpose; a plugin that needs to move a file should ask the backend + * to do it, not dial the backend itself. See #82140. */ +export interface PluginConnection { + /** The live mode, read at call time. */ + mode: () => HermesConnectionMode | null + /** Subscribe to mode changes (connection switch, profile switch, reconnect). + * Fires immediately with the current value. Returns an unsubscribe; it is + * also registered with `onDispose`, so a plugin that ignores the return + * value still stops listening when it unloads. */ + onModeChange: (listener: (mode: HermesConnectionMode | null) => void) => () => void +} + export interface PluginContext { /** The resolved plugin source tag, e.g. `'plugin:cost-meter'`. */ readonly source: string @@ -81,6 +103,9 @@ export interface PluginContext { * manager, clipboard — attributed to this plugin, result-shaped (never * throws for a missing capability). */ os: PluginOs + /** Is the backend's filesystem the machine the user is looking at? The + * supported answer to that question — mode only, no credentials. */ + connection: PluginConnection /** Plugin-scoped persistence. */ storage: PluginStorage /** Plugin-scoped i18n: ship + register locale bundles under this plugin, @@ -156,6 +181,36 @@ function createPluginOs(pluginId: string): PluginOs { } } +// Reads the resolved mode off the live connection atom rather than calling the +// Electron bridge: the atom is what stays in lockstep with the ACTIVE profile +// (syncConnectionToActiveProfile), so a plugin sees the same mode the session +// RPCs announce. A raw bridge.getConnection() would describe the primary window +// backend, which is the wrong answer whenever a background profile is active. +function createPluginConnection(track: (dispose: () => void) => () => void): PluginConnection { + return { + mode: () => resolveConnectionMode($connection.get()), + onModeChange: listener => { + let previous = resolveConnectionMode($connection.get()) + + listener(previous) + + // $connection changes on every reconnect and descriptor refresh, most of + // which don't move the mode. Only forward real transitions so a plugin + // can put its transfer/cleanup work straight in the listener. + return track( + $connection.subscribe(connection => { + const next = resolveConnectionMode(connection) + + if (next !== previous) { + previous = next + listener(next) + } + }) + ) + } + } +} + /** Build the scoped context handed to a plugin's `register`. `onDispose` * receives every registration's disposer (the loader's unload/reload hook). */ export function createPluginContext(pluginId: string, onDispose?: (dispose: () => void) => void): PluginContext { @@ -176,6 +231,7 @@ export function createPluginContext(pluginId: string, onDispose?: (dispose: () = rest: (path: string, opts?: PluginRestOptions) => pluginRest(pluginId, path, opts), socket: (path, onMessage) => track(pluginSocket(pluginId, path, onMessage)), os: createPluginOs(pluginId), + connection: createPluginConnection(track), storage: createPluginStorage(pluginId), i18n: createPluginI18n(pluginId, track) } diff --git a/apps/desktop/src/lib/connection-mode.test.ts b/apps/desktop/src/lib/connection-mode.test.ts new file mode 100644 index 000000000000..7b7f3ec87ce3 --- /dev/null +++ b/apps/desktop/src/lib/connection-mode.test.ts @@ -0,0 +1,64 @@ +import { describe, expect, it } from 'vitest' + +import type { HermesConnection } from '@/global' + +import { resolveConnectionMode, withConnectionMode } from './connection-mode' + +const conn = (over: Partial = {}) => + ({ baseUrl: 'http://127.0.0.1:8787', ...over }) as HermesConnection + +describe('resolveConnectionMode', () => { + it.each(['local', 'remote'] as const)('passes through the resolved %s mode', mode => { + expect(resolveConnectionMode(conn({ mode }))).toBe(mode) + }) + + it.each([ + ['no descriptor yet', null], + ['bridge unavailable', undefined] + ])('resolves null when there is %s', (_label, value) => { + expect(resolveConnectionMode(value)).toBeNull() + }) + + it('resolves null rather than guessing local for an unset mode', () => { + // An older shell that predates the field. Claiming "local" would tell an + // extension a gateway-side file is openable here when it may not be. + expect(resolveConnectionMode(conn())).toBeNull() + expect(resolveConnectionMode(conn({ mode: 'cloud' as never }))).toBeNull() + }) +}) + +describe('withConnectionMode', () => { + it.each(['session.create', 'session.resume', 'prompt.submit'])('stamps the mode onto %s', method => { + expect(withConnectionMode(method, { text: 'hi' }, 'remote')).toEqual({ + connection_mode: 'remote', + text: 'hi' + }) + }) + + it('leaves unrelated RPCs untouched', () => { + const params = { limit: 40 } + + expect(withConnectionMode('session.list', params, 'remote')).toBe(params) + }) + + it('adds no key when the mode is unknown', () => { + // Omitting is deliberate: it leaves any previously-announced value intact + // on the backend instead of clearing it during a reconnect window. + const params = { text: 'hi' } + + expect(withConnectionMode('prompt.submit', params, null)).toBe(params) + }) + + it('never overrides a mode a caller set explicitly', () => { + expect(withConnectionMode('prompt.submit', { connection_mode: 'local' }, 'remote')).toEqual({ + connection_mode: 'local' + }) + }) + + it('does not mutate the caller params', () => { + const params = { text: 'hi' } + withConnectionMode('prompt.submit', params, 'local') + + expect(params).toEqual({ text: 'hi' }) + }) +}) diff --git a/apps/desktop/src/lib/connection-mode.ts b/apps/desktop/src/lib/connection-mode.ts new file mode 100644 index 000000000000..b38675afc1ed --- /dev/null +++ b/apps/desktop/src/lib/connection-mode.ts @@ -0,0 +1,66 @@ +/** + * The resolved Desktop connection mode — the one connection fact extensions are + * allowed to see (NousResearch/hermes-agent#82140). + * + * `local` — this Desktop drives its own local backend, so a path the agent + * reports is already a path on the machine the user is looking at. + * `remote` — this Desktop drives an SSH/URL/cloud backend, so a gateway-side + * path has to be transferred before the Desktop can open it. + * + * That distinction is the whole point: without it an extension can't tell + * whether `/home/user/report.md` is openable here, which is what makes `MEDIA:` + * and file-link handling ambiguous on remote gateways. + * + * Everything else on the connection descriptor — base URL, host, identity, + * tokens, auth mode — stays behind the Electron bridge. Extensions get the + * shape of the connection, never the credentials for it. + */ + +import type { HermesConnection } from '@/global' + +export type HermesConnectionMode = 'local' | 'remote' + +/** RPCs on which the renderer announces its live mode to the backend. Session + * lifecycle pins it for new/resumed chats; `prompt.submit` re-announces every + * turn so switching the active connection or profile lands immediately rather + * than being stuck at whatever was true when the chat opened. */ +const CONNECTION_MODE_METHODS = new Set(['prompt.submit', 'session.create', 'session.resume']) + +/** + * Narrow a connection descriptor to its mode. + * + * The descriptor's `mode` is already resolved (a `cloud` saved config resolves + * to a `remote` connection), so this only has to guard the "no descriptor yet" + * and "older shell that predates the field" cases — both of which resolve to + * null. Null means "unknown", never "local": telling an extension a remote file + * is local hands the user a link to a file that isn't on their machine. + */ +export function resolveConnectionMode(connection: HermesConnection | null | undefined): HermesConnectionMode | null { + const mode = connection?.mode + + return mode === 'local' || mode === 'remote' ? mode : null +} + +/** + * Stamp `connection_mode` onto the params of an RPC that carries it. + * + * Applied at the single `requestGateway` choke point rather than at each of the + * ~10 call sites, so a new session/prompt path announces correctly by + * construction instead of by remembering to. + * + * An explicit param already on `params` wins (nothing sets one today; this + * keeps the helper from silently overriding a deliberate caller). An unknown + * mode adds no key at all, which leaves any previously-announced value intact + * on the backend rather than clearing it during a reconnect window. + */ +export function withConnectionMode( + method: string, + params: Record, + mode: HermesConnectionMode | null +): Record { + if (!mode || !CONNECTION_MODE_METHODS.has(method) || 'connection_mode' in params) { + return params + } + + return { ...params, connection_mode: mode } +} diff --git a/apps/desktop/src/sdk/index.ts b/apps/desktop/src/sdk/index.ts index 032a58bd4d44..7fa76698d478 100644 --- a/apps/desktop/src/sdk/index.ts +++ b/apps/desktop/src/sdk/index.ts @@ -523,7 +523,9 @@ export { Textarea } from '@/components/ui/textarea' export { Tip, Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from '@/components/ui/tooltip' export type { GatewayEventListener } from '@/contrib/events' export type { + HermesConnectionMode, HermesPlugin, + PluginConnection, PluginContext, PluginContribution, PluginNativeNotificationInput, From 0f4082d685bf51b6835a9e75e40d33548b5ec3ae Mon Sep 17 00:00:00 2001 From: Jack Lau <72348727+jackulau@users.noreply.github.com> Date: Sat, 8 Aug 2026 21:15:45 -0500 Subject: [PATCH 06/25] docs(desktop): document the Desktop connection-mode API One page covering all three read paths (skill env var, MCP _meta key, plugin ctx.connection), what each returns when there is no Desktop session, why the source of truth is a contextvar rather than an environment variable, and the fact that only the mode -- never a URL, host, token, or key -- crosses the boundary. Goal: 001-desktop-connection-mode (deliverable 6) --- .../desktop-connection-mode.md | 159 ++++++++++++++++++ website/sidebars.ts | 1 + 2 files changed, 160 insertions(+) create mode 100644 website/docs/developer-guide/desktop-connection-mode.md diff --git a/website/docs/developer-guide/desktop-connection-mode.md b/website/docs/developer-guide/desktop-connection-mode.md new file mode 100644 index 000000000000..0836c3801c11 --- /dev/null +++ b/website/docs/developer-guide/desktop-connection-mode.md @@ -0,0 +1,159 @@ +--- +sidebar_label: "Desktop Connection Mode" +title: "Desktop Connection Mode" +description: "Read whether the Desktop app is driving a local or a remote backend — from a skill, an MCP server, or a Desktop plugin — so file links point somewhere the user can actually open." +--- + +# Desktop Connection Mode + +Hermes can execute on a gateway while you sit in front of the Desktop app on a +different machine. When the agent produces a path like `/home/user/report.md`, +that path is real *on the gateway* — and may be meaningless on the machine +rendering the chat. + +**Connection mode** is the one fact that disambiguates it: + +| Mode | Meaning | +|------|---------| +| `local` | The Desktop app is driving its own local backend. A path the agent reports is already a path on the machine the user is looking at. | +| `remote` | The Desktop app is driving an SSH, URL, or Hermes Cloud backend. A gateway-side path must be transferred before the Desktop can open it. | +| unavailable / `null` | Not a Desktop session (CLI, TUI, messaging, cron, API server), or the client didn't announce a mode. | + +The typical use: + +```text +if mode == "local": present the file directly +elif mode == "remote": copy it to the Desktop machine first, then present it +else: don't claim the file is locally openable +``` + +:::info Only the mode is exposed +Every read path below returns the connection's *shape* and nothing else. Base +URL, remote host, identity, tokens, SSH keys, and auth mode stay behind the +Electron bridge — a plugin that needs to move a file asks the backend to do it +rather than dialling the backend itself. +::: + +## Where the value comes from + +The Desktop shell already resolves the mode for its own use via +`window.hermesDesktop.getConnection()`; a `cloud` saved config resolves to a +`remote` connection, so only `local` and `remote` ever come out. + +The renderer announces that resolved mode to the backend on `session.create`, +`session.resume`, and — critically — on **every `prompt.submit`**. The per-turn +re-announcement is what makes switching the active connection or profile land +immediately, instead of pinning the answer to whatever was true when the chat +was opened. + +The gateway stores it on the live session and binds it into session context for +the turn. It is bound only for sessions whose `source` is `desktop`, so a stray +parameter from another client is ignored. + +:::warning Not an environment variable +The source of truth is a task-local context variable, not configuration. A +`HERMES_DESKTOP_CONNECTION_MODE` exported in your shell is **not** read anywhere +— on the subprocess path below it is actively stripped. That is deliberate: an +extension convinced a remote file is local hands the user a link to a file that +isn't on their machine. +::: + +## Reading it from a skill + +Skills invoke helper scripts through the `terminal` tool, and the subprocess +bridge stamps the mode onto every child environment as +`HERMES_DESKTOP_CONNECTION_MODE`. The variable is **absent** when there is no +Desktop session, so treat absence as "unknown", never as "local". + +```python +import os + +mode = os.environ.get("HERMES_DESKTOP_CONNECTION_MODE") # 'local' | 'remote' | None + +if mode == "local": + present(path) +elif mode == "remote": + present(transfer_to_desktop(path)) +else: + print(f"Path is on the Hermes host: {path}") +``` + +The stamp is re-derived on every spawn, so a mid-session connection switch is +reflected on the next command the skill runs. + +## Reading it from an MCP server + +A stdio MCP server's environment is fixed at spawn time, while the mode is +per-session — one gateway can serve a local Desktop client and a remote one at +the same moment. So the mode rides each request as MCP `_meta`: + +```json +{ + "_meta": { + "hermes-agent.nousresearch.com/desktop-connection-mode": "remote" + } +} +``` + +The key is absent for non-Desktop sessions, so those requests keep exactly the +shape they have today. It is also omitted when the installed `mcp` SDK predates +per-call metadata. + +Reading it with the Python SDK: + +```python +MODE_KEY = "hermes-agent.nousresearch.com/desktop-connection-mode" + +@server.call_tool() +async def handle(name: str, arguments: dict, context) -> list: + mode = (context.meta or {}).get(MODE_KEY) + ... +``` + +## Reading it from a Desktop plugin + +`PluginContext` carries a `connection` door — the supported alternative to +reaching through the raw Electron bridge: + +```ts +export default { + id: 'file-delivery', + register(ctx) { + // Point-in-time read. + const mode = ctx.connection.mode() // 'local' | 'remote' | null + + // Or react to switches. Fires immediately with the current value, then on + // every real transition (connection switch, profile switch, reconnect). + ctx.connection.onModeChange(next => { + if (next === 'remote') { + enableTransferBeforeOpen() + } + }) + } +} +``` + +`onModeChange` returns an unsubscribe, and also registers one with the plugin's +disposers — a plugin that ignores the return value still stops listening when it +unloads. It fires only on genuine transitions; a reconnect that re-mints the +descriptor on the same mode is not a change. + +The value is read from the app's live connection atom rather than from +`getConnection()` directly, so it tracks the **active** profile. A raw bridge +call describes the primary window backend, which is the wrong answer whenever a +background profile is active. + +## Non-Desktop surfaces + +CLI, TUI, messaging platforms, cron, and the API server are unaffected: the +Python accessor returns `None`, the environment variable is not stamped, and MCP +requests carry no extra `_meta` key. + +## Reference + +| Surface | Read path | Absent value | +|---------|-----------|--------------| +| Python (core, tools) | `gateway.session_context.desktop_connection_mode()` | `None` | +| Skill scripts | `HERMES_DESKTOP_CONNECTION_MODE` | variable not set | +| MCP servers | `_meta["hermes-agent.nousresearch.com/desktop-connection-mode"]` | key not present | +| Desktop plugins | `ctx.connection.mode()` / `ctx.connection.onModeChange()` | `null` | diff --git a/website/sidebars.ts b/website/sidebars.ts index d857d33ed666..717aeca0d8ac 100644 --- a/website/sidebars.ts +++ b/website/sidebars.ts @@ -778,6 +778,7 @@ const sidebars: SidebarsConfig = { 'developer-guide/plugin-llm-access', 'developer-guide/subagent-lifecycle-api', 'developer-guide/desktop-plugin-sdk', + 'developer-guide/desktop-connection-mode', 'developer-guide/memory-provider-plugin', 'developer-guide/context-engine-plugin', 'developer-guide/secret-source-plugin', From ec06a7e7e672d5348a92caa95e8b7da4e9ab4355 Mon Sep 17 00:00:00 2001 From: Jack Lau <72348727+jackulau@users.noreply.github.com> Date: Sat, 8 Aug 2026 21:30:17 -0500 Subject: [PATCH 07/25] refactor(desktop): tighten the connection-mode announcement seams Two follow-ups on the same feature: - gateway/session_context.py: derive the contextvar's name from DESKTOP_CONNECTION_MODE_ENV instead of repeating the literal, so the subprocess stamp and the var can't drift apart. - use-gateway-request.ts: resolve the announced mode per ATTEMPT rather than once per call. The reconnect path rewrites $connection, so a retry that lands on a freshly reconnected backend now announces that backend's mode instead of the pre-reconnect one. Goal: 001-desktop-connection-mode (deliverable 7) --- .../src/app/gateway/hooks/use-gateway-request.ts | 13 +++++++------ gateway/session_context.py | 9 +++++---- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/apps/desktop/src/app/gateway/hooks/use-gateway-request.ts b/apps/desktop/src/app/gateway/hooks/use-gateway-request.ts index 4a3e698da805..31531e35d357 100644 --- a/apps/desktop/src/app/gateway/hooks/use-gateway-request.ts +++ b/apps/desktop/src/app/gateway/hooks/use-gateway-request.ts @@ -113,13 +113,14 @@ export function useGatewayRequest() { } // Announce the live connection mode on session/prompt RPCs (#82140). - // Read here, per request, so a connection or profile switch reaches the - // backend on the very next turn — $connection is kept in lockstep with - // the active profile by syncConnectionToActiveProfile. - const params = withConnectionMode(method, rawParams, resolveConnectionMode($connection.get())) + // Resolved per attempt, not per call: $connection is kept in lockstep + // with the active profile by syncConnectionToActiveProfile and is + // rewritten by the reconnect below, so re-reading on the retry sends the + // mode of the connection the retry actually lands on. + const announce = () => withConnectionMode(method, rawParams, resolveConnectionMode($connection.get())) try { - return await gateway.request(method, params, timeoutMs, signal) + return await gateway.request(method, announce(), timeoutMs, signal) } catch (error) { const message = error instanceof Error ? error.message : String(error) @@ -145,7 +146,7 @@ export function useGatewayRequest() { throw error } - return recovered.request(method, params, timeoutMs, signal) + return recovered.request(method, announce(), timeoutMs, signal) } }, [ensureGatewayOpen] diff --git a/gateway/session_context.py b/gateway/session_context.py index 026f1e6a02b5..9d63fc9c7cb2 100644 --- a/gateway/session_context.py +++ b/gateway/session_context.py @@ -135,6 +135,10 @@ def session_context_engaged() -> bool: _CRON_AUTO_DELIVER_CHAT_ID: ContextVar = ContextVar("HERMES_CRON_AUTO_DELIVER_CHAT_ID", default=_UNSET) _CRON_AUTO_DELIVER_THREAD_ID: ContextVar = ContextVar("HERMES_CRON_AUTO_DELIVER_THREAD_ID", default=_UNSET) +# The env var name used for the write-only subprocess stamp (see +# tools/environments/local.py). Never read back as configuration. +DESKTOP_CONNECTION_MODE_ENV = "HERMES_DESKTOP_CONNECTION_MODE" + # The resolved Desktop connection mode for this turn: 'local' when the Desktop # app drives its own local backend, 'remote' when it drives an SSH/URL/cloud # backend on another machine. ``None`` for every non-Desktop surface (CLI, TUI, @@ -152,10 +156,7 @@ def session_context_engaged() -> bool: # ``tools/environments/local.py`` stamps it onto child environments write-only # (always overwritten, stripped when unset) so skills and their helper scripts # can branch on it without it ever becoming an input. -_DESKTOP_CONNECTION_MODE: ContextVar = ContextVar("HERMES_DESKTOP_CONNECTION_MODE", default=_UNSET) - -# The env var name used for the write-only subprocess stamp. Not read anywhere. -DESKTOP_CONNECTION_MODE_ENV = "HERMES_DESKTOP_CONNECTION_MODE" +_DESKTOP_CONNECTION_MODE: ContextVar = ContextVar(DESKTOP_CONNECTION_MODE_ENV, default=_UNSET) # Saved-config connection modes that resolve to a backend on another machine. # The Desktop descriptor already collapses these to 'remote', but the RPC edge From b3f167517b89821180667f4f2992bc7457df0cab Mon Sep 17 00:00:00 2001 From: Jack Lau <72348727+jackulau@users.noreply.github.com> Date: Sat, 8 Aug 2026 21:31:57 -0500 Subject: [PATCH 08/25] fix(gateway): refresh the connection mode when reopening a live chat session.resume's fast path returns an already-live session without touching it, so a client that switched connection or profile since that session was registered kept announcing into a stale stored mode until its next prompt.submit. prompt.submit still refreshes before any turn runs, so nothing acted on the stale value -- this just closes the window between reopening a chat and sending in it. Goal: 001-desktop-connection-mode (deliverable 7) --- tui_gateway/methods_session.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tui_gateway/methods_session.py b/tui_gateway/methods_session.py index 0dd4598b13e1..ec94db0122d5 100644 --- a/tui_gateway/methods_session.py +++ b/tui_gateway/methods_session.py @@ -448,6 +448,11 @@ def _reuse_live_payload(sid: str, session: dict) -> dict: with _session_resume_lock: live = _find_live_session_by_key(target) if live is not None: + # Reopening a live chat is also a re-announcement: the client may + # have switched connection/profile since this session was + # registered (#82140). prompt.submit refreshes it again before + # any turn runs, so this only tightens the window. + _remember_connection_mode(live[1], params) return _ok(rid, _reuse_live_payload(*live)) # Lazy/watch resume: register the live session WITHOUT building an agent. From a23e1ee488183492706fea28e9b6fe9dee565a27 Mon Sep 17 00:00:00 2001 From: Jack Lau <72348727+jackulau@users.noreply.github.com> Date: Wed, 12 Aug 2026 22:35:16 -0500 Subject: [PATCH 09/25] fix(gateway): carry the Desktop connection mode across compute-host turn isolation With dashboard turn isolation enabled, the compute-host child rebuilds the Desktop session from the turn.start frame, which did not carry the resolved connection mode. _set_session_context therefore bound None, and skill subprocesses, MCP per-call _meta, and desktop_connection_mode() all lost the announcement for the whole isolated turn. - _compute_host_turn_frame now sends the parent's resolved mode (None for non-Desktop sessions, same gating as every other read path) - _ensure_server_session applies it on create (_init_session kwarg and the minimal fallback session) and refreshes it on reuse, so a mid-session connection switch lands on the next isolated turn; an omitted key from an older parent leaves the stored mode alone - fixes a latent AttributeError in the fallback path, which referenced a server._sanitize_client_source that never existed; it now uses _resolve_session_source, matching _init_session - regression tests cover the frame, both create paths, reuse-with-switch observed through the child's own context bind, and the omitted-key case Addresses the blocking review on #82187. --- .../test_desktop_connection_mode_rpc.py | 124 ++++++++++++++++++ tui_gateway/compute_host.py | 13 +- tui_gateway/server.py | 5 + 3 files changed, 141 insertions(+), 1 deletion(-) diff --git a/tests/tui_gateway/test_desktop_connection_mode_rpc.py b/tests/tui_gateway/test_desktop_connection_mode_rpc.py index 9a2b457b00be..2a901a34c5a9 100644 --- a/tests/tui_gateway/test_desktop_connection_mode_rpc.py +++ b/tests/tui_gateway/test_desktop_connection_mode_rpc.py @@ -9,6 +9,8 @@ standing up a gateway. """ +import threading + import pytest from gateway.session_context import _DESKTOP_CONNECTION_MODE, _UNSET, _VAR_MAP @@ -120,3 +122,125 @@ def test_new_session_records_carry_a_connection_mode_slot(): connection_mode="remote", ) assert record["connection_mode"] == "remote" + + +def _host(): + import io + + from tui_gateway.compute_host import ComputeHost + + return ComputeHost(stdout=io.StringIO(), heartbeat_secs=0) + + +def _live_session(**extra) -> dict: + return { + "session_key": "k", + "source": "desktop", + "history": [], + "history_lock": threading.Lock(), + "history_version": 0, + "attached_images": [], + "cols": 80, + "cwd": "/w", + **extra, + } + + +class TestComputeHostBoundary: + """Dashboard turn isolation must not erase the Desktop connection mode. + + The compute-host child rebuilds the session from the ``turn.start`` frame, + so the frame must carry the parent's resolved mode and + ``_ensure_server_session`` must apply it on create and refresh it on reuse + — otherwise every isolated Desktop turn binds ``None`` and skills/MCP lose + the announcement (#82140). + """ + + def test_turn_frame_carries_the_resolved_mode(self): + frame = _srv()._compute_host_turn_frame( + "rid", "s1", _live_session(connection_mode="remote"), "hi" + ) + assert frame["connection_mode"] == "remote" + + def test_turn_frame_for_non_desktop_session_carries_none(self): + """A stray mode on a non-Desktop session must not cross the boundary.""" + frame = _srv()._compute_host_turn_frame( + "rid", "s1", _live_session(source="tui", connection_mode="local"), "hi" + ) + assert frame["connection_mode"] is None + + def test_child_new_session_receives_the_frame_mode(self, monkeypatch): + """The create path hands the frame mode to _init_session.""" + srv = _srv() + host = _host() + received = {} + + def _fake_init_session(sid, key, agent, history, **kwargs): + received.update(kwargs) + srv._sessions[sid] = { + "agent": agent, + "session_key": key, + "history": list(history), + "history_lock": threading.Lock(), + "source": srv._resolve_session_source(kwargs.get("source")), + "connection_mode": kwargs.get("connection_mode"), + } + + monkeypatch.setattr(srv, "_sessions", {}, raising=False) + monkeypatch.setattr(srv, "_make_agent", lambda *a, **k: object()) + monkeypatch.setattr(srv, "_transfer_db_to_agent", lambda *a, **k: False) + monkeypatch.setattr(srv, "_init_session", _fake_init_session) + frame = _srv()._compute_host_turn_frame( + "rid", "s1", _live_session(connection_mode="remote"), "hi" + ) + session = host._ensure_server_session(srv, frame) + assert received["connection_mode"] == "remote" + assert session["connection_mode"] == "remote" + + def test_child_fallback_session_keeps_the_frame_mode(self, monkeypatch): + """The minimal host-owned session (init machinery unavailable) too.""" + srv = _srv() + host = _host() + + def _boom(*a, **k): + raise RuntimeError("slash worker unavailable") + + monkeypatch.setattr(srv, "_sessions", {}, raising=False) + monkeypatch.setattr(srv, "_make_agent", lambda *a, **k: object()) + monkeypatch.setattr(srv, "_transfer_db_to_agent", lambda *a, **k: False) + monkeypatch.setattr(srv, "_init_session", _boom) + frame = _srv()._compute_host_turn_frame( + "rid", "s1", _live_session(connection_mode="remote"), "hi" + ) + session = host._ensure_server_session(srv, frame) + assert session["connection_mode"] == "remote" + + def test_child_reuse_refreshes_the_mode_and_binds_it(self, monkeypatch): + """A remote turn, then a switch to local: the reused child session must + refresh and the child's own turn context must observe the new mode.""" + from gateway.session_context import desktop_connection_mode + + srv = _srv() + host = _host() + child_session = _live_session(connection_mode="remote") + monkeypatch.setattr(srv, "_sessions", {"s1": child_session}, raising=False) + + frame = srv._compute_host_turn_frame( + "rid", "s1", _live_session(connection_mode="local"), "hi" + ) + reused = host._ensure_server_session(srv, frame) + assert reused is child_session + assert reused["connection_mode"] == "local" + + # What _run_prompt_submit's context bind now sees in the child. + srv._set_session_context("k") + assert desktop_connection_mode() == "local" + + def test_child_reuse_with_an_older_parent_frame_keeps_the_mode(self, monkeypatch): + """A frame without the key (older parent) must not erase the mode.""" + srv = _srv() + host = _host() + child_session = _live_session(connection_mode="remote") + monkeypatch.setattr(srv, "_sessions", {"s1": child_session}, raising=False) + host._ensure_server_session(srv, {"sid": "s1", "session_key": "k"}) + assert child_session["connection_mode"] == "remote" diff --git a/tui_gateway/compute_host.py b/tui_gateway/compute_host.py index c4d5a6ae7c7b..d9f8b784883c 100644 --- a/tui_gateway/compute_host.py +++ b/tui_gateway/compute_host.py @@ -536,6 +536,11 @@ def _ensure_server_session(self, server: Any, frame: dict[str, Any]) -> dict: session["profile_home"] = str(frame.get("profile_home")) if isinstance(frame.get("attached_images"), list): session["attached_images"] = list(frame.get("attached_images") or []) + if "connection_mode" in frame: + # Refresh so a mid-session Desktop connection switch lands on + # the very next isolated turn (#82140). An OMITTED key (older + # parent) must not erase a mode a newer frame already carried. + session["connection_mode"] = frame.get("connection_mode") return session history = frame.get("history") if isinstance(frame.get("history"), list) else [] @@ -598,6 +603,7 @@ def _ensure_server_session(self, server: Any, frame: dict[str, Any]) -> dict: cwd=str(frame.get("cwd") or "") or None, session_db=session_db, source=frame.get("source"), + connection_mode=frame.get("connection_mode"), ) finally: reset_transport(token) @@ -625,7 +631,12 @@ def _ensure_server_session(self, server: Any, frame: dict[str, Any]) -> dict: "edit_snapshots": {}, "tool_started_at": {}, "model_override": frame.get("model_override"), - "source": server._sanitize_client_source(frame.get("source")), + # _resolve_session_source, same as _init_session: the previous + # _sanitize_client_source reference never existed on the server + # module, so this fallback died with AttributeError instead of + # keeping a minimal host-owned session. + "source": server._resolve_session_source(frame.get("source")), + "connection_mode": frame.get("connection_mode"), "transport": self._transport, } session = server._sessions[sid] diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 83c9b0cf072b..0301fd1ab227 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -1784,6 +1784,11 @@ def _compute_host_turn_frame( "reasoning_config_override": session.get("create_reasoning_override"), "service_tier_override": session.get("create_service_tier_override"), "source": _session_source(session), + # Resolved Desktop connection mode (#82140). The compute-host child + # rebuilds the session from this frame, so without it an isolated turn + # would bind None and skills/MCP would lose the mode the Desktop + # announced to the parent. + "connection_mode": _session_connection_mode(session), "attached_images": attached_images, "queued_prompt_generation": queued_prompt_generation, } From f87b84998c1f9d3801c938ac646c0c802c4b848d Mon Sep 17 00:00:00 2001 From: Jack Lau <72348727+jackulau@users.noreply.github.com> Date: Thu, 13 Aug 2026 01:16:32 -0500 Subject: [PATCH 10/25] fix(gateway): inherit the Desktop connection mode in background and preview agents prompt.background and preview.restart bind fresh bg_*/preview_* task IDs that are not in _sessions, so the lookup-based derivation in _set_session_context found nothing and the detached agent ran the whole task with no mode: no Python accessor value, no subprocess env stamp, no MCP per-call _meta. _set_session_context now takes an explicit connection_mode keyword (sentinel default keeps the session-map derivation for every other caller, and an explicit None is honored rather than second-guessed), and both ephemeral-agent handlers pass the parent session's resolved mode. Regression tests read all three surfaces inside the detached agent thread for both handlers, plus the non-Desktop-parent and explicit-None cases. --- .../test_desktop_connection_mode_rpc.py | 126 ++++++++++++++++++ tui_gateway/methods_prompt.py | 17 ++- tui_gateway/server.py | 18 ++- 3 files changed, 156 insertions(+), 5 deletions(-) diff --git a/tests/tui_gateway/test_desktop_connection_mode_rpc.py b/tests/tui_gateway/test_desktop_connection_mode_rpc.py index 2a901a34c5a9..488641839897 100644 --- a/tests/tui_gateway/test_desktop_connection_mode_rpc.py +++ b/tests/tui_gateway/test_desktop_connection_mode_rpc.py @@ -113,6 +113,27 @@ def test_unknown_session_key_binds_none(self, monkeypatch): srv._set_session_context("no-such-key") assert desktop_connection_mode() is None + def test_explicit_mode_wins_for_ephemeral_ids(self, monkeypatch): + """bg_*/preview_* task IDs aren't session keys; the caller-supplied + parent mode must bind instead of the (empty) lookup result.""" + from gateway.session_context import desktop_connection_mode + + srv = _srv() + monkeypatch.setattr(srv, "_sessions", {}, raising=False) + srv._set_session_context("bg_abc123", connection_mode="remote") + assert desktop_connection_mode() == "remote" + + def test_explicit_none_is_not_second_guessed(self, monkeypatch): + """An explicit None ('parent has no Desktop mode') must not be + overridden by a coincidental session-map hit.""" + from gateway.session_context import desktop_connection_mode + + srv = _srv() + session = _desktop_session(connection_mode="remote") + monkeypatch.setattr(srv, "_sessions", {"s1": session}, raising=False) + srv._set_session_context("k", connection_mode=None) + assert desktop_connection_mode() is None + def test_new_session_records_carry_a_connection_mode_slot(): """Both live-session record shapes must have the field _set_session_context reads.""" @@ -244,3 +265,108 @@ def test_child_reuse_with_an_older_parent_frame_keeps_the_mode(self, monkeypatch monkeypatch.setattr(srv, "_sessions", {"s1": child_session}, raising=False) host._ensure_server_session(srv, {"sid": "s1", "session_key": "k"}) assert child_session["connection_mode"] == "remote" + + +class TestEphemeralAgentInheritance: + """Background and preview agents must inherit the parent Desktop mode. + + prompt.background and preview.restart bind fresh ``bg_*`` / ``preview_*`` + task IDs that are not in ``_sessions``, so the lookup-based derivation in + ``_set_session_context`` finds nothing; the handlers must hand the parent + session's resolved mode across explicitly (#82187 follow-up review, item 1). + Each probe reads all three surfaces INSIDE the detached agent thread: the + Python accessor, the subprocess env stamp, and the MCP per-call ``_meta``. + """ + + def _capture_inside_detached_agent(self, monkeypatch, method_name, params, session): + import queue + + import run_agent + + srv = _srv() + captured: queue.Queue = queue.Queue() + + class _ProbeAgent: + def __init__(self, **kwargs): + pass + + def run_conversation(self, **kwargs): + from gateway.session_context import ( + DESKTOP_CONNECTION_MODE_ENV, + desktop_connection_mode, + ) + from tools.environments.local import _make_run_env + from tools.mcp_tool import _call_tool_meta + + captured.put( + { + "accessor": desktop_connection_mode(), + "env": _make_run_env({}).get(DESKTOP_CONNECTION_MODE_ENV), + "meta": _call_tool_meta(), + } + ) + return {"final_response": "done"} + + monkeypatch.setattr(run_agent, "AIAgent", _ProbeAgent) + monkeypatch.setattr(srv, "_sessions", {"s1": session}, raising=False) + monkeypatch.setattr( + srv, "_background_agent_kwargs", lambda agent, task_id: {}, raising=False + ) + monkeypatch.setattr( + srv, "_ephemeral_preview_agent_kwargs", lambda agent, task_id: {}, raising=False + ) + monkeypatch.setattr( + srv, "_preview_restart_callbacks", lambda parent, task_id: {}, raising=False + ) + monkeypatch.setattr(srv, "_emit", lambda *a, **k: None, raising=False) + resp = srv._methods[method_name]("rid", {"session_id": "s1", **params}) + assert resp.get("error") is None, resp + return captured.get(timeout=15) + + def _parent(self, **extra) -> dict: + return { + "session_key": "k", + "source": "desktop", + "agent": object(), + "history": [], + "history_lock": threading.Lock(), + "cwd": "", + **extra, + } + + def test_background_agent_sees_the_parent_mode(self, monkeypatch): + from tools.mcp_tool import MCP_DESKTOP_CONNECTION_MODE_META_KEY + + seen = self._capture_inside_detached_agent( + monkeypatch, + "prompt.background", + {"text": "hi"}, + self._parent(connection_mode="remote"), + ) + assert seen["accessor"] == "remote" + assert seen["env"] == "remote" + assert seen["meta"] == {MCP_DESKTOP_CONNECTION_MODE_META_KEY: "remote"} + + def test_preview_agent_sees_the_parent_mode(self, monkeypatch): + from tools.mcp_tool import MCP_DESKTOP_CONNECTION_MODE_META_KEY + + seen = self._capture_inside_detached_agent( + monkeypatch, + "preview.restart", + {"url": "http://localhost:3000"}, + self._parent(connection_mode="remote"), + ) + assert seen["accessor"] == "remote" + assert seen["env"] == "remote" + assert seen["meta"] == {MCP_DESKTOP_CONNECTION_MODE_META_KEY: "remote"} + + def test_non_desktop_parent_spawns_modeless_children(self, monkeypatch): + """A TUI parent's stray connection_mode must not leak into children.""" + seen = self._capture_inside_detached_agent( + monkeypatch, + "prompt.background", + {"text": "hi"}, + self._parent(source="tui", connection_mode="local"), + ) + assert seen["accessor"] is None + assert seen["meta"] is None diff --git a/tui_gateway/methods_prompt.py b/tui_gateway/methods_prompt.py index 23fe73a11227..9fbbb9b5923e 100644 --- a/tui_gateway/methods_prompt.py +++ b/tui_gateway/methods_prompt.py @@ -1214,7 +1214,14 @@ def _(rid, params: dict) -> dict: task_id = f"bg_{uuid.uuid4().hex[:6]}" def run(): - session_tokens = _set_session_context(task_id, cwd=_session_cwd(session)) + # task_id is ephemeral (not in _sessions), so the context bind cannot + # derive the Desktop connection mode by lookup — inherit the parent + # session's resolved mode explicitly (#82140). + session_tokens = _set_session_context( + task_id, + cwd=_session_cwd(session), + connection_mode=_session_connection_mode(session), + ) try: from run_agent import AIAgent @@ -1327,7 +1334,13 @@ def _(rid, params: dict) -> dict: def run(): # Pin the validated preview cwd, else the parent workspace — never an # invalid client path, which would silently fall back to the launch dir. - session_tokens = _set_session_context(task_id, cwd=(preview_cwd or _session_cwd(session))) + # Ephemeral preview task: inherit the parent's Desktop connection mode + # explicitly, same as prompt.background (#82140). + session_tokens = _set_session_context( + task_id, + cwd=(preview_cwd or _session_cwd(session)), + connection_mode=_session_connection_mode(session), + ) try: from run_agent import AIAgent from tools.terminal_tool import register_task_env_overrides diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 0301fd1ab227..9390ccc3c81e 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -3461,11 +3461,18 @@ def _cwd_for_session_key(session_key: str) -> str: return "" +# Sentinel for _set_session_context: "caller did not supply a mode, derive it +# from the live session map". Distinct from None, which is a real answer +# ("no Desktop mode") that must not be second-guessed by the lookup. +_DERIVE_CONNECTION_MODE = object() + + def _set_session_context( session_key: str, cwd: str | None = None, *, ui_session_id: str = "", + connection_mode: object = _DERIVE_CONNECTION_MODE, ) -> list: try: from gateway.session_context import set_session_vars @@ -3486,7 +3493,11 @@ def _set_session_context( # fall back to the session_key (matching the id derivation used at # session-finalize), so an identified session is never left blank. session_id = session_key - connection_mode = None + # Ephemeral task IDs (background, preview) aren't in `_sessions` either, + # so the loop below can't find a mode for them. Callers that hold the + # parent session pass its resolved mode explicitly (#82140); the + # session-map derivation only runs when nothing was supplied. + mode = None if connection_mode is _DERIVE_CONNECTION_MODE else connection_mode with _sessions_lock: for sess in list(_sessions.values()): if sess.get("session_key") == session_key: @@ -3494,7 +3505,8 @@ def _set_session_context( session_id = ( getattr(sess.get("agent"), "session_id", None) or session_key ) - connection_mode = _session_connection_mode(sess) + if connection_mode is _DERIVE_CONNECTION_MODE: + mode = _session_connection_mode(sess) break return set_session_vars( session_key=session_key, @@ -3503,7 +3515,7 @@ def _set_session_context( cwd=resolved, ui_session_id=ui_session_id, cron_session="", - desktop_connection_mode=connection_mode, + desktop_connection_mode=mode, ) except Exception: return [] From 6d1293f4762e30119db0c0589093f04a53c055ed Mon Sep 17 00:00:00 2001 From: Jack Lau <72348727+jackulau@users.noreply.github.com> Date: Thu, 13 Aug 2026 01:16:33 -0500 Subject: [PATCH 11/25] fix(agent): route SKILL.md inline shell through the central subprocess env factory The !`cmd` expansion in skill preprocessing called subprocess.run() with no env at all, so unlike every terminal/tool spawn the snippet got the raw process environment: no session-context stamps, no write-only Desktop connection-mode stamp, and no scrub of a HERMES_DESKTOP_CONNECTION_MODE value inherited from the user's shell. It now builds the child env with build_subprocess_env(), the same factory as every other spawn surface (best-effort: a factory failure falls back to inheriting, matching the tool's degraded paths). Tests pin: an env is supplied; a bound mode is stamped; a live remote ContextVar overrides an ambient shell value; and an engaged session context with no bound mode strips the inherited variable. --- agent/skill_preprocessing.py | 12 +++ tests/agent/test_skill_preprocessing_env.py | 95 +++++++++++++++++++++ 2 files changed, 107 insertions(+) create mode 100644 tests/agent/test_skill_preprocessing_env.py diff --git a/agent/skill_preprocessing.py b/agent/skill_preprocessing.py index 44c5714b9b39..c8437ac5f333 100644 --- a/agent/skill_preprocessing.py +++ b/agent/skill_preprocessing.py @@ -69,6 +69,17 @@ def run_inline_shell(command: str, cwd: Path | None, timeout: int) -> str: raising, so one bad snippet can't wreck the whole skill message. """ _popen_kwargs = {"creationflags": windows_hide_flags()} if IS_WINDOWS else {} + # Same central env factory as every other spawn surface: the snippet gets + # the session-context stamps (HERMES_SESSION_*, the write-only Desktop + # connection-mode stamp) and passes through the inherited-value scrub — + # a value inherited from the user's shell is stripped, never honored. + try: + from tools.environments.local import build_subprocess_env + + _run_env = build_subprocess_env() + except Exception: + logger.debug("build_subprocess_env unavailable for inline shell", exc_info=True) + _run_env = None try: completed = subprocess.run( ["bash", "-c", command], @@ -78,6 +89,7 @@ def run_inline_shell(command: str, cwd: Path | None, timeout: int) -> str: timeout=max(1, int(timeout)), check=False, stdin=subprocess.DEVNULL, + env=_run_env, **_popen_kwargs, ) except subprocess.TimeoutExpired: diff --git a/tests/agent/test_skill_preprocessing_env.py b/tests/agent/test_skill_preprocessing_env.py new file mode 100644 index 000000000000..c9b88a266894 --- /dev/null +++ b/tests/agent/test_skill_preprocessing_env.py @@ -0,0 +1,95 @@ +"""SKILL.md inline-shell snippets must use the central subprocess env factory. + +``!`cmd``` expansion used to call ``subprocess.run()`` with no ``env`` at all, +so — unlike every terminal/tool spawn — the snippet inherited the raw process +environment: no session-context stamps, no Desktop connection-mode stamp, and +no scrub of a ``HERMES_DESKTOP_CONNECTION_MODE`` value inherited from the +user's shell (#82187 follow-up review, item 2). +""" + +import os +import subprocess +from types import SimpleNamespace + +import pytest + +import gateway.session_context as sc +from gateway.session_context import ( + DESKTOP_CONNECTION_MODE_ENV as MODE_ENV, + _VAR_MAP, + set_desktop_connection_mode, + set_session_vars, +) + +from agent.skill_preprocessing import run_inline_shell + + +@pytest.fixture(autouse=True) +def _isolate_session_context(): + """Clean ContextVar + os.environ + engaged-latch slate per test, restored.""" + tracked = list(_VAR_MAP.keys()) + [MODE_ENV] + saved_env = {k: os.environ.get(k) for k in tracked} + saved_ctx = {name: var.get() for name, var in _VAR_MAP.items()} + saved_mode = sc._DESKTOP_CONNECTION_MODE.get() + saved_engaged = sc._session_context_engaged + for var in _VAR_MAP.values(): + var.set(sc._UNSET) + sc._DESKTOP_CONNECTION_MODE.set(sc._UNSET) + sc._session_context_engaged = False + try: + yield + finally: + for var, val in zip(_VAR_MAP.values(), saved_ctx.values()): + var.set(val) + sc._DESKTOP_CONNECTION_MODE.set(saved_mode) + sc._session_context_engaged = saved_engaged + for k, v in saved_env.items(): + if v is None: + os.environ.pop(k, None) + else: + os.environ[k] = v + + +def _capture_spawn_env(monkeypatch) -> dict: + """Run one inline snippet with subprocess.run stubbed; return its env kwarg.""" + captured = {} + + def _fake_run(argv, **kwargs): + captured.update({"argv": argv, "env": kwargs.get("env")}) + return SimpleNamespace(stdout="ok\n", stderr="", returncode=0) + + monkeypatch.setattr(subprocess, "run", _fake_run) + assert run_inline_shell("echo hi", None, timeout=5) == "ok" + return captured + + +def test_inline_shell_passes_a_factory_built_env(monkeypatch): + """The spawn must supply an explicit env, not inherit the raw process one.""" + captured = _capture_spawn_env(monkeypatch) + assert captured["env"] is not None + + +def test_live_mode_is_stamped_for_the_snippet(monkeypatch): + set_session_vars(session_key="k", source="desktop") + set_desktop_connection_mode("remote") + captured = _capture_spawn_env(monkeypatch) + assert captured["env"][MODE_ENV] == "remote" + + +def test_live_mode_overrides_an_ambient_shell_value(monkeypatch): + """A live remote ContextVar wins over HERMES_DESKTOP_CONNECTION_MODE=local + inherited from the user's shell.""" + monkeypatch.setenv(MODE_ENV, "local") + set_session_vars(session_key="k", source="desktop") + set_desktop_connection_mode("remote") + captured = _capture_spawn_env(monkeypatch) + assert captured["env"][MODE_ENV] == "remote" + + +def test_ambient_value_is_stripped_when_no_mode_is_bound(monkeypatch): + """Engaged session context with no bound mode: the inherited shell value is + stripped rather than honored (write-only stamp contract).""" + monkeypatch.setenv(MODE_ENV, "remote") + set_session_vars(session_key="k", source="tui") + captured = _capture_spawn_env(monkeypatch) + assert MODE_ENV not in captured["env"] From cce6013d0ecdc5ccabfd8eb4d21ca4be2d123051 Mon Sep 17 00:00:00 2001 From: Jack Lau <72348727+jackulau@users.noreply.github.com> Date: Thu, 13 Aug 2026 01:16:54 -0500 Subject: [PATCH 12/25] fix(desktop): announce the connection mode on plugin host.request RPCs The plugin SDK's host.request sent straight through $gateway.get().request(), bypassing the withConnectionMode stamp that useGatewayRequest applies, so a runtime plugin could create or drive a Desktop session whose skills/MCP context never learned the mode even while ctx.connection.mode() reported remote. announceConnectionMode() in lib/connection-mode is now the one shared announcement helper: it reads the live $connection at call time and stamps session.create / session.resume / prompt.submit. Both the hook and host.request go through it. Tests drive all three stamped methods through host.request, plus the unrelated-RPC, unknown-mode, and no-gateway cases. --- .../app/gateway/hooks/use-gateway-request.ts | 14 ++-- apps/desktop/src/lib/connection-mode.ts | 14 ++++ apps/desktop/src/sdk/index.test.ts | 73 ++++++++++++++++++- apps/desktop/src/sdk/index.ts | 8 +- 4 files changed, 98 insertions(+), 11 deletions(-) diff --git a/apps/desktop/src/app/gateway/hooks/use-gateway-request.ts b/apps/desktop/src/app/gateway/hooks/use-gateway-request.ts index 31531e35d357..e4c1bdf28d39 100644 --- a/apps/desktop/src/app/gateway/hooks/use-gateway-request.ts +++ b/apps/desktop/src/app/gateway/hooks/use-gateway-request.ts @@ -3,10 +3,10 @@ import { useStore } from '@nanostores/react' import { useCallback, useEffect, useRef } from 'react' import type { HermesGateway } from '@/hermes' -import { resolveConnectionMode, withConnectionMode } from '@/lib/connection-mode' +import { announceConnectionMode } from '@/lib/connection-mode' import { $gateway, ensureActiveGatewayOpen, isActivePrimary } from '@/store/gateway' import { $activeGatewayProfile } from '@/store/profile' -import { $connection, $gatewayState, setConnection } from '@/store/session' +import { $gatewayState, setConnection } from '@/store/session' export function useGatewayRequest() { const gatewayState = useStore($gatewayState) @@ -113,11 +113,11 @@ export function useGatewayRequest() { } // Announce the live connection mode on session/prompt RPCs (#82140). - // Resolved per attempt, not per call: $connection is kept in lockstep - // with the active profile by syncConnectionToActiveProfile and is - // rewritten by the reconnect below, so re-reading on the retry sends the - // mode of the connection the retry actually lands on. - const announce = () => withConnectionMode(method, rawParams, resolveConnectionMode($connection.get())) + // Resolved per attempt, not per call: $connection is published in the + // same synchronous frame as a profile switch (ensureGatewayProfile) and + // is rewritten by the reconnect below, so re-reading on the retry sends + // the mode of the connection the retry actually lands on. + const announce = () => announceConnectionMode(method, rawParams) try { return await gateway.request(method, announce(), timeoutMs, signal) diff --git a/apps/desktop/src/lib/connection-mode.ts b/apps/desktop/src/lib/connection-mode.ts index b38675afc1ed..5b9b9147fbd3 100644 --- a/apps/desktop/src/lib/connection-mode.ts +++ b/apps/desktop/src/lib/connection-mode.ts @@ -17,6 +17,7 @@ */ import type { HermesConnection } from '@/global' +import { $connection } from '@/store/session' export type HermesConnectionMode = 'local' | 'remote' @@ -64,3 +65,16 @@ export function withConnectionMode( return { ...params, connection_mode: mode } } + +/** + * The one announcement helper every gateway-request door shares. + * + * Reads the live `$connection` at CALL time (so a retry announces the mode of + * the connection it actually lands on) and stamps it via `withConnectionMode`. + * Both `useGatewayRequest` (app/hook callers) and the plugin SDK's + * `host.request` go through here — a request door that skips it lets a plugin + * drive a Desktop session whose skills/MCP context never learns the mode. + */ +export function announceConnectionMode(method: string, params: Record): Record { + return withConnectionMode(method, params, resolveConnectionMode($connection.get())) +} diff --git a/apps/desktop/src/sdk/index.test.ts b/apps/desktop/src/sdk/index.test.ts index 83c68d9a7b38..f8d8170711bc 100644 --- a/apps/desktop/src/sdk/index.test.ts +++ b/apps/desktop/src/sdk/index.test.ts @@ -1,8 +1,15 @@ -import { afterEach, describe, expect, it } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { HermesConnection } from '@/global' import { createClientSessionState } from '@/lib/chat-runtime' import { host } from '@/sdk' -import { setActiveSessionId, setAwaitingResponse, setBusy } from '@/store/session' +import { $gateway } from '@/store/gateway' +import { + setActiveSessionId, + setAwaitingResponse, + setBusy, + setConnection +} from '@/store/session' import { clearAllSessionStates, publishSessionState } from '@/store/session-states' describe('host.state turn flags', () => { @@ -107,3 +114,65 @@ describe('host.state turn flags', () => { $sessionTiles.set([]) }) }) + +/** + * The plugin SDK's `host.request` door must announce the Desktop connection + * mode on session/prompt RPCs exactly like `useGatewayRequest` does — it used + * to send straight through `$gateway.get().request()`, letting a runtime + * plugin create or drive a session whose skills/MCP context never learned the + * mode (#82187 follow-up review, item 3). + */ + +const conn = (mode?: 'local' | 'remote') => + ({ baseUrl: 'http://127.0.0.1:8787', mode }) as unknown as HermesConnection + +describe('host.request connection-mode announcement', () => { + afterEach(() => { + $gateway.set(null as never) + setConnection(null) + }) + + const installGateway = () => { + const request = vi.fn().mockResolvedValue('ok') + $gateway.set({ request } as never) + + return request + } + + it.each(['session.create', 'session.resume', 'prompt.submit'])( + 'stamps the live mode onto %s', + async method => { + setConnection(conn('remote')) + const request = installGateway() + + await expect(host.request(method, { text: 'hi' })).resolves.toBe('ok') + expect(request).toHaveBeenCalledWith(method, { connection_mode: 'remote', text: 'hi' }) + } + ) + + it('leaves unrelated RPCs untouched', async () => { + setConnection(conn('remote')) + const request = installGateway() + const params = { limit: 3 } + + await host.request('session.list', params) + + expect(request).toHaveBeenCalledWith('session.list', params) + }) + + it('adds no key when the mode is unknown', async () => { + // Null descriptor (reconnect window / older shell): omit rather than + // clear, matching withConnectionMode semantics. + const request = installGateway() + + await host.request('prompt.submit', { text: 'hi' }) + + expect(request).toHaveBeenCalledWith('prompt.submit', { text: 'hi' }) + }) + + it('still throws when no gateway socket is live', async () => { + setConnection(conn('remote')) + + await expect(host.request('prompt.submit', {})).rejects.toThrow('Hermes gateway unavailable') + }) +}) diff --git a/apps/desktop/src/sdk/index.ts b/apps/desktop/src/sdk/index.ts index 7fa76698d478..d11291db0321 100644 --- a/apps/desktop/src/sdk/index.ts +++ b/apps/desktop/src/sdk/index.ts @@ -26,6 +26,7 @@ import type { ClientSessionState } from '@/app/types' import { $narrowViewport } from '@/components/pane-shell/tree/store' import { onGatewayEvent } from '@/contrib/events' import { deleteProfile, getLogs, getStatus, type HermesGateway } from '@/hermes' +import { announceConnectionMode } from '@/lib/connection-mode' import { $gateway, openGatewayForAgent, @@ -401,7 +402,10 @@ export const host = { ): Promise => requestPluginProfile(route, method, params), /** Gateway JSON-RPC — sessions, config, skills, cron, kanban, everything - * the app itself uses. Lazy: resolves the LIVE socket per call. */ + * the app itself uses. Lazy: resolves the LIVE socket per call. Session and + * prompt RPCs announce the live Desktop connection mode through the same + * helper `useGatewayRequest` uses, so a plugin-driven session's skills/MCP + * context sees the mode a hook-driven one would (#82140). */ request: async (method: string, params: Record = {}): Promise => { const gateway = $gateway.get() @@ -409,7 +413,7 @@ export const host = { throw new Error('Hermes gateway unavailable') } - return gateway.request(method, params) + return gateway.request(method, announceConnectionMode(method, params)) }, /** The LIVE gateway instance for the active profile (null before the first From b9cfc652707d36dff2f0118257ad4a9607d50fc9 Mon Sep 17 00:00:00 2001 From: Jack Lau <72348727+jackulau@users.noreply.github.com> Date: Thu, 13 Aug 2026 01:16:54 -0500 Subject: [PATCH 13/25] fix(desktop): publish gateway, profile, and connection descriptor atomically on a profile switch ensureGatewayProfile used to activate the target gateway and set $activeGatewayProfile while the connection descriptor fetch was still in flight, so during that window $gateway already targeted the new backend while $connection still described the previous one, and any request or plugin mode-listener firing then announced the wrong mode to the new backend. A failed descriptor fetch made the mismatch permanent. prepareGatewayForProfile (new gateway-store seam) opens the socket and returns a synchronous activation thunk without publishing anything; ensureGatewayForProfile now delegates to it. The switch resolves the descriptor and opens the socket first, then flips the active gateway, the profile atom, and $connection in one synchronous frame. A descriptor failure aborts the switch as a unit: nothing is published and every atom still consistently describes the previous profile. The deferred-descriptor test holds the fetch open and asserts the public atoms never disagree, then releases it and asserts all three flipped together; the failure test asserts no partial publication. --- apps/desktop/src/store/gateway.ts | 53 +++++++++++++----- apps/desktop/src/store/profile.test.ts | 57 ++++++++++++++++++-- apps/desktop/src/store/profile.ts | 74 ++++++++++++++++---------- 3 files changed, 137 insertions(+), 47 deletions(-) diff --git a/apps/desktop/src/store/gateway.ts b/apps/desktop/src/store/gateway.ts index b9640203143b..f9bfaa97dcfa 100644 --- a/apps/desktop/src/store/gateway.ts +++ b/apps/desktop/src/store/gateway.ts @@ -656,27 +656,34 @@ export async function ensureGatewayForAgent(connectionId: null | string, profile return activated } -// Make `profile` the active gateway, lazily opening its socket if needed. The -// primary is a no-op fast path. Background sockets are never closed here. -export async function ensureGatewayForProfile(profile: string): Promise { +// Open `profile`'s socket if needed and hand back a synchronous activation +// thunk — the publication seam for atomic profile switches. The caller invokes +// the thunk in the same synchronous frame as its own atom writes (profile +// pointer, connection descriptor), so no subscriber can observe the active +// gateway pointing at one backend while companion state still describes +// another. Nothing is published until the thunk runs. +export async function prepareGatewayForProfile(profile: string): Promise<() => void> { const key = normKey(profile) const activationEpoch = beginGatewayActivation() if (key === g.primaryProfile) { - applyActive(key, activationEpoch) - - return + return () => { + applyActive(key, activationEpoch) + } } // Global-remote share (routing case 3): one remote host serves every // profile through the PRIMARY socket, scoped per request. Activate the // primary instead of dialing a doomed duplicate socket at the same - // descriptor — $activeGatewayProfile still moves to `key`, so request - // scoping and profile-aware surfaces behave identically. + // descriptor - $activeGatewayProfile still moves to `key`, so request + // scoping and profile-aware surfaces behave identically. Checked BEFORE + // createSecondary so a shared-remote profile never mints a secondary + // entry, and returned as a thunk like every other path here so this + // switch publishes as atomically as a dedicated-socket one. if (await sharedPrimaryRoute(key)) { - applyActive(g.primaryProfile, activationEpoch) - - return + return () => { + applyActive(g.primaryProfile, activationEpoch) + } } let entry = g.secondaries.get(key) @@ -699,11 +706,31 @@ export async function ensureGatewayForProfile(profile: string): Promise { } } - if (entry.wantOpen && g.secondaries.get(key) === entry && applyActive(key, activationEpoch) && entry.connection) { - publishActiveConnection(entry.connection) + // Bind the entry the await settled on. `g.secondaries.get(key)` can be a + // DIFFERENT object by the time the thunk runs (a teardown + redial between + // prepare and publish), and publishing that one's descriptor would be the + // very mismatch this seam exists to prevent, so the identity re-check below + // compares against this exact entry. + const prepared = entry + + return () => { + if ( + prepared.wantOpen && + g.secondaries.get(key) === prepared && + applyActive(key, activationEpoch) && + prepared.connection + ) { + publishActiveConnection(prepared.connection) + } } } +// Make `profile` the active gateway, lazily opening its socket if needed. The +// primary is a no-op fast path. Background sockets are never closed here. +export async function ensureGatewayForProfile(profile: string): Promise { + ;(await prepareGatewayForProfile(profile))() +} + // Reconnect the active gateway after a transient request failure. Primary // reconnects are owned by use-gateway-boot, so we only drive secondaries here. export async function ensureActiveGatewayOpen(): Promise { diff --git a/apps/desktop/src/store/profile.test.ts b/apps/desktop/src/store/profile.test.ts index c2e7cf36c02b..95b9f6957f44 100644 --- a/apps/desktop/src/store/profile.test.ts +++ b/apps/desktop/src/store/profile.test.ts @@ -6,13 +6,21 @@ import type { ProfileInfo } from '@/types/hermes' // Keep profile.ts's side-effecting imports inert: the gateway socket layer and // the REST query client must not run for real in a unit test. +const activateGateway = vi.fn() const ensureGatewayForProfile = vi.fn(async () => undefined) const ensureGatewayForAgent = vi.fn(async () => undefined) +const prepareGatewayForProfile = vi.fn(async (_profile: string) => activateGateway) const openGatewayForProfile = vi.fn(async (_profile: string) => undefined) const $gateway = atom({ id: 'live-socket' }) const resetStarmapGraph = vi.fn() -vi.mock('@/store/gateway', () => ({ $gateway, ensureGatewayForAgent, ensureGatewayForProfile, openGatewayForProfile })) +vi.mock('@/store/gateway', () => ({ + $gateway, + ensureGatewayForAgent, + ensureGatewayForProfile, + openGatewayForProfile, + prepareGatewayForProfile +})) vi.mock('@/hermes', () => ({ getProfiles: vi.fn(async () => ({ profiles: [] })), setApiRequestProfile: vi.fn() @@ -53,7 +61,9 @@ const getConnection = vi.fn<(profile?: string | null) => Promise { getConnection.mockReset() + activateGateway.mockClear() ensureGatewayForProfile.mockClear() + prepareGatewayForProfile.mockClear() openGatewayForProfile.mockClear() $gateway.set({ id: 'live-socket' }) $activeGatewayProfile.set('default') @@ -79,7 +89,8 @@ describe('ensureGatewayProfile → $connection sync (#46651)', () => { await ensureGatewayProfile('vps-remote') - expect(ensureGatewayForProfile).toHaveBeenCalledWith('vps-remote') + expect(prepareGatewayForProfile).toHaveBeenCalledWith('vps-remote') + expect(activateGateway).toHaveBeenCalledTimes(1) expect(getConnection).toHaveBeenCalledWith('vps-remote') expect($connection.get()?.mode).toBe('remote') expect($connection.get()?.profile).toBe('vps-remote') @@ -96,13 +107,49 @@ describe('ensureGatewayProfile → $connection sync (#46651)', () => { expect($connection.get()?.mode).toBe('local') }) - it('leaves the prior connection intact when the descriptor fetch fails', async () => { + it('fails as a unit when the descriptor fetch fails — no mixed state', async () => { + // Previously the gateway was activated and $activeGatewayProfile set even + // when the descriptor lookup failed, leaving $gateway on the new backend + // while $connection kept describing the old one for the rest of the + // session. Now nothing is published: every atom still consistently + // describes the previous profile and the user can retry. getConnection.mockRejectedValue(new Error('backend unreachable')) await ensureGatewayProfile('vps-remote') - // Best-effort: boot/reconnect resyncs later; we must not null it out here. + expect(activateGateway).not.toHaveBeenCalled() + expect($activeGatewayProfile.get()).toBe('default') + expect($connection.get()?.mode).toBe('local') + }) + + it('never publishes the new gateway before its connection descriptor', async () => { + // The exact mixed-state window from the follow-up review: a slow + // descriptor fetch must not leave $gateway/$activeGatewayProfile on the + // remote backend while $connection still says local. All three flip + // together only once the descriptor is in hand. + let resolveDescriptor: (conn: HermesConnection) => void = () => undefined + getConnection.mockReturnValue( + new Promise(resolve => { + resolveDescriptor = resolve + }) + ) + + const switching = ensureGatewayProfile('vps-remote') + // Let the socket-open half of the switch settle; the descriptor is still + // deliberately pending. + await Promise.resolve() + await Promise.resolve() + + expect(activateGateway).not.toHaveBeenCalled() + expect($activeGatewayProfile.get()).toBe('default') expect($connection.get()?.mode).toBe('local') + + resolveDescriptor(remoteConn()) + await switching + + expect(activateGateway).toHaveBeenCalledTimes(1) + expect($activeGatewayProfile.get()).toBe('vps-remote') + expect($connection.get()?.mode).toBe('remote') }) it('does not churn $connection when the target is already the active profile', async () => { @@ -112,7 +159,7 @@ describe('ensureGatewayProfile → $connection sync (#46651)', () => { await ensureGatewayProfile('vps-remote') expect(getConnection).not.toHaveBeenCalled() - expect(ensureGatewayForProfile).not.toHaveBeenCalled() + expect(prepareGatewayForProfile).not.toHaveBeenCalled() expect($connection.get()?.mode).toBe('remote') }) }) diff --git a/apps/desktop/src/store/profile.ts b/apps/desktop/src/store/profile.ts index 99fd3dd7054c..1f9e17236250 100644 --- a/apps/desktop/src/store/profile.ts +++ b/apps/desktop/src/store/profile.ts @@ -12,7 +12,12 @@ import { storedStringRecord } from '@/lib/storage' import { invalidateCronModelImpactScopeState } from '@/store/cron-model-impact-scope' -import { $gateway, ensureGatewayForAgent, ensureGatewayForProfile, openGatewayForProfile } from '@/store/gateway' +import { + $gateway, + ensureGatewayForAgent, + openGatewayForProfile, + prepareGatewayForProfile +} from '@/store/gateway' import { setConnection } from '@/store/session' import { resetStarmapGraph } from '@/store/starmap' import type { ProfileInfo } from '@/types/hermes' @@ -258,30 +263,24 @@ export function prewarmProfileBackend(name: string): void { let gatewaySwitch: Promise | null = null -// Keep the renderer's $connection (mode / baseUrl / profile) in lockstep with -// the profile the live gateway is now on. $connection seeds from the PRIMARY +// The target profile's connection descriptor (mode / baseUrl / …), fetched +// BEFORE activation so the switch can publish it in the same synchronous frame +// as the gateway and profile pointer. $connection seeds from the PRIMARY // (window) backend at boot and otherwise only refreshes on a sleep/wake -// reconnect — so activating a *background* profile left $connection describing -// the primary, with the wrong `mode` for everything that branches on -// local-vs-remote. Headline symptom: with a local primary and a remote pool -// profile active, image attachments went out via the path-based `image.attach` -// instead of `image.attach_bytes`, handing the remote gateway a client-only -// path it can't resolve ("image not found: C:\…"), while the /api/fs/* file -// browser and /api/media fetches targeted the wrong machine (#46651). -// Best-effort: a failed descriptor fetch leaves the prior connection intact for -// boot/reconnect to resync. -async function syncConnectionToActiveProfile(profile: string): Promise { +// reconnect — so activating a *background* profile without this left +// $connection describing the primary, with the wrong `mode` for everything +// that branches on local-vs-remote (#46651: path-based `image.attach` against +// a remote gateway, /api/fs/* and /api/media on the wrong machine). +// +// Null means "no desktop bridge" (plain browser) — there is no descriptor to +// sync then. A bridge REJECTION propagates: the caller aborts the whole switch +// rather than activating a backend whose descriptor (and thus mode) is +// unknown, which previously left $gateway on the new backend while +// $connection kept describing the old one for the rest of the session. +async function resolveConnectionForProfile(profile: string) { const getConnection = window.hermesDesktop?.getConnection - if (!getConnection) { - return - } - - try { - setConnection(await getConnection(profile)) - } catch { - // Leave the prior connection in place; boot/reconnect resyncs it later. - } + return getConnection ? getConnection(profile) : null } // Make `profile`'s backend the active gateway, lazily opening its socket if it @@ -320,14 +319,31 @@ export async function ensureGatewayProfile(profile: string | null | undefined): $gatewaySwapTarget.set(target) gatewaySwitch = (async () => { - // ensureGatewayForProfile opens (or reuses) the target's socket and points - // the active gateway at it — without closing the profile you came from. - await ensureGatewayForProfile(target) + // Resolve the target's connection descriptor and open (or reuse) its + // socket BEFORE anything is published — without closing the profile you + // came from. The gateway used to be activated (and the profile atom set) + // while the descriptor fetch was still in flight, so during that window + // $gateway already targeted the new backend while $connection still + // described the previous one — and any request or plugin mode-listener + // firing then announced the WRONG mode to the new backend. + const [connection, activate] = await Promise.all([ + resolveConnectionForProfile(target), + prepareGatewayForProfile(target) + ]) + + // One synchronous publication frame — no awaits from here down, so the + // active gateway, $activeGatewayProfile, and $connection flip together. + activate() $activeGatewayProfile.set(target) - // The active backend just changed; resync $connection so remote-aware - // paths (image.attach_bytes vs image.attach, /api/fs/*, /api/media) follow. - await syncConnectionToActiveProfile(target) - })() + + if (connection) { + setConnection(connection) + } + })().catch(() => { + // Descriptor lookup failed: the switch fails as a unit. Nothing was + // published, so every atom still consistently describes the previous + // profile; the user can retry the switch. + }) try { await gatewaySwitch From c5975704a286a4525fef6d9bb251dc91c910ef5b Mon Sep 17 00:00:00 2001 From: Jack Lau <72348727+jackulau@users.noreply.github.com> Date: Thu, 13 Aug 2026 01:17:15 -0500 Subject: [PATCH 14/25] fix(desktop): contain plugin connection-mode listener exceptions ctx.connection.onModeChange invoked plugin callbacks bare, both for the immediate notification and from the $connection subscription. The subscription runs inside core setConnection (boot, reconnect, profile switches), so one throwing plugin listener could abort profile synchronization/reconnect code and starve sibling listeners. Each callback invocation is now wrapped and reported with the plugin id attribution, matching the containment contract gateway event listeners already have. Tests cover a throw from the immediate call, a throw on a real transition (sibling listeners keep running, the thrower stays subscribed), and the attribution in the reported error. --- apps/desktop/src/contrib/plugin.test.ts | 58 +++++++++++++++++++++++++ apps/desktop/src/contrib/plugin.ts | 23 +++++++--- 2 files changed, 76 insertions(+), 5 deletions(-) diff --git a/apps/desktop/src/contrib/plugin.test.ts b/apps/desktop/src/contrib/plugin.test.ts index 2417043e1fd6..495887ff0413 100644 --- a/apps/desktop/src/contrib/plugin.test.ts +++ b/apps/desktop/src/contrib/plugin.test.ts @@ -118,4 +118,62 @@ describe('createPluginContext.connection', () => { expect(listener).toHaveBeenCalledTimes(1) expect($connection.get()?.mode).toBe('remote') }) + + it('contains a throw from the immediate notification', () => { + // The immediate call runs inside the plugin's register; a throw must not + // escape into the loader. + setConnection(conn('local')) + const error = vi.spyOn(console, 'error').mockImplementation(() => undefined) + + let unsubscribe = () => {} + + try { + expect(() => { + unsubscribe = createPluginContext('demo').connection.onModeChange(() => { + throw new Error('plugin bug') + }) + }).not.toThrow() + expect(error).toHaveBeenCalledWith(expect.stringContaining('demo'), expect.any(Error)) + } finally { + unsubscribe() + error.mockRestore() + } + }) + + it('contains a throw on a real transition and keeps other listeners running', () => { + // The subscription fires inside core setConnection (boot, reconnect, + // profile switch); a plugin throw escaping there would abort profile + // synchronization. It must be contained, attributed, and must not starve + // sibling listeners — including the thrower staying subscribed for + // later transitions. + setConnection(conn('local')) + const error = vi.spyOn(console, 'error').mockImplementation(() => undefined) + const unsubscribes: Array<() => void> = [] + + try { + const seen: Array<'local' | 'remote' | null> = [] + unsubscribes.push( + createPluginContext('bad').connection.onModeChange(mode => { + if (mode === 'remote') { + throw new Error('plugin bug') + } + }) + ) + unsubscribes.push(createPluginContext('good').connection.onModeChange(mode => seen.push(mode))) + + expect(() => setConnection(conn('remote'))).not.toThrow() + expect(seen).toEqual(['local', 'remote']) + expect(error).toHaveBeenCalledWith(expect.stringContaining('bad'), expect.any(Error)) + + // The throwing listener is still subscribed and hears later transitions + // (a non-throwing one this time — nothing new is reported). + error.mockClear() + expect(() => setConnection(conn('local'))).not.toThrow() + expect(seen).toEqual(['local', 'remote', 'local']) + expect(error).not.toHaveBeenCalled() + } finally { + unsubscribes.forEach(unsubscribe => unsubscribe()) + error.mockRestore() + } + }) }) diff --git a/apps/desktop/src/contrib/plugin.ts b/apps/desktop/src/contrib/plugin.ts index 58ee9c5d1fba..50b64c933990 100644 --- a/apps/desktop/src/contrib/plugin.ts +++ b/apps/desktop/src/contrib/plugin.ts @@ -183,16 +183,29 @@ function createPluginOs(pluginId: string): PluginOs { // Reads the resolved mode off the live connection atom rather than calling the // Electron bridge: the atom is what stays in lockstep with the ACTIVE profile -// (syncConnectionToActiveProfile), so a plugin sees the same mode the session +// (published atomically with a profile switch), so a plugin sees the same mode the session // RPCs announce. A raw bridge.getConnection() would describe the primary window // backend, which is the wrong answer whenever a background profile is active. -function createPluginConnection(track: (dispose: () => void) => () => void): PluginConnection { +function createPluginConnection(pluginId: string, track: (dispose: () => void) => () => void): PluginConnection { + // Isolate every listener call: the subscription below runs inside core + // connection updates (setConnection during boot, reconnect, and profile + // switches), so a plugin throw escaping here would abort profile + // synchronization/reconnect code — one bad plugin destabilizing the + // renderer. Same containment contract as gateway event listeners. + const invoke = (listener: (mode: HermesConnectionMode | null) => void, mode: HermesConnectionMode | null) => { + try { + listener(mode) + } catch (error) { + console.error(`[plugins] ${pluginId}: connection mode listener failed`, error) + } + } + return { mode: () => resolveConnectionMode($connection.get()), onModeChange: listener => { let previous = resolveConnectionMode($connection.get()) - listener(previous) + invoke(listener, previous) // $connection changes on every reconnect and descriptor refresh, most of // which don't move the mode. Only forward real transitions so a plugin @@ -203,7 +216,7 @@ function createPluginConnection(track: (dispose: () => void) => () => void): Plu if (next !== previous) { previous = next - listener(next) + invoke(listener, next) } }) ) @@ -231,7 +244,7 @@ export function createPluginContext(pluginId: string, onDispose?: (dispose: () = rest: (path: string, opts?: PluginRestOptions) => pluginRest(pluginId, path, opts), socket: (path, onMessage) => track(pluginSocket(pluginId, path, onMessage)), os: createPluginOs(pluginId), - connection: createPluginConnection(track), + connection: createPluginConnection(pluginId, track), storage: createPluginStorage(pluginId), i18n: createPluginI18n(pluginId, track) } From c19786b27e57ac6b3fc30b43f31e64660e3c81aa Mon Sep 17 00:00:00 2001 From: Jack Lau <72348727+jackulau@users.noreply.github.com> Date: Thu, 13 Aug 2026 01:17:15 -0500 Subject: [PATCH 15/25] docs(desktop): fix the FastMCP connection-mode example for the pinned mcp SDK The published example used @server.call_tool() as a decorator and read context.meta; with this tree's mcp==1.28.1, call_tool is the dispatch method (self, name, arguments), not a decorator factory, and Context has no meta attribute. The supported shape is a @server.tool() handler reading ctx.request_context.meta, where the namespaced key lands in the metadata model's extra fields (model_extra). A docs smoke test extracts the snippet from the page and executes it against the installed SDK (decorator registration inspects the handler signature, so drift fails loudly), verifies the documented meta access on the real RequestParams.Meta model, and pins the absence of the old shapes so an SDK bump prompts a docs revisit. --- tests/tools/test_mcp_connection_mode_meta.py | 57 +++++++++++++++++++ .../desktop-connection-mode.md | 16 ++++-- 2 files changed, 69 insertions(+), 4 deletions(-) diff --git a/tests/tools/test_mcp_connection_mode_meta.py b/tests/tools/test_mcp_connection_mode_meta.py index 71cb7e2585a6..6308ca425fdc 100644 --- a/tests/tools/test_mcp_connection_mode_meta.py +++ b/tests/tools/test_mcp_connection_mode_meta.py @@ -99,3 +99,60 @@ async def call_tool(self, name, arguments=None, meta=None): assert _call_tool_supports_meta() is True finally: _call_tool_supports_meta.cache_clear() + + +class TestDocsExample: + """The published FastMCP example must be executable against the pinned SDK. + + The original example showed ``@server.call_tool()`` (not a decorator in + this tree's ``mcp`` SDK) and ``context.meta`` (absent on ``Context``); + the supported shape is a ``@server.tool()`` handler reading + ``ctx.request_context.meta`` (#82187 follow-up review, item 6). This pins + the docs snippet to the real SDK so a future SDK bump or docs edit that + breaks the pairing fails here instead of on a reader's machine. + """ + + def _docs_fastmcp_snippet(self) -> str: + import pathlib + import re + + doc = ( + pathlib.Path(__file__).resolve().parents[2] + / "website" + / "docs" + / "developer-guide" + / "desktop-connection-mode.md" + ) + blocks = re.findall(r"```python\n(.*?)```", doc.read_text(encoding="utf-8"), re.DOTALL) + sdk_blocks = [block for block in blocks if "FastMCP" in block] + assert len(sdk_blocks) == 1, "expected exactly one FastMCP example in the docs page" + return sdk_blocks[0] + + def test_example_executes_against_the_pinned_sdk(self): + pytest.importorskip("mcp.server.fastmcp") + snippet = self._docs_fastmcp_snippet() + namespace: dict = {} + # Executing (not just compiling) registers the tool: FastMCP inspects + # the handler signature at decoration time, so an unsupported decorator + # or Context parameter shape fails right here. + exec(compile(snippet, "desktop-connection-mode.md", "exec"), namespace) + assert namespace["MODE_KEY"] == META_KEY + + def test_documented_meta_access_reads_the_namespaced_key(self): + mcp_types = pytest.importorskip("mcp.types") + meta = mcp_types.RequestParams.Meta(**{META_KEY: "remote"}) + # The exact expression the docs show, on the real metadata model. + assert (meta.model_extra or {}).get(META_KEY) == "remote" + + def test_pinned_sdk_still_lacks_the_shapes_the_old_example_used(self): + """If the SDK grows Context.meta or a call_tool decorator, revisit the + docs example rather than silently drifting.""" + fastmcp = pytest.importorskip("mcp.server.fastmcp") + import inspect + + assert "meta" not in dir(fastmcp.Context) + assert "request_context" in dir(fastmcp.Context) + # call_tool is the dispatch method (self, name, arguments), not a + # decorator factory like tool(). + params = list(inspect.signature(fastmcp.FastMCP.call_tool).parameters) + assert params[:3] == ["self", "name", "arguments"] diff --git a/website/docs/developer-guide/desktop-connection-mode.md b/website/docs/developer-guide/desktop-connection-mode.md index 0836c3801c11..572cf4331cdd 100644 --- a/website/docs/developer-guide/desktop-connection-mode.md +++ b/website/docs/developer-guide/desktop-connection-mode.md @@ -99,14 +99,22 @@ The key is absent for non-Desktop sessions, so those requests keep exactly the shape they have today. It is also omitted when the installed `mcp` SDK predates per-call metadata. -Reading it with the Python SDK: +Reading it with the Python SDK (FastMCP): ```python +from mcp.server.fastmcp import Context, FastMCP + +server = FastMCP("file-delivery") + MODE_KEY = "hermes-agent.nousresearch.com/desktop-connection-mode" -@server.call_tool() -async def handle(name: str, arguments: dict, context) -> list: - mode = (context.meta or {}).get(MODE_KEY) + +@server.tool() +async def deliver(path: str, ctx: Context) -> str: + meta = ctx.request_context.meta + # The key is namespaced (slashes/dots), so it lands in the metadata + # model's extra fields rather than as a declared attribute. + mode = (meta.model_extra or {}).get(MODE_KEY) if meta is not None else None ... ``` From abbf890583d6b2b48eef29005f62460d62b0d08d Mon Sep 17 00:00:00 2001 From: Jack Lau <72348727+jackulau@users.noreply.github.com> Date: Sun, 16 Aug 2026 04:30:16 -0500 Subject: [PATCH 16/25] fix(desktop): announce the connection mode through host.getGateway() too `main` added `host.getGateway()` after the follow-up review, handing plugins the live `HermesGateway` for SDK components that take it as a prop. It is also the SDK's second request door: a plugin reaching `getGateway().request('prompt.submit', ...)` bypassed `announceConnectionMode` and could drive a Desktop session whose skills/MCP context never learned the mode. That is the same gap item 3 of the follow-up review closed for `host.request`, reopened through a different door, so close it the way the review asked: every gateway-request door shares one announcement helper. Wrapped in a Proxy rather than a spread copy or a subclass, because `HermesGateway` is the live socket wrapper and its methods close over connection state that only exists on the real instance. Every member except `request` delegates straight through, bound to the target so a delegated method never runs with the proxy as `this`. Wrappers are cached per real gateway in a WeakMap so repeated calls hand back a stable reference, which matters because SDK components take this as a React prop and a fresh wrapper per render would churn every memo and effect dependency keyed on it. 5 tests: announcement on a stamped RPC, pass-through on an unstamped one, delegation of non-request members, reference stability, and the null-before-first-socket case. --- apps/desktop/src/sdk/index.test.ts | 60 ++++++++++++++++++++++++++++++ apps/desktop/src/sdk/index.ts | 58 ++++++++++++++++++++++++++++- 2 files changed, 116 insertions(+), 2 deletions(-) diff --git a/apps/desktop/src/sdk/index.test.ts b/apps/desktop/src/sdk/index.test.ts index f8d8170711bc..dcda18452066 100644 --- a/apps/desktop/src/sdk/index.test.ts +++ b/apps/desktop/src/sdk/index.test.ts @@ -176,3 +176,63 @@ describe('host.request connection-mode announcement', () => { await expect(host.request('prompt.submit', {})).rejects.toThrow('Hermes gateway unavailable') }) }) + +/** + * `host.getGateway()` is the SDK's OTHER request door. It hands out the live + * instance for components that take a `HermesGateway` prop, so a plugin can + * reach `getGateway().request(...)` — which would otherwise bypass the + * announcement that `host.request` performs and reopen exactly the gap item 3 + * of the follow-up review closed. + */ +describe('host.getGateway connection-mode announcement', () => { + afterEach(() => { + $gateway.set(null as never) + setConnection(null) + }) + + it('announces on session/prompt RPCs through the returned instance', async () => { + setConnection(conn('remote')) + const request = vi.fn().mockResolvedValue('ok') + $gateway.set({ request } as never) + + await host.getGateway()?.request('prompt.submit', { text: 'hi' }) + + expect(request).toHaveBeenCalledWith('prompt.submit', { connection_mode: 'remote', text: 'hi' }) + }) + + it('leaves unrelated RPCs untouched', async () => { + setConnection(conn('remote')) + const request = vi.fn().mockResolvedValue('ok') + $gateway.set({ request } as never) + const params = { limit: 3 } + + await host.getGateway()?.request('session.list', params) + + expect(request).toHaveBeenCalledWith('session.list', params) + }) + + it('delegates non-request members to the real instance', () => { + const close = vi.fn() + setConnection(conn('remote')) + $gateway.set({ close, request: vi.fn(), wsUrl: 'ws://127.0.0.1:8787' } as never) + + const gateway = host.getGateway() as unknown as { close: () => void; wsUrl: string } + + expect(gateway.wsUrl).toBe('ws://127.0.0.1:8787') + gateway.close() + expect(close).toHaveBeenCalledOnce() + }) + + it('hands back a stable reference for one live gateway', () => { + // SDK components take this as a React prop; a fresh wrapper per call would + // churn every memo/effect dependency keyed on it. + setConnection(conn('remote')) + $gateway.set({ request: vi.fn() } as never) + + expect(host.getGateway()).toBe(host.getGateway()) + }) + + it('stays null before the first socket opens', () => { + expect(host.getGateway()).toBeNull() + }) +}) diff --git a/apps/desktop/src/sdk/index.ts b/apps/desktop/src/sdk/index.ts index d11291db0321..ef88c532c9ee 100644 --- a/apps/desktop/src/sdk/index.ts +++ b/apps/desktop/src/sdk/index.ts @@ -92,6 +92,52 @@ export interface PluginProfileRoute { profile: string /** Backend Hermes profile served by that route. */ targetProfile: string + +// One announcing view per real gateway, so repeated `getGateway()` calls hand +// back a stable reference — SDK components take this as a React prop, and a +// fresh wrapper per render would churn every memo and effect dependency on it. +const announcingGateways = new WeakMap() + +/** A gateway whose `request` announces the connection mode, like `host.request`. + * + * A Proxy rather than a spread copy or a subclass: `HermesGateway` is the live + * socket wrapper, so its methods close over connection state that only exists + * on the real instance. Every member except `request` passes straight through, + * bound to the target — calling a delegated method with the proxy as `this` + * would break any private-field access inside it. + */ +const announcingGateway = (gateway: HermesGateway): HermesGateway => { + const cached = announcingGateways.get(gateway) + + if (cached) { + return cached + } + + const wrapped = new Proxy(gateway, { + get(target, prop) { + if (prop === 'request') { + // Mirror the FULL HermesGateway.request signature. A two-argument + // wrapper silently swallows `timeoutMs` and `signal`, so any SDK + // caller passing them lost its custom deadline and its ability to + // abort - a wrapper must not narrow the contract it stands in for. + return ( + method: string, + params: Record = {}, + timeoutMs?: number, + signal?: AbortSignal + ): Promise => + target.request(method, announceConnectionMode(method, params), timeoutMs, signal) + } + + const value = Reflect.get(target, prop) + + return typeof value === 'function' ? value.bind(target) : value + } + }) + + announcingGateways.set(gateway, wrapped) + + return wrapped } /** Window geometry + the app's responsive posture, one readonly rect. */ @@ -420,8 +466,16 @@ export const host = { * socket opens). Most plugins want `host.request`; this exists for SDK * components that take a `HermesGateway` prop directly (e.g. `McpTab`), * which need the instance, not just a JSON-RPC door. Re-read per use — the - * active instance changes on a profile swap. */ - getGateway: (): HermesGateway | null => $gateway.get() + * active instance changes on a profile swap. + * + * Announcing, like `host.request`: this is the SDK's other request door, and + * a door that skips the announcement lets a plugin drive a Desktop session + * whose skills/MCP context never learns the mode (#82140). */ + getGateway: (): HermesGateway | null => { + const gateway = $gateway.get() + + return gateway ? announcingGateway(gateway) : gateway + } } // -- react bridge ------------------------------------------------------------- From 0b4193c3105b2e8e155ba5e158f2da602e8ec1e8 Mon Sep 17 00:00:00 2001 From: Jack Lau <72348727+jackulau@users.noreply.github.com> Date: Sun, 16 Aug 2026 04:57:55 -0500 Subject: [PATCH 17/25] fix(desktop): publish the agent activation atomically too `ensureGatewayAgent` is the (connectionId, profile) door the SDK's `ensureAgent` goes through, and it landed on main after the profile path was made atomic. It published in the order the profile path used to: await ensureGatewayForAgent(connection, target) // $gateway flips here $activeGatewayProfile.set(target) await syncConnectionToActiveAgent(connection, target) // $connection here The trailing await is the same mixed-state window: $gateway and $activeGatewayProfile already name the agent's backend while $connection still describes the previous one, so any request or plugin mode-listener firing in that window announces the wrong mode to the new backend. Both doors now share one seam: * `prepareGatewayForAgent` mirrors `prepareGatewayForProfile`: dial the socket, publish nothing, return the synchronous activation thunk. A local/null connection falls through to the profile seam, so the two paths cannot drift. `ensureGatewayForAgent` becomes `(await prepareGatewayForAgent(...))()`, exactly how `ensureGatewayForProfile` relates to its own prepare. * `syncConnectionToActiveAgent` splits into `resolveConnectionForActiveAgent`, which resolves only. `ensureGatewayAgent` resolves the descriptor and dials the socket concurrently, then activates, moves the profile pointer and sets the descriptor with no awaits between them. The best-effort contract on this path is unchanged on purpose: a descriptor lookup that fails still leaves the previous `$connection` in place rather than aborting the switch, which is what the profile path does instead. That difference is deliberate and called out for review rather than quietly harmonised. Tests: `profile-agent-activation.test.ts` gains `never publishes the agent gateway before its connection descriptor`, the mirror of the profile-path test, asserting a pending `getConnectionFor` leaves all three atoms on the old backend and that they flip together once it resolves. The existing mutex and resync tests move onto the prepare/publish mocks, which also repairs them: that file mocked `@/store/gateway` without `prepareGatewayForProfile`, so its profile-path cases called an undefined mock after the rebase. --- apps/desktop/src/store/gateway.ts | 52 ++++++--- .../store/profile-agent-activation.test.ts | 101 ++++++++++++++---- apps/desktop/src/store/profile.test.ts | 4 +- apps/desktop/src/store/profile.ts | 92 +++++++++++----- 4 files changed, 189 insertions(+), 60 deletions(-) diff --git a/apps/desktop/src/store/gateway.ts b/apps/desktop/src/store/gateway.ts index f9bfaa97dcfa..a869bf8e7ad6 100644 --- a/apps/desktop/src/store/gateway.ts +++ b/apps/desktop/src/store/gateway.ts @@ -606,13 +606,30 @@ export async function openGatewayForAgent(connectionId: null | string, profile: } } -export async function ensureGatewayForAgent(connectionId: null | string, profile: string): Promise { +// The agent-scoped analogue of prepareGatewayForProfile, and the same +// publication seam: dial the agent's socket without publishing anything, and +// hand back the synchronous activation thunk. A local/null connection falls +// through to the profile seam, so both doors into an activation share one +// atomicity contract instead of drifting apart. +// +// The thunk reports whether it actually published, preserving the `activated` +// contract callers rely on: a source edit/remove can dispose this entry while +// its dial is in flight, and a caller must be able to tell "switched" from +// "the target stopped existing" rather than assume the former. +export async function prepareGatewayForAgent( + connectionId: null | string, + profile: string +): Promise<() => boolean> { const scope = registryBackendScopeKey(connectionId, profile) if (scope === normKey(profile)) { - await ensureGatewayForProfile(profile) + const activate = await prepareGatewayForProfile(profile) + + return () => { + activate() - return true + return true + } } if (!window.hermesDesktop?.getConnectionFor) { @@ -641,19 +658,28 @@ export async function ensureGatewayForAgent(connectionId: null | string, profile } } - // A source edit/remove may dispose this entry while its dial is still in - // flight. Only the still-registered, still-owned activation may publish. - const activated = - entry.wantOpen && - g.secondaries.get(scope) === entry && - Boolean(entry.connection) && - applyActive(scope, activationEpoch) + // Bind the entry this dial settled on; see prepareGatewayForProfile. + const prepared = entry - if (activated && entry.connection) { - publishActiveConnection(entry.connection) + return () => { + // A source edit/remove may dispose this entry while its dial is still in + // flight. Only the still-registered, still-owned activation may publish. + const activated = + prepared.wantOpen && + g.secondaries.get(scope) === prepared && + Boolean(prepared.connection) && + applyActive(scope, activationEpoch) + + if (activated && prepared.connection) { + publishActiveConnection(prepared.connection) + } + + return activated } +} - return activated +export async function ensureGatewayForAgent(connectionId: null | string, profile: string): Promise { + return (await prepareGatewayForAgent(connectionId, profile))() } // Open `profile`'s socket if needed and hand back a synchronous activation diff --git a/apps/desktop/src/store/profile-agent-activation.test.ts b/apps/desktop/src/store/profile-agent-activation.test.ts index 9e4105629272..528a5d8e1aa1 100644 --- a/apps/desktop/src/store/profile-agent-activation.test.ts +++ b/apps/desktop/src/store/profile-agent-activation.test.ts @@ -13,18 +13,40 @@ import type { HermesConnection } from '@/global' // 2. Agent activations share the gatewaySwitch mutex with profile switches — // without it, two rapid activations could complete out of order and the // EARLIER setActive() landed last. - -const ensureGatewayForAgent = vi.fn(async (_connectionId: null | string, _profile: string) => true) -const ensureGatewayForProfile = vi.fn(async (_profile: string) => undefined) +// 3. An activation publishes the gateway, the profile pointer and the +// connection descriptor in ONE synchronous frame. Activating first and +// awaiting the descriptor after left $gateway on the new backend while +// $connection still described the old one. +// +// Both doors go through the prepare/publish seam (prepareGatewayFor*, which +// dial without publishing and return the activation thunk), so these mocks +// hand back a spy thunk instead of activating on call. + +// Distinct gateway identities so a listener can tell WHICH backend it was +// handed. A bare vi.fn() thunk never touches $gateway, which would let an +// out-of-order publication pass unnoticed. +const INITIAL_GATEWAY = { id: 'live-socket' } +const AGENT_GATEWAY = { id: 'agent-socket' } +const PROFILE_GATEWAY = { id: 'profile-socket' } +const activateAgent = vi.fn(() => { + $gateway.set(AGENT_GATEWAY) + + return true +}) +const activateProfile = vi.fn(() => { + $gateway.set(PROFILE_GATEWAY) +}) +const prepareGatewayForAgent = vi.fn(async (_connectionId: null | string, _profile: string) => activateAgent) +const prepareGatewayForProfile = vi.fn(async (_profile: string) => activateProfile) const openGatewayForProfile = vi.fn(async (_profile: string) => undefined) -const $gateway = atom({ id: 'live-socket' }) +const $gateway = atom(INITIAL_GATEWAY) const resetStarmapGraph = vi.fn() vi.mock('@/store/gateway', () => ({ $gateway, - ensureGatewayForAgent, - ensureGatewayForProfile, - openGatewayForProfile + openGatewayForProfile, + prepareGatewayForAgent, + prepareGatewayForProfile })) vi.mock('@/hermes', () => ({ getProfiles: vi.fn(async () => ({ profiles: [] })), @@ -60,8 +82,12 @@ function deferred(): { promise: Promise; resolve: () => void } { beforeEach(() => { getConnection.mockReset() getConnectionFor.mockReset() - ensureGatewayForAgent.mockClear() - ensureGatewayForProfile.mockClear() + prepareGatewayForAgent.mockReset() + prepareGatewayForAgent.mockResolvedValue(activateAgent) + prepareGatewayForProfile.mockReset() + prepareGatewayForProfile.mockResolvedValue(activateProfile) + activateAgent.mockClear() + activateProfile.mockClear() $gateway.set({ id: 'live-socket' }) $activeGatewayProfile.set('default') $connection.set(localConn()) @@ -81,7 +107,8 @@ describe('ensureGatewayAgent → $connection / $activeGatewayProfile sync', () = await ensureGatewayAgent('homelab', 'research') - expect(ensureGatewayForAgent).toHaveBeenCalledWith('homelab', 'research') + expect(prepareGatewayForAgent).toHaveBeenCalledWith('homelab', 'research') + expect(activateAgent).toHaveBeenCalledTimes(1) expect(getConnectionFor).toHaveBeenCalledWith({ connectionId: 'homelab', profile: 'research' }) expect($activeGatewayProfile.get()).toBe('research') expect($connection.get()?.mode).toBe('remote') @@ -113,8 +140,8 @@ describe('ensureGatewayAgent → $connection / $activeGatewayProfile sync', () = await ensureGatewayAgent(null, 'research') - expect(ensureGatewayForProfile).toHaveBeenCalledWith('research') - expect(ensureGatewayForAgent).not.toHaveBeenCalled() + expect(prepareGatewayForProfile).toHaveBeenCalledWith('research') + expect(prepareGatewayForAgent).not.toHaveBeenCalled() expect(getConnectionFor).not.toHaveBeenCalled() }) @@ -123,10 +150,40 @@ describe('ensureGatewayAgent → $connection / $activeGatewayProfile sync', () = await ensureGatewayAgent('local', 'research') - expect(ensureGatewayForAgent).toHaveBeenCalledWith('local', 'research') - expect(ensureGatewayForProfile).not.toHaveBeenCalled() + expect(prepareGatewayForAgent).toHaveBeenCalledWith('local', 'research') + expect(prepareGatewayForProfile).not.toHaveBeenCalled() expect(getConnectionFor).toHaveBeenCalledWith({ connectionId: 'local', profile: 'research' }) }) + + it('never publishes the agent gateway before its connection descriptor', async () => { + // The same mixed-state window the profile path closes, through the door + // added for the SDK's ensureAgent. A slow getConnectionFor must not leave + // $gateway/$activeGatewayProfile on the agent's backend while $connection + // still describes the previous one — anything requesting in that window + // announces the WRONG mode to the new backend. + let resolveDescriptor: (conn: HermesConnection) => void = () => undefined + getConnectionFor.mockReturnValue( + new Promise(resolve => { + resolveDescriptor = resolve + }) + ) + + const switching = ensureGatewayAgent('homelab', 'research') + // Let the socket-dial half settle; the descriptor is still pending. + await Promise.resolve() + await Promise.resolve() + + expect(activateAgent).not.toHaveBeenCalled() + expect($activeGatewayProfile.get()).toBe('default') + expect($connection.get()?.mode).toBe('local') + + resolveDescriptor(agentConn()) + await switching + + expect(activateAgent).toHaveBeenCalledTimes(1) + expect($activeGatewayProfile.get()).toBe('research') + expect($connection.get()?.mode).toBe('remote') + }) }) describe('ensureGatewayAgent shares the gatewaySwitch mutex with profile switches', () => { @@ -134,14 +191,16 @@ describe('ensureGatewayAgent shares the gatewaySwitch mutex with profile switche const profileGate = deferred() const order: string[] = [] - ensureGatewayForProfile.mockImplementation(async (profile: string) => { + prepareGatewayForProfile.mockImplementation(async (profile: string) => { order.push(`profile:${profile}`) await profileGate.promise + + return activateProfile }) - ensureGatewayForAgent.mockImplementation(async (_connectionId, profile) => { + prepareGatewayForAgent.mockImplementation(async (_connectionId, profile) => { order.push(`agent:${profile}`) - return true + return activateAgent }) getConnection.mockResolvedValue(localConn({ profile: 'worker' })) getConnectionFor.mockResolvedValue(agentConn()) @@ -170,14 +229,16 @@ describe('ensureGatewayAgent shares the gatewaySwitch mutex with profile switche const agentGate = deferred() const order: string[] = [] - ensureGatewayForAgent.mockImplementation(async (_connectionId, profile) => { + prepareGatewayForAgent.mockImplementation(async (_connectionId, profile) => { order.push(`agent:${profile}`) await agentGate.promise - return true + return activateAgent }) - ensureGatewayForProfile.mockImplementation(async (profile: string) => { + prepareGatewayForProfile.mockImplementation(async (profile: string) => { order.push(`profile:${profile}`) + + return activateProfile }) getConnection.mockResolvedValue(localConn({ profile: 'worker' })) getConnectionFor.mockResolvedValue(agentConn()) diff --git a/apps/desktop/src/store/profile.test.ts b/apps/desktop/src/store/profile.test.ts index 95b9f6957f44..1732e0e726c3 100644 --- a/apps/desktop/src/store/profile.test.ts +++ b/apps/desktop/src/store/profile.test.ts @@ -8,7 +8,7 @@ import type { ProfileInfo } from '@/types/hermes' // the REST query client must not run for real in a unit test. const activateGateway = vi.fn() const ensureGatewayForProfile = vi.fn(async () => undefined) -const ensureGatewayForAgent = vi.fn(async () => undefined) +const prepareGatewayForAgent = vi.fn(async (_connectionId: null | string, _profile: string) => activateGateway) const prepareGatewayForProfile = vi.fn(async (_profile: string) => activateGateway) const openGatewayForProfile = vi.fn(async (_profile: string) => undefined) const $gateway = atom({ id: 'live-socket' }) @@ -16,9 +16,9 @@ const resetStarmapGraph = vi.fn() vi.mock('@/store/gateway', () => ({ $gateway, - ensureGatewayForAgent, ensureGatewayForProfile, openGatewayForProfile, + prepareGatewayForAgent, prepareGatewayForProfile })) vi.mock('@/hermes', () => ({ diff --git a/apps/desktop/src/store/profile.ts b/apps/desktop/src/store/profile.ts index 1f9e17236250..05d9f1c20f62 100644 --- a/apps/desktop/src/store/profile.ts +++ b/apps/desktop/src/store/profile.ts @@ -1,5 +1,6 @@ -import { atom, computed } from 'nanostores' +import { atom, batch, computed } from 'nanostores' +import type { HermesConnection } from '@/global' import { getProfiles, setApiRequestProfile, STARTUP_REQUEST_TIMEOUT_MS } from '@/hermes' import { invalidateProfileScopedQueries } from '@/lib/query-client' import { @@ -14,8 +15,8 @@ import { import { invalidateCronModelImpactScopeState } from '@/store/cron-model-impact-scope' import { $gateway, - ensureGatewayForAgent, openGatewayForProfile, + prepareGatewayForAgent, prepareGatewayForProfile } from '@/store/gateway' import { setConnection } from '@/store/session' @@ -331,14 +332,19 @@ export async function ensureGatewayProfile(profile: string | null | undefined): prepareGatewayForProfile(target) ]) - // One synchronous publication frame — no awaits from here down, so the - // active gateway, $activeGatewayProfile, and $connection flip together. - activate() - $activeGatewayProfile.set(target) - - if (connection) { - setConnection(connection) - } + // ONE publication. batch() defers Nanostores' notifications to the end of + // the callback, so the active gateway, $activeGatewayProfile and + // $connection become visible together. Without it these are sequential + // .set() calls that each drain their listeners synchronously, and a + // $gateway listener runs while the other two still name the old backend. + batch(() => { + activate() + $activeGatewayProfile.set(target) + + if (connection) { + setConnection(connection) + } + }) })().catch(() => { // Descriptor lookup failed: the switch fails as a unit. Nothing was // published, so every atom still consistently describes the previous @@ -356,24 +362,32 @@ export async function ensureGatewayProfile(profile: string | null | undefined): // Registry-aware sibling of syncConnectionToActiveProfile: a connection-scoped // agent's descriptor comes from getConnectionFor (its SOURCE connection), not // getConnection (the local pool). Same best-effort contract. -async function syncConnectionToActiveAgent(connectionId: string, profile: string): Promise { +// Resolve only — publication is the caller's, so the descriptor can be in hand +// BEFORE the activation frame rather than an await after it. Null means "no +// descriptor to publish" (no desktop bridge, or the lookup failed): the caller +// leaves the prior connection in place and boot/reconnect resyncs it later, +// which is this path's established best-effort contract. +async function resolveConnectionForActiveAgent( + connectionId: string, + profile: string +): Promise { const getConnectionFor = window.hermesDesktop?.getConnectionFor if (!getConnectionFor) { - return + return null } try { - setConnection(await getConnectionFor({ connectionId, profile })) + return await getConnectionFor({ connectionId, profile }) } catch { - // Leave the prior connection in place; boot/reconnect resyncs it later. + return null } } // Activate a connection-scoped agent's gateway — the (connectionId, profile) // analogue of ensureGatewayProfile, and the door the SDK's ensureAgent goes -// through. Two invariants the raw store call (ensureGatewayForAgent) does not -// provide on its own: +// through. Three invariants the raw store call (ensureGatewayForAgent) does +// not provide on its own: // - Every activation moves $activeGatewayProfile and resyncs $connection, // exactly like the profile path — otherwise activating an ALREADY-OPEN // registry agent left both describing the previous backend, routing @@ -382,6 +396,11 @@ async function syncConnectionToActiveAgent(connectionId: string, profile: string // - Activations share the gatewaySwitch mutex with profile switches, so a // rapid agent↔profile (or agent↔agent) interleave can't finish out of // order and leave the EARLIER setActive() as the last write. +// - The gateway, the profile pointer and the connection descriptor publish in +// ONE synchronous frame, via the same prepare/publish seam the profile path +// uses, so no subscriber sees the new backend paired with the old +// descriptor. +// A local/null connectionId falls through to the profile path verbatim. // Only a null connectionId falls through to the legacy profile path. Explicit // `local` is a registry identity and must use the genuinely-local route. export async function ensureGatewayAgent(connectionId: null | string, profile: string): Promise { @@ -399,16 +418,39 @@ export async function ensureGatewayAgent(connectionId: null | string, profile: s $gatewaySwapTarget.set(target) gatewaySwitch = (async () => { - const activated = await ensureGatewayForAgent(connection, target) - - if (!activated) { - return - } + // Dial the agent's socket and resolve its descriptor without publishing + // either, exactly like the profile path above. Activating first and then + // awaiting the descriptor left $gateway on the new backend while + // $connection still described the old one, so anything requesting during + // that window announced the WRONG mode to the new backend. + const [descriptor, activate] = await Promise.all([ + resolveConnectionForActiveAgent(connection, target), + prepareGatewayForAgent(connection, target) + ]) - $activeGatewayProfile.set(target) - // The active backend just changed; resync $connection so remote-aware - // paths (image.attach_bytes vs image.attach, /api/fs/*, /api/media) follow. - await syncConnectionToActiveAgent(connection, target) + // ONE publication. batch() defers Nanostores' notifications to the end of + // the callback, so a $gateway listener cannot run while the profile + // pointer and the connection descriptor still name the previous backend. + // Without it these are three sequential .set() calls, each draining its + // listeners synchronously, and the first listener observes exactly the + // mismatch this seam exists to prevent. + batch(() => { + // A disposed target (source edited/removed mid-dial) publishes nothing + // at all, rather than moving the profile pointer to a backend that no + // longer has a socket. + if (!activate()) { + return + } + + $activeGatewayProfile.set(target) + + // Remote-aware paths (image.attach_bytes vs image.attach, /api/fs/*, + // /api/media) follow $connection. A null descriptor keeps the previous + // one rather than clearing it, per resolveConnectionForActiveAgent. + if (descriptor) { + setConnection(descriptor) + } + }) })() try { From 79a362d69c223b96ea3fd577d02ea9fbf59d9bf2 Mon Sep 17 00:00:00 2001 From: Jack Lau <72348727+jackulau@users.noreply.github.com> Date: Sun, 16 Aug 2026 08:13:29 -0500 Subject: [PATCH 18/25] fix(desktop): fail the agent switch closed when its descriptor lookup rejects Review caught that the agent path fixed the pending-descriptor race but not the failure path. `resolveConnectionForActiveAgent` caught a `getConnectionFor` rejection and returned null, so `Promise.all` resolved as `[null, activate]` and the switch published anyway: the activation thunk ran and `$activeGatewayProfile` advanced, while only `setConnection` was skipped. That is the same mixed state this PR exists to remove, except it does not close on its own. The pending-descriptor window ends when the descriptor arrives; a failed lookup never arrives, so `$gateway` named the new backend while `$connection` described the old one until an unrelated reconnect or switch happened to repair it. Anything branching on connection mode in between (plugins, `MEDIA:`, `/api/fs/*`, `/api/media`, image attach) saw the pair disagree. Let the rejection propagate, matching `resolveConnectionForProfile`, whose contract is already exactly this: null means "no desktop bridge" and nothing else, and a bridge rejection aborts the whole switch before anything is published. Both doors now fail closed identically, and the caller can retry. The existing "leaves the prior connection intact when the descriptor fetch fails" test asserted the old best-effort behaviour, so it pinned the defect rather than a contract worth keeping. Replaced with a rejected-descriptor test that asserts none of the three atoms moved and the activation thunk was never called. The pending-descriptor case keeps its own separate test, so the success and failure contracts are pinned independently. Also reworded the publication comments: these are sequential atom writes with no asynchronous gap between them, not a transaction, and describing them as one "frame" overstated the guarantee. --- .../store/profile-agent-activation.test.ts | 29 +++++++++++++----- apps/desktop/src/store/profile.ts | 30 +++++++++---------- 2 files changed, 35 insertions(+), 24 deletions(-) diff --git a/apps/desktop/src/store/profile-agent-activation.test.ts b/apps/desktop/src/store/profile-agent-activation.test.ts index 528a5d8e1aa1..93c50fbb20ba 100644 --- a/apps/desktop/src/store/profile-agent-activation.test.ts +++ b/apps/desktop/src/store/profile-agent-activation.test.ts @@ -13,10 +13,15 @@ import type { HermesConnection } from '@/global' // 2. Agent activations share the gatewaySwitch mutex with profile switches — // without it, two rapid activations could complete out of order and the // EARLIER setActive() landed last. -// 3. An activation publishes the gateway, the profile pointer and the -// connection descriptor in ONE synchronous frame. Activating first and -// awaiting the descriptor after left $gateway on the new backend while -// $connection still described the old one. +// 3. A SUCCEEDING activation publishes the gateway, the profile pointer and +// the connection descriptor with no asynchronous gap between them. +// Activating first and awaiting the descriptor after left $gateway on the +// new backend while $connection still described the old one. +// 4. A FAILING descriptor lookup publishes none of the three. Swallowing the +// rejection and publishing anyway produced the same mixed state as (3), +// except permanent: (3) closes when the descriptor arrives, whereas a +// failed lookup never arrives and the split survived until an unrelated +// reconnect or switch repaired it. // // Both doors go through the prepare/publish seam (prepareGatewayFor*, which // dial without publishing and return the activation thunk), so these mocks @@ -115,14 +120,22 @@ describe('ensureGatewayAgent → $connection / $activeGatewayProfile sync', () = expect($connection.get()?.profile).toBe('research') }) - it('leaves the prior connection intact when the descriptor fetch fails', async () => { + it('fails the switch closed when the descriptor lookup rejects', async () => { + // Previously this path swallowed the rejection and published anyway, which + // left $gateway and $activeGatewayProfile on the NEW backend while + // $connection still described the old one. Unlike the pending-descriptor + // race below, that state did not close on its own: it survived until some + // later reconnect or switch happened to repair it. getConnectionFor.mockRejectedValue(new Error('source unreachable')) - await ensureGatewayAgent('homelab', 'research') + await expect(ensureGatewayAgent('homelab', 'research')).rejects.toThrow('source unreachable') - expect($activeGatewayProfile.get()).toBe('research') - // Best-effort: boot/reconnect resyncs later; we must not null it out here. + // Nothing published: all three still describe the previous backend, and the + // caller can retry the switch. + expect(activateAgent).not.toHaveBeenCalled() + expect($activeGatewayProfile.get()).toBe('default') expect($connection.get()?.mode).toBe('local') + expect($connection.get()?.profile).toBe('default') }) it('does not republish a registry identity invalidated during activation', async () => { diff --git a/apps/desktop/src/store/profile.ts b/apps/desktop/src/store/profile.ts index 05d9f1c20f62..42cfed61ef2b 100644 --- a/apps/desktop/src/store/profile.ts +++ b/apps/desktop/src/store/profile.ts @@ -361,27 +361,24 @@ export async function ensureGatewayProfile(profile: string | null | undefined): // Registry-aware sibling of syncConnectionToActiveProfile: a connection-scoped // agent's descriptor comes from getConnectionFor (its SOURCE connection), not -// getConnection (the local pool). Same best-effort contract. +// getConnection (the local pool). // Resolve only — publication is the caller's, so the descriptor can be in hand -// BEFORE the activation frame rather than an await after it. Null means "no -// descriptor to publish" (no desktop bridge, or the lookup failed): the caller -// leaves the prior connection in place and boot/reconnect resyncs it later, -// which is this path's established best-effort contract. +// BEFORE the activation frame rather than an await after it. +// +// Null means "no desktop bridge" (plain browser) and nothing else, matching +// resolveConnectionForProfile. A bridge REJECTION propagates so the caller +// aborts the whole switch. Collapsing the two into null instead let a failed +// lookup publish the new gateway and profile while $connection kept describing +// the OLD backend, and unlike the pending-descriptor race that state did not +// close on its own: it survived until some later reconnect or switch happened +// to repair it, which is the same invariant this path exists to establish. async function resolveConnectionForActiveAgent( connectionId: string, profile: string ): Promise { const getConnectionFor = window.hermesDesktop?.getConnectionFor - if (!getConnectionFor) { - return null - } - - try { - return await getConnectionFor({ connectionId, profile }) - } catch { - return null - } + return getConnectionFor ? getConnectionFor({ connectionId, profile }) : null } // Activate a connection-scoped agent's gateway — the (connectionId, profile) @@ -445,8 +442,9 @@ export async function ensureGatewayAgent(connectionId: null | string, profile: s $activeGatewayProfile.set(target) // Remote-aware paths (image.attach_bytes vs image.attach, /api/fs/*, - // /api/media) follow $connection. A null descriptor keeps the previous - // one rather than clearing it, per resolveConnectionForActiveAgent. + // /api/media) follow $connection. Null here is only the no-bridge case, + // so keeping the previous descriptor is correct; a failed lookup + // rejected above and never reached this frame. if (descriptor) { setConnection(descriptor) } From 869476d32bde0372676e3af3b617065d9386f9ac Mon Sep 17 00:00:00 2001 From: Jack Lau <72348727+jackulau@users.noreply.github.com> Date: Sun, 16 Aug 2026 19:34:30 -0500 Subject: [PATCH 19/25] fix(desktop): publish a gateway switch in one nanostores batch The prepare/publish seam removed the *await* between activating the gateway and setting the profile pointer and connection descriptor, but not the *notification* gap. Nanostores drains a store's listeners synchronously inside .set(), so three sequential sets still let a $gateway listener run while $activeGatewayProfile and $connection named the previous backend. That is the same mixed state the seam exists to prevent, just narrowed from an async window to a synchronous one, and it is worse to debug because it is invisible in an await-shaped reading of the code. batch() defers every notification to the end of the callback, so the three become one observable transition on both the profile path and the agent path. Pinned with a test that attaches a real $gateway listener and asserts the companions are already current in the first callback; the mock thunks now publish distinct gateway identities so an out-of-order publication cannot pass unnoticed, and three existing tests assert $gateway is still the ORIGINAL object (by identity) on every path that must publish nothing. --- .../store/profile-agent-activation.test.ts | 63 +++++++++++++++++-- apps/desktop/src/store/profile.ts | 7 +-- 2 files changed, 59 insertions(+), 11 deletions(-) diff --git a/apps/desktop/src/store/profile-agent-activation.test.ts b/apps/desktop/src/store/profile-agent-activation.test.ts index 93c50fbb20ba..b5738b0174b3 100644 --- a/apps/desktop/src/store/profile-agent-activation.test.ts +++ b/apps/desktop/src/store/profile-agent-activation.test.ts @@ -33,16 +33,25 @@ import type { HermesConnection } from '@/global' const INITIAL_GATEWAY = { id: 'live-socket' } const AGENT_GATEWAY = { id: 'agent-socket' } const PROFILE_GATEWAY = { id: 'profile-socket' } + const activateAgent = vi.fn(() => { $gateway.set(AGENT_GATEWAY) return true }) + const activateProfile = vi.fn(() => { $gateway.set(PROFILE_GATEWAY) }) -const prepareGatewayForAgent = vi.fn(async (_connectionId: null | string, _profile: string) => activateAgent) -const prepareGatewayForProfile = vi.fn(async (_profile: string) => activateProfile) + +// Annotated with the SEAM's thunk types, not the spies' own. Inferred, the +// resolved type is the MockInstance itself, and a test can no longer hand back +// a plain `() => false` to stand in for a disposed entry. +const prepareGatewayForAgent = vi.fn( + async (_connectionId: null | string, _profile: string): Promise<() => boolean> => activateAgent +) + +const prepareGatewayForProfile = vi.fn(async (_profile: string): Promise<() => void> => activateProfile) const openGatewayForProfile = vi.fn(async (_profile: string) => undefined) const $gateway = atom(INITIAL_GATEWAY) const resetStarmapGraph = vi.fn() @@ -93,7 +102,7 @@ beforeEach(() => { prepareGatewayForProfile.mockResolvedValue(activateProfile) activateAgent.mockClear() activateProfile.mockClear() - $gateway.set({ id: 'live-socket' }) + $gateway.set(INITIAL_GATEWAY) $activeGatewayProfile.set('default') $connection.set(localConn()) vi.stubGlobal('window', { hermesDesktop: { getConnection, getConnectionFor } }) @@ -133,19 +142,62 @@ describe('ensureGatewayAgent → $connection / $activeGatewayProfile sync', () = // Nothing published: all three still describe the previous backend, and the // caller can retry the switch. expect(activateAgent).not.toHaveBeenCalled() + expect($gateway.get()).toBe(INITIAL_GATEWAY) expect($activeGatewayProfile.get()).toBe('default') expect($connection.get()?.mode).toBe('local') expect($connection.get()?.profile).toBe('default') }) it('does not republish a registry identity invalidated during activation', async () => { - ensureGatewayForAgent.mockResolvedValueOnce(false) + // The thunk reports false: the entry was disposed (source edited/removed) + // between dial and publish. Nothing may publish, $gateway included. + prepareGatewayForAgent.mockResolvedValueOnce(() => false) await ensureGatewayAgent('removed-source', 'research') expect($activeGatewayProfile.get()).toBe('default') expect($connection.get()?.mode).toBe('local') - expect(getConnectionFor).not.toHaveBeenCalled() + expect($gateway.get()).toBe(INITIAL_GATEWAY) + // The descriptor lookup DOES run: it is issued concurrently with the dial + // so both can be resolved before anything is published, which is the whole + // point of the seam. Resolving it lazily (only after the thunk reports a + // live entry) would put an await between the identity check and the + // publication and reopen the gap. The cost is one redundant read-only + // lookup in the rare disposed-entry case; the invariant that matters - + // nothing is PUBLISHED - is asserted above. + expect(getConnectionFor).toHaveBeenCalledTimes(1) + }) + + it('never shows a $gateway listener the new backend beside stale companions', async () => { + // The assertion the earlier tests could not make. A spy thunk that never + // touches $gateway proves only that it was CALLED at the right moment; + // it cannot prove that the three public stores become visible together. + // Nanostores drains listeners synchronously on every .set(), so without + // batch() a $gateway listener runs between the writes and reads the new + // gateway next to the previous profile and descriptor. + getConnectionFor.mockResolvedValue(agentConn()) + const seen: { connection?: string; gateway: unknown; profile: string }[] = [] + + const stop = $gateway.listen(gateway => { + seen.push({ + connection: $connection.get()?.profile, + gateway, + profile: $activeGatewayProfile.get() + }) + }) + + try { + await ensureGatewayAgent('homelab', 'research') + } finally { + stop() + } + + expect(seen).toHaveLength(1) + // When the listener sees the agent's gateway, the profile pointer and the + // descriptor must ALREADY identify that same backend. + expect(seen[0].gateway).toBe(AGENT_GATEWAY) + expect(seen[0].profile).toBe('research') + expect(seen[0].connection).toBe('research') }) it('falls through to the profile path for a null connectionId', async () => { @@ -187,6 +239,7 @@ describe('ensureGatewayAgent → $connection / $activeGatewayProfile sync', () = await Promise.resolve() expect(activateAgent).not.toHaveBeenCalled() + expect($gateway.get()).toBe(INITIAL_GATEWAY) expect($activeGatewayProfile.get()).toBe('default') expect($connection.get()?.mode).toBe('local') diff --git a/apps/desktop/src/store/profile.ts b/apps/desktop/src/store/profile.ts index 42cfed61ef2b..72af3f44d607 100644 --- a/apps/desktop/src/store/profile.ts +++ b/apps/desktop/src/store/profile.ts @@ -13,12 +13,7 @@ import { storedStringRecord } from '@/lib/storage' import { invalidateCronModelImpactScopeState } from '@/store/cron-model-impact-scope' -import { - $gateway, - openGatewayForProfile, - prepareGatewayForAgent, - prepareGatewayForProfile -} from '@/store/gateway' +import { $gateway, openGatewayForProfile, prepareGatewayForAgent, prepareGatewayForProfile } from '@/store/gateway' import { setConnection } from '@/store/session' import { resetStarmapGraph } from '@/store/starmap' import type { ProfileInfo } from '@/types/hermes' From 5a778ac67587c57deba871c4da3b483299d07792 Mon Sep 17 00:00:00 2001 From: Jack Lau <72348727+jackulau@users.noreply.github.com> Date: Sun, 16 Aug 2026 19:35:12 -0500 Subject: [PATCH 20/25] fix(desktop): keep the full request signature through host.getGateway() The announcing Proxy wrapped HermesGateway.request with a two-argument function, so a plugin calling getGateway().request(method, params, timeoutMs, signal) silently lost both its custom deadline and its ability to abort. A wrapper must not narrow the contract it stands in for, and this one narrowed it invisibly: the call still succeeded, it just could never be cancelled and always used the default timeout. The tail is forwarded as a rest spread rather than two named parameters so the delegated call carries exactly the arguments the caller made. Naming them re-materializes omitted arguments as explicit undefined, which is invisible to a defaulted parameter but not to anything reading arguments.length, and it makes every pass-through call site un-assertable on its real shape. Two regressions: one asserting the timeout and the AbortSignal reach the underlying request (including signal IDENTITY, not just presence) on a stamped RPC, and one asserting the same on an RPC this door does not stamp. --- apps/desktop/src/sdk/index.test.ts | 71 +++++++++++++++++++++--------- apps/desktop/src/sdk/index.ts | 14 ++++-- 2 files changed, 60 insertions(+), 25 deletions(-) diff --git a/apps/desktop/src/sdk/index.test.ts b/apps/desktop/src/sdk/index.test.ts index dcda18452066..28815af1927f 100644 --- a/apps/desktop/src/sdk/index.test.ts +++ b/apps/desktop/src/sdk/index.test.ts @@ -4,12 +4,7 @@ import type { HermesConnection } from '@/global' import { createClientSessionState } from '@/lib/chat-runtime' import { host } from '@/sdk' import { $gateway } from '@/store/gateway' -import { - setActiveSessionId, - setAwaitingResponse, - setBusy, - setConnection -} from '@/store/session' +import { setActiveSessionId, setAwaitingResponse, setBusy, setConnection } from '@/store/session' import { clearAllSessionStates, publishSessionState } from '@/store/session-states' describe('host.state turn flags', () => { @@ -123,8 +118,7 @@ describe('host.state turn flags', () => { * mode (#82187 follow-up review, item 3). */ -const conn = (mode?: 'local' | 'remote') => - ({ baseUrl: 'http://127.0.0.1:8787', mode }) as unknown as HermesConnection +const conn = (mode?: 'local' | 'remote') => ({ baseUrl: 'http://127.0.0.1:8787', mode }) as unknown as HermesConnection describe('host.request connection-mode announcement', () => { afterEach(() => { @@ -139,16 +133,13 @@ describe('host.request connection-mode announcement', () => { return request } - it.each(['session.create', 'session.resume', 'prompt.submit'])( - 'stamps the live mode onto %s', - async method => { - setConnection(conn('remote')) - const request = installGateway() + it.each(['session.create', 'session.resume', 'prompt.submit'])('stamps the live mode onto %s', async method => { + setConnection(conn('remote')) + const request = installGateway() - await expect(host.request(method, { text: 'hi' })).resolves.toBe('ok') - expect(request).toHaveBeenCalledWith(method, { connection_mode: 'remote', text: 'hi' }) - } - ) + await expect(host.request(method, { text: 'hi' })).resolves.toBe('ok') + expect(request).toHaveBeenCalledWith(method, { connection_mode: 'remote', text: 'hi' }) + }) it('leaves unrelated RPCs untouched', async () => { setConnection(conn('remote')) @@ -160,14 +151,17 @@ describe('host.request connection-mode announcement', () => { expect(request).toHaveBeenCalledWith('session.list', params) }) - it('adds no key when the mode is unknown', async () => { - // Null descriptor (reconnect window / older shell): omit rather than - // clear, matching withConnectionMode semantics. + it('announces an explicit null when the mode is unknown', async () => { + // Null descriptor (reconnect window / older shell). Announcing an explicit + // null CLEARS the backend's remembered mode; omitting the key leaves it + // alone (`_remember_connection_mode` only writes when the key is present), + // so a `local` announced before the reconnect would survive into turns + // that can no longer prove it. Unknown must never read as local. const request = installGateway() await host.request('prompt.submit', { text: 'hi' }) - expect(request).toHaveBeenCalledWith('prompt.submit', { text: 'hi' }) + expect(request).toHaveBeenCalledWith('prompt.submit', { connection_mode: null, text: 'hi' }) }) it('still throws when no gateway socket is live', async () => { @@ -211,6 +205,41 @@ describe('host.getGateway connection-mode announcement', () => { expect(request).toHaveBeenCalledWith('session.list', params) }) + it('forwards the timeout and abort signal the caller passed', async () => { + // HermesGateway.request is (method, params, timeoutMs, signal). The + // announcing wrapper stands in for it, so a two-argument wrapper silently + // dropped arguments three and four: the request fell back to the default + // deadline and could no longer be aborted. Every other test here calls the + // two-argument form, so green CI did not cover it. + setConnection(conn('remote')) + const request = vi.fn().mockResolvedValue('ok') + $gateway.set({ request } as never) + const controller = new AbortController() + + await host.getGateway()?.request('prompt.submit', { text: 'hi' }, 1234, controller.signal) + + expect(request).toHaveBeenCalledWith( + 'prompt.submit', + { connection_mode: 'remote', text: 'hi' }, + 1234, + controller.signal + ) + // Identity, not shape: a copied-but-equal signal would abort nothing. + expect(request.mock.calls[0][3]).toBe(controller.signal) + }) + + it('forwards a timeout and signal on RPCs it does not stamp', async () => { + setConnection(conn('remote')) + const request = vi.fn().mockResolvedValue('ok') + $gateway.set({ request } as never) + const controller = new AbortController() + const params = { limit: 3 } + + await host.getGateway()?.request('session.list', params, 99, controller.signal) + + expect(request).toHaveBeenCalledWith('session.list', params, 99, controller.signal) + }) + it('delegates non-request members to the real instance', () => { const close = vi.fn() setConnection(conn('remote')) diff --git a/apps/desktop/src/sdk/index.ts b/apps/desktop/src/sdk/index.ts index ef88c532c9ee..aa80685f4e76 100644 --- a/apps/desktop/src/sdk/index.ts +++ b/apps/desktop/src/sdk/index.ts @@ -92,6 +92,7 @@ export interface PluginProfileRoute { profile: string /** Backend Hermes profile served by that route. */ targetProfile: string +} // One announcing view per real gateway, so repeated `getGateway()` calls hand // back a stable reference — SDK components take this as a React prop, and a @@ -120,13 +121,18 @@ const announcingGateway = (gateway: HermesGateway): HermesGateway => { // wrapper silently swallows `timeoutMs` and `signal`, so any SDK // caller passing them lost its custom deadline and its ability to // abort - a wrapper must not narrow the contract it stands in for. + // + // The tail is forwarded as a REST spread rather than as two named + // parameters so the delegated call carries exactly the arguments the + // caller made. Naming them re-materializes omitted arguments as + // explicit `undefined`, which is invisible to a defaulted parameter + // but not to anything reading `arguments.length` - and it makes every + // pass-through call site un-assertable on its real shape. return ( method: string, params: Record = {}, - timeoutMs?: number, - signal?: AbortSignal - ): Promise => - target.request(method, announceConnectionMode(method, params), timeoutMs, signal) + ...rest: [timeoutMs?: number, signal?: AbortSignal] + ): Promise => target.request(method, announceConnectionMode(method, params), ...rest) } const value = Reflect.get(target, prop) From 287806ce9d47289227ac101b0e44beb37e1d80b6 Mon Sep 17 00:00:00 2001 From: Jack Lau <72348727+jackulau@users.noreply.github.com> Date: Sun, 16 Aug 2026 19:35:51 -0500 Subject: [PATCH 21/25] fix(agent): fail closed when the inline-shell env factory is unavailable run_inline_shell fell back to env=None when build_subprocess_env could not be imported or raised. subprocess.run(env=None) inherits the RAW parent environment, which is the one door the scrub exists to shut: an ambient HERMES_DESKTOP_CONNECTION_MODE=local set in the user's shell reaches the snippet verbatim and is read as the resolved Desktop mode. So the error branch was strictly more permissive than the success branch, and it was the branch nobody would look at. Refusing to run the snippet is the safe outcome and costs nothing the caller cannot absorb: it already treats an [inline-shell error: ...] marker as a non-fatal result, so one skipped snippet degrades the skill message instead of silently handing it a spoofable environment. The regression asserts the strongest available property, that the child never existed, and fails against the old code with the leaked env printed in the message. --- agent/skill_preprocessing.py | 14 ++++++-- tests/agent/test_skill_preprocessing_env.py | 39 +++++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/agent/skill_preprocessing.py b/agent/skill_preprocessing.py index c8437ac5f333..5bbfaf034d95 100644 --- a/agent/skill_preprocessing.py +++ b/agent/skill_preprocessing.py @@ -78,8 +78,18 @@ def run_inline_shell(command: str, cwd: Path | None, timeout: int) -> str: _run_env = build_subprocess_env() except Exception: - logger.debug("build_subprocess_env unavailable for inline shell", exc_info=True) - _run_env = None + # FAIL CLOSED. Falling back to ``env=None`` makes subprocess.run + # inherit the RAW parent environment, which is the one door the scrub + # above exists to shut: an ambient HERMES_DESKTOP_CONNECTION_MODE=local + # set in the user's shell would reach the snippet and be read as the + # resolved Desktop mode, re-opening the spoofing path. Not running the + # snippet is the safe outcome and costs nothing the caller cannot + # absorb — it already treats a marker as a non-fatal result. + logger.warning( + "build_subprocess_env unavailable for inline shell; refusing to run the snippet", + exc_info=True, + ) + return "[inline-shell error: sanitized environment unavailable]" try: completed = subprocess.run( ["bash", "-c", command], diff --git a/tests/agent/test_skill_preprocessing_env.py b/tests/agent/test_skill_preprocessing_env.py index c9b88a266894..026f3630018d 100644 --- a/tests/agent/test_skill_preprocessing_env.py +++ b/tests/agent/test_skill_preprocessing_env.py @@ -93,3 +93,42 @@ def test_ambient_value_is_stripped_when_no_mode_is_bound(monkeypatch): set_session_vars(session_key="k", source="tui") captured = _capture_spawn_env(monkeypatch) assert MODE_ENV not in captured["env"] + + +def test_snippet_is_not_spawned_when_the_env_factory_fails(monkeypatch): + """A sanitizer that cannot be built must fail CLOSED, not fall back to env=None. + + ``subprocess.run(env=None)`` inherits the raw parent environment, so the + old ``except: _run_env = None`` fallback handed the snippet an ambient + ``HERMES_DESKTOP_CONNECTION_MODE=local`` verbatim — reopening exactly the + spoofing path the scrub exists to close, and only on the error branch where + nobody would look for it (#82187 follow-up review, item 3). + """ + monkeypatch.setenv(MODE_ENV, "local") + set_session_vars(session_key="k", source="desktop") + set_desktop_connection_mode("remote") + + import tools.environments.local as local_env + + def _boom(): + raise RuntimeError("sanitizer unavailable") + + monkeypatch.setattr(local_env, "build_subprocess_env", _boom) + + spawned = [] + + def _record(argv, **kwargs): + spawned.append({"argv": argv, "env": kwargs.get("env")}) + return SimpleNamespace(stdout="ok\n", stderr="", returncode=0) + + monkeypatch.setattr(subprocess, "run", _record) + + result = run_inline_shell("echo hi", None, timeout=5) + + # The strongest assertion available: the child never existed, so there is + # no environment for it to have inherited. + assert spawned == [], ( + "the snippet was spawned with a non-sanitized environment: " + f"{spawned[0]['env'] if spawned else None}" + ) + assert "inline-shell error" in result From 7608633c4ee9ed848a28e4a8ec081323ba478237 Mon Sep 17 00:00:00 2001 From: Jack Lau <72348727+jackulau@users.noreply.github.com> Date: Sun, 16 Aug 2026 19:35:51 -0500 Subject: [PATCH 22/25] fix(desktop): make connection_mode a renderer-owned field withConnectionMode honored a caller-supplied connection_mode and skipped the stamp when the live mode was unresolved. Both are spoofing paths, and the plugin SDK's host.request reaches this same choke point: - A plugin driving a live REMOTE session could pass connection_mode: 'local' and have the backend hand its skills and MCP context paths as though they were on the user's machine. Only the renderer can see the descriptor, so only the renderer may answer. - Omitting the key on an unresolved mode is not neutral. _remember_connection_mode (server.py) only writes when the key is PRESENT, so omission means "keep what you have": a 'local' announced before a reconnect survived into turns that could no longer prove it. An explicit null clears it, because normalize_desktop_connection_mode(None) is None. The field is now reserved: whatever the caller put there is discarded and the live resolved mode, null included, is written in its place. Four regressions replace the caller-wins test, covering each direction: announcing null when unknown, overriding a caller value with the live mode, refusing to let a caller 'local' survive an unknown mode, and clearing a previously announced 'local'. --- apps/desktop/src/lib/connection-mode.test.ts | 38 +++++++++++++++----- apps/desktop/src/lib/connection-mode.ts | 21 ++++++++--- 2 files changed, 46 insertions(+), 13 deletions(-) diff --git a/apps/desktop/src/lib/connection-mode.test.ts b/apps/desktop/src/lib/connection-mode.test.ts index 7b7f3ec87ce3..e6f1b486aab8 100644 --- a/apps/desktop/src/lib/connection-mode.test.ts +++ b/apps/desktop/src/lib/connection-mode.test.ts @@ -41,20 +41,42 @@ describe('withConnectionMode', () => { expect(withConnectionMode('session.list', params, 'remote')).toBe(params) }) - it('adds no key when the mode is unknown', () => { - // Omitting is deliberate: it leaves any previously-announced value intact - // on the backend instead of clearing it during a reconnect window. - const params = { text: 'hi' } - - expect(withConnectionMode('prompt.submit', params, null)).toBe(params) + it('announces an explicit null when the mode is unknown', () => { + // NOT omission. Omitting the key means "leave the stored value alone" to + // _remember_connection_mode, so a `local` announced before a reconnect + // would survive into turns that can no longer prove it. An explicit null + // normalizes to None and clears it. + expect(withConnectionMode('prompt.submit', { text: 'hi' }, null)).toEqual({ + connection_mode: null, + text: 'hi' + }) }) - it('never overrides a mode a caller set explicitly', () => { + it('overrides a caller-supplied mode with the live one', () => { + // connection_mode is renderer-owned. The plugin SDK reaches this same + // door, so a caller value winning would let a plugin on a live REMOTE + // session announce `local` and be believed. expect(withConnectionMode('prompt.submit', { connection_mode: 'local' }, 'remote')).toEqual({ - connection_mode: 'local' + connection_mode: 'remote' + }) + }) + + it('never lets a caller-supplied local survive an unknown live mode', () => { + expect(withConnectionMode('prompt.submit', { connection_mode: 'local' }, null)).toEqual({ + connection_mode: null }) }) + it('clears a previously announced local when the live mode goes unknown', () => { + // The reconnect window: the backend is holding `local` from an earlier + // turn and this turn cannot resolve a descriptor. Unknown must never be + // guessed as local, so the announcement has to clear rather than skip. + const reconnecting = withConnectionMode('prompt.submit', { text: 'hi' }, null) + + expect(reconnecting).toHaveProperty('connection_mode', null) + expect('connection_mode' in reconnecting).toBe(true) + }) + it('does not mutate the caller params', () => { const params = { text: 'hi' } withConnectionMode('prompt.submit', params, 'local') diff --git a/apps/desktop/src/lib/connection-mode.ts b/apps/desktop/src/lib/connection-mode.ts index 5b9b9147fbd3..289c61636ab3 100644 --- a/apps/desktop/src/lib/connection-mode.ts +++ b/apps/desktop/src/lib/connection-mode.ts @@ -49,17 +49,28 @@ export function resolveConnectionMode(connection: HermesConnection | null | unde * ~10 call sites, so a new session/prompt path announces correctly by * construction instead of by remembering to. * - * An explicit param already on `params` wins (nothing sets one today; this - * keeps the helper from silently overriding a deliberate caller). An unknown - * mode adds no key at all, which leaves any previously-announced value intact - * on the backend rather than clearing it during a reconnect window. + * `connection_mode` is a RESERVED, renderer-owned field: whatever the caller + * put there is discarded and the live resolved mode is written in its place. + * The plugin SDK's `host.request` reaches this same door, so honouring a + * caller value would let a plugin driving a live REMOTE session announce + * `local` and have the backend hand its skills and MCP context paths as though + * they were on the user's machine — the exact spoof the field exists to + * prevent. Only the renderer can see the descriptor, so only the renderer may + * answer. + * + * An unresolved mode announces an explicit `null` rather than omitting the + * key. Omitting it means "leave the stored value alone" to the backend + * (`_remember_connection_mode`), so a `local` announced before a reconnect + * would survive into turns that can no longer prove it — and "unknown must + * never be guessed as local" is the whole safety rule here. + * `normalize_desktop_connection_mode(None)` is `None`, so this clears it. */ export function withConnectionMode( method: string, params: Record, mode: HermesConnectionMode | null ): Record { - if (!mode || !CONNECTION_MODE_METHODS.has(method) || 'connection_mode' in params) { + if (!CONNECTION_MODE_METHODS.has(method)) { return params } From 44f3f190fd78619603d030e846390f4f20fcf508 Mon Sep 17 00:00:00 2001 From: Jack Lau <72348727+jackulau@users.noreply.github.com> Date: Sun, 16 Aug 2026 19:38:45 -0500 Subject: [PATCH 23/25] refactor(desktop): wrap the prepareGatewayForAgent signature at the project width --- apps/desktop/src/store/gateway.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/apps/desktop/src/store/gateway.ts b/apps/desktop/src/store/gateway.ts index a869bf8e7ad6..866a93461bb1 100644 --- a/apps/desktop/src/store/gateway.ts +++ b/apps/desktop/src/store/gateway.ts @@ -616,10 +616,7 @@ export async function openGatewayForAgent(connectionId: null | string, profile: // contract callers rely on: a source edit/remove can dispose this entry while // its dial is in flight, and a caller must be able to tell "switched" from // "the target stopped existing" rather than assume the former. -export async function prepareGatewayForAgent( - connectionId: null | string, - profile: string -): Promise<() => boolean> { +export async function prepareGatewayForAgent(connectionId: null | string, profile: string): Promise<() => boolean> { const scope = registryBackendScopeKey(connectionId, profile) if (scope === normKey(profile)) { From 9105b70324c1f496df2679a327fbb6f3a1d29aa6 Mon Sep 17 00:00:00 2001 From: Jack Lau <72348727+jackulau@users.noreply.github.com> Date: Sun, 16 Aug 2026 21:26:38 -0500 Subject: [PATCH 24/25] fix(desktop): guard the profile publication on its activation result too The agent path already declined to publish when applyActive() rejected its activation, but the profile path discarded the same boolean and published unconditionally. applyActive() returns false when its captured epoch has been superseded, which happens whenever a newer switch or a teardown lands while this preparation is still awaiting its route or socket. The result was not a torn publication. batch() makes those writes observer-atomic either way. It was something subtler: ONE complete, internally inconsistent tuple, the CURRENT gateway paired with the stale target's profile pointer and descriptor. Atomicity cannot make a rejected activation correct, so the caller has to decline to publish at all. prepareGatewayForProfile now returns Promise<() => boolean> like its agent counterpart. The primary and shared-primary thunks return applyActive() directly; the secondary thunk reports whether the prepared entry was still current AND the epoch was accepted, keeping the descriptor publish conditional on having a cached connection so an accepted activation with no descriptor still moves the companions. prepareGatewayForAgent's genuinely-local fallthrough now returns the profile thunk unchanged instead of wrapping it to return an unconditional true, which had been reporting a rejected activation to the agent caller as a successful one. Two regressions on the profile door: a superseded activation leaves all three stores on the existing complete route with no subscriber notified at all, and an accepted one still publishes, so a thunk that always reported false could not pass. The mock thunks in profile.test.ts now return true, since a bare vi.fn() returns undefined and would read as "superseded". --- apps/desktop/src/store/gateway.ts | 39 +++++++------- .../store/profile-agent-activation.test.ts | 53 ++++++++++++++++++- apps/desktop/src/store/profile.test.ts | 6 ++- apps/desktop/src/store/profile.ts | 12 ++++- 4 files changed, 87 insertions(+), 23 deletions(-) diff --git a/apps/desktop/src/store/gateway.ts b/apps/desktop/src/store/gateway.ts index 866a93461bb1..841e5fa0e55b 100644 --- a/apps/desktop/src/store/gateway.ts +++ b/apps/desktop/src/store/gateway.ts @@ -619,14 +619,12 @@ export async function openGatewayForAgent(connectionId: null | string, profile: export async function prepareGatewayForAgent(connectionId: null | string, profile: string): Promise<() => boolean> { const scope = registryBackendScopeKey(connectionId, profile) + // Genuinely-local scope: the profile door owns this route, so hand back ITS + // thunk unchanged. Wrapping it to return an unconditional `true` would have + // reported a rejected activation as a successful one and let the agent + // caller publish companion state for a switch that never happened. if (scope === normKey(profile)) { - const activate = await prepareGatewayForProfile(profile) - - return () => { - activate() - - return true - } + return prepareGatewayForProfile(profile) } if (!window.hermesDesktop?.getConnectionFor) { @@ -685,14 +683,12 @@ export async function ensureGatewayForAgent(connectionId: null | string, profile // pointer, connection descriptor), so no subscriber can observe the active // gateway pointing at one backend while companion state still describes // another. Nothing is published until the thunk runs. -export async function prepareGatewayForProfile(profile: string): Promise<() => void> { +export async function prepareGatewayForProfile(profile: string): Promise<() => boolean> { const key = normKey(profile) const activationEpoch = beginGatewayActivation() if (key === g.primaryProfile) { - return () => { - applyActive(key, activationEpoch) - } + return () => applyActive(key, activationEpoch) } // Global-remote share (routing case 3): one remote host serves every @@ -704,9 +700,7 @@ export async function prepareGatewayForProfile(profile: string): Promise<() => v // entry, and returned as a thunk like every other path here so this // switch publishes as atomically as a dedicated-socket one. if (await sharedPrimaryRoute(key)) { - return () => { - applyActive(g.primaryProfile, activationEpoch) - } + return () => applyActive(g.primaryProfile, activationEpoch) } let entry = g.secondaries.get(key) @@ -736,15 +730,20 @@ export async function prepareGatewayForProfile(profile: string): Promise<() => v // compares against this exact entry. const prepared = entry + // Reports whether the ACTIVATION was accepted, which is a different question + // from whether a descriptor was published: an accepted activation with no + // cached connection still moved the gateway, so the caller must still move + // its companion state. Only a rejected activation (disposed entry, or an + // epoch superseded by a newer switch while this one was dialing) must leave + // every companion store alone. return () => { - if ( - prepared.wantOpen && - g.secondaries.get(key) === prepared && - applyActive(key, activationEpoch) && - prepared.connection - ) { + const activated = prepared.wantOpen && g.secondaries.get(key) === prepared && applyActive(key, activationEpoch) + + if (activated && prepared.connection) { publishActiveConnection(prepared.connection) } + + return activated } } diff --git a/apps/desktop/src/store/profile-agent-activation.test.ts b/apps/desktop/src/store/profile-agent-activation.test.ts index b5738b0174b3..124da3010eac 100644 --- a/apps/desktop/src/store/profile-agent-activation.test.ts +++ b/apps/desktop/src/store/profile-agent-activation.test.ts @@ -42,6 +42,8 @@ const activateAgent = vi.fn(() => { const activateProfile = vi.fn(() => { $gateway.set(PROFILE_GATEWAY) + + return true }) // Annotated with the SEAM's thunk types, not the spies' own. Inferred, the @@ -51,7 +53,7 @@ const prepareGatewayForAgent = vi.fn( async (_connectionId: null | string, _profile: string): Promise<() => boolean> => activateAgent ) -const prepareGatewayForProfile = vi.fn(async (_profile: string): Promise<() => void> => activateProfile) +const prepareGatewayForProfile = vi.fn(async (_profile: string): Promise<() => boolean> => activateProfile) const openGatewayForProfile = vi.fn(async (_profile: string) => undefined) const $gateway = atom(INITIAL_GATEWAY) const resetStarmapGraph = vi.fn() @@ -252,6 +254,55 @@ describe('ensureGatewayAgent → $connection / $activeGatewayProfile sync', () = }) }) +describe('ensureGatewayProfile publishes under the same activation guard', () => { + it('publishes nothing when the profile activation is superseded', async () => { + // The profile-door mirror of "does not republish a registry identity + // invalidated during activation". applyActive() returns false when its + // captured epoch has been superseded — a newer switch or a teardown + // landed while this preparation was awaiting its route or socket. + // + // Discarding that boolean does not produce a torn publication; batch() + // makes the writes observer-atomic either way. It produces something + // subtler and worse: ONE complete, internally inconsistent tuple, the + // CURRENT gateway paired with the stale target's profile pointer and + // descriptor. Atomicity cannot make a rejected activation correct, so the + // caller has to decline to publish at all. + getConnection.mockResolvedValue(localConn({ profile: 'worker' })) + prepareGatewayForProfile.mockResolvedValueOnce(() => false) + + const seen: unknown[] = [] + const stop = $gateway.listen(gateway => seen.push(gateway)) + + try { + await ensureGatewayProfile('worker') + } finally { + stop() + } + + // All three still describe the complete route that was already active. + expect($gateway.get()).toBe(INITIAL_GATEWAY) + expect($activeGatewayProfile.get()).toBe('default') + expect($connection.get()?.profile).toBe('default') + expect($connection.get()?.mode).toBe('local') + // And no subscriber was handed a tuple to disagree about. + expect(seen).toEqual([]) + }) + + it('publishes the companions when the profile activation is accepted', async () => { + // The other half: the guard must not swallow a legitimate switch. Without + // this, returning a constant false from every thunk would pass the test + // above and break the feature. + getConnection.mockResolvedValue(localConn({ profile: 'worker' })) + + await ensureGatewayProfile('worker') + + expect(activateProfile).toHaveBeenCalledTimes(1) + expect($gateway.get()).toBe(PROFILE_GATEWAY) + expect($activeGatewayProfile.get()).toBe('worker') + expect($connection.get()?.profile).toBe('worker') + }) +}) + describe('ensureGatewayAgent shares the gatewaySwitch mutex with profile switches', () => { it('serializes an agent activation behind an in-flight profile switch', async () => { const profileGate = deferred() diff --git a/apps/desktop/src/store/profile.test.ts b/apps/desktop/src/store/profile.test.ts index 1732e0e726c3..7ac9dbb6bc8a 100644 --- a/apps/desktop/src/store/profile.test.ts +++ b/apps/desktop/src/store/profile.test.ts @@ -6,7 +6,11 @@ import type { ProfileInfo } from '@/types/hermes' // Keep profile.ts's side-effecting imports inert: the gateway socket layer and // the REST query client must not run for real in a unit test. -const activateGateway = vi.fn() +// Returns true: both prepare seams hand back a thunk reporting whether the +// activation was ACCEPTED, and a caller publishes its companion state only on +// true. A bare vi.fn() returns undefined, which now reads as "superseded" and +// would silently suppress every publication these tests assert on. +const activateGateway = vi.fn(() => true) const ensureGatewayForProfile = vi.fn(async () => undefined) const prepareGatewayForAgent = vi.fn(async (_connectionId: null | string, _profile: string) => activateGateway) const prepareGatewayForProfile = vi.fn(async (_profile: string) => activateGateway) diff --git a/apps/desktop/src/store/profile.ts b/apps/desktop/src/store/profile.ts index 72af3f44d607..cbaa1df7ab8f 100644 --- a/apps/desktop/src/store/profile.ts +++ b/apps/desktop/src/store/profile.ts @@ -333,7 +333,17 @@ export async function ensureGatewayProfile(profile: string | null | undefined): // .set() calls that each drain their listeners synchronously, and a // $gateway listener runs while the other two still name the old backend. batch(() => { - activate() + // A rejected activation publishes NOTHING, exactly like the agent path. + // applyActive() returns false when its captured epoch was superseded -- + // a newer switch (or a teardown) landed while this one was awaiting its + // route or socket. Publishing the companions anyway would leave the + // CURRENT gateway paired with the stale profile pointer and descriptor, + // and batch() cannot rescue that: it would make the mismatched tuple + // atomically observable rather than prevent it. + if (!activate()) { + return + } + $activeGatewayProfile.set(target) if (connection) { From 22d27d8fa325a8f96918e1ecd1be9b6a4d979c03 Mon Sep 17 00:00:00 2001 From: Jack Lau <72348727+jackulau@users.noreply.github.com> Date: Sun, 16 Aug 2026 21:49:41 -0500 Subject: [PATCH 25/25] docs(desktop): state one fallthrough contract at the agent seam The comment above ensureGatewayAgent carried both the old and the new contract on consecutive lines: "a local/null connectionId falls through to the profile path verbatim", immediately contradicted by "only a null connectionId falls through, explicit local is a registry identity". Dropped the stale line. Same wording above prepareGatewayForAgent in gateway.ts, tightened to match what the code actually does: registryBackendScopeKey only collapses to the bare profile key for a null or empty id, so an explicit local id scopes to conn:local:: and stays on the registry route. Comments only, no behavior change. tsc --noEmit, eslint and the three affected suites (43 passed) re-verified. Refs #82140 --- apps/desktop/src/store/gateway.ts | 6 ++++-- apps/desktop/src/store/profile.ts | 1 - 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/apps/desktop/src/store/gateway.ts b/apps/desktop/src/store/gateway.ts index 841e5fa0e55b..c907ca7b4415 100644 --- a/apps/desktop/src/store/gateway.ts +++ b/apps/desktop/src/store/gateway.ts @@ -608,9 +608,11 @@ export async function openGatewayForAgent(connectionId: null | string, profile: // The agent-scoped analogue of prepareGatewayForProfile, and the same // publication seam: dial the agent's socket without publishing anything, and -// hand back the synchronous activation thunk. A local/null connection falls +// hand back the synchronous activation thunk. A null connection id falls // through to the profile seam, so both doors into an activation share one -// atomicity contract instead of drifting apart. +// atomicity contract instead of drifting apart; an explicit `local` id is a +// registry identity (`registryBackendScopeKey` keeps its own scope for it) and +// stays on the registry route. // // The thunk reports whether it actually published, preserving the `activated` // contract callers rely on: a source edit/remove can dispose this entry while diff --git a/apps/desktop/src/store/profile.ts b/apps/desktop/src/store/profile.ts index cbaa1df7ab8f..60bd3d44841c 100644 --- a/apps/desktop/src/store/profile.ts +++ b/apps/desktop/src/store/profile.ts @@ -402,7 +402,6 @@ async function resolveConnectionForActiveAgent( // ONE synchronous frame, via the same prepare/publish seam the profile path // uses, so no subscriber sees the new backend paired with the old // descriptor. -// A local/null connectionId falls through to the profile path verbatim. // Only a null connectionId falls through to the legacy profile path. Explicit // `local` is a registry identity and must use the genuinely-local route. export async function ensureGatewayAgent(connectionId: null | string, profile: string): Promise {