diff --git a/hermes_cli/plugins_discovery.py b/hermes_cli/plugins_discovery.py index a5c8b54caa..7eba65b277 100644 --- a/hermes_cli/plugins_discovery.py +++ b/hermes_cli/plugins_discovery.py @@ -24,10 +24,7 @@ from hermes_cli.relay_plugin_cutover import LEGACY_RELAY_PLUGIN_KEYS, RELAY_PLUG logger = logging.getLogger("hermes_cli.plugins") - ENTRY_POINTS_GROUP = "hermes_agent.plugins" - - ENTRY_POINT_CAPABILITIES_GROUP = "hermes_agent.plugin_capabilities" @@ -56,30 +53,24 @@ def discover_entrypoint_manifests() -> List["PluginManifest"]: except Exception as exc: logger.debug("Entry-point scan failed: %s", exc) return manifests - for ep in group_eps: try: - capabilities = [] - for capability in VALID_CAPABILITY_IDS: - declaration_name = f"{ep.name}.{capability}" + capabilities = [ + capability for capability in VALID_CAPABILITY_IDS if any( - declaration.name == declaration_name and declaration.value == ep.value + declaration.name == f"{ep.name}.{capability}" and declaration.value == ep.value for declaration in capability_eps - ): - capabilities.append(capability) + ) + ] dist = getattr(ep, "dist", None) metadata = getattr(dist, "metadata", None) - manifest = PluginManifest( + manifests.append(PluginManifest( name=ep.name, version=str(getattr(dist, "version", "") or ""), - description=( - str(metadata.get("Summary", "") or "") if metadata is not None else "" - ), source="entrypoint", path=ep.value, key=ep.name, - capabilities=_parse_declared_capabilities( - capabilities, ep.name - ), - ) - manifest.kind = _classify_entrypoint_value_kind(ep.value) - manifests.append(manifest) + description=str(metadata.get("Summary", "") or "") if metadata is not None else "", + source="entrypoint", path=ep.value, key=ep.name, + kind=_classify_entrypoint_value_kind(ep.value), + capabilities=_parse_declared_capabilities(capabilities, ep.name), + )) except Exception as exc: logger.debug("Entry-point manifest for %r skipped: %s", getattr(ep, "name", "?"), exc) return manifests @@ -100,8 +91,7 @@ def _get_disabled_plugins() -> set: """Read ``plugins.disabled`` — a deny-list that wins over ``plugins.enabled``.""" try: from hermes_cli.config import load_config - config = load_config() - disabled = cfg_get(config, "plugins", "disabled", default=[]) + disabled = cfg_get(load_config(), "plugins", "disabled", default=[]) return set(disabled) if isinstance(disabled, list) else set() except Exception: return set() @@ -115,8 +105,7 @@ def _get_enabled_plugins() -> Optional[set]: """ try: from hermes_cli.config import load_config - plugins_cfg = load_config().get("plugins") - enabled = plugins_cfg.get("enabled") if isinstance(plugins_cfg, dict) else None + enabled = cfg_get(load_config(), "plugins", "enabled") return set(enabled) if isinstance(enabled, list) else None except Exception: return None @@ -134,9 +123,7 @@ def scan_directory( if not path.is_dir(): return manifests for child in sorted(path.iterdir()): - if not child.is_dir(): - continue - if depth == 0 and skip_names and child.name in skip_names: + if not child.is_dir() or (depth == 0 and skip_names and child.name in skip_names): continue manifest_file = child / "plugin.yaml" if not manifest_file.exists(): @@ -167,31 +154,25 @@ def collect_directory_manifests() -> List[PluginManifest]: precedence/containment rules of the real discovery sweep.""" from hermes_cli import plugins as _origin # patched names resolve through the origin manifests: List[PluginManifest] = [] + + def _scan(label: str, directory: Path, source: str, skip_names: Optional[Set[str]] = None) -> None: + found = scan_directory(directory, source, skip_names=skip_names) + logger.debug(" %s: %d manifest(s)", label, len(found)) + manifests.extend(found) + # Excluded bundled top-level categories have their own discovery; platforms scan separately. repo_plugins = _origin.get_bundled_plugins_dir() logger.debug("Scanning bundled plugins: %s", repo_plugins) - bundled = scan_directory( - repo_plugins, "bundled", - skip_names={"memory", "context_engine", "platforms", "model-providers"}, - ) - logger.debug(" bundled (top-level): %d manifest(s)", len(bundled)) - manifests.extend(bundled) - bundled_platforms = scan_directory(repo_plugins / "platforms", "bundled") - logger.debug(" bundled/platforms: %d manifest(s)", len(bundled_platforms)) - manifests.extend(bundled_platforms) - + _scan("bundled (top-level)", repo_plugins, "bundled", + {"memory", "context_engine", "platforms", "model-providers"}) + _scan("bundled/platforms", repo_plugins / "platforms", "bundled") user_dir = get_hermes_home() / "plugins" logger.debug("Scanning user plugins: %s", user_dir) - user_manifests = scan_directory(user_dir, "user") - logger.debug(" user: %d manifest(s)", len(user_manifests)) - manifests.extend(user_manifests) - + _scan("user", user_dir, "user") if _origin._env_enabled("HERMES_ENABLE_PROJECT_PLUGINS"): project_dir = Path.cwd() / ".hermes" / "plugins" logger.debug("Scanning project plugins: %s", project_dir) - project_manifests = scan_directory(project_dir, "project") - logger.debug(" project: %d manifest(s)", len(project_manifests)) - manifests.extend(project_manifests) + _scan("project", project_dir, "project") else: logger.debug("Project plugins disabled (set HERMES_ENABLE_PROJECT_PLUGINS=1 to enable)") return manifests @@ -214,9 +195,9 @@ def gate_manifest( disable, category-owned kinds (exclusive / model-provider), bundled auto-loads (backend now, platform deferred), then ``plugins.enabled`` opt-in (path-derived key or legacy bare name).""" lookup_key = manifest_key(manifest) - name = manifest.name + names = {lookup_key, manifest.name} # Relay lifecycle is core-owned; an old plugin copy would compete for its registries. - if lookup_key in LEGACY_RELAY_PLUGIN_KEYS or name in LEGACY_RELAY_PLUGIN_KEYS: + if names & LEGACY_RELAY_PLUGIN_KEYS: error = ( "removed — Relay lifecycle is owned by Hermes core; configure " f"{RELAY_PLUGINS_CONFIG_ENV} instead" @@ -225,7 +206,7 @@ def gate_manifest( "placeholder", error=error, log=(logging.WARNING, "Refusing to load removed Hermes Relay plugin '%s'; %s", lookup_key, error), ) - if lookup_key in disabled or name in disabled: + if names & disabled: return ManifestGate( "placeholder", error="disabled via config", log=(logging.DEBUG, "Skipping disabled plugin '%s'", lookup_key), @@ -243,14 +224,15 @@ def gate_manifest( "placeholder", enabled=True, log=(logging.DEBUG, "Skipping '%s' (model-provider, handled by providers/ discovery)", lookup_key), ) - # Bundled backends auto-load; selection among them is ``.provider`` config. - if manifest.source == "bundled" and manifest.kind == "backend": - return ManifestGate("load_now") - # Bundled platforms register LAZILY: eagerly importing ~20 heavy SDKs added seconds to every - # `hermes` invocation. A deferred loader keeps every platform available on first use. - if manifest.source == "bundled" and manifest.kind == "platform": - return ManifestGate("defer") - if enabled is None or not (lookup_key in enabled or name in enabled): + if manifest.source == "bundled": + # Bundled backends auto-load; selection among them is ``.provider`` config. + if manifest.kind == "backend": + return ManifestGate("load_now") + # Bundled platforms register LAZILY: eagerly importing ~20 heavy SDKs added seconds to every + # `hermes` invocation. A deferred loader keeps every platform available on first use. + if manifest.kind == "platform": + return ManifestGate("defer") + if enabled is None or not names & enabled: return ManifestGate( "placeholder", error=f"not enabled in config (run `hermes plugins enable {lookup_key}` to activate)", diff --git a/hermes_cli/plugins_ledger.py b/hermes_cli/plugins_ledger.py index 33139828fb..9489102ee1 100644 --- a/hermes_cli/plugins_ledger.py +++ b/hermes_cli/plugins_ledger.py @@ -59,18 +59,16 @@ class PluginLedgerMixin: self, manifest: PluginManifest, kind: str, key: str, release: Callable[[], None], *, persistent: bool = False, ) -> PluginRegistration: - """Record one registration under its canonical plugin key. - - ``persistent`` ones (process-global host infrastructure) stay in the ownership ledger for - attribution but NOT in ``_registration_order``, so a routine unload cannot dispose them; - the handle still releases on explicit ``dispose()``. - """ - plugin_key = manifest_key(manifest) + """Record one registration under its canonical plugin key. ``persistent`` ones + (process-global host infrastructure) stay in the ownership ledger for attribution but NOT in + ``_registration_order``, so a routine unload cannot dispose them; the handle still releases + on explicit ``dispose()``.""" registration = PluginRegistration( - kind=kind, key=key, release=release, plugin_key=plugin_key, persistent=persistent, + kind=kind, key=key, release=release, plugin_key=manifest_key(manifest), + persistent=persistent, ) registration._on_dispose = lambda disposed: self._forget_registrations([disposed]) - self._ownership_ledger.setdefault(plugin_key, []).append(registration) + self._ownership_ledger.setdefault(registration.plugin_key, []).append(registration) if not persistent: self._registration_order.append(registration) return registration @@ -79,11 +77,9 @@ class PluginLedgerMixin: self, manifest: PluginManifest, kind: str, name: str, registry: Any, current: Any, previous: Any, *, finalize: Optional[Callable[[], None]] = None, ) -> PluginRegistration: - """Lease one ``(kind, scope, name)`` slot of a scope-keyed process-global registry. - - Unload calls ``registry.restore_registration(name, current, replacement, scope=...)`` — - identity-conditional, so a later generation is never removed by an earlier owner. - """ + """Lease one ``(kind, scope, name)`` slot of a scope-keyed process-global registry. Unload + calls ``registry.restore_registration(name, current, replacement, scope=...)`` — + identity-conditional, so a later generation is never removed by an earlier owner.""" scope = self.scope_key lease = replacement_coordinator.acquire( (kind, scope, name), current=current, previous=previous, @@ -93,22 +89,22 @@ class PluginLedgerMixin: ) return self._track_registration(manifest, kind, name, lease.dispose) + def _active_persistent(self) -> List[PluginRegistration]: + """Live persistent registrations across every plugin in the ownership ledger.""" + return [ + registration for owned in self._ownership_ledger.values() + for registration in owned if registration.persistent and registration.active + ] + def _evict_stale_persistent_registrations(self) -> None: """After re-discovery, dispose parked persistent handles whose plugin did not re-register the same ``(kind, key)``. Re-registered ones are dropped WITHOUT disposing — the same object re-registered would pass the identity check and evict the live entry.""" if not self._persistent_carryover: return - parked = self._persistent_carryover - self._persistent_carryover = [] - current = { - (registration.kind, registration.key) for owned in self._ownership_ledger.values() - for registration in owned if registration.persistent and registration.active - } - stale = [ - registration for registration in parked if registration.active - and (registration.kind, registration.key) not in current - ] + parked, self._persistent_carryover = self._persistent_carryover, [] + current = {(r.kind, r.key) for r in self._active_persistent()} + stale = [r for r in parked if r.active and (r.kind, r.key) not in current] for registration in stale: logger.info( "Evicting persistent registration %s/%s: plugin '%s' no " @@ -158,8 +154,7 @@ class PluginLedgerMixin: def _remove_name_if_unowned(self, kind: str, names: Set[str], name: str) -> None: """Drop *name* from the manager-local name set once no active ledger entry owns it.""" if not any( - registration.active and registration.kind == kind and registration.key == name - for registration in self._registration_order + r.active and r.kind == kind and r.key == name for r in self._registration_order ): names.discard(name) @@ -172,15 +167,10 @@ class PluginLedgerMixin: def _forget_registrations(self, registrations: List[PluginRegistration]) -> None: if not registrations: return - registration_ids = {id(registration) for registration in registrations} - self._registration_order = [ - registration for registration in self._registration_order - if id(registration) not in registration_ids - ] + ids = {id(r) for r in registrations} + self._registration_order = [r for r in self._registration_order if id(r) not in ids] for plugin_key, owned in list(self._ownership_ledger.items()): - remaining = [ - registration for registration in owned if id(registration) not in registration_ids - ] + remaining = [r for r in owned if id(r) not in ids] if remaining: self._ownership_ledger[plugin_key] = remaining else: @@ -204,7 +194,7 @@ class PluginLedgerMixin: if isinstance(plugin, LoadedPlugin): return manifest_key(plugin.manifest) if isinstance(plugin, PluginManifest): - return plugin.key or plugin.name + return manifest_key(plugin) return str(plugin) def unload(self, plugin: Union[str, PluginManifest, LoadedPlugin, None] = None) -> bool: @@ -213,41 +203,32 @@ class PluginLedgerMixin: return self._unload_scoped(plugin) def _unload_scoped(self, plugin: Union[str, PluginManifest, LoadedPlugin, None] = None) -> bool: - """Unload one plugin (or all when ``plugin=None``, as force rediscovery does). - - Every ledger registration — including on_unload callbacks and supervised tasks — is disposed - in reverse acquisition order with identity-conditional inverses. Returns ``True`` when - anything was found. - """ + """Unload one plugin (or all when ``plugin=None``, as force rediscovery does). Every ledger + registration — including on_unload callbacks and supervised tasks — is disposed in reverse + acquisition order with identity-conditional inverses. Returns ``True`` when anything was + found.""" unload_all = plugin is None if unload_all: target_keys = set(self._ownership_ledger) | set(self._plugins) registrations = list(self._registration_order) else: target_keys = self._unload_target_keys(self._resolve_plugin_key(plugin)) - registrations = [ - registration for registration in self._registration_order - if registration.plugin_key in target_keys - ] + registrations = [r for r in self._registration_order if r.plugin_key in target_keys] # Persistent registrations are absent from _registration_order (unload-all keeps them), # but a *targeted* unload is the disable/uninstall path: a disabled auth plugin's # provider must NOT stay live process-wide. registrations.extend( - registration for key in target_keys - for registration in self._ownership_ledger.get(key, []) - if registration.persistent and registration.active + r for key in target_keys for r in self._ownership_ledger.get(key, []) + if r.persistent and r.active ) - found = bool(target_keys or registrations) self._dispose_registrations(registrations) self._forget_registrations(registrations) - if unload_all: self._reset_after_unload_all(registrations) else: for key in target_keys: self._plugins.pop(key, None) - return found def _unload_target_keys(self, requested: str) -> Set[str]: @@ -266,12 +247,8 @@ class PluginLedgerMixin: platform_registry.unregister(platform_name) # Ledger-owned tool names are excluded: their handles already restored the previous entry, # and blanket deregistration would remove what the ledger just restored. - ledger_tool_names = { - registration.key for registration in registrations if registration.kind == "tool" - } - preledger_tools = tuple( - name for name in self._plugin_tool_names if name not in ledger_tool_names - ) + ledger_tool_names = {r.key for r in registrations if r.kind == "tool"} + preledger_tools = tuple(n for n in self._plugin_tool_names if n not in ledger_tool_names) if preledger_tools: try: from tools.registry import registry as tool_registry @@ -285,11 +262,9 @@ class PluginLedgerMixin: logger.debug("unload: tool deregister %s failed: %s", tool_name, exc) # Persistent registrations survive unload-all but must not be orphaned by the ledger clear: # carry them over so force re-discovery can evict the ones whose plugin does not come back. - carryover_ids = {id(registration) for registration in self._persistent_carryover} + carryover_ids = {id(r) for r in self._persistent_carryover} self._persistent_carryover.extend( - registration for owned in self._ownership_ledger.values() for registration in owned - if registration.persistent and registration.active - and id(registration) not in carryover_ids + r for r in self._active_persistent() if id(r) not in carryover_ids ) for container in ( self._ownership_ledger, self._plugins, self._hooks, self._middleware, diff --git a/hermes_cli/plugins_manifest.py b/hermes_cli/plugins_manifest.py index d2b0e5e2ce..545076a6d8 100644 --- a/hermes_cli/plugins_manifest.py +++ b/hermes_cli/plugins_manifest.py @@ -23,22 +23,38 @@ except ImportError: # pragma: no cover – yaml is optional at import time logger = logging.getLogger("hermes_cli.plugins") +_VALID_PLUGIN_KINDS: Set[str] = {"standalone", "backend", "exclusive", "platform", "model-provider"} + +# Unknown plugin.yaml fields are forward-compat surface: warn (debug for v1 files, warning for v2+) +# and continue loading. ``capabilities``/``emits``/``listens``/``hermes``/``depends`` are reserved. +_KNOWN_MANIFEST_FIELDS: Set[str] = { + "name", "version", "description", "author", "requires_env", "provides_tools", "provides_hooks", + "kind", "hooks", "label", "optional_env", "platforms", "external_dependencies", + "pip_dependencies", "provides_browser_providers", "provides_web_providers", + "manifest_version", "api_version", "requires_plugins", "python_dependencies", "config_schema", + "license", "homepage", "tags", "capabilities", "emits", "listens", "hermes", "depends", +} + +# Highest manifest schema version this Hermes understands. +SUPPORTED_MANIFEST_VERSION = 2 + +_CONFIG_SCHEMA_TYPES: Dict[str, tuple] = { + "str": (str,), "string": (str,), "int": (int,), "integer": (int,), "float": (int, float), + "number": (int, float), "bool": (bool,), "boolean": (bool,), "list": (list,), "array": (list,), + "dict": (dict,), "object": (dict,), +} + def _plugins_debug() -> bool: from hermes_cli import plugins as _origin return _origin._PLUGINS_DEBUG -_VALID_PLUGIN_KINDS: Set[str] = {"standalone", "backend", "exclusive", "platform", "model-provider"} - - def _portable_skill_namespace(key: str) -> str: """Return a readable, collision-resistant namespace for a portable plugin.""" - slug = "".join( ch if ch.isascii() and (ch.isalnum() or ch in "_-") else "-" for ch in key.lower() - ) - slug = slug.strip("-_") or "plugin" + ).strip("-_") or "plugin" digest = hashlib.sha256(key.encode("utf-8")).hexdigest()[:8] return f"agent-plugin-{slug}-{digest}" @@ -46,39 +62,10 @@ def _portable_skill_namespace(key: str) -> str: def _display_author(value: object) -> str: """Normalize a manifest author value for the string PluginManifest field.""" if isinstance(value, Mapping): - return ", ".join( - str(value[field]) for field in ("name", "email", "url") if value.get(field) - ) + return ", ".join(str(value[f]) for f in ("name", "email", "url") if value.get(f)) return "" if value is None else str(value) -# Manifest v2 parsing. Unknown plugin.yaml fields are forward-compat surface: warn (debug for v1 -# files, warning for v2+) and continue loading. -_KNOWN_MANIFEST_FIELDS: Set[str] = { - # v1 - "name", "version", "description", "author", "requires_env", - "provides_tools", "provides_hooks", "kind", "hooks", "label", - "optional_env", "platforms", "external_dependencies", "pip_dependencies", - "provides_browser_providers", "provides_web_providers", - # v2 - "manifest_version", "api_version", "requires_plugins", - "python_dependencies", "config_schema", "license", "homepage", "tags", - # owned by sibling sub-issues but reserved so their manifests don't warn - "capabilities", "emits", "listens", "hermes", "depends", -} - - -# Highest manifest schema version this Hermes understands. -SUPPORTED_MANIFEST_VERSION = 2 - - -_CONFIG_SCHEMA_TYPES: Dict[str, tuple] = { - "str": (str,), "string": (str,), "int": (int,), "integer": (int,), "float": (int, float), - "number": (int, float), "bool": (bool,), "boolean": (bool,), "list": (list,), "array": (list,), - "dict": (dict,), "object": (dict,), -} - - def _manifest_field_of_type(data: Mapping, key: str, field_name: str, typ, what: str): """Return ``data[field_name]`` when absent or of ``typ``; warn and return None otherwise.""" raw = data.get(field_name) @@ -91,7 +78,6 @@ def _manifest_field_of_type(data: Mapping, key: str, field_name: str, typ, what: def _parse_manifest_v2_fields(data: Mapping, key: str) -> Dict[str, Any]: """Validate/normalize manifest v2 fields into PluginManifest kwargs (warnings, never failures).""" out: Dict[str, Any] = {} - # manifest_version — absent means v1 (supported forever). raw_mv = data.get("manifest_version", 1) try: @@ -108,7 +94,6 @@ def _parse_manifest_v2_fields(data: Mapping, key: str) -> Dict[str, Any]: key, mv, SUPPORTED_MANIFEST_VERSION, ) out["manifest_version"] = mv - # api_version — plugin API generation (independent of manifest_version). raw_api = data.get("api_version") out["api_version"] = None @@ -117,7 +102,6 @@ def _parse_manifest_v2_fields(data: Mapping, key: str) -> Dict[str, Any]: out["api_version"] = int(raw_api) except (TypeError, ValueError): logger.warning("Plugin %s: api_version %r is not an integer; ignoring", key, raw_api) - # requires_plugins — list of {id, version_range?} (str shorthand ok). deps: List[Dict[str, Any]] = [] for item in _manifest_field_of_type(data, key, "requires_plugins", list, "a list") or []: @@ -132,7 +116,6 @@ def _parse_manifest_v2_fields(data: Mapping, key: str) -> Dict[str, Any]: "string or a {id, version_range} mapping; skipping", key, item, ) out["requires_plugins"] = deps - # python_dependencies — validated and surfaced ONLY; never auto-installed. pydeps: List[str] = [] raw_pydeps = _manifest_field_of_type( @@ -147,7 +130,6 @@ def _parse_manifest_v2_fields(data: Mapping, key: str) -> Dict[str, Any]: "requirement string; skipping", key, item, ) out["python_dependencies"] = pydeps - # config_schema — mapping of key -> {type?, default?, description?, required?}. schema: Dict[str, Any] = {} raw_schema = _manifest_field_of_type(data, key, "config_schema", Mapping, "a mapping") @@ -167,13 +149,10 @@ def _parse_manifest_v2_fields(data: Mapping, key: str) -> Dict[str, Any]: ) schema[str(skey)] = dict(spec) out["config_schema"] = schema - - # Standard metadata. out["license"] = str(data.get("license") or "") out["homepage"] = str(data.get("homepage") or "") raw_tags = _manifest_field_of_type(data, key, "tags", list, "a list") out["tags"] = [str(t) for t in (raw_tags or [])] - # Forward compat: unknown fields warn (never fail); v1 manifests only at debug. unknown = sorted(set(data.keys()) - _KNOWN_MANIFEST_FIELDS) if unknown: @@ -182,7 +161,6 @@ def _parse_manifest_v2_fields(data: Mapping, key: str) -> Dict[str, Any]: "Plugin %s: unknown manifest field(s) ignored: %s " "(newer manifest schema or typo; plugin still loads)", key, ", ".join(unknown), ) - return out @@ -194,8 +172,7 @@ def validate_config_schema(plugin_id: str, schema: Mapping, settings: Mapping) - for skey, spec in schema.items(): if not isinstance(spec, Mapping): continue - present = skey in settings - if not present: + if skey not in settings: if spec.get("required") and "default" not in spec: warnings.append( f"plugins.entries.{plugin_id}.settings.{skey} is required " @@ -204,17 +181,15 @@ def validate_config_schema(plugin_id: str, schema: Mapping, settings: Mapping) - continue stype = spec.get("type") expected = _CONFIG_SCHEMA_TYPES.get(str(stype).lower()) if stype else None - if expected is not None: - value = settings[skey] - # bool is an int subclass — don't let True satisfy int/float. - ok = isinstance(value, expected) and not ( - isinstance(value, bool) and bool not in expected + if expected is None: + continue + value = settings[skey] + # bool is an int subclass — don't let True satisfy int/float. + if not isinstance(value, expected) or (isinstance(value, bool) and bool not in expected): + warnings.append( + f"plugins.entries.{plugin_id}.settings.{skey} should be " + f"{stype} (got {type(value).__name__})" ) - if not ok: - warnings.append( - f"plugins.entries.{plugin_id}.settings.{skey} should be " - f"{stype} (got {type(value).__name__})" - ) return warnings @@ -228,22 +203,15 @@ def resolve_plugin_load_order(manifests: Mapping[str, "PluginManifest"]) -> List keys = sorted(manifests.keys()) by_name: Dict[str, str] = {} for k in keys: - name = manifests[k].name - if name and name not in by_name: - by_name[name] = k - - def _resolve_dep(dep_id: str) -> Optional[str]: - if dep_id in manifests: - return dep_id - return by_name.get(dep_id) - + if manifests[k].name: + by_name.setdefault(manifests[k].name, k) edges: Dict[str, Set[str]] = {k: set() for k in keys} for k in keys: for dep in manifests[k].requires_plugins: dep_id = dep.get("id") if isinstance(dep, Mapping) else None if not dep_id: continue - resolved = _resolve_dep(dep_id) + resolved = dep_id if dep_id in manifests else by_name.get(dep_id) if resolved is None: logger.warning( "Plugin %s requires plugin '%s' which is not enabled/" @@ -251,12 +219,10 @@ def resolve_plugin_load_order(manifests: Mapping[str, "PluginManifest"]) -> List "via ctx.has_plugin). Run `hermes plugins enable %s` if it is installed.", k, dep_id, dep_id, ) - continue - if resolved == k: + elif resolved == k: logger.warning("Plugin %s declares a dependency on itself; ignoring", k) - continue - edges[k].add(resolved) - + else: + edges[k].add(resolved) sorter = graphlib.TopologicalSorter(edges) try: sorter.prepare() @@ -267,7 +233,6 @@ def resolve_plugin_load_order(manifests: Mapping[str, "PluginManifest"]) -> List "alphabetical load order for all plugins", " -> ".join(str(c) for c in cycle), ) return keys - ordered: List[str] = [] while sorter.is_active(): ready = sorted(sorter.get_ready()) @@ -277,11 +242,9 @@ def resolve_plugin_load_order(manifests: Mapping[str, "PluginManifest"]) -> List def _detect_kind_from_source(source_text: str) -> Optional[str]: - """Return the kind implied by source markers (mirrors plugins/memory ``_is_memory_provider_dir``). - - Memory-provider markers -> ``exclusive``; ``register_provider`` + ``ProviderProfile`` -> - ``model-provider``; else ``None``. Keeps both kinds out of the general manager's eager import. - """ + """Kind implied by source markers (mirrors plugins/memory ``_is_memory_provider_dir``): + memory-provider markers -> ``exclusive``; ``register_provider`` + ``ProviderProfile`` -> + ``model-provider``; else ``None``. Keeps both kinds out of the general manager's eager import.""" if "register_memory_provider" in source_text or "MemoryProvider" in source_text: return "exclusive" if "register_provider" in source_text and "ProviderProfile" in source_text: @@ -293,14 +256,11 @@ def _read_source_from_origin(origin: Optional[str], limit: int = 8192) -> str: """First ``limit`` chars of a module's source (``.pyc`` mapped back to ``.py``); "" on failure.""" if not origin: return "" - if origin.endswith((".pyc", ".pyo")): - try: - origin = importlib.util.source_from_cache(origin) - except Exception: - return "" - if not origin.endswith(".py"): - return "" try: + if origin.endswith((".pyc", ".pyo")): + origin = importlib.util.source_from_cache(origin) + if not origin.endswith(".py"): + return "" return Path(origin).read_text(encoding="utf-8", errors="replace")[:limit] except Exception: return "" @@ -322,7 +282,6 @@ def resolve_module_origin(module_name: str) -> Optional[str]: return None if len(parts) == 1: return spec.origin - search_paths = spec.submodule_search_locations if not search_paths: return None diff --git a/hermes_cli/plugins_state.py b/hermes_cli/plugins_state.py index f8d08adc73..c0afd31898 100644 --- a/hermes_cli/plugins_state.py +++ b/hermes_cli/plugins_state.py @@ -13,22 +13,11 @@ from typing import Any, Dict, Mapping from hermes_constants import get_hermes_home from hermes_cli.plugins_manifest import _portable_skill_namespace - _PLUGIN_SETTING_SEGMENT_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_-]{0,127}$") - - _PLUGIN_SETTING_RESERVED_ROOTS = frozenset({"model", "plugins", "security", "settings"}) - - _PLUGIN_STATE_KEY_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_.:-]{0,127}$") - - _PLUGIN_STATE_QUOTA_BYTES = 10 * 1024 * 1024 - - _PLUGIN_STATE_LOCKS: Dict[str, threading.RLock] = {} - - _PLUGIN_STATE_LOCKS_GUARD = threading.Lock() @@ -40,9 +29,8 @@ def _plugin_relative_segments(key: str) -> tuple[str, ...]: segments = tuple(key.split(".")) if ( not key or "/" in key or "\\" in key - or any( - not _PLUGIN_SETTING_SEGMENT_RE.fullmatch(segment) for segment in segments - ) or segments[0].lower() in _PLUGIN_SETTING_RESERVED_ROOTS + or not all(_PLUGIN_SETTING_SEGMENT_RE.fullmatch(segment) for segment in segments) + or segments[0].lower() in _PLUGIN_SETTING_RESERVED_ROOTS ): raise ValueError( "Expected a plugin-relative config key such as 'endpoint' or " @@ -52,6 +40,7 @@ def _plugin_relative_segments(key: str) -> tuple[str, ...]: def _nested_plugin_value(root: object, segments: tuple[str, ...], default: Any) -> Any: + """Walk ``segments`` through nested mappings; ``default`` on the first miss.""" current = root for segment in segments: if not isinstance(current, Mapping) or segment not in current: @@ -61,6 +50,7 @@ def _nested_plugin_value(root: object, segments: tuple[str, ...], default: Any) def _nested_plugin_mapping(segments: tuple[str, ...], value: Any) -> dict[str, Any]: + """Wrap ``value`` in nested single-key dicts, outermost first.""" nested: Any = value for segment in reversed(segments): nested = {segment: nested} @@ -80,24 +70,20 @@ def _plugin_data_namespace(plugin_id: str, skill_namespace: str) -> str: return _portable_skill_namespace(candidate) -def _state_thread_lock(path: Path) -> threading.RLock: - key = str(path.resolve(strict=False)) - with _PLUGIN_STATE_LOCKS_GUARD: - return _PLUGIN_STATE_LOCKS.setdefault(key, threading.RLock()) - - @contextmanager def _locked_plugin_state(path: Path): """Serialize state read-modify-write across threads/processes (fcntl / msvcrt). The lock lives in a sibling file because atomic replacement changes the target's inode.""" lock_path = path.with_name(f".{path.name}.lock") - thread_lock = _state_thread_lock(lock_path) + with _PLUGIN_STATE_LOCKS_GUARD: + thread_lock = _PLUGIN_STATE_LOCKS.setdefault( + str(lock_path.resolve(strict=False)), threading.RLock() + ) with thread_lock: lock_path.parent.mkdir(parents=True, exist_ok=True) with open(lock_path, "a+b") as handle: if os.name == "nt": # pragma: no cover - exercised on Windows CI import msvcrt - if handle.seek(0, os.SEEK_END) == 0: handle.write(b"\0") handle.flush() @@ -105,7 +91,6 @@ def _locked_plugin_state(path: Path): msvcrt.locking(handle.fileno(), msvcrt.LK_LOCK, 1) else: import fcntl - fcntl.flock(handle.fileno(), fcntl.LOCK_EX) try: yield @@ -138,7 +123,7 @@ class PluginState: @staticmethod def _validate_key(key: str) -> None: - if (not isinstance(key, str) or not _PLUGIN_STATE_KEY_RE.fullmatch(key) or ".." in key): + if not isinstance(key, str) or not _PLUGIN_STATE_KEY_RE.fullmatch(key) or ".." in key: raise ValueError( "Plugin state keys must be 1-128 characters using letters, " "numbers, '_', '-', '.', or ':' (without '..')" @@ -180,5 +165,4 @@ class PluginState: f"than the {self.quota_bytes}-byte per-plugin quota" ) from utils import atomic_json_write - atomic_json_write(self.path, data, mode=0o600)