From 85020f2238d254456bb428de6ea5503a19dc9633 Mon Sep 17 00:00:00 2001 From: doncazper Date: Sat, 1 Aug 2026 20:03:12 -0700 Subject: [PATCH] fix(plugins): isolate ownership by profile --- agent/agent_init.py | 10 + agent/browser_registry.py | 69 +- agent/image_gen_registry.py | 47 +- agent/secret_sources/registry.py | 164 ++- agent/tool_executor.py | 1 + agent/transcription_registry.py | 42 +- agent/tts_registry.py | 42 +- agent/video_gen_registry.py | 47 +- agent/web_search_registry.py | 47 +- gateway/authz_mixin.py | 1 + gateway/platform_registry.py | 345 +++++- gateway/run.py | 3 + hermes_cli/dashboard_auth/registry.py | 60 +- hermes_cli/env_loader.py | 14 +- hermes_cli/plugins.py | 676 +++++++--- hermes_cli/tools_config.py | 43 +- hermes_constants.py | 12 + model_tools.py | 24 +- registration_lifecycle.py | 128 ++ tests/gateway/test_google_chat.py | 6 +- .../test_plugin_ownership_ledger.py | 1096 ++++++++++++++++- tests/hermes_cli/test_plugins.py | 18 +- .../secret_sources/test_error_remediation.py | 35 +- .../test_secret_source_registry.py | 32 + ...st_get_tool_definitions_cache_isolation.py | 28 + .../test_browser_cloud_provider_cache.py | 173 +++ tools/browser_tool.py | 58 +- tools/registry.py | 357 +++++- toolsets.py | 2 +- 29 files changed, 3101 insertions(+), 479 deletions(-) create mode 100644 registration_lifecycle.py diff --git a/agent/agent_init.py b/agent/agent_init.py index 797c18c6df..c2021bd16e 100644 --- a/agent/agent_init.py +++ b/agent/agent_init.py @@ -1441,6 +1441,16 @@ def init_agent( print(f"🔄 Fallback chain ({len(agent._fallback_chain)} providers): " + " → ".join(f"{f['model']} ({f['provider']})" for f in agent._fallback_chain)) + # A multiplexed gateway may enter a different HERMES_HOME after + # ``model_tools`` was first imported. Ensure that profile's keyed plugin + # manager has discovered its registrations before taking the tool snapshot. + try: + from hermes_cli.plugins import discover_plugins + + discover_plugins() + except Exception: + logger.warning("Plugin discovery failed during agent setup", exc_info=True) + # Get available tools with filtering. Capture the registry generation this # snapshot is derived from FIRST, so a later concurrent refresh can tell # whether it holds a newer or staler view (see refresh_agent_mcp_tools). diff --git a/agent/browser_registry.py b/agent/browser_registry.py index 8a665fb1bb..4348237af1 100644 --- a/agent/browser_registry.py +++ b/agent/browser_registry.py @@ -41,15 +41,19 @@ import threading from typing import Dict, List, Optional from agent.browser_provider import BrowserProvider +from hermes_constants import hermes_home_key logger = logging.getLogger(__name__) _providers: Dict[str, BrowserProvider] = {} +_scoped_providers: Dict[str, Dict[str, BrowserProvider]] = {} +_generation = 0 +_scoped_generations: Dict[str, int] = {} _lock = threading.Lock() -def register_provider(provider: BrowserProvider) -> None: +def register_provider(provider: BrowserProvider, *, scope: Optional[str] = None) -> None: """Register a cloud browser provider. Re-registration (same ``name``) overwrites the previous entry and logs @@ -61,12 +65,19 @@ def register_provider(provider: BrowserProvider) -> None: f"register_provider() expects a BrowserProvider instance, " f"got {type(provider).__name__}" ) - name = provider.name - if not isinstance(name, str) or not name.strip(): + raw_name = provider.name + if not isinstance(raw_name, str) or not raw_name.strip(): raise ValueError("Browser provider .name must be a non-empty string") + name = raw_name.strip() + global _generation with _lock: - existing = _providers.get(name) - _providers[name] = provider + target = _providers if scope is None else _scoped_providers.setdefault(scope, {}) + existing = target.get(name) + target[name] = provider + if scope is None: + _generation += 1 + else: + _scoped_generations[scope] = _scoped_generations.get(scope, 0) + 1 if existing is not None: logger.debug( "Browser provider '%s' re-registered (was %r)", @@ -79,34 +90,63 @@ def register_provider(provider: BrowserProvider) -> None: ) -def list_providers() -> List[BrowserProvider]: +def list_providers(*, scope: Optional[str] = None) -> List[BrowserProvider]: """Return all registered providers, sorted by name.""" with _lock: - items = list(_providers.values()) + merged = dict(_providers) + merged.update(_scoped_providers.get(scope or hermes_home_key(), {})) + items = list(merged.values()) return sorted(items, key=lambda p: p.name) -def get_provider(name: str) -> Optional[BrowserProvider]: +def get_provider(name: str, *, scope: Optional[str] = None) -> Optional[BrowserProvider]: """Return the provider registered under *name*, or None.""" if not isinstance(name, str): return None with _lock: - return _providers.get(name.strip()) + key = name.strip() + return _scoped_providers.get(scope or hermes_home_key(), {}).get(key) or _providers.get(key) + + +def snapshot_registration( + name: str, *, scope: Optional[str] = None +) -> Optional[BrowserProvider]: + with _lock: + target = _providers if scope is None else _scoped_providers.get(scope, {}) + return target.get(name.strip()) + + +def registry_generation(*, scope: Optional[str] = None) -> tuple[int, int]: + """Return a cache fingerprint for the global base and one profile.""" + active_scope = scope or hermes_home_key() + with _lock: + return _generation, _scoped_generations.get(active_scope, 0) def restore_registration( name: str, current: BrowserProvider, previous: Optional[BrowserProvider], + *, + scope: Optional[str] = None, ) -> bool: """Restore a plugin registration only when *current* is still installed.""" + key = name.strip() + global _generation with _lock: - if _providers.get(name) is not current: + target = _providers if scope is None else _scoped_providers.setdefault(scope, {}) + if target.get(key) is not current: return False if previous is None: - _providers.pop(name, None) + target.pop(key, None) else: - _providers[name] = previous + target[key] = previous + if scope is None: + _generation += 1 + else: + _scoped_generations[scope] = _scoped_generations.get(scope, 0) + 1 + if not target: + _scoped_providers.pop(scope, None) return True @@ -161,6 +201,7 @@ def _resolve(configured: Optional[str]) -> Optional[BrowserProvider]: """ with _lock: snapshot = dict(_providers) + snapshot.update(_scoped_providers.get(hermes_home_key(), {})) def _is_available_safe(p: BrowserProvider) -> bool: """Wrap ``is_available()`` so a buggy provider doesn't kill resolution.""" @@ -204,5 +245,9 @@ def _resolve(configured: Optional[str]) -> Optional[BrowserProvider]: def _reset_for_tests() -> None: """Clear the registry. **Test-only.**""" + global _generation with _lock: _providers.clear() + _scoped_providers.clear() + _scoped_generations.clear() + _generation += 1 diff --git a/agent/image_gen_registry.py b/agent/image_gen_registry.py index 9e29a491ab..82393e88db 100644 --- a/agent/image_gen_registry.py +++ b/agent/image_gen_registry.py @@ -25,15 +25,17 @@ import threading from typing import Dict, List, Optional from agent.image_gen_provider import ImageGenProvider +from hermes_constants import hermes_home_key logger = logging.getLogger(__name__) _providers: Dict[str, ImageGenProvider] = {} +_scoped_providers: Dict[str, Dict[str, ImageGenProvider]] = {} _lock = threading.Lock() -def register_provider(provider: ImageGenProvider) -> None: +def register_provider(provider: ImageGenProvider, *, scope: Optional[str] = None) -> None: """Register an image generation provider. Re-registration (same ``name``) overwrites the previous entry and logs @@ -45,46 +47,65 @@ def register_provider(provider: ImageGenProvider) -> None: f"register_provider() expects an ImageGenProvider instance, " f"got {type(provider).__name__}" ) - name = provider.name - if not isinstance(name, str) or not name.strip(): + raw_name = provider.name + if not isinstance(raw_name, str) or not raw_name.strip(): raise ValueError("Image gen provider .name must be a non-empty string") + name = raw_name.strip() with _lock: - existing = _providers.get(name) - _providers[name] = provider + target = _providers if scope is None else _scoped_providers.setdefault(scope, {}) + existing = target.get(name) + target[name] = provider if existing is not None: logger.debug("Image gen provider '%s' re-registered (was %r)", name, type(existing).__name__) else: logger.debug("Registered image gen provider '%s' (%s)", name, type(provider).__name__) -def list_providers() -> List[ImageGenProvider]: +def list_providers(*, scope: Optional[str] = None) -> List[ImageGenProvider]: """Return all registered providers, sorted by name.""" with _lock: - items = list(_providers.values()) + merged = dict(_providers) + merged.update(_scoped_providers.get(scope or hermes_home_key(), {})) + items = list(merged.values()) return sorted(items, key=lambda p: p.name) -def get_provider(name: str) -> Optional[ImageGenProvider]: +def get_provider(name: str, *, scope: Optional[str] = None) -> Optional[ImageGenProvider]: """Return the provider registered under *name*, or None.""" if not isinstance(name, str): return None with _lock: - return _providers.get(name.strip()) + key = name.strip() + return _scoped_providers.get(scope or hermes_home_key(), {}).get(key) or _providers.get(key) + + +def snapshot_registration( + name: str, *, scope: Optional[str] = None +) -> Optional[ImageGenProvider]: + with _lock: + target = _providers if scope is None else _scoped_providers.get(scope, {}) + return target.get(name.strip()) def restore_registration( name: str, current: ImageGenProvider, previous: Optional[ImageGenProvider], + *, + scope: Optional[str] = None, ) -> bool: """Restore a plugin registration only when *current* is still installed.""" + key = name.strip() with _lock: - if _providers.get(name) is not current: + target = _providers if scope is None else _scoped_providers.setdefault(scope, {}) + if target.get(key) is not current: return False if previous is None: - _providers.pop(name, None) + target.pop(key, None) else: - _providers[name] = previous + target[key] = previous + if scope is not None and not target: + _scoped_providers.pop(scope, None) return True @@ -120,6 +141,7 @@ def get_active_provider() -> Optional[ImageGenProvider]: with _lock: snapshot = dict(_providers) + snapshot.update(_scoped_providers.get(hermes_home_key(), {})) def _is_available_safe(p: ImageGenProvider) -> bool: """Wrap ``is_available()`` so a buggy provider doesn't kill resolution.""" @@ -159,3 +181,4 @@ def _reset_for_tests() -> None: """Clear the registry. **Test-only.**""" with _lock: _providers.clear() + _scoped_providers.clear() diff --git a/agent/secret_sources/registry.py b/agent/secret_sources/registry.py index c7ed8964f7..1881820d2a 100644 --- a/agent/secret_sources/registry.py +++ b/agent/secret_sources/registry.py @@ -30,6 +30,7 @@ from __future__ import annotations import concurrent.futures import logging import os +import threading from dataclasses import dataclass, field from pathlib import Path from typing import Dict, List, MutableMapping, Optional @@ -43,6 +44,7 @@ from agent.secret_sources.base import ( reset_source_environment, set_source_environment, ) +from hermes_constants import hermes_home_key logger = logging.getLogger(__name__) @@ -51,7 +53,9 @@ logger = logging.getLogger(__name__) # recorded beside each source so consumers never infer ownership from names. _SOURCES: Dict[str, SecretSource] = {} _SOURCE_ORIGINS: Dict[str, str] = {} +_SCOPED_SOURCES: Dict[str, Dict[str, SecretSource]] = {} _BUILTINS_LOADED = False +_REGISTRY_LOCK = threading.RLock() @dataclass @@ -101,6 +105,7 @@ def register_source( *, replace: bool = False, builtin: bool = False, + scope: Optional[str] = None, ) -> bool: """Register a secret source. Returns True on success. @@ -133,48 +138,80 @@ def register_source( name, getattr(source, "shape", None), ) return False - if name in _SOURCES and not replace: - logger.warning("Secret source '%s' already registered; ignoring duplicate", name) - return False - scheme = getattr(source, "scheme", None) - if scheme: - for other_name, other in _SOURCES.items(): - if other_name != name and getattr(other, "scheme", None) == scheme: - logger.warning( - "Ignoring secret source '%s': scheme '%s://' is already " - "owned by source '%s'", - name, scheme, other_name, - ) - return False - _SOURCES[name] = source - _SOURCE_ORIGINS[name] = "builtin" if builtin else "plugin" + with _REGISTRY_LOCK: + effective = dict(_SOURCES) + if scope is not None: + effective.update(_SCOPED_SOURCES.get(scope, {})) + if name in effective and not replace: + logger.warning( + "Secret source '%s' already registered; ignoring duplicate", name + ) + return False + scheme = getattr(source, "scheme", None) + if scheme: + for other_name, other in effective.items(): + if other_name != name and getattr(other, "scheme", None) == scheme: + logger.warning( + "Ignoring secret source '%s': scheme '%s://' is already " + "owned by source '%s'", + name, + scheme, + other_name, + ) + return False + target = _SOURCES if scope is None else _SCOPED_SOURCES.setdefault(scope, {}) + target[name] = source + if scope is None: + _SOURCE_ORIGINS[name] = "builtin" if builtin else "plugin" return True -def get_source(name: str) -> Optional[SecretSource]: +def get_source(name: str, *, scope: Optional[str] = None) -> Optional[SecretSource]: _ensure_builtin_sources() - return _SOURCES.get(name) + with _REGISTRY_LOCK: + return _SCOPED_SOURCES.get(scope or hermes_home_key(), {}).get( + name + ) or _SOURCES.get(name) + + +def snapshot_registration( + name: str, *, scope: Optional[str] = None +) -> Optional[SecretSource]: + """Return the registration owned by exactly one registry layer.""" + _ensure_builtin_sources() + with _REGISTRY_LOCK: + target = _SOURCES if scope is None else _SCOPED_SOURCES.get(scope, {}) + return target.get(name) def restore_registration( name: str, current: SecretSource, previous: Optional[SecretSource], + *, + scope: Optional[str] = None, ) -> bool: """Restore a host-owned source registration if it is still current.""" _ensure_builtin_sources() - if _SOURCES.get(name) is not current: - return False - if previous is None: - _SOURCES.pop(name, None) - else: - _SOURCES[name] = previous + with _REGISTRY_LOCK: + target = _SOURCES if scope is None else _SCOPED_SOURCES.setdefault(scope, {}) + if target.get(name) is not current: + return False + if previous is None: + target.pop(name, None) + else: + target[name] = previous + if scope is not None and not target: + _SCOPED_SOURCES.pop(scope, None) return True -def list_sources() -> List[SecretSource]: +def list_sources(*, scope: Optional[str] = None) -> List[SecretSource]: _ensure_builtin_sources() - return list(_SOURCES.values()) + with _REGISTRY_LOCK: + merged = dict(_SOURCES) + merged.update(_SCOPED_SOURCES.get(scope or hermes_home_key(), {})) + return list(merged.values()) def list_plugin_sources() -> List[SecretSource]: @@ -194,37 +231,46 @@ def _ensure_builtin_sources() -> None: source can never break registration of the others. """ global _BUILTINS_LOADED - if _BUILTINS_LOADED: - return - _BUILTINS_LOADED = True - try: - from agent.secret_sources.bitwarden import BitwardenSource + with _REGISTRY_LOCK: + if _BUILTINS_LOADED: + return + _BUILTINS_LOADED = True + try: + from agent.secret_sources.bitwarden import BitwardenSource - register_source(BitwardenSource(), builtin=True) - except Exception: # noqa: BLE001 — never block startup - logger.warning("Failed to register bundled Bitwarden secret source", - exc_info=True) - try: - from agent.secret_sources.onepassword import OnePasswordSource + register_source(BitwardenSource(), builtin=True) + except Exception: # noqa: BLE001 — never block startup + logger.warning( + "Failed to register bundled Bitwarden secret source", + exc_info=True, + ) + try: + from agent.secret_sources.onepassword import OnePasswordSource - register_source(OnePasswordSource(), builtin=True) - except Exception: # noqa: BLE001 — never block startup - logger.warning("Failed to register bundled 1Password secret source", - exc_info=True) - try: - from agent.secret_sources.command import CommandSource + register_source(OnePasswordSource(), builtin=True) + except Exception: # noqa: BLE001 — never block startup + logger.warning( + "Failed to register bundled 1Password secret source", + exc_info=True, + ) + try: + from agent.secret_sources.command import CommandSource - register_source(CommandSource(), builtin=True) - except Exception: # noqa: BLE001 — never block startup - logger.warning("Failed to register bundled command secret source", - exc_info=True) + register_source(CommandSource(), builtin=True) + except Exception: # noqa: BLE001 — never block startup + logger.warning( + "Failed to register bundled command secret source", + exc_info=True, + ) def _reset_registry_for_tests() -> None: global _BUILTINS_LOADED - _SOURCES.clear() - _SOURCE_ORIGINS.clear() - _BUILTINS_LOADED = False + with _REGISTRY_LOCK: + _SOURCES.clear() + _SOURCE_ORIGINS.clear() + _SCOPED_SOURCES.clear() + _BUILTINS_LOADED = False # --------------------------------------------------------------------------- @@ -287,7 +333,9 @@ def _fetch_with_timeout( return result -def _ordered_enabled_sources(secrets_cfg: dict) -> List[SecretSource]: +def _ordered_enabled_sources( + secrets_cfg: dict, *, scope: Optional[str] = None +) -> List[SecretSource]: """Resolve which sources run, in which order. Order: the optional ``secrets.sources`` list wins; sources not named @@ -295,28 +343,28 @@ def _ordered_enabled_sources(secrets_cfg: dict) -> List[SecretSource]: ``is_enabled`` says so for its config section. Mapped-vs-bulk precedence is applied on top of this order by :func:`apply_all`. """ - _ensure_builtin_sources() + sources = {source.name: source for source in list_sources(scope=scope)} explicit = secrets_cfg.get("sources") order: List[str] = [] if isinstance(explicit, list): for entry in explicit: - if isinstance(entry, str) and entry in _SOURCES and entry not in order: + if isinstance(entry, str) and entry in sources and entry not in order: order.append(entry) unknown = [e for e in explicit - if isinstance(e, str) and e not in _SOURCES] + if isinstance(e, str) and e not in sources] if unknown: logger.warning( "secrets.sources names unknown source(s): %s (known: %s)", - ", ".join(unknown), ", ".join(_SOURCES) or "none", + ", ".join(unknown), ", ".join(sources) or "none", ) - for name in _SOURCES: + for name in sources: if name not in order: order.append(name) enabled: List[SecretSource] = [] for name in order: - source = _SOURCES[name] + source = sources[name] cfg = secrets_cfg.get(name) cfg = cfg if isinstance(cfg, dict) else {} try: @@ -398,7 +446,9 @@ def apply_all(secrets_cfg: dict, home_path: Path, report = ApplyReport() secrets_cfg = secrets_cfg if isinstance(secrets_cfg, dict) else {} - enabled = _ordered_enabled_sources(secrets_cfg) + enabled = _ordered_enabled_sources( + secrets_cfg, scope=hermes_home_key(home_path) + ) if not enabled: return report diff --git a/agent/tool_executor.py b/agent/tool_executor.py index 9645199ef0..00029aabdd 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -355,6 +355,7 @@ def _tool_search_scoped_names(agent) -> frozenset: enabled = getattr(agent, "enabled_toolsets", None) disabled = getattr(agent, "disabled_toolsets", None) cache_key = ( + _registry.current_scope_key(), getattr(_registry, "_generation", 0), frozenset(enabled) if enabled is not None else None, frozenset(disabled) if disabled is not None else None, diff --git a/agent/transcription_registry.py b/agent/transcription_registry.py index db028a75e2..a167e710b5 100644 --- a/agent/transcription_registry.py +++ b/agent/transcription_registry.py @@ -24,6 +24,7 @@ import threading from typing import Dict, List, Optional from agent.transcription_provider import TranscriptionProvider +from hermes_constants import hermes_home_key logger = logging.getLogger(__name__) @@ -50,10 +51,11 @@ _BUILTIN_NAMES = frozenset({ _providers: Dict[str, TranscriptionProvider] = {} +_scoped_providers: Dict[str, Dict[str, TranscriptionProvider]] = {} _lock = threading.Lock() -def register_provider(provider: TranscriptionProvider) -> None: +def register_provider(provider: TranscriptionProvider, *, scope: Optional[str] = None) -> None: """Register a transcription provider. Rejects: @@ -85,8 +87,9 @@ def register_provider(provider: TranscriptionProvider) -> None: ) return with _lock: - existing = _providers.get(key) - _providers[key] = provider + target = _providers if scope is None else _scoped_providers.setdefault(scope, {}) + existing = target.get(key) + target[key] = provider if existing is not None: logger.debug( "Transcription provider '%s' re-registered (was %r)", @@ -99,14 +102,16 @@ def register_provider(provider: TranscriptionProvider) -> None: ) -def list_providers() -> List[TranscriptionProvider]: +def list_providers(*, scope: Optional[str] = None) -> List[TranscriptionProvider]: """Return all registered providers, sorted by name.""" with _lock: - items = list(_providers.values()) + merged = dict(_providers) + merged.update(_scoped_providers.get(scope or hermes_home_key(), {})) + items = list(merged.values()) return sorted(items, key=lambda p: p.name) -def get_provider(name: str) -> Optional[TranscriptionProvider]: +def get_provider(name: str, *, scope: Optional[str] = None) -> Optional[TranscriptionProvider]: """Return the provider registered under *name*, or None. Name matching is case-insensitive and whitespace-tolerant — mirrors @@ -115,23 +120,39 @@ def get_provider(name: str) -> Optional[TranscriptionProvider]: """ if not isinstance(name, str): return None - return _providers.get(name.strip().lower()) + key = name.strip().lower() + with _lock: + return _scoped_providers.get(scope or hermes_home_key(), {}).get(key) or _providers.get(key) + + +def snapshot_registration( + name: str, *, scope: Optional[str] = None +) -> Optional[TranscriptionProvider]: + key = name.strip().lower() + with _lock: + target = _providers if scope is None else _scoped_providers.get(scope, {}) + return target.get(key) def restore_registration( name: str, current: TranscriptionProvider, previous: Optional[TranscriptionProvider], + *, + scope: Optional[str] = None, ) -> bool: """Restore a plugin registration only when *current* is still installed.""" key = name.strip().lower() with _lock: - if _providers.get(key) is not current: + target = _providers if scope is None else _scoped_providers.setdefault(scope, {}) + if target.get(key) is not current: return False if previous is None: - _providers.pop(key, None) + target.pop(key, None) else: - _providers[key] = previous + target[key] = previous + if scope is not None and not target: + _scoped_providers.pop(scope, None) return True @@ -139,3 +160,4 @@ def _reset_for_tests() -> None: """Clear the registry. **Test-only.**""" with _lock: _providers.clear() + _scoped_providers.clear() diff --git a/agent/tts_registry.py b/agent/tts_registry.py index e3671dd7d5..5e8f94b459 100644 --- a/agent/tts_registry.py +++ b/agent/tts_registry.py @@ -33,6 +33,7 @@ import threading from typing import Dict, List, Optional from agent.tts_provider import TTSProvider +from hermes_constants import hermes_home_key logger = logging.getLogger(__name__) @@ -61,10 +62,11 @@ _BUILTIN_NAMES = frozenset({ _providers: Dict[str, TTSProvider] = {} +_scoped_providers: Dict[str, Dict[str, TTSProvider]] = {} _lock = threading.Lock() -def register_provider(provider: TTSProvider) -> None: +def register_provider(provider: TTSProvider, *, scope: Optional[str] = None) -> None: """Register a TTS provider. Rejects: @@ -95,8 +97,9 @@ def register_provider(provider: TTSProvider) -> None: ) return with _lock: - existing = _providers.get(key) - _providers[key] = provider + target = _providers if scope is None else _scoped_providers.setdefault(scope, {}) + existing = target.get(key) + target[key] = provider if existing is not None: logger.debug( "TTS provider '%s' re-registered (was %r)", @@ -109,14 +112,16 @@ def register_provider(provider: TTSProvider) -> None: ) -def list_providers() -> List[TTSProvider]: +def list_providers(*, scope: Optional[str] = None) -> List[TTSProvider]: """Return all registered providers, sorted by name.""" with _lock: - items = list(_providers.values()) + merged = dict(_providers) + merged.update(_scoped_providers.get(scope or hermes_home_key(), {})) + items = list(merged.values()) return sorted(items, key=lambda p: p.name) -def get_provider(name: str) -> Optional[TTSProvider]: +def get_provider(name: str, *, scope: Optional[str] = None) -> Optional[TTSProvider]: """Return the provider registered under *name*, or None. Name matching is case-insensitive and whitespace-tolerant — mirrors @@ -125,23 +130,39 @@ def get_provider(name: str) -> Optional[TTSProvider]: """ if not isinstance(name, str): return None - return _providers.get(name.strip().lower()) + key = name.strip().lower() + with _lock: + return _scoped_providers.get(scope or hermes_home_key(), {}).get(key) or _providers.get(key) + + +def snapshot_registration( + name: str, *, scope: Optional[str] = None +) -> Optional[TTSProvider]: + key = name.strip().lower() + with _lock: + target = _providers if scope is None else _scoped_providers.get(scope, {}) + return target.get(key) def restore_registration( name: str, current: TTSProvider, previous: Optional[TTSProvider], + *, + scope: Optional[str] = None, ) -> bool: """Restore a plugin registration only when *current* is still installed.""" key = name.strip().lower() with _lock: - if _providers.get(key) is not current: + target = _providers if scope is None else _scoped_providers.setdefault(scope, {}) + if target.get(key) is not current: return False if previous is None: - _providers.pop(key, None) + target.pop(key, None) else: - _providers[key] = previous + target[key] = previous + if scope is not None and not target: + _scoped_providers.pop(scope, None) return True @@ -149,3 +170,4 @@ def _reset_for_tests() -> None: """Clear the registry. **Test-only.**""" with _lock: _providers.clear() + _scoped_providers.clear() diff --git a/agent/video_gen_registry.py b/agent/video_gen_registry.py index 51559c2e34..ff46b087c0 100644 --- a/agent/video_gen_registry.py +++ b/agent/video_gen_registry.py @@ -29,15 +29,17 @@ import threading from typing import Dict, List, Optional from agent.video_gen_provider import VideoGenProvider +from hermes_constants import hermes_home_key logger = logging.getLogger(__name__) _providers: Dict[str, VideoGenProvider] = {} +_scoped_providers: Dict[str, Dict[str, VideoGenProvider]] = {} _lock = threading.Lock() -def register_provider(provider: VideoGenProvider) -> None: +def register_provider(provider: VideoGenProvider, *, scope: Optional[str] = None) -> None: """Register a video generation provider. Re-registration (same ``name``) overwrites the previous entry and logs @@ -49,46 +51,65 @@ def register_provider(provider: VideoGenProvider) -> None: f"register_provider() expects a VideoGenProvider instance, " f"got {type(provider).__name__}" ) - name = provider.name - if not isinstance(name, str) or not name.strip(): + raw_name = provider.name + if not isinstance(raw_name, str) or not raw_name.strip(): raise ValueError("Video gen provider .name must be a non-empty string") + name = raw_name.strip() with _lock: - existing = _providers.get(name) - _providers[name] = provider + target = _providers if scope is None else _scoped_providers.setdefault(scope, {}) + existing = target.get(name) + target[name] = provider if existing is not None: logger.debug("Video gen provider '%s' re-registered (was %r)", name, type(existing).__name__) else: logger.debug("Registered video gen provider '%s' (%s)", name, type(provider).__name__) -def list_providers() -> List[VideoGenProvider]: +def list_providers(*, scope: Optional[str] = None) -> List[VideoGenProvider]: """Return all registered providers, sorted by name.""" with _lock: - items = list(_providers.values()) + merged = dict(_providers) + merged.update(_scoped_providers.get(scope or hermes_home_key(), {})) + items = list(merged.values()) return sorted(items, key=lambda p: p.name) -def get_provider(name: str) -> Optional[VideoGenProvider]: +def get_provider(name: str, *, scope: Optional[str] = None) -> Optional[VideoGenProvider]: """Return the provider registered under *name*, or None.""" if not isinstance(name, str): return None with _lock: - return _providers.get(name.strip()) + key = name.strip() + return _scoped_providers.get(scope or hermes_home_key(), {}).get(key) or _providers.get(key) + + +def snapshot_registration( + name: str, *, scope: Optional[str] = None +) -> Optional[VideoGenProvider]: + with _lock: + target = _providers if scope is None else _scoped_providers.get(scope, {}) + return target.get(name.strip()) def restore_registration( name: str, current: VideoGenProvider, previous: Optional[VideoGenProvider], + *, + scope: Optional[str] = None, ) -> bool: """Restore a plugin registration only when *current* is still installed.""" + key = name.strip() with _lock: - if _providers.get(name) is not current: + target = _providers if scope is None else _scoped_providers.setdefault(scope, {}) + if target.get(key) is not current: return False if previous is None: - _providers.pop(name, None) + target.pop(key, None) else: - _providers[name] = previous + target[key] = previous + if scope is not None and not target: + _scoped_providers.pop(scope, None) return True @@ -113,6 +134,7 @@ def get_active_provider() -> Optional[VideoGenProvider]: with _lock: snapshot = dict(_providers) + snapshot.update(_scoped_providers.get(hermes_home_key(), {})) if configured: provider = snapshot.get(configured) @@ -147,3 +169,4 @@ def _reset_for_tests() -> None: """Clear the registry. **Test-only.**""" with _lock: _providers.clear() + _scoped_providers.clear() diff --git a/agent/web_search_registry.py b/agent/web_search_registry.py index d182fce050..2e0c116ec0 100644 --- a/agent/web_search_registry.py +++ b/agent/web_search_registry.py @@ -37,15 +37,17 @@ import threading from typing import Dict, List, Optional from agent.web_search_provider import WebSearchProvider +from hermes_constants import hermes_home_key logger = logging.getLogger(__name__) _providers: Dict[str, WebSearchProvider] = {} +_scoped_providers: Dict[str, Dict[str, WebSearchProvider]] = {} _lock = threading.Lock() -def register_provider(provider: WebSearchProvider) -> None: +def register_provider(provider: WebSearchProvider, *, scope: Optional[str] = None) -> None: """Register a web search/extract provider. Re-registration (same ``name``) overwrites the previous entry and logs @@ -57,12 +59,14 @@ def register_provider(provider: WebSearchProvider) -> None: f"register_provider() expects a WebSearchProvider instance, " f"got {type(provider).__name__}" ) - name = provider.name - if not isinstance(name, str) or not name.strip(): + raw_name = provider.name + if not isinstance(raw_name, str) or not raw_name.strip(): raise ValueError("Web provider .name must be a non-empty string") + name = raw_name.strip() with _lock: - existing = _providers.get(name) - _providers[name] = provider + target = _providers if scope is None else _scoped_providers.setdefault(scope, {}) + existing = target.get(name) + target[name] = provider if existing is not None: logger.debug( "Web provider '%s' re-registered (was %r)", @@ -75,34 +79,51 @@ def register_provider(provider: WebSearchProvider) -> None: ) -def list_providers() -> List[WebSearchProvider]: +def list_providers(*, scope: Optional[str] = None) -> List[WebSearchProvider]: """Return all registered providers, sorted by name.""" with _lock: - items = list(_providers.values()) + merged = dict(_providers) + merged.update(_scoped_providers.get(scope or hermes_home_key(), {})) + items = list(merged.values()) return sorted(items, key=lambda p: p.name) -def get_provider(name: str) -> Optional[WebSearchProvider]: +def get_provider(name: str, *, scope: Optional[str] = None) -> Optional[WebSearchProvider]: """Return the provider registered under *name*, or None.""" if not isinstance(name, str): return None with _lock: - return _providers.get(name.strip()) + key = name.strip() + return _scoped_providers.get(scope or hermes_home_key(), {}).get(key) or _providers.get(key) + + +def snapshot_registration( + name: str, *, scope: Optional[str] = None +) -> Optional[WebSearchProvider]: + with _lock: + target = _providers if scope is None else _scoped_providers.get(scope, {}) + return target.get(name.strip()) def restore_registration( name: str, current: WebSearchProvider, previous: Optional[WebSearchProvider], + *, + scope: Optional[str] = None, ) -> bool: """Restore a plugin registration only when *current* is still installed.""" + key = name.strip() with _lock: - if _providers.get(name) is not current: + target = _providers if scope is None else _scoped_providers.setdefault(scope, {}) + if target.get(key) is not current: return False if previous is None: - _providers.pop(name, None) + target.pop(key, None) else: - _providers[name] = previous + target[key] = previous + if scope is not None and not target: + _scoped_providers.pop(scope, None) return True @@ -178,6 +199,7 @@ def _resolve(configured: Optional[str], *, capability: str) -> Optional[WebSearc """ with _lock: snapshot = dict(_providers) + snapshot.update(_scoped_providers.get(hermes_home_key(), {})) def _capable(p: WebSearchProvider) -> bool: if capability == "search": @@ -318,3 +340,4 @@ def _reset_for_tests() -> None: """Clear the registry. **Test-only.**""" with _lock: _providers.clear() + _scoped_providers.clear() diff --git a/gateway/authz_mixin.py b/gateway/authz_mixin.py index 1b3103f49f..fae4b74a5e 100644 --- a/gateway/authz_mixin.py +++ b/gateway/authz_mixin.py @@ -564,6 +564,7 @@ class GatewayAuthorizationMixin: if source.platform not in platform_env_map: try: from gateway.platform_registry import platform_registry + entry = platform_registry.get(source.platform.value) if entry: if entry.allowed_users_env: diff --git a/gateway/platform_registry.py b/gateway/platform_registry.py index 82f7e15d02..541fb75696 100644 --- a/gateway/platform_registry.py +++ b/gateway/platform_registry.py @@ -29,12 +29,36 @@ Usage (gateway side): """ import logging +import sys +import threading from dataclasses import dataclass, field from typing import Any, Awaitable, Callable, Optional +from hermes_constants import hermes_home_key + logger = logging.getLogger(__name__) +def _plugin_scope_from_callable(callback: Callable) -> Optional[str]: + """Infer a plugin profile from code registered outside PluginContext.""" + try: + from tools.registry import registry as tool_registry + + return tool_registry.plugin_scope_for_callable(callback) + except (ImportError, AttributeError): + return None + + +def _caller_plugin_scope() -> Optional[str]: + try: + module_name = sys._getframe(2).f_globals.get("__name__", "") or "" + except Exception: + return None + return _plugin_scope_from_callable( + type("_Caller", (), {"__module__": module_name}) + ) + + @dataclass class PlatformEntry: """Metadata and factory for a single platform adapter.""" @@ -208,12 +232,17 @@ class PlatformEntry: class PlatformRegistry: """Central registry of platform adapters. - Thread-safe for reads (dict lookups are atomic under GIL). - Writes happen at startup during sequential discovery. + Registrations are serialized, and concurrent lazy lookups share an + in-flight event while the loader runs outside the registry lock. """ def __init__(self) -> None: + self._lock = threading.RLock() + # Process-global registrations (for example the built-in relay). self._entries: dict[str, PlatformEntry] = {} + # Plugin adapters are isolated per resolved HERMES_HOME and overlay the + # process-global entries for lookups in that profile's runtime scope. + self._scoped_entries: dict[str, dict[str, PlatformEntry]] = {} # Deferred platform loaders: name -> zero-arg callable that imports the # owning plugin module (which calls register() and populates _entries). # @@ -227,10 +256,50 @@ class PlatformRegistry: # actually asks for that platform (gateway start, cron delivery, # `hermes setup`/`gateway status`, send_message). self._deferred: dict[str, Callable[[], None]] = {} + self._scoped_deferred: dict[str, dict[str, Callable[[], None]]] = {} + self._inflight: dict[tuple[Optional[str], str], threading.Event] = {} + self._inflight_loaders: dict[ + tuple[Optional[str], str], Callable[[], None] + ] = {} + self._inflight_owners: dict[tuple[Optional[str], str], int] = {} + self._cancelled_inflight: set[tuple[Optional[str], str]] = set() + # A failed loader is no longer discoverable, but its identity remains + # until ownership teardown can CAS-restore the displaced predecessor. + self._consumed_loaders: dict[ + tuple[Optional[str], str], Callable[[], None] + ] = {} + + @staticmethod + def current_scope_key() -> str: + return hermes_home_key() + + def _scope_maps( + self, + scope: Optional[str], + *, + create: bool = False, + ) -> tuple[dict[str, PlatformEntry], dict[str, Callable[[], None]]]: + if scope is None: + return self._entries, self._deferred + if create: + return ( + self._scoped_entries.setdefault(scope, {}), + self._scoped_deferred.setdefault(scope, {}), + ) + return ( + self._scoped_entries.get(scope, {}), + self._scoped_deferred.get(scope, {}), + ) # -- deferred loading ---------------------------------------------------- - def register_deferred(self, name: str, loader: Callable[[], None]) -> None: + def register_deferred( + self, + name: str, + loader: Callable[[], None], + *, + scope: Optional[str] = None, + ) -> None: """Register a lazy loader for a platform that hasn't been imported yet. *loader* is a zero-arg callable that imports the owning plugin module, @@ -240,14 +309,19 @@ class PlatformRegistry: registered directly (e.g. a built-in) takes precedence -- the deferred loader is then dropped. """ - if name in self._entries: - # Already concretely registered; no need to defer. - return - self._deferred[name] = loader + with self._lock: + entries, deferred = self._scope_maps(scope, create=True) + self._consumed_loaders.pop((scope, name), None) + if name in entries: + # Already concretely registered; no need to defer. + return + deferred[name] = loader def snapshot_registration( self, name: str, + *, + scope: Optional[str] = None, ) -> tuple[Optional[PlatformEntry], Optional[Callable[[], None]]]: """Return the concrete and deferred state for *name* without resolving it. @@ -256,13 +330,22 @@ class PlatformRegistry: importing the displaced adapter as a side effect of taking the snapshot. """ - return self._entries.get(name), self._deferred.get(name) + with self._lock: + entries, deferred = self._scope_maps(scope) + loader = deferred.get(name) + if entries.get(name) is None and loader is None: + loader = self._inflight_loaders.get((scope, name)) + if entries.get(name) is None and loader is None: + loader = self._consumed_loaders.get((scope, name)) + return entries.get(name), loader def restore_registration( self, name: str, current: tuple[Optional[PlatformEntry], Optional[Callable[[], None]]], previous: tuple[Optional[PlatformEntry], Optional[Callable[[], None]]], + *, + scope: Optional[str] = None, ) -> bool: """Restore a platform registration if its full state is still current. @@ -271,29 +354,89 @@ class PlatformRegistry: it displaced. Both concrete entries and deferred loaders are part of the state because bundled platform plugins load lazily. """ - current_state = self.snapshot_registration(name) - if ( - current_state[0] is not current[0] - or current_state[1] is not current[1] - ): - return False + with self._lock: + entries, deferred = self._scope_maps(scope, create=True) + entry = entries.get(name) + loader = deferred.get(name) + load_key = (scope, name) + if entry is None and loader is None: + loader = self._inflight_loaders.get(load_key) + if entry is None and loader is None: + loader = self._consumed_loaders.get(load_key) + current_state = (entry, loader) + is_current = not ( + current_state[0] is not current[0] + or current_state[1] is not current[1] + ) + if not is_current: + return False - entry, loader = previous - if entry is None: - self._entries.pop(name, None) - else: - self._entries[name] = entry - if loader is None: - self._deferred.pop(name, None) - else: - self._deferred[name] = loader - return True + previous_entry, previous_loader = previous + if previous_entry is None: + entries.pop(name, None) + else: + entries[name] = previous_entry + if previous_loader is None: + deferred.pop(name, None) + else: + deferred[name] = previous_loader + if load_key in self._inflight: + self._cancelled_inflight.add(load_key) + self._consumed_loaders.pop(load_key, None) + if scope is not None: + if not entries: + self._scoped_entries.pop(scope, None) + if not deferred: + self._scoped_deferred.pop(scope, None) + return True - def _resolve(self, name: str) -> None: + def _resolve(self, name: str, scope: Optional[str] = None) -> None: """Run the deferred loader for *name* if one is pending.""" - loader = self._deferred.pop(name, None) - if loader is None: + loader: Optional[Callable[[], None]] = None + event: Optional[threading.Event] = None + load_key: tuple[Optional[str], str] + is_loader = False + with self._lock: + active_scope = scope or self.current_scope_key() + entries, deferred = self._scope_maps(active_scope) + scoped_key = (active_scope, name) + global_key = (None, name) + event = self._inflight.get(scoped_key) + load_key = scoped_key + if event is None and name not in entries: + loader = deferred.pop(name, None) + if event is None and loader is None and name not in entries: + event = self._inflight.get(global_key) + load_key = global_key + if event is None and loader is None and name not in entries: + loader = self._deferred.pop(name, None) + load_key = global_key + if event is None and loader is not None: + event = threading.Event() + self._inflight[load_key] = event + self._inflight_loaders[load_key] = loader + self._inflight_owners[load_key] = threading.get_ident() + is_loader = True + if event is None: + return + if ( + not is_loader + and self._inflight_owners.get(load_key) == threading.get_ident() + ): + logger.warning( + "Deferred platform '%s' recursively requested while loading", + name, + ) + return + + if not is_loader: + event.wait() + # Teardown may have restored an older deferred generation while + # cancelling the one we waited for. Resolve that predecessor in + # the same lookup instead of returning a one-shot false negative. + self._resolve(name, active_scope) return + try: loader() except Exception as e: @@ -303,6 +446,34 @@ class PlatformRegistry: e, exc_info=True, ) + finally: + with self._lock: + was_cancelled = load_key in self._cancelled_inflight + load_scope, _load_name = load_key + entries, deferred = self._scope_maps(load_scope) + if ( + not was_cancelled + and name not in entries + and name not in deferred + ): + self._consumed_loaders[load_key] = loader + self._inflight.pop(load_key, None) + self._inflight_loaders.pop(load_key, None) + self._inflight_owners.pop(load_key, None) + self._cancelled_inflight.discard(load_key) + event.set() + if was_cancelled: + self._resolve(name, active_scope) + + def is_deferred_load_cancelled( + self, + name: str, + *, + scope: Optional[str] = None, + ) -> bool: + """Return whether ownership teardown cancelled an in-flight loader.""" + with self._lock: + return (scope, name) in self._cancelled_inflight def _resolve_all(self) -> None: """Run every pending deferred loader. @@ -312,59 +483,119 @@ class PlatformRegistry: gateway startup, ``hermes setup``/``gateway status``, channel directory. CLI chat never iterates the full set. """ - if not self._deferred: - return - # Snapshot keys -- loaders mutate _deferred as they resolve. - for name in list(self._deferred): - self._resolve(name) + active_scope = self.current_scope_key() + with self._lock: + _entries, scoped_deferred = self._scope_maps(active_scope) + scoped_names = set(scoped_deferred) + global_names = set(self._deferred) + for inflight_scope, name in self._inflight: + if inflight_scope == active_scope: + scoped_names.add(name) + elif inflight_scope is None: + global_names.add(name) + # Load outside the registry lock; each name has an in-flight event so + # concurrent readers wait for the same materialization. + for name in sorted(scoped_names): + self._resolve(name, active_scope) + for name in sorted(global_names): + self._resolve(name, active_scope) - def register(self, entry: PlatformEntry) -> None: + def register( + self, + entry: PlatformEntry, + *, + scope: Optional[str] = None, + ) -> None: """Register a platform adapter entry. If an entry with the same name exists, it is replaced (last writer wins -- this lets plugins override built-in adapters if desired). """ - # A concrete registration supersedes any pending deferred loader. - self._deferred.pop(entry.name, None) - if entry.name in self._entries: - prev = self._entries[entry.name] - logger.info( - "Platform '%s' re-registered (was %s, now %s)", - entry.name, - prev.source, - entry.source, - ) - self._entries[entry.name] = entry - logger.debug("Registered platform adapter: %s (%s)", entry.name, entry.source) + with self._lock: + if scope is None and entry.source == "plugin": + scope = _caller_plugin_scope() + if scope is None: + scope = _plugin_scope_from_callable(entry.adapter_factory) + if scope is None: + scope = _plugin_scope_from_callable(entry.check_fn) + # A concrete registration supersedes any pending deferred loader. + entries, deferred = self._scope_maps(scope, create=True) + self._consumed_loaders.pop((scope, entry.name), None) + deferred.pop(entry.name, None) + if entry.name in entries: + prev = entries[entry.name] + logger.info( + "Platform '%s' re-registered (was %s, now %s)", + entry.name, + prev.source, + entry.source, + ) + entries[entry.name] = entry + logger.debug("Registered platform adapter: %s (%s)", entry.name, entry.source) - def unregister(self, name: str) -> bool: + def unregister(self, name: str, *, scope: Optional[str] = None) -> bool: """Remove a platform entry. Returns True if it existed.""" - self._deferred.pop(name, None) - removed = self._entries.pop(name, None) is not None - return removed + with self._lock: + inferred_scope = scope if scope is not None else _caller_plugin_scope() + active_scope = inferred_scope or self.current_scope_key() + entries, deferred = self._scope_maps(active_scope) + if inferred_scope is not None or name in entries or name in deferred: + deferred.pop(name, None) + removed = entries.pop(name, None) is not None + if not entries: + self._scoped_entries.pop(active_scope, None) + if not deferred: + self._scoped_deferred.pop(active_scope, None) + return removed + self._deferred.pop(name, None) + return self._entries.pop(name, None) is not None def get(self, name: str) -> Optional[PlatformEntry]: """Look up a platform entry by name.""" - if name not in self._entries: - self._resolve(name) - return self._entries.get(name) + scope = self.current_scope_key() + with self._lock: + entries, deferred = self._scope_maps(scope) + needs_resolve = name not in entries and ( + name in deferred + or (name not in self._entries and name in self._deferred) + or (scope, name) in self._inflight + or (None, name) in self._inflight + ) + if needs_resolve: + self._resolve(name, scope) + with self._lock: + entries, _deferred = self._scope_maps(scope) + return entries.get(name) or self._entries.get(name) def all_entries(self) -> list[PlatformEntry]: """Return all registered platform entries.""" self._resolve_all() - return list(self._entries.values()) + with self._lock: + entries = dict(self._entries) + entries.update(self._scoped_entries.get(self.current_scope_key(), {})) + return list(entries.values()) def plugin_entries(self) -> list[PlatformEntry]: """Return only plugin-registered platform entries.""" self._resolve_all() - return [e for e in self._entries.values() if e.source == "plugin"] + return [e for e in self.all_entries() if e.source == "plugin"] def is_registered(self, name: str) -> bool: # A deferred (not-yet-imported) platform still counts as registered -- # the loader will materialize it on first real use. This keeps cheap # membership checks (toolset resolution, webhook deliver-target checks) # from triggering a heavy import. - return name in self._entries or name in self._deferred + with self._lock: + scope = self.current_scope_key() + entries, deferred = self._scope_maps(scope) + return ( + name in entries + or name in deferred + or name in self._entries + or name in self._deferred + or (scope, name) in self._inflight + or (None, name) in self._inflight + ) def create_adapter(self, name: str, config: Any) -> Optional[Any]: """Create an adapter instance for the given platform name. @@ -376,9 +607,7 @@ class PlatformRegistry: - validate_config() returns False (misconfigured) - The factory raises an exception """ - if name not in self._entries: - self._resolve(name) - entry = self._entries.get(name) + entry = self.get(name) if entry is None: return None diff --git a/gateway/run.py b/gateway/run.py index 09d2352134..a817919915 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -13778,6 +13778,9 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew with _profile_runtime_scope(profile_home): profile_runtime_cfg = _load_gateway_runtime_config() + from hermes_cli.plugins import discover_plugins + + discover_plugins() profile_cfg = load_gateway_config() violation = _own_policy_open_startup_violation(profile_cfg) self._snapshot_profile_busy_modes(profile_name, profile_runtime_cfg) diff --git a/hermes_cli/dashboard_auth/registry.py b/hermes_cli/dashboard_auth/registry.py index d9f9047547..5ad0df82c9 100644 --- a/hermes_cli/dashboard_auth/registry.py +++ b/hermes_cli/dashboard_auth/registry.py @@ -10,6 +10,7 @@ import logging import threading from typing import List, Optional +from hermes_constants import hermes_home_key from hermes_cli.dashboard_auth.base import ( DashboardAuthProvider, assert_protocol_compliance, @@ -18,9 +19,20 @@ from hermes_cli.dashboard_auth.base import ( _log = logging.getLogger(__name__) _lock = threading.Lock() _providers: dict[str, DashboardAuthProvider] = {} +_scoped_providers: dict[str, dict[str, DashboardAuthProvider]] = {} -def register_provider(provider: DashboardAuthProvider) -> None: +def _merged(scope: Optional[str] = None) -> dict[str, DashboardAuthProvider]: + providers = dict(_providers) + providers.update(_scoped_providers.get(scope or hermes_home_key(), {})) + return providers + + +def register_provider( + provider: DashboardAuthProvider, + *, + scope: Optional[str] = None, +) -> None: """Register a provider. Raises: @@ -29,43 +41,64 @@ def register_provider(provider: DashboardAuthProvider) -> None: """ assert_protocol_compliance(type(provider)) with _lock: - if provider.name in _providers: + target = _providers if scope is None else _scoped_providers.setdefault(scope, {}) + effective = target if scope is None else _merged(scope) + if provider.name in effective: raise ValueError( f"dashboard-auth provider already registered: {provider.name!r}" ) - _providers[provider.name] = provider + target[provider.name] = provider _log.info( "dashboard-auth: registered provider %r (%s)", provider.name, provider.display_name, ) -def get_provider(name: str) -> Optional[DashboardAuthProvider]: +def get_provider( + name: str, + *, + scope: Optional[str] = None, +) -> Optional[DashboardAuthProvider]: """Return the registered provider for ``name``, or None if unknown.""" with _lock: - return _providers.get(name) + return _merged(scope).get(name) + + +def snapshot_registration( + name: str, + *, + scope: Optional[str] = None, +) -> Optional[DashboardAuthProvider]: + with _lock: + target = _providers if scope is None else _scoped_providers.get(scope, {}) + return target.get(name) def restore_registration( name: str, current: DashboardAuthProvider, previous: Optional[DashboardAuthProvider], + *, + scope: Optional[str] = None, ) -> bool: """Restore a host-owned provider registration if it is still current.""" with _lock: - if _providers.get(name) is not current: + target = _providers if scope is None else _scoped_providers.setdefault(scope, {}) + if target.get(name) is not current: return False if previous is None: - _providers.pop(name, None) + target.pop(name, None) else: - _providers[name] = previous + target[name] = previous + if scope is not None and not target: + _scoped_providers.pop(scope, None) return True -def list_providers() -> List[DashboardAuthProvider]: +def list_providers(*, scope: Optional[str] = None) -> List[DashboardAuthProvider]: """All registered providers, in registration order.""" with _lock: - return list(_providers.values()) + return list(_merged(scope).values()) def list_token_providers() -> List[DashboardAuthProvider]: @@ -78,8 +111,7 @@ def list_token_providers() -> List[DashboardAuthProvider]: no token provider is registered — a token-authable route then fails closed (401), never open. """ - with _lock: - return [p for p in _providers.values() if getattr(p, "supports_token", False)] + return [p for p in list_providers() if getattr(p, "supports_token", False)] def list_session_providers() -> List[DashboardAuthProvider]: @@ -87,11 +119,11 @@ def list_session_providers() -> List[DashboardAuthProvider]: sessions). The login page, /auth/login, and the gate's verify/refresh loops consult only these. Mirror of list_token_providers. """ - with _lock: - return [p for p in _providers.values() if getattr(p, "supports_session", True)] + return [p for p in list_providers() if getattr(p, "supports_session", True)] def clear_providers() -> None: """Test-only: drop all registrations.""" with _lock: _providers.clear() + _scoped_providers.clear() diff --git a/hermes_cli/env_loader.py b/hermes_cli/env_loader.py index 0f0a98c4f0..311a30647f 100644 --- a/hermes_cli/env_loader.py +++ b/hermes_cli/env_loader.py @@ -685,7 +685,9 @@ def _apply_external_secret_sources(home_path: Path) -> None: ) if src.result.error: print(f" {src.label}: {src.result.error}", file=sys.stderr) - hint = _remediation_hint(src.name, src.result.error_kind, cfg) + hint = _remediation_hint( + src.name, src.result.error_kind, cfg, scope=home_key + ) if hint: print(f" {src.label}: → {hint}", file=sys.stderr) for warn in src.result.warnings: @@ -694,7 +696,13 @@ def _apply_external_secret_sources(home_path: Path) -> None: print(f" Secret sources: {conflict}", file=sys.stderr) -def _remediation_hint(source_name: str, error_kind, secrets_cfg: dict) -> str: +def _remediation_hint( + source_name: str, + error_kind, + secrets_cfg: dict, + *, + scope: str | None = None, +) -> str: """Ask the failed source for its one-line fix-it hint. Defensive wrapper: remediation() is a pure mapping and shouldn't @@ -704,7 +712,7 @@ def _remediation_hint(source_name: str, error_kind, secrets_cfg: dict) -> str: try: from agent.secret_sources.registry import get_source - source = get_source(source_name) + source = get_source(source_name, scope=scope) if source is None: return "" src_cfg = secrets_cfg.get(source_name) diff --git a/hermes_cli/plugins.py b/hermes_cli/plugins.py index 8dec25c520..6b24481439 100644 --- a/hermes_cli/plugins.py +++ b/hermes_cli/plugins.py @@ -49,11 +49,17 @@ import threading import types from contextlib import contextmanager from dataclasses import dataclass, field +from functools import wraps from pathlib import Path from typing import Any, Callable, Dict, List, Mapping, Optional, Set, Union - -from hermes_constants import get_hermes_home +from hermes_constants import ( + get_hermes_home, + hermes_home_key, + reset_hermes_home_override, + set_hermes_home_override, +) +from registration_lifecycle import replacement_coordinator from utils import env_var_enabled, fast_safe_load from hermes_cli.config import cfg_get, load_config_readonly from hermes_cli.middleware import OBSERVER_SCHEMA_VERSION, VALID_MIDDLEWARE @@ -311,6 +317,28 @@ _EVENT_PENDING_CAP = 64 _EVENT_WORKER_STOP = object() _NS_PARENT = "hermes_plugins" +_MODULE_NAMESPACE_LOCK = threading.RLock() +_BARE_MODULE_SCOPE: Dict[str, str] = {} + + +def _serialized_replacement(method): + """Make snapshot → write → lease attachment one atomic transaction.""" + @wraps(method) + def wrapped(*args, **kwargs): + with replacement_coordinator.transaction(): + return method(*args, **kwargs) + + return wrapped + + +@contextmanager +def _plugin_home_scope(home: Path): + """Bind discovery and loading to the manager's immutable Hermes home.""" + token = set_hermes_home_override(home) + try: + yield + finally: + reset_hermes_home_override(token) def _env_enabled(name: str) -> bool: @@ -1162,6 +1190,27 @@ class PluginContext: self.manifest, kind, key, release ) + def _track_replacement( + self, + kind: str, + key: str, + *, + slot: tuple, + current: Any, + previous: Any, + restore: Callable[[Any], bool], + finalize: Optional[Callable[[], None]] = None, + ) -> PluginRegistration: + """Track one generation in a replaceable registration slot.""" + lease = replacement_coordinator.acquire( + slot, + current=current, + previous=previous, + restore=restore, + finalize=finalize, + ) + return self._track(kind, key, lease.dispose) + # -- host-owned LLM access ---------------------------------------------- @property @@ -1240,6 +1289,7 @@ class PluginContext: # -- tool registration -------------------------------------------------- + @_serialized_replacement def register_tool( self, name: str, @@ -1279,7 +1329,16 @@ class PluginContext: from tools.registry import registry - previous = registry.get_entry(name) + scope = self._manager.scope_key + previous = registry.snapshot_registration(name, scope=scope) + effective = registry.get_entry(name, scope=scope) + if previous is None and effective is not None and not override: + logger.warning( + "Plugin %s tried to shadow global tool %s without override=True", + self.manifest.name, + name, + ) + return None registry.register( name=name, toolset=toolset, @@ -1291,15 +1350,29 @@ class PluginContext: description=description, emoji=emoji, override=override, + scope=scope, ) - registered = registry.get_entry(name) - if registered is not None and registered.handler is handler: + registered = registry.snapshot_registration(name, scope=scope) + if ( + registered is not None + and registered is not previous + and registered.handler is handler + ): self._manager._plugin_tool_names.add(name) - def _release_tool() -> None: - registry.restore_registration(name, registered, previous) - self._manager._remove_tool_name_if_unowned(name) + def _restore_tool(replacement: Any) -> bool: + return registry.restore_registration( + name, registered, replacement, scope=scope + ) - handle = self._track("tool", name, _release_tool) + handle = self._track_replacement( + "tool", + name, + slot=("tool", scope, name), + current=registered, + previous=previous, + restore=_restore_tool, + finalize=lambda: self._manager._remove_tool_name_if_unowned(name), + ) else: handle = None logger.debug( @@ -1343,10 +1416,21 @@ class PluginContext: source = getattr(self.manifest, "source", "") or "" if source == "bundled": return True + try: + from hermes_cli.config import load_config + + with _plugin_home_scope(self._manager.home_path): + cfg = load_config() or {} + except Exception: + # If we can't load config, fail closed — better to break the + # override than silently grant it. + return False plugin_id = self.manifest.key or self.manifest.name # Fail-closed by construction: any failure to read consent state - # inside plugin_capability_granted returns False. - return plugin_capability_granted(plugin_id, "tools.override") + # inside plugin_capability_granted returns False. The profile-scoped + # config is passed through so a multi-profile process consults THIS + # manager's home, never the active profile's (#65593 constraint). + return plugin_capability_granted(plugin_id, "tools.override", config=cfg) # -- message injection -------------------------------------------------- @@ -1442,6 +1526,7 @@ class PluginContext: # -- CLI command registration -------------------------------------------- + @_serialized_replacement def register_cli_command( self, name: str, @@ -1466,10 +1551,14 @@ class PluginContext: "plugin_key": self.manifest.key or self.manifest.name, } self._manager._cli_commands[name] = entry - handle = self._track( - "cli_command", name, - lambda: self._manager._restore_mapping( - self._manager._cli_commands, name, entry, previous + handle = self._track_replacement( + "cli_command", + name, + slot=("manager_mapping", id(self._manager._cli_commands), name), + current=entry, + previous=previous, + restore=lambda replacement: self._manager._restore_mapping( + self._manager._cli_commands, name, entry, replacement ), ) logger.debug("Plugin %s registered CLI command: %s", self.manifest.name, name) @@ -1477,6 +1566,7 @@ class PluginContext: # -- slash command registration ------------------------------------------- + @_serialized_replacement def register_command( self, name: str, @@ -1532,10 +1622,14 @@ class PluginContext: "args_hint": (args_hint or "").strip(), } self._manager._plugin_commands[clean] = entry - handle = self._track( - "command", clean, - lambda: self._manager._restore_mapping( - self._manager._plugin_commands, clean, entry, previous + handle = self._track_replacement( + "command", + clean, + slot=("manager_mapping", id(self._manager._plugin_commands), clean), + current=entry, + previous=previous, + restore=lambda replacement: self._manager._restore_mapping( + self._manager._plugin_commands, clean, entry, replacement ), ) logger.debug("Plugin %s registered command: /%s", self.manifest.name, clean) @@ -1570,10 +1664,13 @@ class PluginContext: if agent is not None: kwargs["parent_agent"] = agent - return registry.dispatch(tool_name, args, **kwargs) + return registry.dispatch( + tool_name, args, scope=self._manager.scope_key, **kwargs + ) # -- context engine registration ----------------------------------------- + @_serialized_replacement def register_context_engine(self, engine) -> Optional[PluginRegistration]: """Register a context engine to replace the built-in ContextCompressor. @@ -1600,10 +1697,14 @@ class PluginContext: return previous = self._manager._context_engine self._manager._context_engine = engine - handle = self._track( - "context_engine", engine.name, - lambda: self._manager._restore_value( - "_context_engine", engine, previous + handle = self._track_replacement( + "context_engine", + engine.name, + slot=("manager_value", id(self._manager), "_context_engine"), + current=engine, + previous=previous, + restore=lambda replacement: self._manager._restore_value( + "_context_engine", engine, replacement ), ) logger.info( @@ -1650,6 +1751,7 @@ class PluginContext: # -- image gen provider registration ------------------------------------ + @_serialized_replacement def register_image_gen_provider(self, provider) -> Optional[PluginRegistration]: """Register an image generation backend. @@ -1661,9 +1763,9 @@ class PluginContext: """ from agent.image_gen_provider import ImageGenProvider from agent.image_gen_registry import ( - get_provider, register_provider, restore_registration, + snapshot_registration, ) if not isinstance(provider, ImageGenProvider): @@ -1673,23 +1775,32 @@ class PluginContext: self.manifest.name, ) return - previous = get_provider(provider.name) - register_provider(provider) - registered = get_provider(provider.name) + registry_name = provider.name.strip() + scope = self._manager.scope_key + previous = snapshot_registration(registry_name, scope=scope) + register_provider(provider, scope=scope) + registered = snapshot_registration(registry_name, scope=scope) if registered is not provider: return None - handle = self._track( - "image_gen_provider", provider.name, - lambda: restore_registration(provider.name, provider, previous), + handle = self._track_replacement( + "image_gen_provider", + registry_name, + slot=("image_gen_provider", scope, registry_name), + current=provider, + previous=previous, + restore=lambda replacement: restore_registration( + registry_name, provider, replacement, scope=scope + ), ) logger.info( "Plugin '%s' registered image_gen provider: %s", - self.manifest.name, provider.name, + self.manifest.name, registry_name, ) return handle # -- dashboard auth provider registration -------------------------------- + @_serialized_replacement def register_dashboard_auth_provider(self, provider) -> Optional[PluginRegistration]: """Register a dashboard authentication provider. @@ -1705,10 +1816,10 @@ class PluginContext: """ from hermes_cli.dashboard_auth import ( DashboardAuthProvider, - get_provider, register_provider, ) from hermes_cli.dashboard_auth.registry import restore_registration + from hermes_cli.dashboard_auth.registry import snapshot_registration if not isinstance(provider, DashboardAuthProvider): logger.warning( @@ -1717,9 +1828,11 @@ class PluginContext: self.manifest.name, ) return - previous = get_provider(provider.name) + registry_name = provider.name + scope = self._manager.scope_key + previous = snapshot_registration(registry_name, scope=scope) try: - register_provider(provider) + register_provider(provider, scope=scope) except (TypeError, ValueError) as e: logger.warning( "Plugin '%s' failed to register dashboard-auth provider " @@ -1727,21 +1840,28 @@ class PluginContext: self.manifest.name, getattr(provider, "name", "?"), e, ) return - registered = get_provider(provider.name) + registered = snapshot_registration(registry_name, scope=scope) if registered is not provider: return None - handle = self._track( - "dashboard_auth_provider", provider.name, - lambda: restore_registration(provider.name, provider, previous), + handle = self._track_replacement( + "dashboard_auth_provider", + registry_name, + slot=("dashboard_auth_provider", scope, registry_name), + current=provider, + previous=previous, + restore=lambda replacement: restore_registration( + registry_name, provider, replacement, scope=scope + ), ) logger.info( "Plugin '%s' registered dashboard-auth provider: %s (%s)", - self.manifest.name, provider.name, provider.display_name, + self.manifest.name, registry_name, provider.display_name, ) return handle # -- video gen provider registration ------------------------------------- + @_serialized_replacement def register_video_gen_provider(self, provider) -> Optional[PluginRegistration]: """Register a video generation backend. @@ -1753,9 +1873,9 @@ class PluginContext: """ from agent.video_gen_provider import VideoGenProvider from agent.video_gen_registry import ( - get_provider, register_provider as _register_video_provider, restore_registration, + snapshot_registration, ) if not isinstance(provider, VideoGenProvider): @@ -1765,23 +1885,32 @@ class PluginContext: self.manifest.name, ) return - previous = get_provider(provider.name) - _register_video_provider(provider) - registered = get_provider(provider.name) + registry_name = provider.name.strip() + scope = self._manager.scope_key + previous = snapshot_registration(registry_name, scope=scope) + _register_video_provider(provider, scope=scope) + registered = snapshot_registration(registry_name, scope=scope) if registered is not provider: return None - handle = self._track( - "video_gen_provider", provider.name, - lambda: restore_registration(provider.name, provider, previous), + handle = self._track_replacement( + "video_gen_provider", + registry_name, + slot=("video_gen_provider", scope, registry_name), + current=provider, + previous=previous, + restore=lambda replacement: restore_registration( + registry_name, provider, replacement, scope=scope + ), ) logger.info( "Plugin '%s' registered video_gen provider: %s", - self.manifest.name, provider.name, + self.manifest.name, registry_name, ) return handle # -- web search/extract provider registration ---------------------------- + @_serialized_replacement def register_web_search_provider(self, provider) -> Optional[PluginRegistration]: """Register a web search/extract backend. @@ -1794,9 +1923,9 @@ class PluginContext: """ from agent.web_search_provider import WebSearchProvider from agent.web_search_registry import ( - get_provider, register_provider as _register_web_provider, restore_registration, + snapshot_registration, ) if not isinstance(provider, WebSearchProvider): @@ -1806,23 +1935,32 @@ class PluginContext: self.manifest.name, ) return - previous = get_provider(provider.name) - _register_web_provider(provider) - registered = get_provider(provider.name) + registry_name = provider.name.strip() + scope = self._manager.scope_key + previous = snapshot_registration(registry_name, scope=scope) + _register_web_provider(provider, scope=scope) + registered = snapshot_registration(registry_name, scope=scope) if registered is not provider: return None - handle = self._track( - "web_search_provider", provider.name, - lambda: restore_registration(provider.name, provider, previous), + handle = self._track_replacement( + "web_search_provider", + registry_name, + slot=("web_search_provider", scope, registry_name), + current=provider, + previous=previous, + restore=lambda replacement: restore_registration( + registry_name, provider, replacement, scope=scope + ), ) logger.info( "Plugin '%s' registered web provider: %s", - self.manifest.name, provider.name, + self.manifest.name, registry_name, ) return handle # -- browser provider registration --------------------------------------- + @_serialized_replacement def register_browser_provider(self, provider) -> Optional[PluginRegistration]: """Register a cloud browser backend. @@ -1839,9 +1977,9 @@ class PluginContext: """ from agent.browser_provider import BrowserProvider from agent.browser_registry import ( - get_provider, register_provider as _register_browser_provider, restore_registration, + snapshot_registration, ) if not isinstance(provider, BrowserProvider): @@ -1851,23 +1989,32 @@ class PluginContext: self.manifest.name, ) return - previous = get_provider(provider.name) - _register_browser_provider(provider) - registered = get_provider(provider.name) + registry_name = provider.name.strip() + scope = self._manager.scope_key + previous = snapshot_registration(registry_name, scope=scope) + _register_browser_provider(provider, scope=scope) + registered = snapshot_registration(registry_name, scope=scope) if registered is not provider: return None - handle = self._track( - "browser_provider", provider.name, - lambda: restore_registration(provider.name, provider, previous), + handle = self._track_replacement( + "browser_provider", + registry_name, + slot=("browser_provider", scope, registry_name), + current=provider, + previous=previous, + restore=lambda replacement: restore_registration( + registry_name, provider, replacement, scope=scope + ), ) logger.info( "Plugin '%s' registered browser provider: %s", - self.manifest.name, provider.name, + self.manifest.name, registry_name, ) return handle # -- secret source registration ------------------------------------------- + @_serialized_replacement def register_secret_source(self, source) -> Optional[PluginRegistration]: """Register an external secret-manager backend. @@ -1896,9 +2043,9 @@ class PluginContext: """ from agent.secret_sources.base import SecretSource from agent.secret_sources.registry import ( - get_source, register_source, restore_registration, + snapshot_registration, ) if not isinstance(source, SecretSource): @@ -1908,24 +2055,33 @@ class PluginContext: self.manifest.name, ) return - previous = get_source(source.name) - if register_source(source): - registered = get_source(source.name) + registry_name = source.name + scope = self._manager.scope_key + previous = snapshot_registration(registry_name, scope=scope) + if register_source(source, scope=scope): + registered = snapshot_registration(registry_name, scope=scope) if registered is not source: return None - handle = self._track( - "secret_source", source.name, - lambda: restore_registration(source.name, source, previous), + handle = self._track_replacement( + "secret_source", + registry_name, + slot=("secret_source", scope, registry_name), + current=source, + previous=previous, + restore=lambda replacement: restore_registration( + registry_name, source, replacement, scope=scope + ), ) logger.info( "Plugin '%s' registered secret source: %s", - self.manifest.name, source.name, + self.manifest.name, registry_name, ) return handle return None # -- TTS provider registration ------------------------------------------- + @_serialized_replacement def register_tts_provider(self, provider) -> Optional[PluginRegistration]: """Register a text-to-speech backend. @@ -1948,9 +2104,9 @@ class PluginContext: """ from agent.tts_provider import TTSProvider from agent.tts_registry import ( - get_provider, register_provider as _register_tts_provider, restore_registration, + snapshot_registration, ) if not isinstance(provider, TTSProvider): @@ -1960,23 +2116,32 @@ class PluginContext: self.manifest.name, ) return - previous = get_provider(provider.name) - _register_tts_provider(provider) - registered = get_provider(provider.name) + registry_name = provider.name.strip().lower() + scope = self._manager.scope_key + previous = snapshot_registration(registry_name, scope=scope) + _register_tts_provider(provider, scope=scope) + registered = snapshot_registration(registry_name, scope=scope) if registered is not provider: return None - handle = self._track( - "tts_provider", provider.name, - lambda: restore_registration(provider.name, provider, previous), + handle = self._track_replacement( + "tts_provider", + registry_name, + slot=("tts_provider", scope, registry_name), + current=provider, + previous=previous, + restore=lambda replacement: restore_registration( + registry_name, provider, replacement, scope=scope + ), ) logger.info( "Plugin '%s' registered TTS provider: %s", - self.manifest.name, provider.name, + self.manifest.name, registry_name, ) return handle # -- transcription (STT) provider registration --------------------------- + @_serialized_replacement def register_transcription_provider(self, provider) -> Optional[PluginRegistration]: """Register a speech-to-text backend. @@ -2005,9 +2170,9 @@ class PluginContext: """ from agent.transcription_provider import TranscriptionProvider from agent.transcription_registry import ( - get_provider, register_provider as _register_stt_provider, restore_registration, + snapshot_registration, ) if not isinstance(provider, TranscriptionProvider): @@ -2017,23 +2182,32 @@ class PluginContext: self.manifest.name, ) return - previous = get_provider(provider.name) - _register_stt_provider(provider) - registered = get_provider(provider.name) + registry_name = provider.name.strip().lower() + scope = self._manager.scope_key + previous = snapshot_registration(registry_name, scope=scope) + _register_stt_provider(provider, scope=scope) + registered = snapshot_registration(registry_name, scope=scope) if registered is not provider: return None - handle = self._track( - "transcription_provider", provider.name, - lambda: restore_registration(provider.name, provider, previous), + handle = self._track_replacement( + "transcription_provider", + registry_name, + slot=("transcription_provider", scope, registry_name), + current=provider, + previous=previous, + restore=lambda replacement: restore_registration( + registry_name, provider, replacement, scope=scope + ), ) logger.info( "Plugin '%s' registered transcription provider: %s", - self.manifest.name, provider.name, + self.manifest.name, registry_name, ) return handle # -- platform adapter registration --------------------------------------- + @_serialized_replacement def register_platform( self, name: str, @@ -2088,17 +2262,23 @@ class PluginContext: source="plugin", **entry_kwargs, ) - previous = platform_registry.snapshot_registration(name) - platform_registry.register(entry) - current = platform_registry.snapshot_registration(name) + scope = self._manager.scope_key + previous = platform_registry.snapshot_registration(name, scope=scope) + platform_registry.register(entry, scope=scope) + current = platform_registry.snapshot_registration(name, scope=scope) if current[0] is not entry or current[1] is not None: return None self._manager._plugin_platform_names.add(name) - handle = self._track( - "platform", name, - lambda: self._release_platform_registration( - platform_registry, name, current, previous + handle = self._track_replacement( + "platform", + name, + slot=("platform", scope, name), + current=current, + previous=previous, + restore=lambda replacement: self._restore_platform_registration( + platform_registry, name, current, replacement, scope ), + finalize=lambda: self._manager._remove_platform_name_if_unowned(name), ) logger.debug( "Plugin %s registered platform: %s", @@ -2107,15 +2287,17 @@ class PluginContext: ) return handle - def _release_platform_registration( + def _restore_platform_registration( self, platform_registry, name: str, current, - previous, - ) -> None: - platform_registry.restore_registration(name, current, previous) - self._manager._remove_platform_name_if_unowned(name) + replacement, + scope: str, + ) -> bool: + return platform_registry.restore_registration( + name, current, replacement, scope=scope + ) # -- slack action handler registration ---------------------------------- @@ -2186,6 +2368,7 @@ class PluginContext: # -- auxiliary task registration --------------------------------------- + @_serialized_replacement def register_auxiliary_task( self, key: str, @@ -2298,10 +2481,14 @@ class PluginContext: "plugin_key": owner_id, } entry = self._manager._aux_tasks[key] - handle = self._track( - "auxiliary_task", key, - lambda: self._manager._restore_mapping( - self._manager._aux_tasks, key, entry, existing + handle = self._track_replacement( + "auxiliary_task", + key, + slot=("manager_mapping", id(self._manager._aux_tasks), key), + current=entry, + previous=existing, + restore=lambda replacement: self._manager._restore_mapping( + self._manager._aux_tasks, key, entry, replacement ), ) logger.debug( @@ -2538,6 +2725,7 @@ class PluginContext: # -- skill registration ------------------------------------------------- + @_serialized_replacement def register_skill( self, name: str, @@ -2586,10 +2774,14 @@ class PluginContext: "frontmatter": dict(frontmatter or {}), } self._manager._plugin_skills[qualified] = entry - handle = self._track( - "skill", qualified, - lambda: self._manager._restore_mapping( - self._manager._plugin_skills, qualified, entry, previous + handle = self._track_replacement( + "skill", + qualified, + slot=("manager_mapping", id(self._manager._plugin_skills), qualified), + current=entry, + previous=previous, + restore=lambda replacement: self._manager._restore_mapping( + self._manager._plugin_skills, qualified, entry, replacement ), ) logger.debug( @@ -2606,7 +2798,13 @@ class PluginContext: class PluginManager: """Central manager that discovers, loads, and invokes plugins.""" - def __init__(self) -> None: + def __init__(self, scope_key: Optional[str] = None) -> None: + # Capture the home immutably. Unload can run from a different ambient + # profile context, but every inverse must target the registration's + # original scope. + self.scope_key = scope_key or hermes_home_key() + self.home_path = Path(self.scope_key) + self._discovery_lock = threading.RLock() self._plugins: Dict[str, LoadedPlugin] = {} self._hooks: Dict[str, List[Callable]] = {} self._middleware: Dict[str, List[Callable]] = {} @@ -2702,8 +2900,8 @@ class PluginManager: if not callbacks: mapping.pop(key, None) - @staticmethod def _restore_mapping( + self, mapping: Dict[str, dict], key: str, current: dict, @@ -2801,6 +2999,14 @@ class PluginManager: def unload( self, plugin: Union[str, PluginManifest, LoadedPlugin, None] = None, + ) -> bool: + """Unload registrations while excluding discovery/deferred loading.""" + with self._discovery_lock, _plugin_home_scope(self.home_path): + return self._unload_scoped(plugin) + + def _unload_scoped( + self, + plugin: Union[str, PluginManifest, LoadedPlugin, None] = None, ) -> bool: """Unload one plugin or all plugins owned by this manager. @@ -2908,33 +3114,33 @@ class PluginManager: changes or newly-added bundled backends become visible in long-lived sessions without requiring a full agent restart. """ - if self._discovered and not force: - return - if force: - # The ledger owns teardown. Clearing manager-local containers by - # itself leaves process-global tools/platforms/providers installed. - self.unload() - if env_var_enabled("HERMES_SAFE_MODE"): - logger.info("HERMES_SAFE_MODE=1 — plugin discovery skipped") + with self._discovery_lock, _plugin_home_scope(self.home_path): + if self._discovered and not force: + return + if force: + # The ledger owns teardown. Clearing manager-local containers by + # itself leaves process-global tools/platforms/providers installed. + self.unload() + if env_var_enabled("HERMES_SAFE_MODE"): + logger.info("HERMES_SAFE_MODE=1 — plugin discovery skipped") + self._discovered = True + return + # Set the flag up front as a re-entrancy guard (a plugin's register() + # can transitively trigger discovery again), but reset it if the sweep + # raises so a failed scan is NOT cached as "discovered with an empty + # registry" — callers swallow the exception and would otherwise be + # permanently stranded on the early-return above (the "No web provider + # configured" class of failures). self._discovered = True - return - - # Set the flag up front as a re-entrancy guard (a plugin's register() - # can transitively trigger discovery again), but reset it if the sweep - # raises so a failed scan is NOT cached as "discovered with an empty - # registry" — callers swallow the exception and would otherwise be - # permanently stranded on the early-return above (the "No web provider - # configured" class of failures). - self._discovered = True - try: - self._discover_and_load_inner() - # Plugin secret sources register during discover; the initial - # load_hermes_dotenv() already ran at import time. Re-pull so the - # first process sees plugin backends (tracking #64177). - self._refresh_secret_sources_after_discovery() - except BaseException: - self._discovered = False - raise + try: + self._discover_and_load_inner() + # Plugin secret sources register during discover; the initial + # load_hermes_dotenv() already ran at import time. Re-pull so the + # first process sees plugin backends (tracking #64177). + self._refresh_secret_sources_after_discovery() + except BaseException: + self._discovered = False + raise def _refresh_secret_sources_after_discovery(self) -> None: """If any plugin secret source is enabled, reset cache and re-apply. @@ -3537,6 +3743,7 @@ class PluginManager: return Path(manifest.path).name return name + @_serialized_replacement def _register_deferred_platform(self, manifest: PluginManifest) -> None: """Register a lazy loader for a bundled platform plugin. @@ -3556,27 +3763,55 @@ class PluginManager: loaded.deferred = True self._plugins[lookup_key] = loaded - def _loader(_manifest: PluginManifest = manifest) -> None: - self._load_plugin(_manifest) - try: from gateway.platform_registry import platform_registry - previous = platform_registry.snapshot_registration(platform_name) - platform_registry.register_deferred(platform_name, _loader) - current = platform_registry.snapshot_registration(platform_name) + scope = self.scope_key + + def _loader(_manifest: PluginManifest = manifest) -> None: + # Acquire the manager lock before checking cancellation. If an + # unload won the race after the registry marked this loader + # in-flight, it restores the predecessor and this loader exits + # without publishing any plugin registrations. If loading won, + # unload waits and then disposes the completed registration set. + with self._discovery_lock, _plugin_home_scope(self.home_path): + if platform_registry.is_deferred_load_cancelled( + platform_name, scope=scope + ): + return + self._load_plugin_scoped(_manifest) + + previous = platform_registry.snapshot_registration( + platform_name, scope=scope + ) + platform_registry.register_deferred( + platform_name, _loader, scope=scope + ) + current = platform_registry.snapshot_registration( + platform_name, scope=scope + ) if current[0] is None and current[1] is _loader: self._plugin_platform_names.add(platform_name) + lease = replacement_coordinator.acquire( + ("platform", scope, platform_name), + current=current, + previous=previous, + restore=lambda replacement: self._restore_deferred_platform( + platform_registry, + platform_name, + current, + replacement, + scope, + ), + finalize=lambda: self._remove_platform_name_if_unowned( + platform_name + ), + ) self._track_registration( manifest, "platform", platform_name, - lambda: self._release_deferred_platform( - platform_registry, - platform_name, - current, - previous, - ), + lease.dispose, ) logger.debug( "Registered deferred platform loader: %s (plugin=%s)", @@ -3660,18 +3895,25 @@ class PluginManager: ): logger.warning("Plugin %s config: %s", plugin_id, warning) - def _release_deferred_platform( + def _restore_deferred_platform( self, platform_registry, name: str, current, - previous, - ) -> None: - platform_registry.restore_registration(name, current, previous) - self._remove_platform_name_if_unowned(name) + replacement, + scope: str, + ) -> bool: + return platform_registry.restore_registration( + name, current, replacement, scope=scope + ) def _load_plugin(self, manifest: PluginManifest) -> None: """Import a plugin module and call its ``register(ctx)`` function.""" + with self._discovery_lock, _plugin_home_scope(self.home_path): + self._load_plugin_scoped(manifest) + + def _load_plugin_scoped(self, manifest: PluginManifest) -> None: + """Load one plugin with the manager's home bound as current.""" loaded = LoadedPlugin(manifest=manifest) logger.debug( "Loading plugin '%s' (source=%s, kind=%s, path=%s)", @@ -3683,17 +3925,40 @@ class PluginManager: return from tools.registry import registry as _registry - _plugin_id = manifest.key or manifest.name - _slug = _plugin_id.replace("/", "__").replace("-", "_") - _registry.register_plugin_override_policy( - f"{_NS_PARENT}.{_slug}", - PluginContext(manifest, self)._tool_override_allowed(""), - ) registration_start = len(self._registration_order) plugin_key = manifest.key or manifest.name + _module_name = self._policy_module_name(manifest) + with replacement_coordinator.transaction(): + previous_policy = _registry.snapshot_plugin_override_policy( + _module_name, scope=self.scope_key + ) + current_policy = _registry.register_plugin_override_policy( + _module_name, + PluginContext(manifest, self)._tool_override_allowed(""), + scope=self.scope_key, + ) + policy_lease = replacement_coordinator.acquire( + ("tool_override_policy", self.scope_key, _module_name), + current=current_policy, + previous=previous_policy, + restore=lambda replacement: _registry.restore_plugin_override_policy( + _module_name, + current_policy, + replacement, + scope=self.scope_key, + ), + ) + self._track_registration( + manifest, + "tool_override_policy", + _module_name, + policy_lease.dispose, + ) try: if manifest.source in {"user", "project", "bundled"}: - module = self._load_directory_module(manifest) + module = self._load_directory_module( + manifest, module_name=_module_name + ) else: module = self._load_entrypoint_module(manifest) @@ -3822,7 +4087,35 @@ class PluginManager: logger.warning("Failed to load Agent Plugin '%s': %s", lookup_key, exc) self._plugins[lookup_key] = loaded - def _load_directory_module(self, manifest: PluginManifest) -> types.ModuleType: + def _directory_module_name(self, manifest: PluginManifest) -> str: + """Return a profile-safe import namespace for a directory plugin.""" + key = manifest.key or manifest.name + slug = key.replace("/", "__").replace("-", "_") + bare_name = f"{_NS_PARENT}.{slug}" + with _MODULE_NAMESPACE_LOCK: + owner = _BARE_MODULE_SCOPE.get(bare_name) + if owner is None: + _BARE_MODULE_SCOPE[bare_name] = self.scope_key + return bare_name + if owner == self.scope_key: + return bare_name + digest = hashlib.sha256(self.scope_key.encode("utf-8")).hexdigest()[:12] + return f"{bare_name}__home_{digest}" + + def _policy_module_name(self, manifest: PluginManifest) -> str: + """Return the module prefix whose callbacks inherit plugin policy.""" + if manifest.source == "entrypoint" and manifest.path: + module_name = str(manifest.path).partition(":")[0].strip() + if module_name: + return module_name + return self._directory_module_name(manifest) + + def _load_directory_module( + self, + manifest: PluginManifest, + *, + module_name: Optional[str] = None, + ) -> types.ModuleType: """Import a directory-based plugin as ``hermes_plugins.``. The module slug is derived from ``manifest.key`` so category-namespaced @@ -3842,9 +4135,7 @@ class PluginManager: ns_pkg.__package__ = _NS_PARENT sys.modules[_NS_PARENT] = ns_pkg - key = manifest.key or manifest.name - slug = key.replace("/", "__").replace("-", "_") - module_name = f"{_NS_PARENT}.{slug}" + module_name = module_name or self._directory_module_name(manifest) # Evict any stale sys.modules entries for this slug before # (re-)importing. A same-slug module may already be cached here @@ -4413,6 +4704,7 @@ _plugin_manager: Optional[PluginManager] = None # seen profile reuses its manager (and picks up any modules it already # imported) instead of rebuilding from scratch every switch. _plugin_managers_by_home: Dict[Path, PluginManager] = {} +_plugin_managers_lock = threading.RLock() def _plugin_home_key() -> Path: @@ -4460,6 +4752,9 @@ def _clear_plugin_submodules(manager: Optional[PluginManager]) -> None: prefix = f"{module_name}." for name in [n for n in sys.modules if n == module_name or n.startswith(prefix)]: del sys.modules[name] + with _MODULE_NAMESPACE_LOCK: + if _BARE_MODULE_SCOPE.get(module_name) == manager.scope_key: + _BARE_MODULE_SCOPE.pop(module_name, None) def get_plugin_manager() -> PluginManager: @@ -4475,28 +4770,25 @@ def get_plugin_manager() -> PluginManager: global _plugin_manager current_home = _plugin_home_key() - # Tests and embedders historically monkeypatch ``_plugin_manager`` - # directly (``monkeypatch.setattr(plugins_mod, "_plugin_manager", mgr)``). - # Detect that specifically by checking whether the single-slot pointer - # references a manager our keyed cache doesn't know about *at all* - # (i.e. it isn't the value cached for *any* home) — that can only - # happen via a direct assignment bypassing this function, not via a - # legitimate home switch (which always leaves ``_plugin_manager`` - # pointing at a manager already stored in the cache). Comparing against - # only ``current_home``'s slot is wrong: it also matches an ordinary - # switch to a *new* home that simply hasn't been cached yet, which - # would incorrectly resurrect the previous home's manager here. - if _plugin_manager is not None and _plugin_manager not in _plugin_managers_by_home.values(): - _plugin_managers_by_home[current_home] = _plugin_manager - return _plugin_manager + with _plugin_managers_lock: + # Tests and embedders historically monkeypatch ``_plugin_manager`` + # directly (``monkeypatch.setattr(plugins_mod, "_plugin_manager", mgr)``). + # Detect that specifically by checking whether the single-slot pointer + # references a manager our keyed cache doesn't know about *at all*. + if ( + _plugin_manager is not None + and _plugin_manager not in _plugin_managers_by_home.values() + ): + _plugin_managers_by_home[current_home] = _plugin_manager + return _plugin_manager - manager = _plugin_managers_by_home.get(current_home) - if manager is None: - manager = PluginManager() - _plugin_managers_by_home[current_home] = manager + manager = _plugin_managers_by_home.get(current_home) + if manager is None: + manager = PluginManager(scope_key=hermes_home_key(current_home)) + _plugin_managers_by_home[current_home] = manager - _plugin_manager = manager - return manager + _plugin_manager = manager + return manager def _reset_plugin_managers_for_tests() -> None: @@ -4507,10 +4799,18 @@ def _reset_plugin_managers_for_tests() -> None: this instead of reaching into the module's private dict directly. """ global _plugin_manager - for manager in _plugin_managers_by_home.values(): - _clear_plugin_submodules(manager) - _plugin_managers_by_home.clear() - _plugin_manager = None + with _plugin_managers_lock: + managers = list(dict.fromkeys(_plugin_managers_by_home.values())) + if _plugin_manager is not None and _plugin_manager not in managers: + managers.append(_plugin_manager) + for manager in managers: + _clear_plugin_submodules(manager) + try: + manager.unload() + except Exception: + logger.debug("test plugin-manager unload failed", exc_info=True) + _plugin_managers_by_home.clear() + _plugin_manager = None def has_enabled_agent_plugin_mcp(raw_config: Mapping[str, Any]) -> bool: diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index a32b5368cf..8ac29e0f5a 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -2732,8 +2732,8 @@ def _prompt_choice(question: str, choices: list, default: int = 0) -> int: # ─── Token Estimation ──────────────────────────────────────────────────────── -# Module-level cache so discovery + tokenization runs at most once per process. -_tool_token_cache: Optional[Dict[str, int]] = None +# Profile-keyed cache so one process can serve distinct plugin tool catalogs. +_tool_token_cache: Optional[Dict[tuple[str, int], Dict[str, int]]] = None def _estimate_tool_tokens() -> Dict[str, int]: @@ -2746,25 +2746,33 @@ def _estimate_tool_tokens() -> Dict[str, int]: Returns an empty dict when tiktoken or the registry is unavailable. """ global _tool_token_cache - if _tool_token_cache is not None: - return _tool_token_cache + from hermes_constants import hermes_home_key + + scope = hermes_home_key() + + try: + # Trigger full tool discovery (imports all tool modules). + import model_tools # noqa: F401 + from tools.registry import registry + cache_key = (scope, registry._generation) + except Exception: + logger.debug("Tool registry unavailable; skipping token estimation") + cache_key = (scope, -1) + _tool_token_cache = _tool_token_cache or {} + _tool_token_cache[cache_key] = {} + return _tool_token_cache[cache_key] + + if _tool_token_cache is not None and cache_key in _tool_token_cache: + return _tool_token_cache[cache_key] try: import tiktoken enc = tiktoken.get_encoding("cl100k_base") except Exception: logger.debug("tiktoken unavailable; skipping tool token estimation") - _tool_token_cache = {} - return _tool_token_cache - - try: - # Trigger full tool discovery (imports all tool modules). - import model_tools # noqa: F401 - from tools.registry import registry - except Exception: - logger.debug("Tool registry unavailable; skipping token estimation") - _tool_token_cache = {} - return _tool_token_cache + _tool_token_cache = _tool_token_cache or {} + _tool_token_cache[cache_key] = {} + return _tool_token_cache[cache_key] counts: Dict[str, int] = {} for name in registry.get_all_tool_names(): @@ -2774,8 +2782,9 @@ def _estimate_tool_tokens() -> Dict[str, int]: # {"type": "function", "function": } text = _json.dumps({"type": "function", "function": schema}) counts[name] = len(enc.encode(text)) - _tool_token_cache = counts - return _tool_token_cache + _tool_token_cache = _tool_token_cache or {} + _tool_token_cache[cache_key] = counts + return counts def _prompt_toolset_checklist( diff --git a/hermes_constants.py b/hermes_constants.py index 96b9227322..a9a56edefe 100644 --- a/hermes_constants.py +++ b/hermes_constants.py @@ -139,6 +139,18 @@ def get_hermes_home() -> Path: return _hermes_home_from_env() +def hermes_home_key(path: str | Path | None = None) -> str: + """Return a stable key for a Hermes home/profile directory. + + Runtime registries use this key to isolate plugin-owned entries while + keeping built-in registrations process-global. ``strict=False`` preserves + useful behavior for profiles whose directories have not been created yet. + """ + candidate = Path(path) if path is not None else get_hermes_home() + resolved = candidate.expanduser().resolve(strict=False) + return os.path.normcase(str(resolved)) + + def get_process_hermes_home() -> Path: """Return the Hermes home for the running process, ignoring task overrides. diff --git a/model_tools.py b/model_tools.py index 655ceb6490..c5c1256c90 100644 --- a/model_tools.py +++ b/model_tools.py @@ -274,7 +274,7 @@ _LEGACY_TOOLSET_MAP = { # ============================================================================= # Module-level memoization for get_tool_definitions(). Keyed on -# (frozenset(enabled_toolsets), frozenset(disabled_toolsets), registry._generation). +# (profile scope, enabled/disabled toolsets, registry generation). # Hot callers (gateway runner, AIAgent.__init__) invoke this on every turn # with quiet_mode=True; caching avoids ~7 ms of registry walking + schema # filtering + check_fn probing per call. Only active when quiet_mode=True @@ -285,6 +285,7 @@ _LEGACY_TOOLSET_MAP = { # inner check_fn TTL cache in registry.py handles environment drift (Docker # daemon start/stop, env var changes, etc.) on a 30 s horizon. _tool_defs_cache: Dict[tuple, List[Dict[str, Any]]] = {} +_tool_defs_cache_lock = threading.Lock() # Hard cap on memoized get_tool_definitions() results. A long-lived Gateway # process sees many distinct toolset/config fingerprints over its lifetime @@ -299,7 +300,8 @@ def _clear_tool_defs_cache() -> None: """Drop memoized get_tool_definitions() results. Called when dynamic schema dependencies change (e.g. discord capability cache reset, execute_code sandbox reconfigured).""" - _tool_defs_cache.clear() + with _tool_defs_cache_lock: + _tool_defs_cache.clear() def get_tool_definitions( @@ -346,6 +348,7 @@ def get_tool_definitions( profile_scope = check_fn_cache_scope() if profile_scope != CHECK_FN_CACHE_BYPASS: cache_key = ( + registry.current_scope_key(), frozenset(enabled_toolsets) if enabled_toolsets is not None else None, frozenset(disabled_toolsets) if disabled_toolsets else None, registry._generation, @@ -356,7 +359,8 @@ def get_tool_definitions( _is_dispatcher_owned_worker(), profile_scope, ) - cached = _tool_defs_cache.get(cache_key) if cache_key is not None else None + with _tool_defs_cache_lock: + cached = _tool_defs_cache.get(cache_key) if cache_key is not None else None if cached is not None: # Update _last_resolved_tool_names so downstream callers see # consistent state even on a cache hit. @@ -379,10 +383,16 @@ def get_tool_definitions( # Bound the cache with LRU eviction so a long-lived Gateway process # doesn't accumulate entries unboundedly across the many distinct # toolset/config fingerprints it sees over its lifetime (#19251). - if len(_tool_defs_cache) >= _TOOL_DEFS_CACHE_MAX: - _tool_defs_cache.pop(next(iter(_tool_defs_cache))) # evict oldest - _tool_defs_cache[cache_key] = result - return list(result) + with _tool_defs_cache_lock: + # Another thread may have populated this exact key while this + # thread computed. Reuse it and serialize capacity eviction. + cached = _tool_defs_cache.get(cache_key) + if cached is None: + if len(_tool_defs_cache) >= _TOOL_DEFS_CACHE_MAX: + _tool_defs_cache.pop(next(iter(_tool_defs_cache))) + _tool_defs_cache[cache_key] = result + cached = result + return list(cached) if quiet_mode: return list(result) return result diff --git a/registration_lifecycle.py b/registration_lifecycle.py new file mode 100644 index 0000000000..3f10a56d18 --- /dev/null +++ b/registration_lifecycle.py @@ -0,0 +1,128 @@ +"""Ownership leases for replaceable runtime registrations. + +The coordinator models registration *generations*, not just value identity. +That distinction matters when the same provider singleton is registered again +after an older ownership generation was unloaded. +""" + +from __future__ import annotations + +import threading +from collections.abc import Callable, Hashable +from contextlib import contextmanager +from dataclasses import dataclass, field +from typing import Any + + +def same_registration(left: Any, right: Any) -> bool: + """Compare opaque registry snapshots using identity only.""" + if isinstance(left, tuple) and isinstance(right, tuple): + return len(left) == len(right) and all( + same_registration(a, b) for a, b in zip(left, right) + ) + return left is right + + +@dataclass +class ReplacementLease: + """One ownership generation in a replaceable registry slot.""" + + coordinator: "ReplacementCoordinator" + slot: Hashable + current: Any + previous: Any + restore: Callable[[Any], bool] + finalize: Callable[[], None] | None = None + predecessor: "ReplacementLease | None" = None + active: bool = field(default=True, init=False) + + def dispose(self) -> None: + self.coordinator.dispose(self) + + +class ReplacementCoordinator: + """Link and remove registration generations in arbitrary unload order.""" + + def __init__(self) -> None: + self._active: dict[Hashable, list[ReplacementLease]] = {} + self._lock = threading.RLock() + + @contextmanager + def transaction(self): + """Serialize a registry snapshot/write/acquire with lease disposal.""" + with self._lock: + yield + + def acquire( + self, + slot: Hashable, + *, + current: Any, + previous: Any, + restore: Callable[[Any], bool], + finalize: Callable[[], None] | None = None, + ) -> ReplacementLease: + """Attach a new live generation to the matching active predecessor.""" + with self._lock: + leases = self._active.setdefault(slot, []) + predecessor = next( + ( + lease + for lease in reversed(leases) + if lease.active and same_registration(lease.current, previous) + ), + None, + ) + lease = ReplacementLease( + coordinator=self, + slot=slot, + current=current, + previous=previous, + restore=restore, + finalize=finalize, + predecessor=predecessor, + ) + leases.append(lease) + return lease + + def dispose(self, lease: ReplacementLease) -> None: + """Remove *lease*, restoring the nearest still-live predecessor.""" + with self._lock: + if not lease.active: + return + leases = self._active.get(lease.slot, []) + latest = next( + (candidate for candidate in reversed(leases) if candidate.active), + None, + ) + lease.active = False + + # An older generation can share the exact same object identity as + # a newer one. Registry-level CAS cannot distinguish those leases, + # so only the latest live generation is allowed to mutate the slot. + try: + try: + if latest is lease: + replacement = lease.previous + predecessor = lease.predecessor + while predecessor is not None: + if predecessor.active: + replacement = predecessor.current + break + replacement = predecessor.previous + predecessor = predecessor.predecessor + + lease.restore(replacement) + finally: + if lease.finalize is not None: + lease.finalize() + finally: + if leases: + self._active[lease.slot] = [ + item for item in leases if item.active + ] + if not self._active[lease.slot]: + self._active.pop(lease.slot, None) + + +replacement_coordinator = ReplacementCoordinator() diff --git a/tests/gateway/test_google_chat.py b/tests/gateway/test_google_chat.py index 175f487231..497a4d8e39 100644 --- a/tests/gateway/test_google_chat.py +++ b/tests/gateway/test_google_chat.py @@ -1567,8 +1567,13 @@ class TestAuthorizationEmailMatch: from gateway.config import GatewayConfig from gateway.run import GatewayRunner from gateway.session import SessionSource + from hermes_cli.plugins import discover_plugins monkeypatch.setenv("GOOGLE_CHAT_ALLOWED_USERS", "alice@example.com") + # Plugin platforms become available during the normal gateway startup + # discovery pass. This unit test constructs GatewayRunner directly, + # so perform that lifecycle step explicitly before testing auth. + discover_plugins() cfg = GatewayConfig() runner = GatewayRunner(cfg) runner.pairing_store = MagicMock() @@ -1739,4 +1744,3 @@ class TestGoogleChatStandaloneSend: assert kwargs["headers"]["Authorization"] == "Bearer the-token" assert kwargs["json"] == {"text": "hello cron"} - diff --git a/tests/hermes_cli/test_plugin_ownership_ledger.py b/tests/hermes_cli/test_plugin_ownership_ledger.py index 22b6cccfcd..f52854f94d 100644 --- a/tests/hermes_cli/test_plugin_ownership_ledger.py +++ b/tests/hermes_cli/test_plugin_ownership_ledger.py @@ -2,7 +2,11 @@ from __future__ import annotations +from concurrent.futures import ThreadPoolExecutor, TimeoutError from pathlib import Path +from threading import Event +from time import monotonic, sleep +from types import MethodType import yaml @@ -67,6 +71,38 @@ def _write_plugin(hermes_home: Path) -> None: ) +def _write_profile_probe(hermes_home: Path, marker: str) -> None: + plugin_dir = hermes_home / "plugins" / "profile_probe" + plugin_dir.mkdir(parents=True) + (plugin_dir / "plugin.yaml").write_text( + yaml.safe_dump( + { + "name": "profile_probe", + "version": "0.1.0", + "description": f"profile probe {marker}", + } + ) + ) + (plugin_dir / "__init__.py").write_text( + "def register(ctx):\n" + " ctx.register_tool(\n" + " name='shared_profile_tool',\n" + " toolset='profile_probe',\n" + " schema={'name': 'shared_profile_tool', 'parameters': {'type': 'object', 'properties': {}}},\n" + f" handler=lambda args, **kwargs: {marker!r},\n" + " )\n" + " ctx.register_platform(\n" + " name='shared_profile_platform',\n" + f" label={marker!r},\n" + f" adapter_factory=lambda config: {marker!r},\n" + " check_fn=lambda: True,\n" + " )\n" + ) + (hermes_home / "config.yaml").write_text( + yaml.safe_dump({"plugins": {"enabled": ["profile_probe"]}}) + ) + + def test_load_force_reload_and_unload_remove_every_manager_registration( tmp_path, monkeypatch, @@ -97,6 +133,7 @@ def test_load_force_reload_and_unload_remove_every_manager_registration( first_command = manager._plugin_commands["ledger-probe-command"] first_cli_command = manager._cli_commands["ledger-probe-cli"] first_skill = manager._plugin_skills["ledger_probe:ledger-probe"] + module_name = manager._plugins["ledger_probe"].module.__name__ assert first_tool is not None assert first_platform is not None @@ -109,6 +146,7 @@ def test_load_force_reload_and_unload_remove_every_manager_registration( "middleware", "auxiliary_task", "skill", + "tool_override_policy", } manager.discover_and_load(force=True) @@ -142,6 +180,10 @@ def test_load_force_reload_and_unload_remove_every_manager_registration( assert "ledger_probe:ledger-probe" not in manager._plugin_skills assert manager._aux_tasks == {} assert manager._ownership_ledger == {} + assert registry.snapshot_plugin_override_policy( + module_name, scope=manager.scope_key + ) is None + assert registry.plugin_scope_for_module(module_name) == manager.scope_key def test_reverse_unload_restores_an_overridden_platform_registration(): @@ -150,9 +192,10 @@ def test_reverse_unload_restores_an_overridden_platform_registration(): from hermes_cli.plugins import PluginContext, PluginManager, PluginManifest name = "ledger_override_platform" - previous = platform_registry.snapshot_registration(name) - manager_a = PluginManager() - manager_b = PluginManager() + scope = platform_registry.current_scope_key() + previous = platform_registry.snapshot_registration(name, scope=scope) + manager_a = PluginManager(scope_key=scope) + manager_b = PluginManager(scope_key=scope) context_a = PluginContext( PluginManifest(name="ledger_owner_a", key="ledger_owner_a"), manager_a ) @@ -183,9 +226,1050 @@ def test_reverse_unload_restores_an_overridden_platform_registration(): handle_b.dispose() assert platform_registry.get(name) is entry_a handle_a.dispose() - assert platform_registry.snapshot_registration(name) == previous + assert platform_registry.snapshot_registration(name, scope=scope) == previous finally: # The test uses a deliberately unique name, but restore any state that # a surrounding test may have installed under it. - current = platform_registry.snapshot_registration(name) - platform_registry.restore_registration(name, current, previous) + current = platform_registry.snapshot_registration(name, scope=scope) + platform_registry.restore_registration( + name, current, previous, scope=scope + ) + + +def test_targeted_unload_does_not_resurrect_an_older_tool_override(): + """The tool overlay follows the same arbitrary-order ownership contract.""" + from hermes_cli.plugins import PluginContext, PluginManager, PluginManifest + from tools.registry import registry + + name = "ledger_out_of_order_tool" + scope = registry.current_scope_key() + previous = registry.snapshot_registration(name, scope=scope) + manager_a = PluginManager(scope_key=scope) + manager_b = PluginManager(scope_key=scope) + context_a = PluginContext(PluginManifest(name="tool_a", key="tool_a"), manager_a) + context_b = PluginContext(PluginManifest(name="tool_b", key="tool_b"), manager_b) + + def register(context, marker): + return context.register_tool( + name=name, + toolset="ledger_test", + schema={ + "name": name, + "parameters": {"type": "object", "properties": {}}, + }, + handler=lambda args, **kwargs: marker, + ) + + try: + assert register(context_a, "a") is not None + old_entry = registry.get_entry(name, scope=scope) + assert register(context_b, "b") is not None + new_entry = registry.get_entry(name, scope=scope) + + assert old_entry is not None and new_entry is not old_entry + manager_a.unload("tool_a") + assert registry.get_entry(name, scope=scope) is new_entry + assert name not in manager_a._plugin_tool_names + manager_b.unload("tool_b") + assert registry.get_entry(name, scope=scope) is not old_entry + assert registry.snapshot_registration(name, scope=scope) is previous + finally: + current = registry.snapshot_registration(name, scope=scope) + if current is not None: + registry.restore_registration(name, current, previous, scope=scope) + + +def test_rejected_tool_registration_does_not_claim_global_fallback(): + """Effective fallback identity cannot masquerade as a successful write.""" + from hermes_cli.plugins import PluginContext, PluginManager, PluginManifest + from tools.registry import registry + + name = "ledger_rejected_tool" + previous = registry.snapshot_registration(name) + + def shared_handler(args, **kwargs): + return "base" + + registry.register( + name=name, + toolset="ledger_base", + schema={"name": name, "parameters": {"type": "object", "properties": {}}}, + handler=shared_handler, + ) + base_entry = registry.snapshot_registration(name) + manager = PluginManager() + context = PluginContext( + PluginManifest(name="rejected_owner", key="rejected_owner"), manager + ) + try: + handle = context.register_tool( + name=name, + toolset="different_toolset", + schema={ + "name": name, + "parameters": {"type": "object", "properties": {}}, + }, + handler=shared_handler, + ) + assert handle is None + assert name not in manager._plugin_tool_names + assert manager._ownership_ledger == {} + assert registry.snapshot_registration(name) is base_entry + finally: + if base_entry is not None: + registry.restore_registration(name, base_entry, previous) + + +def test_plugin_context_cannot_shadow_same_toolset_global_with_core_callable(): + """Explicit context scope cannot launder an imported/core handler.""" + from hermes_cli.plugins import PluginContext, PluginManager, PluginManifest + from tools.registry import registry + + name = "ledger_same_toolset_global" + previous = registry.snapshot_registration(name) + registry.register( + name=name, + toolset="ledger_same_toolset", + schema={"name": name, "parameters": {"type": "object", "properties": {}}}, + handler=lambda args, **kwargs: "base", + ) + base_entry = registry.snapshot_registration(name) + manager = PluginManager() + context = PluginContext( + PluginManifest(name="core_callable", key="core_callable"), manager + ) + try: + assert context.register_tool( + name=name, + toolset="ledger_same_toolset", + schema={ + "name": name, + "parameters": {"type": "object", "properties": {}}, + }, + handler=str, + ) is None + assert registry.get_entry(name, scope=manager.scope_key) is base_entry + assert registry.snapshot_registration(name, scope=manager.scope_key) is None + finally: + if base_entry is not None: + registry.restore_registration(name, base_entry, previous) + + +def test_rejected_tool_registration_does_not_claim_local_predecessor(): + """A same-handler rejection cannot manufacture a replacement lease.""" + from hermes_cli.plugins import PluginContext, PluginManager, PluginManifest + from tools.registry import registry + + name = "ledger_rejected_local_tool" + scope = registry.current_scope_key() + previous = registry.snapshot_registration(name, scope=scope) + + def shared_handler(args, **kwargs): + return "shared" + + manager_a = PluginManager(scope_key=scope) + manager_b = PluginManager(scope_key=scope) + context_a = PluginContext(PluginManifest(name="local_owner", key="local_owner"), manager_a) + context_b = PluginContext(PluginManifest(name="false_owner", key="false_owner"), manager_b) + try: + assert context_a.register_tool( + name=name, + toolset="owner_toolset", + schema={"name": name, "parameters": {"type": "object", "properties": {}}}, + handler=shared_handler, + ) is not None + owner_entry = registry.snapshot_registration(name, scope=scope) + assert context_b.register_tool( + name=name, + toolset="different_toolset", + schema={"name": name, "parameters": {"type": "object", "properties": {}}}, + handler=shared_handler, + ) is None + assert manager_b._ownership_ledger == {} + manager_a.unload("local_owner") + assert registry.snapshot_registration(name, scope=scope) is previous + assert owner_entry is not None + finally: + current = registry.snapshot_registration(name, scope=scope) + if current is not None: + registry.restore_registration(name, current, previous, scope=scope) + + +def test_scoped_plugin_cannot_deregister_a_process_global_tool(): + """Profile-local plugin cleanup must never mutate the shared base layer.""" + from unittest.mock import patch + + import pytest + + from tools.registry import ToolRegistry + + registry = ToolRegistry() + name = "ledger_global_base" + registry.register( + name=name, + toolset="ledger_base", + schema={"name": name, "parameters": {"type": "object", "properties": {}}}, + handler=lambda args, **kwargs: "base", + ) + module_name = "hermes_plugins.scoped_cleanup" + registry.register_plugin_override_policy( + module_name, True, scope="/profiles/isolated" + ) + + with patch.object(ToolRegistry, "_caller_module", return_value=module_name): + with pytest.raises(PermissionError, match="process-global"): + registry.deregister(name) + + assert registry.snapshot_registration(name) is not None + + +def test_shared_entrypoint_module_uses_the_active_profile_scope(tmp_path): + """One pip module can serve A and B without becoming process-global.""" + import pytest + + from hermes_constants import reset_hermes_home_override, set_hermes_home_override + from tools.registry import ToolRegistry + + registry = ToolRegistry() + module_name = "third_party.shared_hermes_plugin" + home_a = str((tmp_path / "entrypoint-a").resolve()) + home_b = str((tmp_path / "entrypoint-b").resolve()) + home_c = str((tmp_path / "entrypoint-c").resolve()) + policy_a = registry.register_plugin_override_policy( + module_name, False, scope=home_a + ) + policy_b = registry.register_plugin_override_policy( + module_name, False, scope=home_b + ) + handler = eval("lambda args, **kwargs: 'shared'", {"__name__": module_name}) + + def register_in(home, name): + token = set_hermes_home_override(home) + try: + registry.register( + name=name, + toolset="entrypoint_shared", + schema={ + "name": name, + "parameters": {"type": "object", "properties": {}}, + }, + handler=handler, + ) + finally: + reset_hermes_home_override(token) + + register_in(home_a, "shared_entrypoint_a") + register_in(home_b, "shared_entrypoint_b") + assert registry.snapshot_registration("shared_entrypoint_a", scope=home_a) is not None + assert registry.snapshot_registration("shared_entrypoint_a", scope=home_b) is None + assert registry.snapshot_registration("shared_entrypoint_b", scope=home_b) is not None + assert registry.snapshot_registration("shared_entrypoint_b") is None + + from unittest.mock import patch + + token = set_hermes_home_override(home_a) + try: + with patch.object(ToolRegistry, "_caller_module", return_value=module_name): + registry.deregister("shared_entrypoint_a") + finally: + reset_hermes_home_override(token) + assert registry.snapshot_registration("shared_entrypoint_a", scope=home_a) is None + assert registry.snapshot_registration("shared_entrypoint_b", scope=home_b) is not None + + # Policy unload revokes authorization but durable scope attribution keeps + # a stale delayed callback out of the process-global registry. + registry.restore_plugin_override_policy( + module_name, policy_a, None, scope=home_a + ) + registry.restore_plugin_override_policy( + module_name, policy_b, None, scope=home_b + ) + register_in(home_a, "shared_entrypoint_stale") + assert registry.snapshot_registration("shared_entrypoint_stale", scope=home_a) + assert registry.snapshot_registration("shared_entrypoint_stale") is None + with pytest.raises(PermissionError, match="multiple profiles"): + register_in(home_c, "shared_entrypoint_ambiguous") + assert registry.snapshot_registration("shared_entrypoint_ambiguous") is None + + +def test_decorated_plugin_callable_keeps_its_defining_module_scope(tmp_path): + """functools.wraps must not replace the plugin wrapper's provenance.""" + import functools + + from tools.registry import ToolRegistry + + registry = ToolRegistry() + module_name = "third_party.decorated_plugin" + scope = str((tmp_path / "decorated-profile").resolve()) + registry.register_plugin_override_policy(module_name, False, scope=scope) + namespace = {"__name__": module_name, "functools": functools} + exec( + "@functools.wraps(str)\n" + "def handler(args, **kwargs):\n" + " return 'decorated'\n", + namespace, + ) + handler = namespace["handler"] + registry.register( + name="decorated_plugin_tool", + toolset="decorated_plugin", + schema={ + "name": "decorated_plugin_tool", + "parameters": {"type": "object", "properties": {}}, + }, + handler=handler, + ) + + assert registry.snapshot_registration( + "decorated_plugin_tool", scope=scope + ) is not None + assert registry.snapshot_registration("decorated_plugin_tool") is None + + +def test_entrypoint_policy_uses_the_most_specific_module_prefix(tmp_path): + from hermes_constants import reset_hermes_home_override, set_hermes_home_override + from tools.registry import ToolRegistry + + registry = ToolRegistry() + broad_scope = str((tmp_path / "broad").resolve()) + narrow_scope = str((tmp_path / "narrow").resolve()) + registry.register_plugin_override_policy("vendor", True, scope=broad_scope) + registry.register_plugin_override_policy( + "vendor.plugin", False, scope=narrow_scope + ) + handler = eval( + "lambda args, **kwargs: 'narrow'", + {"__name__": "vendor.plugin.handlers"}, + ) + token = set_hermes_home_override(narrow_scope) + try: + registry.register( + name="specific_entrypoint_tool", + toolset="specific_entrypoint", + schema={ + "name": "specific_entrypoint_tool", + "parameters": {"type": "object", "properties": {}}, + }, + handler=handler, + ) + finally: + reset_hermes_home_override(token) + + assert registry.snapshot_registration( + "specific_entrypoint_tool", scope=narrow_scope + ) is not None + assert registry.snapshot_registration( + "specific_entrypoint_tool", scope=broad_scope + ) is None + + +def test_targeted_unload_does_not_resurrect_an_older_override(): + """Removing A under B tombstones A so B cannot restore it later.""" + from gateway.platform_registry import platform_registry + from hermes_cli.plugins import PluginContext, PluginManager, PluginManifest + + name = "ledger_out_of_order_platform" + scope = platform_registry.current_scope_key() + previous = platform_registry.snapshot_registration(name, scope=scope) + manager_a = PluginManager(scope_key=scope) + manager_b = PluginManager(scope_key=scope) + context_a = PluginContext( + PluginManifest(name="ledger_old", key="ledger_old"), manager_a + ) + context_b = PluginContext( + PluginManifest(name="ledger_new", key="ledger_new"), manager_b + ) + + try: + context_a.register_platform( + name=name, + label="old", + adapter_factory=lambda config: "old", + check_fn=lambda: True, + ) + old_entry = platform_registry.get(name) + context_b.register_platform( + name=name, + label="new", + adapter_factory=lambda config: "new", + check_fn=lambda: True, + ) + new_entry = platform_registry.get(name) + + assert old_entry is not None and new_entry is not None + assert manager_a.unload("ledger_old") is True + assert platform_registry.get(name) is new_entry + assert name not in manager_a._plugin_platform_names + assert manager_b.unload("ledger_new") is True + assert platform_registry.get(name) is not old_entry + assert platform_registry.snapshot_registration(name, scope=scope) == previous + finally: + current = platform_registry.snapshot_registration(name, scope=scope) + platform_registry.restore_registration( + name, current, previous, scope=scope + ) + + +def test_manager_local_override_does_not_resurrect_after_targeted_unload(): + from hermes_cli.plugins import PluginContext, PluginManager, PluginManifest + + manager = PluginManager() + context_a = PluginContext(PluginManifest(name="local_a", key="local_a"), manager) + context_b = PluginContext(PluginManifest(name="local_b", key="local_b"), manager) + + context_a.register_cli_command("shared-local", "A", lambda parser: None) + context_b.register_cli_command("shared-local", "B", lambda parser: None) + + assert manager.unload("local_a") is True + assert manager._cli_commands["shared-local"]["plugin"] == "local_b" + assert manager.unload("local_b") is True + assert "shared-local" not in manager._cli_commands + + +def test_provider_overlay_switches_profiles_and_reveals_fresh_global_fallback( + tmp_path, + monkeypatch, +): + """Provider consumers see A→B→A, and unload never pins a stale base.""" + from agent.image_gen_provider import ImageGenProvider + import agent.image_gen_registry as image_registry + from hermes_constants import reset_hermes_home_override, set_hermes_home_override + from hermes_cli.plugins import PluginContext, PluginManager, PluginManifest + + class Provider(ImageGenProvider): + def __init__(self, marker): + self.marker = marker + + @property + def name(self): + return "ledger_profile_provider" + + def generate(self, prompt, aspect_ratio="landscape", **kwargs): + return {"marker": self.marker} + + name = "ledger_profile_provider" + baseline = image_registry.snapshot_registration(name) + global_a = Provider("global-a") + global_b = Provider("global-b") + provider_a = Provider("profile-a") + provider_b = Provider("profile-b") + home_a = str((tmp_path / "provider-a").resolve()) + home_b = str((tmp_path / "provider-b").resolve()) + manager_a = PluginManager(scope_key=home_a) + manager_b = PluginManager(scope_key=home_b) + context_a = PluginContext(PluginManifest(name="provider_a", key="provider_a"), manager_a) + context_b = PluginContext(PluginManifest(name="provider_b", key="provider_b"), manager_b) + monkeypatch.setattr( + "hermes_cli.config.load_config_readonly", + lambda: {"image_gen": {"provider": name}}, + ) + + def active_for(home): + token = set_hermes_home_override(home) + try: + return image_registry.get_active_provider() + finally: + reset_hermes_home_override(token) + + image_registry.register_provider(global_a) + try: + assert context_a.register_image_gen_provider(provider_a) is not None + assert context_b.register_image_gen_provider(provider_b) is not None + assert active_for(home_a) is provider_a + assert active_for(home_b) is provider_b + assert active_for(home_a) is provider_a + + assert manager_a.unload("provider_a") is True + assert active_for(home_a) is global_a + image_registry.register_provider(global_b) + assert active_for(home_a) is global_b + assert active_for(home_b) is provider_b + + assert manager_b.unload("provider_b") is True + assert active_for(home_b) is global_b + finally: + manager_a.unload("provider_a") + manager_b.unload("provider_b") + current = image_registry.snapshot_registration(name) + if current is not None: + image_registry.restore_registration(name, current, baseline) + + +def test_reused_provider_singleton_keeps_registration_generations_distinct(): + """Reusing one object after unload must not revive its retired generation.""" + from agent.image_gen_provider import ImageGenProvider + from agent.image_gen_registry import ( + get_provider, + restore_registration, + snapshot_registration, + ) + from hermes_cli.plugins import PluginContext, PluginManager, PluginManifest + + class ProbeProvider(ImageGenProvider): + def __init__(self, marker): + self.marker = marker + + @property + def name(self): + return "ledger_generation_provider" + + def generate(self, prompt, aspect_ratio="landscape", **kwargs): + return {"marker": self.marker} + + managers = [PluginManager() for _ in range(4)] + scope = managers[0].scope_key + baseline = get_provider("ledger_generation_provider", scope=scope) + baseline_local = snapshot_registration("ledger_generation_provider", scope=scope) + provider_a = ProbeProvider("a") + provider_b = ProbeProvider("b") + provider_c = ProbeProvider("c") + contexts = [ + PluginContext( + PluginManifest(name=f"provider_{index}", key=f"provider_{index}"), + manager, + ) + for index, manager in enumerate(managers) + ] + + try: + contexts[0].register_image_gen_provider(provider_a) + contexts[1].register_image_gen_provider(provider_b) + managers[0].unload("provider_0") + assert get_provider(provider_a.name, scope=scope) is provider_b + + # A fresh ownership generation deliberately reuses the same singleton. + contexts[2].register_image_gen_provider(provider_a) + contexts[3].register_image_gen_provider(provider_c) + managers[3].unload("provider_3") + assert get_provider(provider_a.name, scope=scope) is provider_a + managers[2].unload("provider_2") + assert get_provider(provider_a.name, scope=scope) is provider_b + managers[1].unload("provider_1") + assert get_provider(provider_a.name, scope=scope) is baseline + finally: + current = snapshot_registration("ledger_generation_provider", scope=scope) + if current is not baseline_local and current is not None: + restore_registration( + "ledger_generation_provider", current, baseline_local, scope=scope + ) + + +def test_same_provider_singleton_can_have_two_live_owners(): + """Retiring an older identical lease must not remove the newer owner.""" + from agent.image_gen_provider import ImageGenProvider + from agent.image_gen_registry import ( + get_provider, + restore_registration, + snapshot_registration, + ) + from hermes_cli.plugins import PluginContext, PluginManager, PluginManifest + + class SharedProvider(ImageGenProvider): + @property + def name(self): + return "ledger_shared_singleton" + + def generate(self, prompt, aspect_ratio="landscape", **kwargs): + return {"prompt": prompt} + + provider = SharedProvider() + manager_a = PluginManager() + manager_b = PluginManager() + scope = manager_a.scope_key + baseline = get_provider("ledger_shared_singleton", scope=scope) + baseline_local = snapshot_registration("ledger_shared_singleton", scope=scope) + context_a = PluginContext(PluginManifest(name="shared_a", key="shared_a"), manager_a) + context_b = PluginContext(PluginManifest(name="shared_b", key="shared_b"), manager_b) + + try: + assert context_a.register_image_gen_provider(provider) is not None + assert context_b.register_image_gen_provider(provider) is not None + manager_a.unload("shared_a") + assert get_provider(provider.name, scope=scope) is provider + manager_b.unload("shared_b") + assert get_provider(provider.name, scope=scope) is baseline + finally: + current = snapshot_registration(provider.name, scope=scope) + if current is not baseline_local and current is not None: + restore_registration( + provider.name, current, baseline_local, scope=scope + ) + + +def test_provider_cleanup_uses_the_captured_normalized_name(): + """Whitespace and later name mutation cannot orphan a provider entry.""" + from agent.image_gen_provider import ImageGenProvider + from agent.image_gen_registry import ( + get_provider, + restore_registration, + snapshot_registration, + ) + from hermes_cli.plugins import PluginContext, PluginManager, PluginManifest + + class MutableProvider(ImageGenProvider): + def __init__(self): + self._name = " ledger_mutable_provider " + + @property + def name(self): + return self._name + + def generate(self, prompt, aspect_ratio="landscape", **kwargs): + return {"prompt": prompt} + + key = "ledger_mutable_provider" + provider = MutableProvider() + manager = PluginManager() + scope = manager.scope_key + baseline = get_provider(key, scope=scope) + baseline_local = snapshot_registration(key, scope=scope) + context = PluginContext( + PluginManifest(name="mutable_provider", key="mutable_provider"), manager + ) + try: + assert context.register_image_gen_provider(provider) is not None + assert get_provider(key, scope=scope) is provider + provider._name = "renamed_after_registration" + manager.unload("mutable_provider") + assert get_provider(key, scope=scope) is baseline + finally: + current = snapshot_registration(key, scope=scope) + if current is not baseline_local and current is not None: + restore_registration(key, current, baseline_local, scope=scope) + + +def test_registration_transaction_excludes_concurrent_disposal(monkeypatch): + """A lease cannot be retired between another generation's write/acquire.""" + from agent.image_gen_provider import ImageGenProvider + import agent.image_gen_registry as image_registry + from hermes_cli.plugins import PluginContext, PluginManager, PluginManifest + + class Provider(ImageGenProvider): + def __init__(self, marker): + self.marker = marker + + @property + def name(self): + return "ledger_transaction_provider" + + def generate(self, prompt, aspect_ratio="landscape", **kwargs): + return {"marker": self.marker} + + provider_a = Provider("a") + provider_b = Provider("b") + manager_a = PluginManager() + manager_b = PluginManager() + scope = manager_a.scope_key + baseline = image_registry.get_provider("ledger_transaction_provider", scope=scope) + baseline_local = image_registry.snapshot_registration( + "ledger_transaction_provider", scope=scope + ) + context_a = PluginContext(PluginManifest(name="tx_a", key="tx_a"), manager_a) + context_b = PluginContext(PluginManifest(name="tx_b", key="tx_b"), manager_b) + context_a.register_image_gen_provider(provider_a) + + original_register = image_registry.register_provider + wrote = Event() + release = Event() + + def paused_register(provider, *, scope=None): + original_register(provider, scope=scope) + if provider is provider_b: + wrote.set() + assert release.wait(timeout=2) + + monkeypatch.setattr(image_registry, "register_provider", paused_register) + try: + with ThreadPoolExecutor(max_workers=2) as pool: + registration = pool.submit(context_b.register_image_gen_provider, provider_b) + assert wrote.wait(timeout=1) + disposal = pool.submit(manager_a.unload, "tx_a") + try: + disposal.result(timeout=0.05) + except TimeoutError: + pass + else: + raise AssertionError("dispose interleaved with registration transaction") + release.set() + assert registration.result(timeout=1) is not None + assert disposal.result(timeout=1) is True + + assert image_registry.get_provider(provider_a.name, scope=scope) is provider_b + manager_b.unload("tx_b") + assert image_registry.get_provider(provider_a.name, scope=scope) is baseline + finally: + release.set() + current = image_registry.snapshot_registration( + "ledger_transaction_provider", scope=scope + ) + if current is not baseline_local and current is not None: + image_registry.restore_registration( + "ledger_transaction_provider", + current, + baseline_local, + scope=scope, + ) + + +def test_deferred_platform_resolution_is_atomic_across_threads(): + """Concurrent first lookups both observe the materialized adapter.""" + from gateway.platform_registry import PlatformEntry, PlatformRegistry + + registry = PlatformRegistry() + entry = PlatformEntry( + name="threaded_platform", + label="Threaded", + adapter_factory=lambda config: object(), + check_fn=lambda: True, + source="plugin", + ) + loads = [] + started = Event() + release = Event() + + def load(): + loads.append(1) + started.set() + assert release.wait(timeout=2) + registry.register(entry) + + registry.register_deferred("threaded_platform", load) + with ThreadPoolExecutor(max_workers=3) as pool: + first = pool.submit(registry.get, "threaded_platform") + assert started.wait(timeout=1) + assert registry.is_registered("threaded_platform") + second = pool.submit(registry.get, "threaded_platform") + try: + second.result(timeout=0.05) + except TimeoutError: + pass + else: + raise AssertionError("concurrent lookup did not wait for materialization") + enumeration = pool.submit(registry.all_entries) + try: + enumeration.result(timeout=0.05) + except TimeoutError: + pass + else: + raise AssertionError("enumeration omitted an in-flight platform") + release.set() + results = [first.result(timeout=1), second.result(timeout=1)] + assert enumeration.result(timeout=1) == [entry] + + assert results == [entry, entry] + assert loads == [1] + + +def test_deferred_platform_recursive_lookup_does_not_deadlock(): + """A loader that asks for its own entry fails fast until registration.""" + from gateway.platform_registry import PlatformEntry, PlatformRegistry + + registry = PlatformRegistry() + nested_results = [] + entry = PlatformEntry( + name="recursive_platform", + label="Recursive", + adapter_factory=lambda config: object(), + check_fn=lambda: True, + source="plugin", + ) + + def load(): + nested_results.append(registry.get("recursive_platform")) + registry.register(entry) + + registry.register_deferred("recursive_platform", load) + assert registry.get("recursive_platform") is entry + assert nested_results == [None] + + +def test_resolved_deferred_platform_restores_its_displaced_loader(): + """Deferred-to-concrete loading remains one replacement chain.""" + from gateway.platform_registry import platform_registry + from hermes_cli.plugins import PluginContext, PluginManager, PluginManifest + + name = "ledger_transfer" + scope = platform_registry.current_scope_key() + previous = platform_registry.snapshot_registration(name, scope=scope) + old_loader = lambda: None + platform_registry.register_deferred(name, old_loader, scope=scope) + displaced = platform_registry.snapshot_registration(name, scope=scope) + manager = PluginManager(scope_key=scope) + manifest = PluginManifest( + name=f"{name}-platform", + key=f"{name}-platform", + source="bundled", + path="unused", + ) + + def load_scoped(self, loaded_manifest): + PluginContext(loaded_manifest, self).register_platform( + name=name, + label="Transferred", + adapter_factory=lambda config: object(), + check_fn=lambda: True, + ) + + manager._load_plugin_scoped = MethodType(load_scoped, manager) + try: + manager._register_deferred_platform(manifest) + entry = platform_registry.get(name) + assert entry is not None and entry.label == "Transferred" + manager.unload(manifest) + assert platform_registry.snapshot_registration(name, scope=scope) == displaced + finally: + current = platform_registry.snapshot_registration(name, scope=scope) + platform_registry.restore_registration( + name, current, previous, scope=scope + ) + + +def test_unload_cancels_a_deferred_platform_before_module_load(): + """Losing the in-flight race cannot publish registrations after unload.""" + from gateway.platform_registry import platform_registry + from hermes_cli.plugins import PluginContext, PluginManager, PluginManifest + + name = "ledger_cancel" + scope = platform_registry.current_scope_key() + previous = platform_registry.snapshot_registration(name, scope=scope) + def old_loader(): + from gateway.platform_registry import PlatformEntry + + platform_registry.register( + PlatformEntry( + name=name, + label="Restored predecessor", + adapter_factory=lambda config: object(), + check_fn=lambda: True, + source="plugin", + ), + scope=scope, + ) + platform_registry.register_deferred(name, old_loader, scope=scope) + displaced = platform_registry.snapshot_registration(name, scope=scope) + manager = PluginManager(scope_key=scope) + manifest = PluginManifest( + name=f"{name}-platform", + key=f"{name}-platform", + source="bundled", + path="unused", + ) + + def load_scoped(self, loaded_manifest): + PluginContext(loaded_manifest, self).register_platform( + name=name, + label="Should not publish", + adapter_factory=lambda config: object(), + check_fn=lambda: True, + ) + + manager._load_plugin_scoped = MethodType(load_scoped, manager) + manager._register_deferred_platform(manifest) + try: + with ThreadPoolExecutor(max_workers=1) as pool: + with manager._discovery_lock: + lookup = pool.submit(platform_registry.get, name) + deadline = monotonic() + 1 + while (scope, name) not in platform_registry._inflight: + if monotonic() >= deadline: + raise AssertionError("deferred loader never became in-flight") + sleep(0.001) + assert manager.unload(manifest) is True + restored_entry = lookup.result(timeout=1) + assert restored_entry is not None + assert restored_entry.label == "Restored predecessor" + assert platform_registry.snapshot_registration(name, scope=scope)[0] is restored_entry + assert name not in manager._plugin_platform_names + finally: + current = platform_registry.snapshot_registration(name, scope=scope) + platform_registry.restore_registration( + name, current, previous, scope=scope + ) + + +def test_direct_plugin_platform_registration_infers_immutable_scope(tmp_path): + """The documented direct registry API cannot leak into another profile.""" + from gateway.platform_registry import PlatformEntry, platform_registry + from hermes_constants import reset_hermes_home_override, set_hermes_home_override + from tools.registry import registry as tool_registry + + home_a = str((tmp_path / "direct-a").resolve()) + home_b = tmp_path / "direct-b" + module_name = "company.hermes.direct_platform_probe" + policy = tool_registry.register_plugin_override_policy( + module_name, False, scope=home_a + ) + factory = eval( + "lambda config: 'direct-a'", + {"__name__": f"{module_name}.handlers"}, + ) + name = "ledger_direct_platform" + previous_a = platform_registry.snapshot_registration(name, scope=home_a) + previous_global = platform_registry.snapshot_registration(name) + token = set_hermes_home_override(home_b) + try: + platform_registry.register( + PlatformEntry( + name=name, + label="Direct A", + adapter_factory=factory, + check_fn=lambda: True, + source="plugin", + ) + ) + assert platform_registry.get(name) is None + finally: + reset_hermes_home_override(token) + + token = set_hermes_home_override(home_a) + try: + assert platform_registry.get(name).label == "Direct A" + finally: + reset_hermes_home_override(token) + current_a = platform_registry.snapshot_registration(name, scope=home_a) + platform_registry.restore_registration( + name, current_a, previous_a, scope=home_a + ) + current_global = platform_registry.snapshot_registration(name) + if current_global != previous_global: + platform_registry.restore_registration( + name, current_global, previous_global + ) + tool_registry.restore_plugin_override_policy( + module_name, policy, None, scope=home_a + ) + + +def test_same_name_tool_and_platform_are_isolated_by_hermes_home( + tmp_path, + monkeypatch, +): + """Real A→B→A profile switching keeps dispatch and adapters isolated.""" + import hermes_cli.plugins as plugins_mod + from gateway.platform_registry import platform_registry + from hermes_constants import reset_hermes_home_override, set_hermes_home_override + from hermes_cli.plugins import PluginManager + from tools.registry import registry + + home_a = tmp_path / "profile-a" + home_b = tmp_path / "profile-b" + _write_profile_probe(home_a, "profile-a") + _write_profile_probe(home_b, "profile-b") + monkeypatch.setattr( + plugins_mod, + "get_bundled_plugins_dir", + lambda: tmp_path / "empty-bundled", + ) + monkeypatch.setattr(PluginManager, "_scan_entry_points", lambda self: []) + + def load_profile(home: Path): + token = set_hermes_home_override(home) + try: + manager = plugins_mod.get_plugin_manager() + manager.discover_and_load() + tool_entry = registry.get_entry("shared_profile_tool") + platform_entry = platform_registry.get("shared_profile_platform") + return manager, tool_entry, platform_entry + finally: + reset_hermes_home_override(token) + + manager_a, tool_a, platform_a = load_profile(home_a) + manager_b, tool_b, platform_b = load_profile(home_b) + + assert manager_a is not manager_b + assert tool_a is not None and tool_b is not None and tool_a is not tool_b + assert platform_a is not None and platform_b is not None and platform_a is not platform_b + + token_a = set_hermes_home_override(home_a) + try: + assert registry.dispatch("shared_profile_tool", {}) == "profile-a" + assert platform_registry.get("shared_profile_platform") is platform_a + finally: + reset_hermes_home_override(token_a) + + token_b = set_hermes_home_override(home_b) + try: + assert registry.dispatch("shared_profile_tool", {}) == "profile-b" + assert platform_registry.get("shared_profile_platform") is platform_b + finally: + reset_hermes_home_override(token_b) + + token_a = set_hermes_home_override(home_a) + try: + assert registry.dispatch("shared_profile_tool", {}) == "profile-a" + assert platform_registry.get("shared_profile_platform") is platform_a + finally: + reset_hermes_home_override(token_a) + + +def test_manager_discovery_uses_its_home_not_the_ambient_profile( + tmp_path, + monkeypatch, +): + """A retained manager cannot scan another concurrently active profile.""" + import hermes_cli.plugins as plugins_mod + from gateway.platform_registry import platform_registry + from hermes_constants import reset_hermes_home_override, set_hermes_home_override + from hermes_cli.plugins import PluginManager + from tools.registry import registry + + home_a = tmp_path / "retained-a" + home_b = tmp_path / "ambient-b" + _write_profile_probe(home_a, "retained-a") + _write_profile_probe(home_b, "ambient-b") + monkeypatch.setattr( + plugins_mod, + "get_bundled_plugins_dir", + lambda: tmp_path / "empty-bundled", + ) + monkeypatch.setattr(PluginManager, "_scan_entry_points", lambda self: []) + + manager_a = PluginManager(scope_key=str(home_a.resolve())) + ambient = set_hermes_home_override(home_b) + try: + manager_a.discover_and_load() + finally: + reset_hermes_home_override(ambient) + + tool_a = registry.get_entry("shared_profile_tool", scope=manager_a.scope_key) + platform_a = platform_registry.snapshot_registration( + "shared_profile_platform", scope=manager_a.scope_key + )[0] + assert tool_a is not None and tool_a.handler({}) == "retained-a" + assert platform_a is not None and platform_a.label == "retained-a" + assert registry.snapshot_registration( + "shared_profile_tool", scope=str(home_b.resolve()) + ) is None + assert platform_registry.snapshot_registration( + "shared_profile_platform", scope=str(home_b.resolve()) + ) == (None, None) + + +def test_same_slug_profiles_allocate_distinct_modules_concurrently( + tmp_path, + monkeypatch, +): + """Policy binding and import use one atomic profile-specific namespace.""" + import hermes_cli.plugins as plugins_mod + from hermes_cli.plugins import PluginManager + + home_a = tmp_path / "concurrent-a" + home_b = tmp_path / "concurrent-b" + _write_profile_probe(home_a, "concurrent-a") + _write_profile_probe(home_b, "concurrent-b") + monkeypatch.setattr( + plugins_mod, + "get_bundled_plugins_dir", + lambda: tmp_path / "empty-bundled", + ) + monkeypatch.setattr(PluginManager, "_scan_entry_points", lambda self: []) + managers = [ + PluginManager(scope_key=str(home_a.resolve())), + PluginManager(scope_key=str(home_b.resolve())), + ] + + with ThreadPoolExecutor(max_workers=2) as pool: + list(pool.map(lambda manager: manager.discover_and_load(), managers)) + + modules = [manager._plugins["profile_probe"].module.__name__ for manager in managers] + assert modules[0] != modules[1] diff --git a/tests/hermes_cli/test_plugins.py b/tests/hermes_cli/test_plugins.py index b44e91769e..fe84c550a7 100644 --- a/tests/hermes_cli/test_plugins.py +++ b/tests/hermes_cli/test_plugins.py @@ -1322,22 +1322,30 @@ class TestPluginCommands: try: manager_a = plugins_mod.get_plugin_manager() manager_a.discover_and_load() + module_a = manager_a._plugins["stateful-plugin"].module finally: reset_hermes_home_override(token_a) - assert "hermes_plugins.stateful_plugin.state" in sys.modules - assert sys.modules["hermes_plugins.stateful_plugin.state"].MARKER == "marker-a" + assert module_a is not None + module_a_state = f"{module_a.__name__}.state" + assert module_a_state in sys.modules + assert sys.modules[module_a_state].MARKER == "marker-a" token_b = set_hermes_home_override(str(home_b)) try: manager_b = plugins_mod.get_plugin_manager() manager_b.discover_and_load() + module_b = manager_b._plugins["stateful-plugin"].module finally: reset_hermes_home_override(token_b) - # The submodule cached under sys.modules must now reflect profile - # b's code, not a leftover from profile a. - assert sys.modules["hermes_plugins.stateful_plugin.state"].MARKER == "marker-b" + # Each profile keeps a stable namespace, so concurrent/runtime relative + # imports cannot resolve another profile's package or submodules. + assert module_b is not None + module_b_state = f"{module_b.__name__}.state" + assert module_a.__name__ != module_b.__name__ + assert sys.modules[module_a_state].MARKER == "marker-a" + assert sys.modules[module_b_state].MARKER == "marker-b" assert ( manager_b._plugin_skills["stateful-plugin::marker"]["marker"] == "marker-b" ) diff --git a/tests/secret_sources/test_error_remediation.py b/tests/secret_sources/test_error_remediation.py index 0874725caa..852c65bd74 100644 --- a/tests/secret_sources/test_error_remediation.py +++ b/tests/secret_sources/test_error_remediation.py @@ -13,7 +13,7 @@ import pytest from agent.secret_sources import bitwarden as bw from agent.secret_sources import onepassword as op -from agent.secret_sources.base import ErrorKind, SecretSource +from agent.secret_sources.base import ErrorKind, FetchResult, SecretSource from agent.secret_sources.bitwarden import ( BitwardenSource, _classify_bws_error, @@ -149,3 +149,36 @@ def test_env_loader_prints_remediation_hint(tmp_path, monkeypatch, capsys): assert "hermes secrets bitwarden token" in err +def test_remediation_hint_uses_explicit_profile_scope(tmp_path, monkeypatch): + from agent.secret_sources import registry + from hermes_cli import env_loader + + class ScopedSource(SecretSource): + name = "scoped_hint" + label = "Scoped hint" + shape = "mapped" + + def __init__(self, marker): + self.marker = marker + + def fetch(self, cfg, home_path): + return FetchResult() + + def remediation(self, kind, cfg): + return self.marker + + monkeypatch.setattr(registry, "_ensure_builtin_sources", lambda: None) + registry._reset_registry_for_tests() + home_a = str((tmp_path / "hint-a").resolve()) + home_b = str((tmp_path / "hint-b").resolve()) + source_a = ScopedSource("profile-a") + source_b = ScopedSource("profile-b") + assert registry.register_source(source_a, scope=home_a) + assert registry.register_source(source_b, scope=home_b) + try: + assert env_loader._remediation_hint( + "scoped_hint", ErrorKind.AUTH_FAILED, {}, scope=home_b + ) == "profile-b" + finally: + registry._reset_registry_for_tests() + diff --git a/tests/secret_sources/test_secret_source_registry.py b/tests/secret_sources/test_secret_source_registry.py index c48cacfc09..3fca184844 100644 --- a/tests/secret_sources/test_secret_source_registry.py +++ b/tests/secret_sources/test_secret_source_registry.py @@ -95,6 +95,38 @@ class TestRegistration: def test_rejects_non_secretsource_instance(self): assert reg.register_source(object()) is False + def test_same_name_is_isolated_by_profile(self, tmp_path): + from hermes_constants import ( + reset_hermes_home_override, + set_hermes_home_override, + ) + + home_a = str((tmp_path / "secrets-a").resolve()) + home_b = str((tmp_path / "secrets-b").resolve()) + source_a = _make_source(name="profile_secret", secrets={"A": "a"}) + source_b = _make_source(name="profile_secret", secrets={"B": "b"}) + assert reg.register_source(source_a, scope=home_a) + assert reg.register_source(source_b, scope=home_b) + + token = set_hermes_home_override(home_a) + try: + assert reg.get_source("profile_secret") is source_a + explicit_b_env = {} + report = reg.apply_all( + {"profile_secret": {"enabled": True}}, + Path(home_b), + environ=explicit_b_env, + ) + assert report.sources[0].result.secrets == {"B": "b"} + assert explicit_b_env == {"B": "b"} + finally: + reset_hermes_home_override(token) + token = set_hermes_home_override(home_b) + try: + assert reg.get_source("profile_secret") is source_b + finally: + reset_hermes_home_override(token) + diff --git a/tests/test_get_tool_definitions_cache_isolation.py b/tests/test_get_tool_definitions_cache_isolation.py index e1698d16e0..d883104481 100644 --- a/tests/test_get_tool_definitions_cache_isolation.py +++ b/tests/test_get_tool_definitions_cache_isolation.py @@ -16,6 +16,9 @@ These tests pin: """ from __future__ import annotations +from concurrent.futures import ThreadPoolExecutor +from threading import Barrier + import pytest import model_tools @@ -80,3 +83,28 @@ class TestQuietModeCacheIsolation: explains why the bug only hit Gateway.""" model_tools.get_tool_definitions(quiet_mode=False) assert len(model_tools._tool_defs_cache) == 0 + + def test_concurrent_capacity_misses_evict_atomically(self, monkeypatch): + """Two profile/toolset misses at capacity cannot race on eviction.""" + barrier = Barrier(2) + + def compute(*args, **kwargs): + barrier.wait(timeout=2) + return [] + + monkeypatch.setattr(model_tools, "_compute_tool_definitions", compute) + for index in range(model_tools._TOOL_DEFS_CACHE_MAX): + model_tools._tool_defs_cache[("old", index)] = [] + + with ThreadPoolExecutor(max_workers=2) as pool: + futures = [ + pool.submit( + model_tools.get_tool_definitions, + enabled_toolsets=[f"concurrent_{index}"], + quiet_mode=True, + ) + for index in range(2) + ] + assert [future.result(timeout=2) for future in futures] == [[], []] + + assert len(model_tools._tool_defs_cache) == model_tools._TOOL_DEFS_CACHE_MAX diff --git a/tests/tools/test_browser_cloud_provider_cache.py b/tests/tools/test_browser_cloud_provider_cache.py index c5b7dbcec6..d7b9671000 100644 --- a/tests/tools/test_browser_cloud_provider_cache.py +++ b/tests/tools/test_browser_cloud_provider_cache.py @@ -25,6 +25,179 @@ def _reset_resolver_state(monkeypatch): class TestCloudProviderCachePolicy: + def test_cache_is_isolated_by_hermes_home(self, tmp_path, monkeypatch): + from hermes_constants import ( + get_hermes_home, + reset_hermes_home_override, + set_hermes_home_override, + ) + + monkeypatch.setattr( + "hermes_cli.config.read_raw_config", + lambda: {"browser": {"cloud_provider": "profile-provider"}}, + ) + providers = {} + resolutions = [] + + def resolve(_name): + home = str(get_hermes_home()) + resolutions.append(home) + return providers[home] + + monkeypatch.setattr(browser_tool, "_ensure_browser_plugins_loaded", lambda: None) + monkeypatch.setattr(browser_tool, "_registry_get_browser_provider", resolve) + home_a = tmp_path / "browser-a" + home_b = tmp_path / "browser-b" + providers[str(home_a)] = Mock(name="provider-a") + providers[str(home_b)] = Mock(name="provider-b") + + def resolve_for(home): + token = set_hermes_home_override(home) + try: + return browser_tool._get_cloud_provider() + finally: + reset_hermes_home_override(token) + + assert resolve_for(home_a) is providers[str(home_a)] + assert resolve_for(home_b) is providers[str(home_b)] + assert resolve_for(home_a) is providers[str(home_a)] + assert resolutions == [str(home_a), str(home_b)] + + def test_same_profile_registry_replacement_invalidates_cache( + self, tmp_path, monkeypatch + ): + from agent.browser_provider import BrowserProvider + import agent.browser_registry as browser_registry + from hermes_constants import ( + reset_hermes_home_override, + set_hermes_home_override, + ) + + class Provider(BrowserProvider): + def __init__(self, marker): + self.marker = marker + + @property + def name(self): + return "cache-replacement" + + def is_available(self): + return True + + def create_session(self, task_id): + return {"marker": self.marker} + + def close_session(self, session_id): + return True + + def emergency_cleanup(self, session_id): + return None + + home = str((tmp_path / "same-profile").resolve()) + first = Provider("first") + second = Provider("second") + monkeypatch.setattr( + "hermes_cli.config.read_raw_config", + lambda: {"browser": {"cloud_provider": "cache-replacement"}}, + ) + monkeypatch.setattr(browser_tool, "_ensure_browser_plugins_loaded", lambda: None) + token = set_hermes_home_override(home) + try: + browser_registry.register_provider(first, scope=home) + assert browser_tool._get_cloud_provider() is first + browser_registry.register_provider(second, scope=home) + assert browser_tool._get_cloud_provider() is second + finally: + current = browser_registry.snapshot_registration( + "cache-replacement", scope=home + ) + if current is not None: + browser_registry.restore_registration( + "cache-replacement", current, None, scope=home + ) + reset_hermes_home_override(token) + + def test_concurrent_registry_replacement_discards_stale_resolution( + self, tmp_path, monkeypatch + ): + from concurrent.futures import ThreadPoolExecutor + from threading import Event + + from agent.browser_provider import BrowserProvider + import agent.browser_registry as browser_registry + from hermes_constants import ( + reset_hermes_home_override, + set_hermes_home_override, + ) + + class Provider(BrowserProvider): + def __init__(self, marker): + self.marker = marker + + @property + def name(self): + return "cache-race" + + def is_available(self): + return True + + def create_session(self, task_id): + return {"marker": self.marker} + + def close_session(self, session_id): + return True + + def emergency_cleanup(self, session_id): + return None + + home = str((tmp_path / "race-profile").resolve()) + first = Provider("first") + second = Provider("second") + paused = Event() + release = Event() + calls = 0 + original_get = browser_registry.get_provider + + def racing_get(name): + nonlocal calls + calls += 1 + resolved = original_get(name, scope=home) + if calls == 1: + paused.set() + assert release.wait(timeout=2) + return resolved + + monkeypatch.setattr( + "hermes_cli.config.read_raw_config", + lambda: {"browser": {"cloud_provider": "cache-race"}}, + ) + monkeypatch.setattr(browser_tool, "_ensure_browser_plugins_loaded", lambda: None) + monkeypatch.setattr(browser_tool, "_registry_get_browser_provider", racing_get) + browser_registry.register_provider(first, scope=home) + + def resolve(): + token = set_hermes_home_override(home) + try: + return browser_tool._get_cloud_provider() + finally: + reset_hermes_home_override(token) + + try: + with ThreadPoolExecutor(max_workers=1) as pool: + future = pool.submit(resolve) + assert paused.wait(timeout=1) + browser_registry.register_provider(second, scope=home) + release.set() + assert future.result(timeout=2) is second + assert calls == 2 + finally: + release.set() + current = browser_registry.snapshot_registration("cache-race", scope=home) + if current is not None: + browser_registry.restore_registration( + "cache-race", current, None, scope=home + ) + def test_explicit_local_caches_permanently(self, monkeypatch): """`cloud_provider: local` is a positive choice and must stick.""" monkeypatch.setattr( diff --git a/tools/browser_tool.py b/tools/browser_tool.py index f2b0ac1314..e0f124231f 100644 --- a/tools/browser_tool.py +++ b/tools/browser_tool.py @@ -69,6 +69,7 @@ from hermes_constants import ( agent_browser_runnable, get_hermes_home, get_hermes_home_override, + hermes_home_key, ) from utils import env_int, is_truthy_value from hermes_cli.config import DEFAULT_CONFIG, cfg_get @@ -165,6 +166,16 @@ from agent.browser_provider import BrowserProvider as CloudBrowserProvider # no from agent.browser_registry import ( # noqa: F401 (test-patchable surface) get_provider as _registry_get_browser_provider, ) +try: + from agent.browser_registry import ( + registry_generation as _browser_registry_generation, + ) +except ImportError: + # A few isolated compatibility tests intentionally install a minimal + # ``agent.browser_registry`` stub exposing only ``get_provider``. Those + # harnesses have no mutable registry, so a constant generation is exact. + def _browser_registry_generation(*, scope=None): + return (0, 0) from plugins.browser.browserbase.provider import ( # noqa: F401 (legacy import surface) BrowserbaseBrowserProvider as BrowserbaseProvider, ) @@ -683,6 +694,11 @@ _DEFAULT_PROVIDER_REGISTRY: Dict[str, type] = dict(_PROVIDER_REGISTRY) _cached_cloud_provider: Optional[CloudBrowserProvider] = None _cloud_provider_resolved = False +_cached_cloud_provider_scope: Optional[str] = None +_cached_cloud_providers: Dict[ + tuple[str, tuple[int, int]], Optional[CloudBrowserProvider] +] = {} +_cloud_provider_cache_lock = threading.RLock() _allow_private_urls_resolved = False _cached_allow_private_urls: Optional[bool] = None _cached_agent_browser: Optional[str] = None @@ -738,6 +754,46 @@ def _ensure_browser_plugins_loaded() -> None: def _get_cloud_provider() -> Optional[CloudBrowserProvider]: + """Return the provider cached for the active Hermes profile.""" + global _cached_cloud_provider, _cloud_provider_resolved + global _cached_cloud_provider_scope + + scope = hermes_home_key() + with _cloud_provider_cache_lock: + # Tests and legacy reset paths clear the boolean. Treat that as a full + # reset even if a previous scoped resolution remains mirrored here. + if not _cloud_provider_resolved: + _cached_cloud_provider_scope = None + _cached_cloud_providers.clear() + while True: + before_generation = _browser_registry_generation(scope=scope) + cache_key = (scope, before_generation) + if cache_key in _cached_cloud_providers: + _cached_cloud_provider = _cached_cloud_providers[cache_key] + _cloud_provider_resolved = True + _cached_cloud_provider_scope = scope + return _cached_cloud_provider + + _cached_cloud_provider = None + _cloud_provider_resolved = False + resolved = _resolve_cloud_provider_uncached() + after_generation = _browser_registry_generation(scope=scope) + if before_generation != after_generation: + # A force reload replaced/unloaded this profile's provider + # while resolution was in progress. Discard the stale result + # and resolve against the new registry generation. + continue + if _cloud_provider_resolved: + _cached_cloud_provider_scope = scope + for stale_key in [ + key for key in _cached_cloud_providers if key[0] == scope + ]: + _cached_cloud_providers.pop(stale_key, None) + _cached_cloud_providers[cache_key] = resolved + return resolved + + +def _resolve_cloud_provider_uncached() -> Optional[CloudBrowserProvider]: """Return the configured cloud browser provider, or None for local mode. Reads ``config["browser"]["cloud_provider"]`` once and caches the result @@ -755,8 +811,6 @@ def _get_cloud_provider() -> Optional[CloudBrowserProvider]: ``_is_legacy_provider_registry_overridden``. """ global _cached_cloud_provider, _cloud_provider_resolved - if _cloud_provider_resolved: - return _cached_cloud_provider resolved: Optional[CloudBrowserProvider] = None try: diff --git a/tools/registry.py b/tools/registry.py index b0f1caaa0a..0810409114 100644 --- a/tools/registry.py +++ b/tools/registry.py @@ -15,6 +15,7 @@ Import chain (circular-import safe): """ import ast +import functools import importlib import json import logging @@ -24,6 +25,8 @@ import time from pathlib import Path from typing import Callable, Dict, List, Optional, Set +from hermes_constants import hermes_home_key + logger = logging.getLogger(__name__) # Cap on a tool error body; only trims runaway interpolated exceptions (static msgs are ~115 chars). @@ -230,6 +233,15 @@ class ToolEntry: self.dynamic_schema_overrides = dynamic_schema_overrides +class _PluginOverridePolicy: + """Identity-bearing authorization record for one plugin generation.""" + + __slots__ = ("allowed",) + + def __init__(self, allowed: bool) -> None: + self.allowed = bool(allowed) + + # --------------------------------------------------------------------------- # check_fn TTL cache # @@ -415,13 +427,20 @@ class ToolRegistry: """Singleton registry that collects tool schemas + handlers from tool files.""" def __init__(self): + # Built-in and other process-global registrations. self._tools: Dict[str, ToolEntry] = {} - # Durable map: plugin module namespace (handler.__globals__["__name__"]) - # -> operator opt-in for built-in override. Populated at plugin load and - # never cleared, so a plugin's override authorization is bound to the - # code that defined the handler, independent of WHEN the register() call - # happens (sync during load, or a delayed/threaded callback afterwards). - self._plugin_override_policy: Dict[str, bool] = {} + # Plugin registrations are overlays keyed by resolved HERMES_HOME. A + # profile sees its own overlay first and then the global built-ins. + self._scoped_tools: Dict[str, Dict[str, ToolEntry]] = {} + # Plugin module namespace -> operator opt-in for built-in override. + # Authorization records are lifecycle-managed; the separate scope map + # remains durable so delayed callbacks stay profile-confined. + self._plugin_override_policy: Dict[ + tuple[Optional[str], str], _PluginOverridePolicy + ] = {} + # Scope attribution stays durable after policy removal so delayed code + # remains confined to the profile where its module was loaded. + self._plugin_module_scopes: Dict[str, Set[Optional[str]]] = {} self._toolset_checks: Dict[str, Callable] = {} self._toolset_aliases: Dict[str, str] = {} # MCP dynamic refresh can mutate the registry while other threads are @@ -435,10 +454,30 @@ class ToolRegistry: # long as the generation hasn't changed. self._generation: int = 0 - def _snapshot_state(self) -> tuple[List[ToolEntry], Dict[str, Callable]]: + @staticmethod + def current_scope_key() -> str: + """Return the active profile's canonical registry scope.""" + return hermes_home_key() + + def _merged_tools(self, scope: Optional[str] = None) -> Dict[str, ToolEntry]: + """Return global tools overlaid with one profile's plugin tools.""" + active_scope = scope or self.current_scope_key() + merged = dict(self._tools) + merged.update(self._scoped_tools.get(active_scope, {})) + return merged + + def _snapshot_state( + self, + scope: Optional[str] = None, + ) -> tuple[List[ToolEntry], Dict[str, Callable]]: """Return a coherent snapshot of registry entries and toolset checks.""" with self._lock: - return list(self._tools.values()), dict(self._toolset_checks) + entries = list(self._merged_tools(scope).values()) + checks = dict(self._toolset_checks) + for entry in entries: + if entry.check_fn is not None: + checks[entry.toolset] = entry.check_fn + return entries, checks def _snapshot_entries(self) -> List[ToolEntry]: """Return a stable snapshot of registered tool entries.""" @@ -468,15 +507,35 @@ class ToolRegistry: return True return False - def get_entry(self, name: str) -> Optional[ToolEntry]: - """Return a registered tool entry by name, or None.""" + def get_entry( + self, + name: str, + *, + scope: Optional[str] = None, + ) -> Optional[ToolEntry]: + """Return the active profile's entry by name, falling back to global.""" with self._lock: - return self._tools.get(name) + return self._merged_tools(scope).get(name) + + def snapshot_registration( + self, + name: str, + *, + scope: Optional[str] = None, + ) -> Optional[ToolEntry]: + """Return the local slot state without following global fallback.""" + with self._lock: + target = self._tools if scope is None else self._scoped_tools.get(scope, {}) + return target.get(name) def get_registered_toolset_names(self) -> List[str]: """Return sorted unique toolset names present in the registry.""" return sorted({entry.toolset for entry in self._snapshot_entries()}) + def get_all_entries(self) -> List[ToolEntry]: + """Return the active profile's merged tool entries.""" + return self._snapshot_entries() + def get_tool_names_for_toolset(self, toolset: str) -> List[str]: """Return sorted tool names registered under a given toolset.""" return sorted( @@ -510,14 +569,62 @@ class ToolRegistry: # Registration # ------------------------------------------------------------------ - def register_plugin_override_policy(self, module_namespace: str, allowed: bool) -> None: - """Bind a plugin module namespace to its operator opt-in for built-in - override. Called once per plugin at load time. Durable: never cleared, - so later (even threaded/delayed) register() calls from that module are - still gated by the same policy. + def register_plugin_override_policy( + self, + module_namespace: str, + allowed: bool, + *, + scope: Optional[str] = None, + ) -> _PluginOverridePolicy: + """Bind a plugin module namespace to its current operator opt-in. + + The identity-bearing result lets plugin unload/reload revoke a stale + authorization without losing durable module-to-profile attribution. """ with self._lock: - self._plugin_override_policy[module_namespace] = bool(allowed) + policy = _PluginOverridePolicy(allowed) + self._plugin_override_policy[(scope, module_namespace)] = policy + self._plugin_module_scopes.setdefault(module_namespace, set()).add(scope) + return policy + + def snapshot_plugin_override_policy( + self, + module_namespace: str, + *, + scope: Optional[str] = None, + ) -> Optional[_PluginOverridePolicy]: + """Return one local authorization generation without fallback.""" + with self._lock: + return self._plugin_override_policy.get((scope, module_namespace)) + + def restore_plugin_override_policy( + self, + module_namespace: str, + current: _PluginOverridePolicy, + previous: Optional[_PluginOverridePolicy], + *, + scope: Optional[str] = None, + ) -> bool: + """CAS-restore policy state while retaining durable scope attribution.""" + with self._lock: + key = (scope, module_namespace) + if self._plugin_override_policy.get(key) is not current: + return False + if previous is None: + self._plugin_override_policy.pop(key, None) + else: + self._plugin_override_policy[key] = previous + return True + + def _plugin_override_allowed( + self, + scope: Optional[str], + module_namespace: str, + ) -> bool: + policy = self._plugin_override_policy.get((scope, module_namespace)) + if policy is None and scope is not None: + policy = self._plugin_override_policy.get((None, module_namespace)) + return bool(policy and policy.allowed) def _plugin_owner_of(self, handler: Callable) -> Optional[str]: """Return the plugin module namespace that defined *handler*, or None @@ -531,18 +638,86 @@ class ToolRegistry: handlers live outside the plugin namespace and return None (unchanged behavior). """ - try: - mod = handler.__globals__.get("__name__", "") # type: ignore[attr-defined] - except AttributeError: + mod = self._callable_module(handler) + if not mod: return None - if mod in self._plugin_override_policy: - return mod + return self._plugin_namespace_of_module(mod) + + @staticmethod + def _callable_module(handler: Callable) -> str: + """Resolve defining module through wrappers, partials, and objects.""" + current = handler + seen: Set[int] = set() + while id(current) not in seen: + seen.add(id(current)) + if isinstance(current, functools.partial): + current = current.func + continue + func = getattr(current, "__func__", None) + if func is not None: + current = func + continue + globals_dict = getattr(current, "__globals__", None) + if isinstance(globals_dict, dict): + module_name = globals_dict.get("__name__", "") + if module_name: + return str(module_name) + wrapped = getattr(current, "__wrapped__", None) + if wrapped is not None: + current = wrapped + continue + break + module_name = getattr(current, "__module__", "") + if module_name: + return str(module_name) + return str(getattr(type(current), "__module__", "") or "") + + def _plugin_namespace_of_module( + self, + module_namespace: str, + ) -> Optional[str]: + """Resolve a module/submodule to its durable plugin namespace.""" + with self._lock: + matches = [ + namespace + for namespace in self._plugin_module_scopes + if module_namespace == namespace + or module_namespace.startswith(f"{namespace}.") + ] + if matches: + return max(matches, key=len) # Also gate plugin modules currently loading but not yet policy-recorded # (defensive: a handler defined in the plugin namespace is plugin code). - if isinstance(mod, str) and mod.startswith("hermes_plugins."): - return mod + if module_namespace.startswith("hermes_plugins."): + return ".".join(module_namespace.split(".")[:2]) return None + def _plugin_scope_of(self, module_namespace: str) -> Optional[str]: + """Return the profile scope bound to a loaded plugin module.""" + with self._lock: + scopes = self._plugin_module_scopes.get(module_namespace) + if not scopes: + return None + active_scope = self.current_scope_key() + if active_scope in scopes: + return active_scope + if len(scopes) == 1: + return next(iter(scopes)) + raise PermissionError( + f"Plugin module {module_namespace!r} is active in multiple " + "profiles and cannot register outside one of those scopes." + ) + + def plugin_scope_for_module(self, module_namespace: str) -> Optional[str]: + """Public host lookup for a loaded plugin module's immutable scope.""" + owner = self._plugin_namespace_of_module(module_namespace) + return self._plugin_scope_of(owner or module_namespace) + + def plugin_scope_for_callable(self, callback: Callable) -> Optional[str]: + """Return the durable plugin scope for any supported callable shape.""" + module_name = self._callable_module(callback) + return self.plugin_scope_for_module(module_name) if module_name else None + @staticmethod def _caller_module() -> str: """Best-effort module name of whoever called the registry method that @@ -573,6 +748,7 @@ class ToolRegistry: max_result_size_chars: int | float | None = None, dynamic_schema_overrides: Callable = None, override: bool = False, + scope: Optional[str] = None, ): """Register a tool. Called at module-import time by each tool file. @@ -582,22 +758,58 @@ class ToolRegistry: registrations that would shadow an existing tool from a different toolset are rejected to prevent accidental overwrites. """ + handler_owner = self._plugin_owner_of(handler) + caller_owner = self._plugin_namespace_of_module(self._caller_module()) + owner = caller_owner or handler_owner + if scope is None and owner is not None: + scope = self._plugin_scope_of(owner) with self._lock: - existing = self._tools.get(name) + target = ( + self._tools + if scope is None + else self._scoped_tools.setdefault(scope, {}) + ) + existing = ( + self._tools.get(name) + if scope is None + else self._merged_tools(scope).get(name) + ) + shadows_global = ( + owner is not None + and scope is not None + and name not in target + and name in self._tools + ) + if shadows_global: + if not override: + logger.error( + "Tool registration REJECTED: plugin %r attempted to " + "shadow global tool %r without override=True", + owner, + name, + ) + return + if not self._plugin_override_allowed(scope, owner): + raise PermissionError( + f"Plugin module {owner!r} cannot override built-in " + f"tool {name!r} without operator opt-in " + f"(allow_tool_override)." + ) if existing and existing.toolset != toolset: if override: - _owner = self._plugin_owner_of(handler) - if _owner is not None and not self._plugin_override_policy.get(_owner, False): + if owner is not None and not self._plugin_override_allowed( + scope, owner + ): logger.error( "Tool registration REJECTED: plugin %r attempted to " "override built-in tool %r (existing toolset %r) without " "operator opt-in. Set " "plugins.entries..allow_tool_override: true " "in config.yaml to allow it.", - _owner, name, existing.toolset, + owner, name, existing.toolset, ) raise PermissionError( - f"Plugin module {_owner!r} cannot override built-in " + f"Plugin module {owner!r} cannot override built-in " f"tool {name!r} without operator opt-in " f"(allow_tool_override)." ) @@ -620,7 +832,7 @@ class ToolRegistry: name, toolset, existing.toolset, ) return - self._tools[name] = ToolEntry( + target[name] = ToolEntry( name=name, toolset=toolset, schema=schema, @@ -639,7 +851,7 @@ class ToolRegistry: # banner.py reads (presence only, never called) to classify an # already-unavailable toolset as lazy-init vs disabled. Keep the # write path for that classification. - if check_fn and toolset not in self._toolset_checks: + if scope is None and check_fn and toolset not in self._toolset_checks: self._toolset_checks[toolset] = check_fn self._generation += 1 @@ -660,11 +872,30 @@ class ToolRegistry: every refresh and has no plugin-override concept. """ with self._lock: - entry = self._tools.get(name) + caller_mod = self._caller_module() + caller_owner = self._plugin_namespace_of_module(caller_mod) + caller_scope = ( + self._plugin_scope_of(caller_owner) + if caller_owner is not None + else None + ) + target = ( + self._scoped_tools.get(caller_scope, {}) + if caller_scope is not None + else self._tools + ) + entry = target.get(name) + if entry is None and caller_scope is not None: + if name in self._tools: + raise PermissionError( + f"Scoped plugin module {caller_mod!r} cannot deregister " + f"process-global tool {name!r}; register a scoped " + "override instead." + ) + return if entry is None: return if not entry.toolset.startswith("mcp-"): - caller_mod = self._caller_module() owner = self._plugin_owner_of(entry.handler) # Ownership check: bind to the plugin package root # (``hermes_plugins.{name}``), not the exact module string. @@ -673,13 +904,13 @@ class ToolRegistry: # string equality would wrongly block root-module cleanup code # from removing tools registered by a submodule of the same # plugin (egilewski review on #55840). - caller_root = ".".join(caller_mod.split(".")[:2]) - owner_root = ".".join(owner.split(".")[:2]) if owner else "" - same_plugin = bool(owner and caller_root == owner_root) + same_plugin = bool(owner and caller_owner == owner) if ( - caller_mod.startswith("hermes_plugins.") + caller_owner is not None and not same_plugin - and not self._plugin_override_policy.get(caller_root, False) + and not self._plugin_override_allowed( + caller_scope, caller_owner + ) ): logger.error( "Tool deregistration REJECTED: plugin %r attempted to " @@ -694,11 +925,14 @@ class ToolRegistry: f"{name!r} (toolset {entry.toolset!r}) without operator " f"opt-in (allow_tool_override)." ) - del self._tools[name] + del target[name] + if caller_scope is not None and not target: + self._scoped_tools.pop(caller_scope, None) # Drop the toolset check and aliases if this was the last tool in # that toolset. toolset_still_exists = any( - e.toolset == entry.toolset for e in self._tools.values() + e.toolset == entry.toolset + for e in self._merged_tools(caller_scope).values() ) if not toolset_still_exists: self._toolset_checks.pop(entry.toolset, None) @@ -715,6 +949,8 @@ class ToolRegistry: name: str, current: ToolEntry, previous: Optional[ToolEntry], + *, + scope: Optional[str] = None, ) -> bool: """Restore a host-owned registration if it is still current. @@ -725,13 +961,20 @@ class ToolRegistry: must leave the newer entry untouched. """ with self._lock: - if self._tools.get(name) is not current: + target = ( + self._tools + if scope is None + else self._scoped_tools.setdefault(scope, {}) + ) + if target.get(name) is not current: return False if previous is None: - self._tools.pop(name, None) + target.pop(name, None) else: - self._tools[name] = previous + target[name] = previous + if scope is not None and not target: + self._scoped_tools.pop(scope, None) # Rebuild the affected toolset checks from the surviving entries. # A plugin may have replaced an entry in the same toolset, so @@ -742,18 +985,23 @@ class ToolRegistry: affected_toolsets.add(previous.toolset) for toolset in affected_toolsets: surviving = [ - entry for entry in self._tools.values() + entry for entry in self._merged_tools(scope).values() if entry.toolset == toolset ] check_fn = next( (entry.check_fn for entry in surviving if entry.check_fn), None, ) - if check_fn is None: - self._toolset_checks.pop(toolset, None) - else: - self._toolset_checks[toolset] = check_fn - if not surviving: + if scope is None: + if check_fn is None: + self._toolset_checks.pop(toolset, None) + else: + self._toolset_checks[toolset] = check_fn + if not surviving and not any( + entry.toolset == toolset + for entries in self._scoped_tools.values() + for entry in entries.values() + ): self._toolset_aliases = { alias: target for alias, target in self._toolset_aliases.items() @@ -851,7 +1099,14 @@ class ToolRegistry: result_type=result_type, ) - def dispatch(self, name: str, args: dict, **kwargs) -> str | dict: + def dispatch( + self, + name: str, + args: dict, + *, + scope: Optional[str] = None, + **kwargs, + ) -> str | dict: """Execute a tool handler by name. * Async handlers are bridged automatically via ``_run_async()``. @@ -860,7 +1115,7 @@ class ToolRegistry: * All exceptions are caught and returned as ``{"error": "..."}`` for consistent error format. """ - entry = self.get_entry(name) + entry = self.get_entry(name, scope=scope) if not entry: return tool_error(f"Unknown tool: {name}") try: diff --git a/toolsets.py b/toolsets.py index 235f341fc7..f33966ebae 100644 --- a/toolsets.py +++ b/toolsets.py @@ -810,7 +810,7 @@ def resolve_toolset(name: str, visited: Set[str] = None, *, include_registry: bo try: from tools.registry import registry plugin_tools.update( - e.name for e in registry._tools.values() + e.name for e in registry.get_all_entries() if e.toolset == platform_name ) except Exception: