diff --git a/agent/agent_init.py b/agent/agent_init.py index bbd6421b1f..c47ccbf244 100644 --- a/agent/agent_init.py +++ b/agent/agent_init.py @@ -873,8 +873,8 @@ def _routed_client_kwargs(agent, fallback_model, _provider_timeout) -> Optional[ from hermes_constants import profile_cli_selector _sel = profile_cli_selector() raise RuntimeError( - f"No LLM provider configured. Run `hermes {_sel}model` to " - f"select a provider, or run `hermes {_sel}setup` for first-time " + "No LLM provider configured. Run `hermes model` to " + "select a provider, or run `hermes setup` for first-time " "configuration." ) diff --git a/agent/anthropic_credentials.py b/agent/anthropic_credentials.py index ed9f45f5ac..1084f7893f 100644 --- a/agent/anthropic_credentials.py +++ b/agent/anthropic_credentials.py @@ -733,6 +733,17 @@ def _get_hermes_oauth_file() -> Path: return get_hermes_home() / ".anthropic_oauth.json" +def _root_hermes_oauth_file() -> Optional[Path]: + """Global-root ``.anthropic_oauth.json`` inside a named profile (None in classic mode); used to commit a + rotation of a grant the profile borrowed via the pool's root fallback.""" + try: + from hermes_constants import get_default_hermes_root + root = get_default_hermes_root() + return None if root.resolve(strict=False) == get_hermes_home().resolve(strict=False) else root / ".anthropic_oauth.json" + except Exception: + return None + + def _generate_pkce() -> tuple: """Generate PKCE code_verifier and code_challenge (S256).""" verifier = base64.urlsafe_b64encode(secrets.token_bytes(32)).rstrip(b"=").decode() @@ -802,13 +813,14 @@ def read_hermes_oauth_credentials() -> Optional[Dict[str, Any]]: def _write_hermes_oauth_credentials( - access_token: str, refresh_token: Optional[str], expires_at_ms: Optional[int], + access_token: str, refresh_token: Optional[str], expires_at_ms: Optional[int], *, target: Optional[Path] = None ) -> None: - """Commit refreshed hermes_pkce tokens to ``/.anthropic_oauth.json`` (``CredentialPersistError`` - on failure); without it the next ``load_pool()`` re-seeds the stale (consumed) pair from the file over the - rotated pool entry.""" + """Commit refreshed hermes_pkce tokens to ~/.hermes/.anthropic_oauth.json (``CredentialPersistError`` on failure). + ``target`` lets a named profile commit a grant it BORROWED from the global root back to the ROOT singleton + instead of forking a copy under its own HERMES_HOME; without this write-through the next ``load_pool()`` + re-seeds the stale (consumed) pair from the file over the rotated pool entry.""" _commit_private_json( - _get_hermes_oauth_file(), + target if target is not None else _get_hermes_oauth_file(), {"accessToken": access_token, "refreshToken": refresh_token, "expiresAt": expires_at_ms}, "Hermes OAuth credentials", ) diff --git a/agent/credential_pool.py b/agent/credential_pool.py index 533435533a..1d345f8b73 100644 --- a/agent/credential_pool.py +++ b/agent/credential_pool.py @@ -36,10 +36,13 @@ from hermes_cli.auth import ( _auth_store_lock, _codex_access_token_is_expiring, _decode_jwt_claims, + _global_auth_file_path, _load_auth_store, _load_provider_state, + _load_provider_state_with_source, _resolve_kimi_base_url, _resolve_zai_base_url, + _same_path, _save_auth_store, _save_provider_state, _store_provider_state, @@ -756,6 +759,181 @@ def resolve_runtime_pool_key(provider: Optional[str], base_url: Optional[str]) - DEFAULT_MAX_CONCURRENT_PER_CREDENTIAL = 1 +# --- Multi-profile root write-through --- + + +def _guarded_global_root(global_path: Optional[Path]) -> Optional[Path]: + """Apply the pytest seat belt to a resolved global-root auth.json path. + + ``None`` means classic mode (profile == root) or "refuse": under pytest, + never write the real user's ``~/.hermes/auth.json`` even when HERMES_HOME + points at a profile path (mirrors the read-side guard in + ``_load_global_auth_store``). Uses the unmodified HOME env, not + ``Path.home()`` which fixtures may monkeypatch. + """ + if global_path is None: + return None + if os.environ.get("PYTEST_CURRENT_TEST"): + real_home_env = os.environ.get("HOME", "") + if real_home_env: + real_root = Path(real_home_env) / ".hermes" / "auth.json" + try: + # Comparing the guard path must not probe the real auth store. + if os.path.normcase(os.path.abspath(global_path)) == os.path.normcase(os.path.abspath(real_root)): + return None + except Exception: + return None + return global_path + + +def _write_through_provider_state_to_global_root( + provider_id: str, state: Dict[str, Any] +) -> None: + """Persist a rotated OAuth ``state`` into the global-root auth.json. + + Best-effort write-through for the multi-profile rotation hazard: nous, + openai-codex, and xai-oauth rotate the refresh_token on refresh, so when + a profile pool refresh rotates a grant it resolved from the root fallback, + the rotated chain must land back in root. Otherwise root keeps a revoked + refresh token and every other profile dies with ``refresh_token_reused`` + / ``invalid_grant`` once its access token expires. + + Only updates ``providers.`` in the root store; never touches + the profile store (the caller already saved that). Swallows all errors — + a failed write-through degrades to root-stale and must never break the + profile's own successful save. Mirrors + ``hermes_cli.auth._write_through_xai_oauth_to_global_root``. + + See #48415. + """ + try: + global_path = _guarded_global_root(auth_mod._global_auth_file_path()) + except Exception: + return + if global_path is None: + return + try: + auth_mod._persist_provider_state_to_store(provider_id, state, global_path, set_active=False) + except Exception as exc: # pragma: no cover - best effort + logger.debug("%s pool refresh: write-through to global root failed: %s", provider_id, exc) + + +def _singleton_target_for_entry(pool: "CredentialPool", entry: "PooledCredential") -> Optional[Path]: + """Root ``.anthropic_oauth.json`` when *entry* is a borrowed hermes_pkce row, else None.""" + if entry.source != "hermes_pkce" or entry.id not in getattr(pool, "_borrowed_root_ids", ()): + return None + try: + from agent.anthropic_credentials import _root_hermes_oauth_file + return _root_hermes_oauth_file() + except Exception: + return None + + +def _profile_owns_pool_provider(provider: str) -> bool: + """True when the ACTIVE auth.json has its own rows for *provider*. + + Named profiles with no local rows read the provider through the + ``read_credential_pool`` global-root fallback ("borrowing"). + """ + try: + pool = _load_auth_store().get("credential_pool") + except Exception: + return True # unreadable store: assume ownership, keep legacy path + entries = pool.get(provider) if isinstance(pool, dict) else None + return isinstance(entries, list) and bool(entries) + + +def _borrowed_single_use_pool_root() -> Optional[Path]: + """Global-root auth.json when persisting a BORROWED single-use pool, else None. + + ``None`` means "persist to the active store as usual": classic mode + (profile == root), or the profile owns its own rows for this provider. + """ + try: + return _guarded_global_root(_global_auth_file_path()) + except Exception: + return None + + +def _update_root_pool_rows( + provider: str, payloads: List[Dict[str, Any]], global_path: Path, + *, status_cleared_ids: Optional[Iterable[str]] = None, +) -> None: + """UPDATE-ONLY merge of *payloads* into the root store's rows for *provider*. + + A borrower may refresh the root's rows (rotation, cooldown state) but + never add or delete them — the root owns their lifecycle. In particular a + profile's singleton-prune (it has no ``.anthropic_oauth.json`` of its own) + must not delete the root grant, so ``removed_ids`` is ignored by callers. + """ + with _auth_store_lock(target_path=global_path): + store = _load_auth_store(global_path) + pool = store.get("credential_pool") + if not isinstance(pool, dict): + pool = {} + store["credential_pool"] = pool + existing = pool.get(provider) + existing_list = existing if isinstance(existing, list) else [] + incoming_by_id = {p.get("id"): p for p in payloads if isinstance(p, dict) and p.get("id")} + cleared = {cid for cid in (status_cleared_ids or ()) if cid} + merged: List[Dict[str, Any]] = [] + changed = False + for disk_entry in existing_list: + did = disk_entry.get("id") if isinstance(disk_entry, dict) else None + incoming = incoming_by_id.get(did) if did else None + if incoming is None: + merged.append(disk_entry) + continue + # A deliberately cleared entry has no disk cooldown worth keeping. + updated = auth_mod._merge_disk_cooldown_state( + incoming, None if did in cleared else disk_entry, provider, + ) + if updated != disk_entry: + changed = True + merged.append(updated) + if changed: + pool[provider] = merged + _save_auth_store(store, target_path=global_path) + + +def persist_pool_entries( + provider: str, + payloads: List[Dict[str, Any]], + *, + removed_ids: Optional[Iterable[str]] = None, + status_cleared_ids: Optional[Iterable[str]] = None, +) -> None: + """Persist a provider's pool rows to the store that OWNS them. + + A named profile that sees a single-use-refresh provider (Anthropic, + Codex, xAI OAuth) only through the global-root fallback must not + materialize a local ``credential_pool.`` copy: that copy forks + the single-use refresh token, the first profile to rotate commits the new + pair only to its own file, and root plus every sibling die with + ``invalid_grant`` (#100339). Such rows are written back to the root store + (under the root lock); everything else goes to the active store. + """ + if provider in SINGLE_USE_REFRESH_POOL_PROVIDERS and not _profile_owns_pool_provider(provider): + global_path = _borrowed_single_use_pool_root() + if global_path is not None: + try: + _update_root_pool_rows( + provider, payloads, global_path, + status_cleared_ids=status_cleared_ids, + ) + except Exception as exc: + # Fail closed on the FORK, not on the save: never fall back to + # writing a local copy (that IS the bug). The in-memory pool + # still holds the rotated pair for this process. + logger.warning( + "%s pool: write-through of borrowed root grant failed (%s); " + "not materializing a profile-local copy", + provider, exc, + ) + return + write_credential_pool( + provider, payloads, removed_ids=removed_ids, status_cleared_ids=status_cleared_ids, + ) # --- Per-provider singleton refresh plumbing ------------------------------- @@ -807,6 +985,9 @@ class CredentialPool(CredentialPoolAdminMixin, CredentialPoolModelCooldownMixin) self.provider = provider self._entries = sorted(entries, key=lambda entry: entry.priority) self._current_id: Optional[str] = None + # Ids of rows read via the global-root fallback (single-use OAuth + # providers only); set by load_pool(), consumed by add_entry(). + self._borrowed_root_ids: Set[str] = set() self._strategy = get_pool_strategy(provider) # RLock: _replace_entry/_persist self-acquire it so the DEFERRED # single-use-token refresh path (network I/O outside the lock by @@ -935,7 +1116,7 @@ class CredentialPool(CredentialPoolAdminMixin, CredentialPoolModelCooldownMixin) ) -> None: # Self-locking: snapshotting self._entries must not race a rotation. with self._lock: - write_credential_pool( + persist_pool_entries( self.provider, [entry.to_dict() for entry in self._entries], removed_ids=removed_ids, @@ -1229,6 +1410,14 @@ class CredentialPool(CredentialPoolAdminMixin, CredentialPoolModelCooldownMixin) ``set_active=False`` everywhere: a sync-back is a token-rotation side effect, not the user choosing a provider; ``_save_provider_state`` would flip ``active_provider`` to whichever provider refreshed last. + + #74339: decide the root write-through on WHERE the state resolved + from (``_load_provider_state_with_source``), not on whether the + profile has a ``providers.`` key — ``_store_provider_state`` + creates that key unconditionally, which self-sealed the check after + the first refresh. When the grant came from the global root, write + back to root ONLY and skip the profile store so it never accrues a + shadowing key that blocks both the fallback and the write-through. """ # Only singleton-seeded entries sync back; ``manual:*`` entries are # independent credentials and must not write to the singleton. @@ -1237,13 +1426,20 @@ class CredentialPool(CredentialPoolAdminMixin, CredentialPoolModelCooldownMixin) try: with _auth_store_lock(): auth_store = _load_auth_store() - state = _load_provider_state(auth_store, self.provider) + state, source_path = _load_provider_state_with_source(auth_store, self.provider) if not isinstance(state, dict): return + global_root = _global_auth_file_path() + is_from_root = bool( + source_path is not None and global_root is not None and _same_path(source_path, global_root) + ) if not self._apply_entry_to_singleton_state(entry, state): return - _store_provider_state(auth_store, self.provider, state, set_active=False) - _save_auth_store(auth_store) + if is_from_root: + _write_through_provider_state_to_global_root(self.provider, state) + else: + _store_provider_state(auth_store, self.provider, state, set_active=False) + _save_auth_store(auth_store) except Exception as exc: logger.debug("Failed to sync %s pool entry back to auth store: %s", self.provider, exc) @@ -1394,10 +1590,11 @@ class CredentialPool(CredentialPoolAdminMixin, CredentialPoolModelCooldownMixin) """Write a rotated Anthropic pair to its authoritative singleton, or fail closed. claude_code -> ~/.claude/.credentials.json (so the fallback resolver - and other profiles see it). hermes_pkce -> /.anthropic_oauth.json - (``_seed_from_singletons`` re-seeds it every load). Not ``endswith``: - manual:hermes_pkce is pool-owned and a singleton for it would be a second - authority for the same refresh-token family. + and other profiles see it). hermes_pkce -> ~/.hermes/.anthropic_oauth.json + (``_seed_from_singletons`` re-seeds it every load; a borrowed row commits + to the ROOT's file, never a new profile-local copy, #100339). Not + ``endswith``: manual:hermes_pkce is pool-owned and a singleton for it + would be a second authority for the same refresh-token family. """ if entry.source == "claude_code": store = "~/.claude/.credentials.json" @@ -1411,7 +1608,7 @@ class CredentialPool(CredentialPoolAdminMixin, CredentialPoolModelCooldownMixin) if entry.source == "claude_code": ac._write_claude_code_credentials(*args, spent_refresh_token=entry.refresh_token or "") else: - ac._write_hermes_oauth_credentials(*args) + ac._write_hermes_oauth_credentials(*args, target=_singleton_target_for_entry(self, entry)) except Exception as wexc: # Authoritative commit failed: do not mark, persist or return the # rotation as successful, and bypass the re-POST recovery path — @@ -2788,6 +2985,10 @@ def _seed_custom_pool(pool_key: str, entries: List[PooledCredential]) -> Tuple[b def load_pool(provider: str) -> CredentialPool: provider = (provider or "").strip().lower() + if provider in SINGLE_USE_REFRESH_POOL_PROVIDERS: + # One-time heal for installs that forked this grant across profiles + # before the clone-strip / root write-through existed (#100339). + auth_mod.heal_forked_single_use_oauth_grants(provider) raw_entries = read_credential_pool(provider) disk_ids = {e.get("id") for e in raw_entries if isinstance(e, dict) and e.get("id")} changed = any( @@ -2802,7 +3003,13 @@ def load_pool(provider: str) -> CredentialPool: ) != payload.get("auth_type", AUTH_TYPE_API_KEY) for payload in raw_entries ) - changed |= raw_needs_auth_normalization + if raw_needs_auth_normalization: + # A profile may be reading this provider from the global-root fallback. + # Keep that fallback read-only: only the owning store may rewrite these + # rows; loading the default/root profile heals global rows. + active_pool = _load_auth_store().get("credential_pool") + active_entries = active_pool.get(provider) if isinstance(active_pool, dict) else None + changed |= bool(active_entries) if provider.startswith(CUSTOM_POOL_PREFIX): custom_changed, custom_sources = _seed_custom_pool(provider, entries) @@ -2814,16 +3021,38 @@ def load_pool(provider: str) -> CredentialPool: changed |= singleton_changed or env_changed # ``load_pool()`` is a non-destructive read for env-seeded entries # (#9331); file-backed singletons still prune when their file is gone. - changed |= _prune_stale_seeded_entries( - entries, singleton_sources | env_sources, prune_env_sources=False, + borrowing_root_grant = ( + provider in SINGLE_USE_REFRESH_POOL_PROVIDERS + and bool(disk_ids) + and not _profile_owns_pool_provider(provider) ) + if borrowing_root_grant: + # Rows read through the global-root fallback are seeded from the + # ROOT's singleton files, which this profile cannot see; pruning + # them would hide (and, via write-through, delete) the shared + # grant. The root's own load_pool() prunes. + borrowed = [e for e in entries if e.id in disk_ids] + others = [e for e in entries if e.id not in disk_ids] + changed |= _prune_stale_seeded_entries( + others, singleton_sources | env_sources, prune_env_sources=False, + ) + entries[:] = borrowed + others + else: + changed |= _prune_stale_seeded_entries( + entries, singleton_sources | env_sources, prune_env_sources=False, + ) changed |= _normalize_pool_priorities(provider, entries) if changed: new_ids = {entry.id for entry in entries} - write_credential_pool( + persist_pool_entries( provider, [entry.to_dict() for entry in sorted(entries, key=lambda item: item.priority)], removed_ids=disk_ids - new_ids, ) - return CredentialPool(provider, entries) + pool = CredentialPool(provider, entries) + # Remember the root's borrowed rows so a later ``add_entry`` in this + # profile leaves them out of the profile's own store (#100339). + if provider in SINGLE_USE_REFRESH_POOL_PROVIDERS and not _profile_owns_pool_provider(provider): + pool._borrowed_root_ids = set(disk_ids) + return pool diff --git a/agent/credential_pool_admin.py b/agent/credential_pool_admin.py index 515a2f09b9..f5b89f7e08 100644 --- a/agent/credential_pool_admin.py +++ b/agent/credential_pool_admin.py @@ -54,12 +54,18 @@ class CredentialPoolAdminMixin: return len(stale) def remove_index(self, index: int) -> Optional[PooledCredential]: + from agent.credential_pool import persist_pool_entries + with self._lock: if index < 1 or index > len(self._entries): return None removed = self._entries.pop(index - 1) self._entries = [replace(e, priority=p) for p, e in enumerate(self._entries)] - self._persist(removed_ids=[removed.id]) + persist_pool_entries( + self.provider, + [entry.to_dict() for entry in self._entries], + removed_ids=[removed.id], + ) if self._current_id == removed.id: self._current_id = None return removed @@ -108,10 +114,21 @@ class CredentialPoolAdminMixin: return None, None, f'No credential matching "{raw}".' def add_entry(self, entry: PooledCredential) -> PooledCredential: - from agent.credential_pool import _next_priority + from agent.credential_pool import _next_priority, write_credential_pool with self._lock: entry = replace(entry, priority=_next_priority(self._entries)) self._entries.append(entry) - self._persist() + borrowed_ids = getattr(self, "_borrowed_root_ids", None) + if borrowed_ids: + # ``hermes -p auth add ``: the + # profile claims its OWN credential. Persist only profile-owned + # rows — copying the borrowed root grant alongside would fork + # its single-use refresh token (#100339). Once the profile owns + # rows, the root fallback for this provider is shadowed. + self._entries = [e for e in self._entries if e.id not in borrowed_ids] + write_credential_pool(self.provider, [e.to_dict() for e in self._entries]) + self._borrowed_root_ids = set() + else: + self._persist() return entry diff --git a/apps/desktop/electron/connection-config.test.ts b/apps/desktop/electron/connection-config.test.ts index 76fc83f5b1..2cdae3d67f 100644 --- a/apps/desktop/electron/connection-config.test.ts +++ b/apps/desktop/electron/connection-config.test.ts @@ -376,7 +376,39 @@ const ROUTES = [ expected: { backend: 'pool', descriptorProfile: null, scopePath: false } }, { - name: 'an unscoped local profile request keeps its pooled backend', + // THE INVARIANT this collapse must not eat: a route the server cannot + // profile-scope has only the backend PROCESS's HERMES_HOME left as a + // scope, so it keeps a pooled backend. /api/files/upload acts on host + // paths and takes no `profile` even after #118275. + name: 'a mutating local request the server cannot scope keeps its pooled backend', + profile: 'coder', + opts: { + primaryProfile: 'default', + globalRemote: false, + profileRemoteOverride: false, + requestMethod: 'POST', + requestPath: '/api/files/upload' + }, + expected: { backend: 'pool', descriptorProfile: null, scopePath: false } + }, + { + // Same unscopable route, safe method: a read cannot corrupt the wrong + // home, and holding reads back would spawn a backend per profile again. + name: 'a read on an unscopable route still shares the host backend', + profile: 'coder', + opts: { + primaryProfile: 'default', + globalRemote: false, + profileRemoteOverride: false, + requestMethod: 'GET', + requestPath: '/api/files/upload' + }, + expected: { backend: 'primary', descriptorProfile: 'coder', scopePath: true } + }, + { + // #118275 taught this handler `?profile=`, so the server CAN vouch for the + // scope and the same destructive call rides the shared host backend. + name: 'a destructive local request the server can scope shares the host backend', profile: 'coder', opts: { primaryProfile: 'default', @@ -385,7 +417,7 @@ const ROUTES = [ requestMethod: 'POST', requestPath: '/api/memory/reset' }, - expected: { backend: 'pool', descriptorProfile: null, scopePath: false } + expected: { backend: 'primary', descriptorProfile: 'coder', scopePath: true } }, { name: 'a remote sub-profile without a local entry routes through the primary remote gateway', @@ -436,7 +468,10 @@ const ROUTES = [ expected: { backend: 'primary', descriptorProfile: 'coder', scopePath: true } }, { - name: 'a local session write keeps its pooled backend', + // Scoped by `body.profile` (rename_session_endpoint -> `_with_db`), not by + // the query: shares the host backend with its path left alone. Appending + // `?profile=` here would advertise a scope the handler ignores. + name: 'a local session write shares the host backend, scoped by its body', profile: 'coder', opts: { primaryProfile: 'default', @@ -445,7 +480,7 @@ const ROUTES = [ requestMethod: 'PATCH', requestPath: '/api/sessions/session-1' }, - expected: { backend: 'pool', descriptorProfile: null, scopePath: false } + expected: { backend: 'primary', descriptorProfile: null, scopePath: false } }, { name: 'a profile-management request uses the primary without a query scope', @@ -472,6 +507,17 @@ const ROUTES = [ requestPath: '/api/config' }, expected: { backend: 'pool', descriptorProfile: null, scopePath: false } + }, + { + name: 'HERMES_DESKTOP_ISOLATED_BACKEND keeps a local profile on its own pooled backend', + profile: 'coder', + opts: { + primaryProfile: 'default', + globalRemote: false, + profileRemoteOverride: false, + isolatedBackend: true + }, + expected: { backend: 'pool', descriptorProfile: null, scopePath: false } } ] @@ -599,13 +645,15 @@ test('pathWithGlobalRemoteProfile does not replace an explicit profile query', ( ) }) -test('pathWithGlobalRemoteProfile skips local and per-profile remote override paths', () => { +test('pathWithGlobalRemoteProfile scopes a shared-host local path and skips per-profile remote overrides', () => { + // Multiplex-only: the local profile now shares the host backend, so its + // path must name the profile or the request reads the launch home. assert.equal( pathWithGlobalRemoteProfile('/api/model/info', 'iris', { globalRemote: false, profileRemoteOverride: false }), - '/api/model/info' + '/api/model/info?profile=iris' ) assert.equal( pathWithGlobalRemoteProfile('/api/model/info', 'iris', { @@ -684,7 +732,7 @@ test('translateSelfProfileQuery no-ops when alias and backend profile agree or a assert.equal(translateSelfProfileQuery('/api/cron/jobs?profile=mara', '', 'default'), '/api/cron/jobs?profile=mara') }) -test('pathWithGlobalRemoteProfile appends local-primary profile scope only for eligible routes', () => { +test('pathWithGlobalRemoteProfile appends the profile scope on the shared host backend', () => { assert.equal( pathWithGlobalRemoteProfile('/api/config', 'iris', { globalRemote: false, @@ -701,7 +749,28 @@ test('pathWithGlobalRemoteProfile appends local-primary profile scope only for e requestMethod: 'POST', requestPath: '/api/memory/reset' }), - '/api/memory/reset' + '/api/memory/reset?profile=iris' + ) + // Still ineligible: the managed-files routes act on host paths, not a profile home. + assert.equal( + pathWithGlobalRemoteProfile('/api/files/upload', 'iris', { + globalRemote: false, + profileRemoteOverride: false, + requestMethod: 'POST', + requestPath: '/api/files/upload' + }), + '/api/files/upload' + ) + // The profile-management family names its target in the path and must not + // be self-scoped. + assert.equal( + pathWithGlobalRemoteProfile('/api/profiles/worker', 'iris', { + globalRemote: false, + profileRemoteOverride: false, + requestMethod: 'DELETE', + requestPath: '/api/profiles/worker' + }), + '/api/profiles/worker' ) }) @@ -752,12 +821,19 @@ test('resolveProfileApiRequest scopes read-only session probes without spawning ) }) -test('resolveProfileApiRequest keeps unscoped destructive routes on the profile backend', () => { +test('resolveProfileApiRequest scopes destructive profile-owned routes to the shared backend', () => { + // These handlers used to read the process home directly, so they had to ride a + // per-profile backend. No per-profile backend exists any more, and they now take + // `?profile=` and refuse an unnamed profile while several are served, so the + // query param is what reaches the right home. for (const [method, path] of [ ['POST', '/api/memory/reset'], ['POST', '/api/curator/run'], ['PUT', '/api/curator/paused'], - ['POST', '/api/webhooks'] + ['POST', '/api/webhooks'], + ['DELETE', '/api/webhooks/alerts'], + ['DELETE', '/api/ops/hooks'], + ['POST', '/api/ops/checkpoints/prune'] ]) { assert.deepEqual( resolveProfileApiRequest('iris', path, { @@ -765,11 +841,62 @@ test('resolveProfileApiRequest keeps unscoped destructive routes on the profile profileRemoteOverride: false, requestMethod: method }), - { backendProfile: 'iris', requestPath: path } + { backendProfile: null, requestPath: `${path}?profile=iris` } ) } }) +test('resolveProfileApiRequest keeps an unscopable mutating route on a process-scoped backend', () => { + // The load-bearing half of the collapse: a route the server cannot scope has + // nothing left but the backend process's own HERMES_HOME, so it must NOT fall + // through to the shared primary. Live proof of the failure mode this pins: + // `POST /api/memory/reset?profile=beta` on an unfixed server deleted ALPHA's + // MEMORY.md and returned ok:true. + for (const [method, path] of [ + ['POST', '/api/files/upload'], + ['DELETE', '/api/files/managed'], + // A hypothetical future route: the gate is derived from + // localPrimaryRequestScope(), not from a hardcoded list, so an endpoint + // nobody has taught `profile` is held back the day it is added. + ['POST', '/api/not-a-real-route/destroy'] + ]) { + assert.deepEqual( + resolveProfileApiRequest('iris', path, { + globalRemote: false, + profileRemoteOverride: false, + requestMethod: method + }), + { backendProfile: 'iris', requestPath: path }, + `${method} ${path} must keep its own backend` + ) + } + + // ...and the gate is about SCOPE, not about the word "destructive": the same + // unscopable paths read fine on the shared backend. + assert.deepEqual( + resolveProfileApiRequest('iris', '/api/files/managed', { + globalRemote: false, + profileRemoteOverride: false, + requestMethod: 'GET' + }), + { backendProfile: null, requestPath: '/api/files/managed?profile=iris' } + ) +}) + +test('resolveProfileApiRequest leaves a body-scoped session write unqueried on the shared backend', () => { + // PATCH /api/sessions/{id} reads its target DB from `body.profile`; the query + // is ignored. apps/desktop/src/api/sessions.ts always names the owner in the + // body (sessionWriteProfile), so this rides the shared host backend. + assert.deepEqual( + resolveProfileApiRequest('iris', '/api/sessions/session-1', { + globalRemote: false, + profileRemoteOverride: false, + requestMethod: 'PATCH' + }), + { backendProfile: null, requestPath: '/api/sessions/session-1' } + ) +}) + test('resolveProfileApiRequest uses exact method and path eligibility for mixed families', () => { assert.deepEqual( resolveProfileApiRequest('iris', '/api/skills', { @@ -777,6 +904,8 @@ test('resolveProfileApiRequest uses exact method and path eligibility for mixed }), { backendProfile: null, requestPath: '/api/skills?profile=iris' } ) + // Only `GET /api/skills` is eligible: the exact method matters, and an + // unlisted mutating method keeps its own process-scoped backend. assert.deepEqual( resolveProfileApiRequest('iris', '/api/skills', { requestMethod: 'POST' @@ -787,15 +916,15 @@ test('resolveProfileApiRequest uses exact method and path eligibility for mixed resolveProfileApiRequest('iris', '/api/config/defaults', { requestMethod: 'GET' }), - { backendProfile: 'iris', requestPath: '/api/config/defaults' } + { backendProfile: null, requestPath: '/api/config/defaults?profile=iris' } ) assert.deepEqual( resolveProfileApiRequest('iris', '/api/model/recommended-default?provider=nous', { requestMethod: 'GET' }), { - backendProfile: 'iris', - requestPath: '/api/model/recommended-default?provider=nous' + backendProfile: null, + requestPath: '/api/model/recommended-default?provider=nous&profile=iris' } ) }) diff --git a/apps/desktop/electron/connection-config.ts b/apps/desktop/electron/connection-config.ts index 735694264b..702088c96f 100644 --- a/apps/desktop/electron/connection-config.ts +++ b/apps/desktop/electron/connection-config.ts @@ -35,6 +35,7 @@ // AT cookie. A liveness check that looked only at the AT cookie would // force a needless full re-login every ~15 min — hence cookiesHaveLiveSession. import { readStatusCode } from './api-transport' +import { sharesHostBackend } from './host-backend-singleton' const AT_COOKIE_VARIANTS = ['__Host-hermes_session_at', '__Secure-hermes_session_at', 'hermes_session_at'] const RT_COOKIE_VARIANTS = ['__Host-hermes_session_rt', '__Secure-hermes_session_rt', 'hermes_session_rt'] @@ -561,6 +562,8 @@ export interface ProfileRouteOptions { primaryRemoteActive?: boolean /** A stored per-profile entry exists for this profile (local or remote). */ ownEntry?: boolean + /** `HERMES_DESKTOP_ISOLATED_BACKEND=1`: opt out of the host singleton. */ + isolatedBackend?: boolean requestMethod?: null | string requestPath?: null | string } @@ -616,7 +619,27 @@ const LOCAL_PRIMARY_SCOPED_ROUTES = new Set([ // backend's own shutdown, which SIGTERMs its gateway-restart child. 'POST /api/gateway/restart', 'POST /api/gateway/start', - 'POST /api/gateway/stop' + 'POST /api/gateway/stop', + // Profile-owned state that used to ride a per-profile backend: with one backend + // per host these handlers take `?profile=` and resolve the home per request. + // Destructive ones (memory reset, curator run, hook delete, checkpoint prune, + // import) REFUSE an unnamed profile while several are served, so the query is + // not optional here. + 'GET /api/memory', + 'PUT /api/memory/provider', + 'POST /api/memory/reset', + 'GET /api/curator', + 'PUT /api/curator/paused', + 'POST /api/curator/run', + 'GET /api/logs', + 'GET /api/portal', + 'GET /api/hermes/update/check', + 'POST /api/local-models/activate', + 'GET /api/dashboard/themes', + 'PUT /api/dashboard/theme', + 'GET /api/dashboard/font', + 'PUT /api/dashboard/font', + 'GET /api/dashboard/plugins' ]) function localPrimaryRequestScope(opts: ProfileRouteOptions): boolean | null { @@ -669,9 +692,61 @@ function localPrimaryRequestScope(opts: ProfileRouteOptions): boolean | null { return false } + // Whole families whose every handler now takes `?profile=` and resolves the + // profile's home per request: webhook subscriptions (`{name}` in the path) and + // the /api/ops maintenance routes (doctor, backup/import, hooks, checkpoints, + // diagnostics). Their action spawns pass `-p ` to the child, and the + // /api/actions poll family above already pins to this same backend. + if (pathname === '/api/webhooks' || pathname.startsWith('/api/webhooks/')) { + return true + } + + if (pathname.startsWith('/api/ops/')) { + return true + } + + // Session WRITES are scoped by `body.profile` (`rename_session_endpoint` -> + // `_with_db(body.profile, ...)`), not by the query. They ARE scopable — just + // not through the URL — so they belong on the shared backend with the path + // left alone; `apps/desktop/src/api/sessions.ts` always names the owner in + // the body. Returning `true` here would append a `?profile=` the handler + // ignores and advertise a scope that is not doing the work. + if (method !== 'GET' && (pathname === '/api/sessions' || pathname.startsWith('/api/sessions/'))) { + return false + } + return null } +const SAFE_REQUEST_METHODS = new Set(['GET', 'HEAD', 'OPTIONS']) + +/** + * True when this is a REST request that CHANGES something and the server cannot + * vouch for its profile scope (`localPrimaryRequestScope` → null: no + * `?profile=`, no `body.profile`, no target named in the path). + * + * Such a route has exactly one scope left — the backend process's own + * `HERMES_HOME` — so it keeps a pooled, profile-scoped backend even though every + * other local request now shares the host one. Mechanical on purpose: the day a + * handler learns to read `profile` it joins `LOCAL_PRIMARY_SCOPED_ROUTES` (or a + * family above), `localPrimaryRequestScope` stops returning null, and this + * predicate stops seeing it — there is no second list to keep in sync. + * + * A call with no `requestPath` is a BACKEND/descriptor resolution (WebSocket + * dial, pool bookkeeping), not a REST call, and is never held back. + */ +export function unscopableMutatingRequest(opts: ProfileRouteOptions = {}): boolean { + if (!String(opts.requestPath || '')) { + return false + } + + if (SAFE_REQUEST_METHODS.has(String(opts.requestMethod || 'GET').toUpperCase())) { + return false + } + + return localPrimaryRequestScope(opts) === null +} + /** * The one place that answers "which backend serves profile P, and does its * REST path need a profile scope?". Six routes, in precedence order: @@ -684,11 +759,16 @@ function localPrimaryRequestScope(opts: ProfileRouteOptions): boolean | null { * 3. A profile inheriting the app-global remote shares the primary backend — * one host serves every profile — so it is scoped per request instead. * 4. An unknown profile under a remote primary shares that remote backend. - * A stored local profile remains isolated in its own backend. - * 5. A local profile REST request that the primary backend can safely scope - * reuses that backend, with `?profile=` when the handler accepts it. - * 6. Any other local profile gets its own pooled backend, spawned with - * `--profile`, so its `HERMES_HOME` scopes it. + * A stored local profile keeps its own pooled backend instead. + * 5. A local profile REST request the primary backend can scope reuses that + * backend, with `?profile=` when the handler reads the query (handlers that + * name their target in the path or `body.profile` get no query). + * 6. Every other LOCAL profile also shares the one host backend + * (multiplex-only: one `hermes serve` per HOST). The two ways out are + * `HERMES_DESKTOP_ISOLATED_BACKEND=1`, which gives this app a private + * backend, and a MUTATING request the server cannot scope at all — that + * one keeps a pooled backend whose HERMES_HOME does the scoping, so a + * destructive call can never fall through to the primary's home. * * Routing used to be spread across three overlapping predicates that each * re-derived part of this table, which is how case 3 ended up registering @@ -744,6 +824,15 @@ function resolveProfileBackendRoute(profile, opts: ProfileRouteOptions = {}): Pr } } + // 6. Multiplex-only: every other LOCAL profile shares the one host backend + // too, carrying `?profile=` / the `profile` RPC param instead of getting + // a `hermes serve` child of its own — UNLESS this request mutates state + // the server cannot scope, in which case the pooled backend's own + // HERMES_HOME is the only scope left and it keeps one. + if (sharesHostBackend({ isolated: opts.isolatedBackend, unscopableRequest: unscopableMutatingRequest(opts) })) { + return { backend: 'primary', descriptorProfile: scopedProfile, scopePath: true } + } + return { backend: 'pool', descriptorProfile: null, scopePath: false } } diff --git a/apps/desktop/electron/host-backend-singleton.test.ts b/apps/desktop/electron/host-backend-singleton.test.ts new file mode 100644 index 0000000000..5e16e73685 --- /dev/null +++ b/apps/desktop/electron/host-backend-singleton.test.ts @@ -0,0 +1,81 @@ +// Multiplex-only invariants for the Desktop backend pool: one host backend +// serves every profile, and the local pooled-spawn path is unreachable. +import assert from 'node:assert/strict' + +import { test } from 'vitest' + +import { resolveProfileBackendRoute, unscopableMutatingRequest } from './connection-config' +import { assertNoSecondLocalBackend, SecondLocalBackendError, sharesHostBackend } from './host-backend-singleton' + +const LOCAL = { globalRemote: false, primaryProfile: 'default', profileRemoteOverride: false } + +test('two local profiles connecting concurrently produce ZERO additional backends, each bound to its own profile', () => { + // The routing decision is the whole spawn decision: `ensureBackend` spawns a + // pooled child if and only if the route says `pool`. Resolve both profiles + // the way two concurrent renderer dials would. + const routes = ['worker', 'venture'].map(profile => resolveProfileBackendRoute(profile, LOCAL)) + + assert.deepEqual( + routes.filter(route => route.backend === 'pool'), + [], + 'a local profile must never resolve to a pooled backend of its own' + ) + + // Both land on the SAME backend and still carry distinct wire identities, so + // each connection's turns bind to its own home (`session.create {profile}` -> + // `profile_home`; sessionless RPCs take the explicit `profile` argument). + assert.deepEqual( + routes.map(route => [route.backend, route.descriptorProfile, route.scopePath]), + [ + ['primary', 'worker', true], + ['primary', 'venture', true] + ] + ) +}) + +test('the local pool spawn path is unreachable, and the escape hatches still reach it', () => { + assert.throws(() => assertNoSecondLocalBackend('worker', { isolated: false }), SecondLocalBackendError) + + // HERMES_DESKTOP_ISOLATED_BACKEND=1: a private backend for this app. + assert.equal(sharesHostBackend({ isolated: true }), false) + assertNoSecondLocalBackend('worker', { isolated: true }) + assert.equal(resolveProfileBackendRoute('worker', { ...LOCAL, isolatedBackend: true }).backend, 'pool') + + // A remote/SSH backend is a DIFFERENT host: out of scope for the singleton, + // and its pooled descriptor never meant a local child anyway. + assert.equal(sharesHostBackend({ profileRemoteOverride: true }), false) + assert.equal(sharesHostBackend({ primaryRemoteActive: true }), false) + assertNoSecondLocalBackend('worker', { profileRemoteOverride: true }) + assert.equal(resolveProfileBackendRoute('worker', { ...LOCAL, profileRemoteOverride: true }).backend, 'pool') + + // A mutating request the server cannot profile-scope is the third way + // through: the pooled backend's HERMES_HOME is its only scope, so the guard + // must let that spawn happen instead of refusing a legitimate route. + assert.equal(sharesHostBackend({ unscopableRequest: true }), false) + assertNoSecondLocalBackend('worker', { unscopableRequest: true }) +}) + +test('the spawn guard and the router agree on which requests keep a backend', () => { + // The guard is a backstop, not a second opinion: every request the router + // sends to the pool must be one the guard admits, or the destructive write + // fails with SecondLocalBackendError instead of reaching the right home. + const cases: Array<[string, string]> = [ + ['POST', '/api/files/upload'], + ['DELETE', '/api/files/managed'], + ['POST', '/api/skills'], + ['POST', '/api/memory/reset'], + ['GET', '/api/config'], + ['PATCH', '/api/sessions/session-1'] + ] + + for (const [requestMethod, requestPath] of cases) { + const opts = { ...LOCAL, requestMethod, requestPath } + const pooled = resolveProfileBackendRoute('worker', opts).backend === 'pool' + + assert.equal( + sharesHostBackend({ unscopableRequest: unscopableMutatingRequest(opts) }), + !pooled, + `${requestMethod} ${requestPath}: guard and router disagree` + ) + } +}) diff --git a/apps/desktop/electron/host-backend-singleton.ts b/apps/desktop/electron/host-backend-singleton.ts new file mode 100644 index 0000000000..0450eb5585 --- /dev/null +++ b/apps/desktop/electron/host-backend-singleton.ts @@ -0,0 +1,90 @@ +// Multiplex-only, Desktop half: ONE `hermes serve` per HOST serves every +// profile, so a local profile never gets a backend process of its own. +// +// `backend-discovery.ts` + `host-backend-attach.ts` made the PRIMARY backend +// attach to a backend the host is already running. This module removes the +// other producer of `hermes serve` children: the per-profile backend pool. +// Every local profile now resolves onto the same host backend and carries its +// own `profile` on the wire — the server binds a SESSION to a profile home +// (`session.create {profile}` -> `profile_home`) and a sessionless RPC to the +// explicit `profile` argument (`tui_gateway/server.py::@_profile_scoped`), so +// one process genuinely serves N homes. +// +// Two things are deliberately NOT collapsed: +// * `HERMES_DESKTOP_ISOLATED_BACKEND=1` — the escape hatch that gives this +// app a private backend instead of the host's. +// * Remote / SSH / Cloud backends — a DIFFERENT host. Its ownership proof +// (`remote-lifecycle.ts` isolated_count === 1) is about that machine's +// process, not ours, and stays pooled. +// +// Pure and dependency-injected so the decision tests without Electron. + +/** Everything the collapse decision needs about one profile's routing. */ +export interface HostBackendCollapseOptions { + /** `HERMES_DESKTOP_ISOLATED_BACKEND=1`: this app wants its own backend. */ + isolated?: boolean + /** This profile points at its own remote host (connection.json / SSH). */ + profileRemoteOverride?: boolean + /** The primary profile's backend is itself a remote host. */ + primaryRemoteActive?: boolean + /** + * This request MUTATES state the server cannot profile-scope + * (`connection-config.ts::unscopableMutatingRequest`). The pooled backend's + * own `HERMES_HOME` is then the only scope there is, so the collapse must + * not swallow it — and the spawn guard below must not refuse it. + */ + unscopableRequest?: boolean +} + +/** + * True when a LOCAL profile shares the one host backend instead of spawning + * its own. False keeps the legacy pooled descriptor — which, for a remote + * route, never meant a local child in the first place. + */ +export function sharesHostBackend(opts: HostBackendCollapseOptions = {}): boolean { + if (opts.isolated || opts.unscopableRequest) { + return false + } + + return !opts.profileRemoteOverride && !opts.primaryRemoteActive +} + +/** Raised when something still tries to start a second local backend. */ +export class SecondLocalBackendError extends Error { + readonly poolKey: string + + constructor(poolKey: string) { + super( + `Refusing to start a second local Hermes backend for "${poolKey}": one backend serves every profile on this host. ` + + 'Set HERMES_DESKTOP_ISOLATED_BACKEND=1 for a private backend.' + ) + this.name = 'SecondLocalBackendError' + this.poolKey = poolKey + } +} + +/** + * The single enforcement point for "never spawn a second backend". + * + * Routing is what normally prevents a pooled local spawn; this guard sits at + * the one place a local `hermes serve` child is actually started, so a future + * caller that reaches it through a path routing does not cover fails loudly + * instead of quietly reintroducing a process per profile. + */ +export function assertNoSecondLocalBackend(poolKey: string, opts: HostBackendCollapseOptions = {}): void { + if (sharesHostBackend(opts)) { + throw new SecondLocalBackendError(poolKey) + } +} + +/** + * A passive read (background tile reconcile, #103375) may only be served by a + * backend that already exists: it never cold-starts a child and never + * refreshes `lastActiveAt`, so an open-but-unviewed tile cannot keep the pool + * saturated. Callers treat the rejection as "nothing to refresh yet". + */ +export function assertNotPassiveSpawn(passive: boolean, poolKey: string): void { + if (passive) { + throw new Error(`Passive read: no warm backend for "${poolKey}"`) + } +} diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index fa00148f50..a48a7bfdac 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -138,6 +138,7 @@ import { pathWithGlobalRemoteProfile, profileHasRemoteConnection, profileRemoteOverride, + type ProfileRouteOptions, profileSshOverride, type RegistryBackendRequestScope, resolveAuthMode, @@ -148,6 +149,7 @@ import { sanitizeRemoteHeaderValue, savedProfileSsh, tokenPreview, + unscopableMutatingRequest, withTransientRetries } from './connection-config' import { applyConnectionConfigAtomically } from './connection-config-apply' @@ -272,6 +274,7 @@ import { spawnLedgerPath, type SpawnReservation } from './host-backend-attach' +import { assertNoSecondLocalBackend, assertNotPassiveSpawn } from './host-backend-singleton' import { requestHudClose } from './hud-close' import { cursorPointInWindow } from './hud-cursor' import { startHudGameOverlayWatch } from './hud-game-overlay' @@ -1560,17 +1563,6 @@ function logPoolSpawnFailure(label: string, error: unknown): void { } } -// A passive read (background tile reconcile, #103375) may only be served by a -// backend that already exists: it never cold-starts a pooled child, never -// takes a slot, and never refreshes lastActiveAt, so an open-but-unviewed tile -// cannot keep the pool saturated. Callers treat the rejection as "nothing to -// refresh yet"; primary-routed profiles are always warm and never reach here. -function assertNotPassiveSpawn(passive: boolean, poolKey: string): void { - if (passive) { - throw new Error(`Passive read: no warm backend for "${poolKey}"`) - } -} - // Apply foreground intent to the dial claim for `scopeKey`: an entry already // in the pool is promoted directly, otherwise the intent is marked for the // spawn the claim owner is about to start. Returns the cleanup that clears a @@ -10290,7 +10282,10 @@ function primaryProfileKey() { } // Options describing the current connection setup for `resolveProfileBackendRoute`. -function profileRouteOptions(profile, request?) { +function profileRouteOptions( + profile: string | null | undefined, + request?: { method?: string; path?: string } +): ProfileRouteOptions { const config = readDesktopConnectionConfig() const sshOverride = profileSshOverride(config, profile) const key = connectionScopeKey(profile) || primaryProfileKey() @@ -10309,6 +10304,7 @@ function profileRouteOptions(profile, request?) { // A stored per-profile entry (local or remote) — pins this profile to // its own backend; absent entries inherit the primary's remote. ownEntry: Boolean((config.profiles || {})[key]), + isolatedBackend: ISOLATED_BACKEND, requestMethod: request?.method, requestPath: request?.path } @@ -10317,7 +10313,10 @@ function profileRouteOptions(profile, request?) { // Resolve a backend connection for the given profile, per the routing table in // resolveProfileBackendRoute(). An empty / unknown profile resolves to the // primary, so legacy callers are unchanged. -async function ensureBackend(profile, opts: { passive?: boolean; spawnPriority?: LocalBackendSpawnPriority } = {}) { +async function ensureBackend( + profile: string | null | undefined, + opts: { passive?: boolean; request?: { method?: string; path?: string }; spawnPriority?: LocalBackendSpawnPriority } = {} +): Promise>> { const key = profile && String(profile).trim() ? String(profile).trim() : primaryProfileKey() const spawnPriority = spawnPriorityFrom(opts.spawnPriority) poolRetirer.assertCanOpen(key, spawnPriority) @@ -10325,7 +10324,12 @@ async function ensureBackend(profile, opts: { passive?: boolean; spawnPriority?: profileDeletionGate.assertCanStart(key) - const route = resolveProfileBackendRoute(key, profileRouteOptions(key)) + // The REQUEST is part of the routing decision (case 5/6): resolving without + // it would collapse a profile onto the shared backend that the caller's + // resolveProfileApiRequest deliberately kept pooled, and the unscopable + // destructive write would execute against the primary's home after all. + const routeOpts = profileRouteOptions(key, opts.request) + const route = resolveProfileBackendRoute(key, routeOpts) if (route.backend === 'primary') { const connection = await startHermes() @@ -10385,7 +10389,9 @@ async function ensureBackend(profile, opts: { passive?: boolean; spawnPriority?: spawnPriority } - entry.connectionPromise = spawnPoolBackend(key, entry).catch(async error => { + entry.connectionPromise = spawnPoolBackend(key, entry, { + unscopableRequest: unscopableMutatingRequest(routeOpts) + }).catch(async error => { // Land the failure in desktop.log: without this a spawn that dies before // its child exists (guard rejection, runtime resolution) leaves no trace // beyond renderer-side rejections users never see in a bundle. @@ -11362,7 +11368,7 @@ function teardownFailedLocalBackend(poolKey: string, entry: any): Promise async function spawnPoolBackend( profile: string, entry: any, - opts: { forceLocal?: boolean; poolKey?: string } = {} + opts: { forceLocal?: boolean; poolKey?: string; unscopableRequest?: boolean } = {} ): Promise>> { const poolKey = opts.poolKey || profile @@ -11393,6 +11399,22 @@ async function spawnPoolBackend( } } + // Everything below starts a LOCAL `hermes serve` child. Multiplex-only says + // the host has exactly one, and routing (resolveProfileBackendRoute case 6) + // keeps local profiles off this path — this is the backstop that makes the + // pool spawn path genuinely unreachable rather than merely unused. + // Same options object the router reads, so the guard cannot drift from it + // (profileRouteOptions folds the per-profile SSH override into + // profileRemoteOverride; a hand-rolled term here missed that). + const guardRoute = profileRouteOptions(profile) + + assertNoSecondLocalBackend(poolKey, { + isolated: guardRoute.isolatedBackend, + primaryRemoteActive: guardRoute.primaryRemoteActive, + profileRemoteOverride: opts.forceLocal ? false : guardRoute.profileRemoteOverride, + unscopableRequest: opts.unscopableRequest + }) + // Bound the slot wait BELOW the renderer's backend-boot budget (45s): once // the renderer has given up on this spawn, a ticket still queued for the // pool-idle window (10 min) would hold the pool key hostage and every @@ -16034,9 +16056,11 @@ async function handleHermesApiRequest(request) { // backend calls ensure_hermes_home() which recreates the profile directory, // defeating the deletion and leaving a zombie process. // - // Safe local-profile REST calls also stay on the primary dashboard and carry - // ?profile=. Endpoints that cannot honor that scope retain their pooled - // backend so a destructive call can never fall through to the primary home. + // Local-profile REST calls stay on the primary dashboard and carry ?profile= + // (or name the profile in the path / PATCH body). A request that MUTATES + // state the server cannot scope at all retains its pooled backend, whose + // HERMES_HOME is then the scope, so a destructive call can never fall + // through to the primary home — `resolveProfileBackendRoute` case 6. // // A profile rename tears down the old-name backend the same way; for a // primary rename the lifecycle has already made `default` the temporary @@ -16051,7 +16075,11 @@ async function handleHermesApiRequest(request) { let connection try { - connection = await ensureBackend(routeProfile, { passive: request?.passive, spawnPriority }) + connection = await ensureBackend(routeProfile, { + passive: request?.passive, + request: { method: request?.method, path: request?.path }, + spawnPriority + }) const timeoutMs = resolveTimeoutMs(request?.timeoutMs, DEFAULT_FETCH_TIMEOUT_MS) const url = `${connection.baseUrl}${apiRoute.requestPath}` diff --git a/apps/desktop/src/api/sessions.test.ts b/apps/desktop/src/api/sessions.test.ts index 3376573b56..f3399eab4a 100644 --- a/apps/desktop/src/api/sessions.test.ts +++ b/apps/desktop/src/api/sessions.test.ts @@ -5,6 +5,7 @@ vi.mock('@/store/transcript-tail', () => ({ recordTranscriptTail: vi.fn() })) vi.mock('./client', () => ({ capabilityScoped: vi.fn(), getApiRequestConnection: vi.fn(() => 'prometheus'), + getApiRequestProfile: vi.fn(() => null), hermesApi: vi.fn(), profileScoped: vi.fn(() => ({})) })) @@ -25,6 +26,7 @@ const hermesApi = vi.mocked(client.hermesApi) beforeEach(() => { vi.clearAllMocks() vi.mocked(client.getApiRequestConnection).mockReturnValue('prometheus') + vi.mocked(client.getApiRequestProfile).mockReturnValue(null) }) describe('deleteSession profile scoping', () => { @@ -124,11 +126,29 @@ describe('setSessionArchived profile scoping', () => { }) }) - it('omits the profile from the body when none is given', async () => { + it('falls back to the ACTIVE profile in the body when no owner is given', async () => { + // Multiplex-only: the PATCH handler resolves its state.db from + // `body.profile` and there is no per-profile backend whose HERMES_HOME + // could stand in. An unnamed owner therefore has to mean "the profile I am + // looking at" — otherwise the archive lands on the shared backend's own + // state.db and silently no-ops. hermesApi.mockResolvedValue({ ok: true } as never) + vi.mocked(client.getApiRequestProfile).mockReturnValue('beta') await setSessionArchived('sess-b', false) + expect(hermesApi.mock.calls[0][0]).toMatchObject({ + method: 'PATCH', + profile: 'beta', + body: { archived: false, profile: 'beta' } + }) + }) + + it('omits the profile from the body only when there is no active profile at all', async () => { + hermesApi.mockResolvedValue({ ok: true } as never) + + await setSessionArchived('sess-b2', false) + const req = hermesApi.mock.calls[0][0] as { body: Record } expect(req).toMatchObject({ method: 'PATCH', body: { archived: false } }) expect(req.body).not.toHaveProperty('profile') @@ -162,14 +182,17 @@ describe('setSessionPinnedRemote / setSessionUnreadRemote profile scoping', () = }) }) - it('omits the profile from the body when none is given', async () => { + it('falls back to the ACTIVE profile in the body when no owner is given', async () => { hermesApi.mockResolvedValue({ ok: true } as never) + vi.mocked(client.getApiRequestProfile).mockReturnValue('beta') await setSessionPinnedRemote('sess-p2', false) - const req = hermesApi.mock.calls[0][0] as { body: Record } - expect(req).toMatchObject({ method: 'PATCH', body: { pinned: false } }) - expect(req.body).not.toHaveProperty('profile') + expect(hermesApi.mock.calls[0][0]).toMatchObject({ + method: 'PATCH', + profile: 'beta', + body: { pinned: false, profile: 'beta' } + }) }) }) diff --git a/apps/desktop/src/api/sessions.ts b/apps/desktop/src/api/sessions.ts index 81c938caf4..d983fc1033 100644 --- a/apps/desktop/src/api/sessions.ts +++ b/apps/desktop/src/api/sessions.ts @@ -42,6 +42,18 @@ function sessionScoped(scope?: ProfileScope): { connectionId?: string; profile?: return scoped } +/** + * The profile a session WRITE must name in its body. The PATCH handler reads + * its target DB from `body.profile` alone (`_with_db(body.profile, ...)`), and + * under multiplex-only there is no per-profile backend whose HERMES_HOME could + * stand in for it: an unnamed owner lands the rename/pin/archive/mark-read on + * the shared backend's own state.db. "Unnamed" therefore means "the profile I + * am looking at", not "whatever home the backend was launched in". + */ +function sessionWriteProfile(profile?: null | string): string | undefined { + return String(profile ?? '').trim() || getApiRequestProfile() || undefined +} + function sessionScopeQuery(scope?: ProfileScope): string { const profile = sessionScoped(scope).profile @@ -344,11 +356,13 @@ export function setSessionArchived(id: string, archived: boolean, profile?: stri // remote gateway with no remoteProfile alias: the archive lands on the wrong // (default) state.db, no-ops on a missing row, and the archived/unarchived // state silently fails to stick — the same class as the unscoped DELETE. + const owner = sessionWriteProfile(profile) + return hermesApi<{ ok: boolean }>({ - ...(profile ? { profile } : {}), + ...(owner ? { profile: owner } : {}), path: `/api/sessions/${encodeURIComponent(id)}`, method: 'PATCH', - body: { archived, ...(profile ? { profile } : {}) } + body: { archived, ...(owner ? { profile: owner } : {}) } }) } @@ -360,11 +374,13 @@ export function setSessionPinnedRemote(id: string, pinned: boolean, profile?: st // Owning profile in the PATCH body (see setSessionArchived / renameSession): // the handler reads its target DB from body.profile, so a remote/foreign // profile's pin must travel in the body or it no-ops on the wrong state.db. + const owner = sessionWriteProfile(profile) + return hermesApi<{ ok: boolean }>({ - ...(profile ? { profile } : {}), + ...(owner ? { profile: owner } : {}), path: `/api/sessions/${encodeURIComponent(id)}`, method: 'PATCH', - body: { pinned, ...(profile ? { profile } : {}) } + body: { pinned, ...(owner ? { profile: owner } : {}) } }) } @@ -377,11 +393,13 @@ export function setSessionUnreadRemote(id: string, unread: boolean, profile?: st // the handler reads its target DB from body.profile, so a remote/foreign // profile's unread toggle must travel in the body or it no-ops on the wrong // state.db. + const owner = sessionWriteProfile(profile) + return hermesApi<{ ok: boolean }>({ - ...(profile ? { profile } : {}), + ...(owner ? { profile: owner } : {}), path: `/api/sessions/${encodeURIComponent(id)}`, method: 'PATCH', - body: { unread, ...(profile ? { profile } : {}) } + body: { unread, ...(owner ? { profile: owner } : {}) } }) } @@ -633,10 +651,12 @@ export function renameSession( title: string, profile?: string | null ): Promise<{ ok: boolean; title: string }> { + const owner = sessionWriteProfile(profile) + return hermesApi<{ ok: boolean; title: string }>({ - ...(profile ? { profile } : {}), + ...(owner ? { profile: owner } : {}), path: `/api/sessions/${encodeURIComponent(id)}`, method: 'PATCH', - body: { title, ...(profile ? { profile } : {}) } + body: { title, ...(owner ? { profile: owner } : {}) } }) } diff --git a/apps/desktop/src/api/system.ts b/apps/desktop/src/api/system.ts index 3e7675eac3..47d2636f2b 100644 --- a/apps/desktop/src/api/system.ts +++ b/apps/desktop/src/api/system.ts @@ -241,18 +241,29 @@ export function getGhAuthStatus(refresh = false): Promise<{ available: boolean; // audit` / `hermes backup` / `hermes debug share` and the dashboard System // page). All except debug share are spawn-based background actions tailed via // getActionStatus(). +// +// Every one carries the ambient profile: Electron pins the whole /api/ops +// family to the shared primary backend (connection-config's +// LOCAL_PRIMARY_SCOPED_ROUTES), so an unprofiled call acts on that backend's +// LAUNCH profile — and debug share uploads a home's logs and config. // --------------------------------------------------------------------------- export function runDoctor(): Promise { - return hermesApi({ path: '/api/ops/doctor', method: 'POST', body: {} }) + return hermesApi({ ...profileScoped(), path: '/api/ops/doctor', method: 'POST', body: {} }) } export function runSecurityAudit(): Promise { - return hermesApi({ path: '/api/ops/security-audit', method: 'POST', body: {} }) + return hermesApi({ + ...profileScoped(), + path: '/api/ops/security-audit', + method: 'POST', + body: {} + }) } export function runBackup(): Promise { return hermesApi({ + ...profileScoped(), path: '/api/ops/backup', method: 'POST', body: {} @@ -261,6 +272,7 @@ export function runBackup(): Promise { export function runDebugShare(): Promise { return hermesApi({ + ...profileScoped(), path: '/api/ops/debug-share', method: 'POST', body: {}, diff --git a/apps/desktop/src/app/gateway/hooks/use-gateway-boot.test.tsx b/apps/desktop/src/app/gateway/hooks/use-gateway-boot.test.tsx index 1e1021cfd0..2413e744c4 100644 --- a/apps/desktop/src/app/gateway/hooks/use-gateway-boot.test.tsx +++ b/apps/desktop/src/app/gateway/hooks/use-gateway-boot.test.tsx @@ -43,7 +43,13 @@ import { setActiveSessionId, setSelectedStoredSessionId } from '@/store/session' -import { $sessionTiles, $workingSessionIds, clearAllSessionStates, publishSessionState } from '@/store/session-states' +import { + $sessionTiles, + $workingSessionIds, + clearAllSessionStates, + publishSessionState, + runtimeSessionOwner +} from '@/store/session-states' import { warnIfTerminalBackendUnavailable } from '@/store/terminal-backend-warning' import { deferred } from '../../../test/deferred' @@ -689,6 +695,67 @@ describe('primary failure foreground isolation', () => { ) }) +describe('shared host backend event provenance', () => { + // Multiplex-only: ONE backend serves every local profile, so the primary + // socket carries profile B's events and no secondary closure exists to stamp + // them. Unstamped, runtimeSessionOwner() is blank for B and the live + // sessions/cron sync falls back to slow polling. + // A LOCAL host backend: no registry connection id, so ownership can only come + // from the stamp (a registry-tagged event already carries its exact owner). + const sharedPrimaryConn = { + ...primaryConn, + baseUrl: 'http://127.0.0.1:8899', + connectionId: '', + profile: 'beta', + sharedPrimary: true, + wsUrl: 'ws://127.0.0.1:8899/api/ws?token=t' + } + + function deliverEvent(socket: FakeWebSocket, frame: Record) { + ;(socket as unknown as { emit: (type: string, ev: unknown) => void }).emit('message', { + data: JSON.stringify({ jsonrpc: '2.0', method: 'event', params: frame }) + }) + } + + it("stamps a shared-primary profile-B event with B, not with the boot-time profile", async () => { + const desktop = fakeDesktop() + + desktop.getConnection.mockResolvedValue(sharedPrimaryConn) + desktop.getGatewayWsUrl.mockResolvedValue(sharedPrimaryConn.wsUrl) + ;(window as { hermesDesktop?: unknown }).hermesDesktop = desktop + + render() + await flushAsync() + + expect($gatewayState.get()).toBe('open') + + // The window moved to profile B after boot; the socket did not. + act(() => { + $connection.set(sharedPrimaryConn as unknown as ReturnType) + $activeGatewayProfile.set('beta') + }) + + act(() => { + deliverEvent(FakeWebSocket.instances[0], { session_id: 'rt-B', type: 'session.info' }) + }) + + expect(runtimeSessionOwner('rt-B')).toBe('beta') + }) + + it('leaves an unshared primary on its exact owner — the socket already IS its profile', async () => { + render() + await flushAsync() + + act(() => { + deliverEvent(FakeWebSocket.instances[0], { session_id: 'rt-A', type: 'session.info' }) + }) + + // The registry (connectionId, profile) owner, NOT a bare-profile marker: + // the stamp is reserved for the shared-primary topology. + expect(runtimeSessionOwner('rt-A')).toEqual({ connectionId: 'primary-vps', profile: 'default' }) + }) +}) + describe('useGatewayBoot remote reconnect loop (real hook, fake socket)', () => { it('parks rejected primary auth across timers and wake signals until explicit recovery', async () => { const desktop = fakeDesktop() diff --git a/apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts b/apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts index 9602d1c281..28176c3135 100644 --- a/apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts +++ b/apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts @@ -85,6 +85,7 @@ import { setCurrentCwd, setSessionsLoading } from '@/store/session' +import { stampSecondaryProfileOwner } from '@/store/session-event-provenance' import { $attentionSessionIds, $sessionOwnerHoldRevision, @@ -1012,10 +1013,15 @@ export function useGatewayBoot({ } }) - const sourceProfile = normalizeProfileKey($activeGatewayProfile.get()) + // Read PER EVENT, never once at boot: under multiplex-only this one socket + // serves every local profile, and the profile moves under it while the + // socket stays open. A boot-time capture stamps every later profile's + // events with whatever was active when the gateway booted. + const sourceProfileNow = () => normalizeProfileKey($activeGatewayProfile.get()) const offEvent = gateway.onEvent(event => { const connectionId = activeGatewayConnectionId() + const sourceProfile = sourceProfileNow() const scopedEvent = { ...event, @@ -1023,12 +1029,25 @@ export function useGatewayBoot({ ...(connectionId ? { connectionId } : {}) } - recordSessionEventScope(scopedEvent) - callbacksRef.current.handleGatewayEvent(scopedEvent) + // On a shared host backend the socket no longer PROVES the profile the + // way a pooled secondary's closure did, so nothing stamps ownership and + // runtimeSessionOwner() stays blank for every non-primary local profile + // — the live sessions/cron sync dies and falls back to slow polling. + // The shared-primary descriptor is exactly the topology where the active + // profile is the authority for this socket's traffic. (The marker is the + // LAST rung of knownOwnerForSession, so durable stored identity still + // outranks it — #97511.) + const ownedEvent = + $connection.get()?.sharedPrimary === true + ? stampSecondaryProfileOwner(scopedEvent, sourceProfile) + : scopedEvent + + recordSessionEventScope(ownedEvent) + callbacksRef.current.handleGatewayEvent(ownedEvent) }) // Secondary sockets reach the same handler through the registry's onServerRequest. - const offRequest = gateway.onRequest(request => dispatchPrimaryServerRequest(request, sourceProfile)) + const offRequest = gateway.onRequest(request => dispatchPrimaryServerRequest(request, sourceProfileNow())) // Wake signals: power resume (macOS/Windows), network coming back, and the // window regaining focus/visibility. Each nudges an immediate reconnect. diff --git a/apps/desktop/src/components/onboarding-chat/setup-profile.ts b/apps/desktop/src/components/onboarding-chat/setup-profile.ts index 4d141fd459..ef1587a959 100644 --- a/apps/desktop/src/components/onboarding-chat/setup-profile.ts +++ b/apps/desktop/src/components/onboarding-chat/setup-profile.ts @@ -292,6 +292,7 @@ export async function ensureSetupProfile(request: GatewayRequest): Promise description: 'Where Hermes met you — walks your first run, then checks in as you find your feet.', name: SETUP_PROFILE, clone_from: 'default', + share_auth: true, no_alias: true, soul: composeSetupSoul() }) diff --git a/apps/desktop/src/plugins/hermes-bots/create-dialog.tsx b/apps/desktop/src/plugins/hermes-bots/create-dialog.tsx index 31747afa2d..12a0a3afb5 100644 --- a/apps/desktop/src/plugins/hermes-bots/create-dialog.tsx +++ b/apps/desktop/src/plugins/hermes-bots/create-dialog.tsx @@ -141,7 +141,7 @@ export function CreateAgentDialog({ open, onClose, roster }: CreateAgentDialogPr const [provider, setProvider] = useState('') const [soul, setSoul] = useState('') const [noSkills, setNoSkills] = useState(false) - const [mirrorCredentials, setMirrorCredentials] = useState(true) + const [shareAuth, setShareAuth] = useState(true) const [advTab, setAdvTab] = useState('general') // Where the profile is created: '' = the active gateway (unchanged default), // else a registry connection id — the profiles.create lands on THAT @@ -274,7 +274,7 @@ export function CreateAgentDialog({ open, onClose, roster }: CreateAgentDialogPr setProvider('') setSoul('') setNoSkills(false) - setMirrorCredentials(true) + setShareAuth(true) setAdvTab('general') setCreatedForCaps(null) setCaps(null) @@ -392,10 +392,10 @@ export function CreateAgentDialog({ open, onClose, roster }: CreateAgentDialogPr // the remote box doesn't have. clone_from: cloneFrom === '__none__' ? null : remoteTarget ? 'default' : cloneFrom, no_skills: noSkills, - // Copies the main profile's API keys (.env + auth.json) into the new profile. OAuth - // logins are never copied (single-use refresh tokens fork) and never inherited: a - // profile only reads its own auth.json, so sign the bot in itself for those. - mirror_credentials: mirrorCredentials, + // Shared (not copied) auth keeps ONE OAuth/token pool with the main + // profile, so refreshes can't invalidate each other. Older gateways + // ignore the param and copy — still functional, just forked. + share_auth: shareAuth, soul: composeSoul({ name: slug, title: botTitle, @@ -792,16 +792,12 @@ export function CreateAgentDialog({ open, onClose, roster }: CreateAgentDialogPr /> )}
- Each profile owns its credentials. API keys are copied; OAuth logins (Claude, Codex, xAI, Nous) are - not — sign the bot in with hermes -p <name> model. Uncheck to start with no - credentials. + Subscriptions, OAuth logins, and API keys stay shared (not copied), so token refreshes never + invalidate each other. Uncheck for an isolated snapshot copy.