From 8cb066404e3edc3501a07a408c59834dc745cc74 Mon Sep 17 00:00:00 2001 From: GodsBoy Date: Fri, 7 Aug 2026 06:45:21 +0200 Subject: [PATCH] fix(plugins): address portable MCP review feedback --- ...eat-agent-plugins-v1-compatibility-plan.md | 304 ------------------ hermes_cli/agent_plugins.py | 92 +----- hermes_cli/plugins.py | 186 ++++++++--- tests/hermes_cli/test_agent_plugins.py | 41 +++ tests/hermes_cli/test_plugins.py | 54 ++++ tests/tools/test_mcp_tool.py | 1 + 6 files changed, 247 insertions(+), 431 deletions(-) delete mode 100644 docs/plans/2026-08-06-001-feat-agent-plugins-v1-compatibility-plan.md diff --git a/docs/plans/2026-08-06-001-feat-agent-plugins-v1-compatibility-plan.md b/docs/plans/2026-08-06-001-feat-agent-plugins-v1-compatibility-plan.md deleted file mode 100644 index e72591811fa3b..0000000000000 --- a/docs/plans/2026-08-06-001-feat-agent-plugins-v1-compatibility-plan.md +++ /dev/null @@ -1,304 +0,0 @@ ---- -title: Agent Plugins v1 Compatibility - Plan -type: feat -date: 2026-08-06 -artifact_contract: ce-unified-plan/v1 -artifact_readiness: implementation-ready -product_contract_source: ce-plan-bootstrap -execution: code -deepened: 2026-08-06 ---- - -# Agent Plugins v1 Compatibility - Plan - -## Goal Capsule - -- **Objective:** Let Hermes install, discover, validate, explicitly enable, and load portable Agent Plugins v1.0.0 directory packages that contain Agent Skills and MCP servers. -- **Authority:** Preserve Hermes native `plugin.yaml` plus `register(ctx)` behavior and the Agent Plugins v1.0.0 normative text. When machine schemas and specification prose differ, the specification prose governs. -- **Execution profile:** Add a compatibility adapter that translates portable components into Hermes' existing namespaced plugin-skill registry and MCP client. Do not add a model-visible core tool or a parallel runtime. -- **Stop conditions:** The supported subset, security boundaries, failure isolation, native regressions, documentation, and focused integration tests are complete. Full Agent Plugins conformance must not be claimed unless every applicable normative requirement is proved. -- **Tail ownership:** This change ships as one focused PR linked to #64182 and explicitly complementary to #69446 and #64181. - ---- - -## Product Contract - -### Summary - -Hermes will recognize root `plugin.json` packages alongside native plugins, but only enabled packages can contribute components. A local compatibility adapter will validate the v1.0.0 format and feed valid skills and MCP entries into existing Hermes machinery. - -### Problem Frame - -Hermes already owns the two portable component runtimes standardized by Agent Plugins v1: Agent Skills and MCP servers. Its native plugin system uses a different package contract based on `plugin.yaml` and Python `register(ctx)`, so standard portable packages are currently invisible even when their components are otherwise compatible. - -The compatibility boundary is security-sensitive. A portable package can expose instructions, supporting files, local executables, process environment, and remote MCP endpoints. Discovery must therefore remain opt-in, path-contained, locally validated, and isolated at the narrowest component boundary. - -The official rendered specification page labels v1.0.0 a Working Draft, while the specification repository changed the same version to Published on 2026-07-24. The implementation targets the normative v1.0.0 content and documentation must record this status discrepancy without treating either label as a runtime rule. - -### Requirements - -#### Package discovery and activation - -- R1. Hermes recognizes a root `plugin.json` as an Agent Plugins v1 package without changing native `plugin.yaml`, `plugin.yml`, Python module, or entry-point plugin behavior. -- R2. User and project portable packages use the existing `plugins.enabled` allow-list and `plugins.disabled` deny-list, with explicit disable taking precedence and no automatic trust or activation. -- R3. `hermes plugins install`, `list`, `enable`, `disable`, `update`, and `remove` accept portable packages through the existing plugin workflow and retain the install-disabled default. -- R4. Portable package identity and component names are deterministic and namespaced so they cannot silently replace native skills or MCP servers. - -#### Local validation and containment - -- R5. Root `plugin.json` is parsed and validated locally for the canonical v1.0.0 `$schema`, closed field set, field types, and plugin-name constraints without retrieving schemas at load time. -- R6. Unknown root manifest fields and a non-object `extensions` field are reported and ignored as the specification's non-fatal exceptions; every other manifest violation rejects the package before component discovery. -- R7. Fixed component locations are exactly immediate `skills/*/SKILL.md` directories and root `mcp.json`; missing locations are valid, while a wrong filesystem kind invalidates only that component type. -- R8. Every package file path is checked against the filesystem-resolved package root, including symlink and junction targets, with the specification's narrow failure boundaries for the manifest, component type, skill, and MCP server entry. - -#### Skills compatibility - -- R9. Each immediate skill is validated against the complete Agent Skills frontmatter contract: required matching `name` and non-empty `description`, plus every optional field's type and limit when present. Invalid skills are reported and skipped without blocking valid siblings or MCP. -- R10. Valid portable skills use Hermes' read-only namespaced plugin-skill registry and existing `skill_view` safeguards, preprocessing, linked-file containment, platform checks, and prompt-injection warnings. -- R11. Portable skill discovery does not mutate the system prompt or past messages during a conversation; any inventory change takes effect only through existing startup or turn-boundary behavior. - -#### MCP compatibility - -- R12. Root `mcp.json` is locally validated for the canonical v1.0.0 schema identifier, a version matching `plugin.json`, the closed top-level shape, and independently closed server variants. -- R13. Invalid top-level MCP configuration disables MCP only for that package, while invalid or unsupported server entries are skipped independently and valid skills and sibling servers continue. -- R14. Stdio commands remain one opaque executable token with arguments passed separately; Hermes never invokes a shell or splits a command string for portable packages. -- R15. The initial portable subset supports stdio MCP only. Package files and plugin-relative commands resolve within `PLUGIN_ROOT`; explicit `cwd` resolves within its selected `PLUGIN_ROOT` or `PLUGIN_DATA` root, and omitted `cwd` becomes `PLUGIN_ROOT`. Streamable HTTP and legacy SSE entries are reported and skipped until Hermes can enforce the v1 cross-origin configured-header rules throughout the native remote client. -- R16. Hermes creates a dedicated persistent writable `PLUGIN_DATA` directory per installed package instance, sets filesystem-resolved `PLUGIN_ROOT` and `PLUGIN_DATA` after configured environment overlays, and prevents packages from overriding either reserved key. -- R17. Placeholder expansion is a single non-recursive replacement of exact `${PLUGIN_ROOT}` and `${PLUGIN_DATA}` occurrences only in `args`, environment values, and `cwd`; no other environment, command, URL, header, key, or path expansion is performed. -- R18. Portable MCP entries pass through existing suspicious-entry filtering, safe subprocess environment construction, async discovery, tool registration, approval middleware, reconnect handling, and per-server connection failure isolation. - -#### Scope and claims - -- R19. Documentation identifies the exact implemented Agent Plugins v1.0.0 subset and does not claim full conformance unless the applicable normative checklist and runtime behavior are fully tested. -- R20. The change adds no catalog, registry admission, portable export format, manifest-v2 policy, capability-consent model, new transactional update mechanism, lifecycle ledger, gateway injection, Desktop UI, or model-visible core tool. R3 only adapts the existing non-transactional update workflow. - -### Acceptance Examples - -- AE1. **Covers R1, R2, R9, R10.** Given an enabled portable package with one valid skill and a native Python plugin in a separate directory beneath the same plugin search root, when discovery runs, then `skill_view` resolves the qualified portable skill and the native plugin still registers normally. -- AE2. **Covers R2, R3.** Given a valid installed package absent from `plugins.enabled`, when Hermes starts, then the package is listed as not enabled and contributes no skills, MCP servers, or subprocesses. -- AE3. **Covers R5, R6.** Given a manifest with one unknown top-level field but otherwise valid content, when discovery runs, then Hermes reports and ignores the field and continues; an unsupported `$schema` rejects the package. -- AE4. **Covers R7, R8.** Given an in-root skills directory and an `mcp.json` symlink escaping the package, when discovery runs, then valid skills remain available and MCP is disabled only for that package. -- AE5. **Covers R9, R13.** Given one valid skill, one invalid skill, one valid MCP server, and one invalid server entry, when the enabled package loads, then only the valid skill and server are registered. -- AE6. **Covers R14, R15, R16, R17.** Given a stdio entry with an opaque command, separate args, plugin placeholders, and omitted `cwd`, when translated, then the command is unchanged, placeholders expand exactly once, reserved environment values are host-owned, and the native process receives the resolved plugin root as `cwd`. -- AE7. **Covers R8, R15.** Given a plugin-relative command or `cwd` whose lexical or symlink resolution escapes the permitted root, when validation runs, then that server entry is skipped before any process starts. -- AE8. **Covers R13, R18.** Given two valid portable MCP entries where one fails to connect, when native MCP discovery runs, then the other server and its tools remain available and the failed entry is reported. - -### Success Criteria - -- Every scenario named in the task brief has focused automated coverage using isolated `HOME` and `HERMES_HOME` fixtures. -- Native plugin discovery, activation, skill registration, and MCP configuration remain backward compatible. -- Portable packages are useful through existing CLI, skill, and MCP surfaces without changing prompt caching or the core tool schema. -- Public documentation and PR wording distinguish the supported subset from catalog, pack, lifecycle, consent, and gateway roadmap work. - -### Scope Boundaries - -#### Deferred to Follow-Up Work - -- Catalog metadata, pinned-source admission, search, and discovery remain owned by #64181 and #69446. -- Hermes-native manifest v2 and plugin pack export or install semantics remain owned by #64165 and #64166. -- Capability declaration, consent, staged updates, and ownership lifecycle remain owned by #64228, #37977, #64229, and #76490. -- Full-profile distribution packaging remains separate from the portable plugin adapter. - -#### Outside This Change - -- Automatic trust, sandbox guarantees, archive formats, registries, signatures, provenance verification, hot reload watchers, Desktop management UI, new component types, and gateway-session injection are not part of Agent Plugins v1 compatibility. - ---- - -## Planning Contract - -### Assumptions - -- The portable package root is an installed plugin directory already visible to the native plugin scanner; this change does not search arbitrary ancestor directories or reinterpret workspace-root files as packages. -- Portable components use a deterministic package namespace when exposed to Hermes. Native user config retains precedence because portable MCP server names cannot collide with unqualified native server keys. -- Hermes supports Agent Plugins stdio through its existing client. Portable Streamable HTTP and legacy SSE remain unsupported in this first subset because the current redirect path cannot prove the v1 configured-header boundary end to end. -- Package state lives below profile-specific `HERMES_HOME` in a dedicated data subtree keyed by canonical package identity. Removal may leave data intact unless existing uninstall policy explicitly owns cleanup. - -### Key Technical Decisions - -- KTD1. **Add a compatibility adapter, not a runtime.** A focused module owns v1 manifest, skill, MCP, path, and placeholder validation, then returns native Hermes records to the existing plugin manager and MCP registry. -- KTD2. **Preserve native discovery contracts.** The plugin scanner recognizes `plugin.json` only when no native manifest owns the directory, records a portable marker on the manifest, and routes enabled portable packages to component registration without importing `__init__.py`. -- KTD3. **Use namespaced read-only skills with collision refusal.** Portable skills use a deterministic `agent-plugin--` namespace derived from the canonical discovered plugin key and enter the same registry as `ctx.register_skill` output. The suffix prevents distinct path-derived keys from collapsing after namespace sanitization. Portable registration reports and skips a duplicate qualified name rather than overwriting another skill, while native registration keeps its existing semantics. Progressive disclosure does not join the editable flat skill tree or force a system-prompt rebuild. -- KTD4. **Merge MCP after native interpolation.** Native `config.yaml` servers keep their current secret interpolation. Portable entries are independently translated and merged afterward so the portable contract can leave unknown placeholders literal and restrict expansion to the two standardized variables. -- KTD5. **Namespace portable MCP servers before registration.** The internal server key combines package and declared server identity, preventing silent collision while leaving the portable manifest unchanged. -- KTD6. **Extend stdio runtime only where the portable contract requires it.** The existing MCP client receives an explicit `cwd` and already executes `command` plus `args` without a shell; no separate process launcher is introduced. -- KTD7. **Report partial failure through existing logging and loader state.** Fatal manifest errors reject a package, while skill, MCP-type, server-entry, and connection failures are logged at their normative narrow boundary and do not roll back independent components. -- KTD8. **Document subset support, not blanket conformance.** The guide binds behavior to the v1.0.0 normative specification, records the current Working Draft/Published source mismatch, identifies stdio as the only portable MCP transport, and lists client-owned trust policy. - -### High-Level Technical Design - -```mermaid -flowchart TB - A[Existing plugin directories] --> B{Native YAML manifest present?} - B -->|yes| C[Existing Python plugin loader] - B -->|no| D[Portable plugin.json validator] - D --> E{Package explicitly enabled?} - E -->|no| F[Record inactive package] - E -->|yes| G[Fixed component discovery] - G --> H[Validate and register namespaced skills] - G --> I[Validate and translate mcp.json] - I --> J[Existing MCP config and security pipeline] - J --> K[Existing MCP lifecycle and tool registry] -``` - -```mermaid -flowchart TB - A[Resolve package root] --> B{Manifest valid?} - B -->|no| C[Reject package] - B -->|yes| D{Component path valid?} - D -->|no| E[Disable only component type] - D -->|yes| F{Entry path valid?} - F -->|no| G[Skip only skill or MCP entry] - F -->|yes| H[Translate into native record] - H --> I{Runtime connection succeeds?} - I -->|no| J[Report server failure] - I -->|yes| K[Register component] -``` - -### System-Wide Impact - -- **Prompt caching:** Package discovery and MCP registration finish before agent tool snapshots. Portable skills use existing progressive disclosure. No system prompt or past message is mutated mid-conversation. -- **Trust:** Agent Plugins v1 does not define enablement, permissions, provenance, or sandboxing. Hermes' existing explicit activation and full-trust plugin warning remain the client-owned boundary. -- **Profiles:** Package data and activation are profile-scoped through `get_hermes_home()`. Tests must isolate both `HOME` and `HERMES_HOME` to prevent cross-profile leakage. -- **MCP security:** Portable configuration receives stricter format validation before existing IOC, suspicious-shell, safe-environment, malware preflight, and connection controls. -- **Startup parity:** Background CLI, TUI, dashboard, and slash-worker gates treat enabled portable MCP entries as configured so a portable-only package reaches discovery before tool snapshots. -- **Compatibility:** Native Python plugins, model providers, memory providers, platform plugins, and entry points remain on their existing loading paths. - -### Risks and Dependencies - -- The official website and repository disagree on v1.0.0 publication status. Runtime behavior must key on canonical schema identifiers, not a mutable status label. -- Agent Plugins delegates `SKILL.md` validity to the Agent Skills specification, which is stricter than Hermes' permissive frontmatter parser. The adapter needs a narrow explicit validator rather than weakening local skill behavior globally. -- Existing MCP config performs broad `${VAR}` secret interpolation, which is incompatible with the portable expansion rules. Merge ordering must keep native and portable interpolation paths separate. -- Existing stdio startup currently omits `cwd` from its SDK parameters. Adding it must be regression-tested for native servers with and without `cwd`. -- Filesystem containment must consider symlinks and platform path forms without pretending to sandbox the launched process. - -### Sources and Research - -- Agent Plugins v1.0.0 normative specification: -- Rendered specification with Working Draft label: -- Agent Skills format specification: -- Hermes roadmap tracking issue: -- Community index issue and catalog PR: and -- Existing gateway plugin PR, outside this adapter: - ---- - -## Implementation Units - -### U1. Portable package validation and translation model - -- **Goal:** Add the local v1.0.0 validator and immutable translation records for portable manifests, skills, MCP entries, package paths, and plugin data. -- **Requirements:** R5-R9, R12-R17. -- **Dependencies:** None. -- **Files:** `hermes_cli/agent_plugins.py`, `tests/hermes_cli/test_agent_plugins.py`. -- **Approach:** - 1. Define canonical schema identifiers and explicit closed-field validators in Python so load never needs network access or a new dependency. - 2. Resolve package, fixed component, skill, command, and working-directory paths through one containment helper with narrow typed diagnostics. - 3. Validate Agent Skills frontmatter without changing Hermes' permissive local-skill parser. - 4. Translate valid stdio and supported remote entries into native MCP configuration, including dedicated data directories and exact single-pass placeholders. -- **Patterns to follow:** `hermes_cli/plugins_cmd.py` safe install-path resolution, `tools/path_security.py` containment, `agent/skill_utils.py` frontmatter parsing, and `hermes_cli/mcp_security.py` fail-closed entry checks. -- **Test scenarios:** - - Minimal and full valid v1 manifests produce portable package metadata without network access. - - Malformed JSON, non-object roots, missing required fields, invalid field types, invalid names, and unsupported schema identifiers reject the package. - - Unknown manifest fields and non-object `extensions` report non-fatal diagnostics and preserve otherwise valid loading. - - Missing fixed locations are accepted; wrong kinds and symlink escapes invalidate only the owning boundary. - - Valid and invalid Agent Skills are distinguished by required frontmatter, name constraints, directory match, and description constraints. - - MCP top-level errors, schema mismatch, unknown server fields, invalid stdio variants, reserved environment keys, and path escapes stop only the applicable MCP boundary; valid remote variants are reported as unsupported and skipped. - - Placeholder replacement is exact, single-pass, non-recursive, field-limited, and leaves unknown forms literal. - - A command string containing shell syntax remains one unchanged executable token and is never split. -- **Verification:** Unit results demonstrate every validation and containment rule with deterministic diagnostics and no external schema fetch. - -### U2. Plugin discovery, activation, CLI, and skills bridge - -- **Goal:** Recognize portable packages through existing plugin roots and CLI workflows, then register valid skills only after explicit activation. -- **Requirements:** R1-R4, R7-R11. -- **Dependencies:** U1. -- **Files:** `hermes_cli/plugins.py`, `hermes_cli/plugins_cmd.py`, `hermes_cli/subcommands/plugins.py`, `tools/skills_tool.py`, `tests/hermes_cli/test_plugins.py`, `tests/hermes_cli/test_plugins_cmd.py`, `tests/test_plugin_skills.py`, `tests/hermes_cli/test_agent_plugins.py`. -- **Approach:** - 1. Extend directory discovery with a portable-manifest branch that does not alter native YAML precedence, kind routing, source precedence, or Python import behavior. - 2. Route enabled portable packages through a component bridge that records loaded state and registers namespaced read-only skills, refusing duplicate qualified names instead of silently replacing an existing entry. - 3. Include portable packages in install validation, list metadata, enable/disable key resolution, and help text while preserving the disabled-by-default installation flow. - 4. Surface portable skills and their supporting files through existing progressive-disclosure APIs without adding them to the editable flat skill tree or rebuilding the system prompt mid-session. Extend qualified skill file reads with resolved-root and symlink containment because the current plugin branch serves only the main `SKILL.md`. -- **Patterns to follow:** `PluginManager._scan_directory_level`, `PluginContext.register_skill`, `_discover_all_plugins`, and the existing allow-list/deny-list migration semantics. -- **Test scenarios:** - - An enabled portable package registers valid qualified skills and an unenabled or explicitly disabled package registers nothing. - - `plugins.disabled` wins when a package is present in both activation lists. - - CLI install accepts a valid `plugin.json` repository but does not enable it unless requested; invalid portable manifests fail without moving an active target. - - CLI list, JSON, plain, enable, disable, update, and remove resolve portable identity through the same canonical key. - - One invalid skill does not hide a valid sibling; a valid supporting file can be read, while traversal and symlink escapes are blocked. - - Native and portable packages that would produce the same qualified skill name do not overwrite one another and emit a deterministic diagnostic. - - A native `plugin.yaml` plus `register(ctx)` plugin and a portable package load in the same isolated process with unchanged native registration. - - Source collision and rescan behavior remain deterministic and do not leave stale portable skill entries. -- **Verification:** Loader-level integration under temporary `HOME` and `HERMES_HOME` proves activation, CLI visibility, namespaced skill access, and native regression behavior. - -### U3. Portable MCP merge and native runtime support - -- **Goal:** Feed validated portable MCP servers into the existing MCP registration and lifecycle path with correct environment, `cwd`, namespacing, and isolation. -- **Requirements:** R4, R12-R18. -- **Dependencies:** U1, U2. -- **Files:** `hermes_cli/agent_plugins.py`, `hermes_cli/plugins.py`, `hermes_cli/mcp_startup.py`, background startup owners that gate MCP discovery, `tools/mcp_tool.py`, `tests/tools/test_mcp_tool.py`, `tests/tools/test_agent_plugins_mcp.py`, `tests/hermes_cli/test_agent_plugins.py`, and focused startup-path tests located during implementation. -- **Approach:** - 1. Expose enabled packages' translated MCP entries from the plugin manager as an in-memory source. - 2. Merge that source after native config interpolation, assign collision-proof internal server names, and make all existing background startup gates recognize a portable-only MCP source after plugin discovery. - 3. Pass validated `cwd` into `StdioServerParameters` while retaining the no-shell command and separate-arguments path. - 4. Run portable stdio entries through existing suspicious-entry filtering, safe environment construction, parallel connection discovery, tool registry, and error reporting. Report and skip remote portable transports before they reach the native client. -- **Patterns to follow:** `_load_mcp_config`, `_filter_suspicious_mcp_servers`, `register_mcp_servers`, `_discover_and_register_server`, and MCP startup before `AIAgent` tool snapshots. -- **Test scenarios:** - - Valid stdio portable entries convert into expected native records without rewriting `config.yaml`; Streamable HTTP and legacy SSE entries are reported as unsupported and absent from the registered map. - - Native config interpolation still resolves secrets, while portable unknown placeholders remain literal and reserved variables are host-owned. - - Portable server names cannot overwrite a native server or another package's server. - - Omitted and explicit valid `cwd` values reach the SDK; lexical and symlink escapes are absent from the registered server map. - - Existing suspicious MCP filtering can reject a translated entry before spawn. - - Mixed valid and invalid entries preserve valid siblings; a real fixture where one connection fails leaves another connected. - - Portable-only MCP configuration starts through background CLI, TUI, dashboard, and slash-worker gates after plugin discovery. - - Safe mode suppresses native and portable MCP discovery. - - Existing native MCP configs with and without `cwd` retain their prior behavior. -- **Verification:** Focused unit tests and a loader-to-native-registry integration exercise prove translation, subprocess parameters, collision handling, and per-server failure isolation. - -### U4. User and developer documentation - -- **Goal:** Document installation, activation, supported subset, security posture, status discrepancy, and roadmap boundaries in the current plugin guide and CLI help. -- **Requirements:** R3, R19, R20. -- **Dependencies:** U1-U3. -- **Files:** `website/docs/developer-guide/plugins/index.md`, `website/docs/user-guide/cli.md`, `hermes_cli/subcommands/plugins.py`. -- **Approach:** - 1. Add a concise portable-package section showing the fixed v1 layout and existing `hermes plugins` workflow. - 2. State supported component types, stdio-only portable MCP transport support, explicit activation, full-trust process posture, profile-scoped data, failure isolation, and unsupported v1/client-owned features. - 3. Record that the rendered site says Working Draft while the repository says Published, and link directly to the normative versioned specification. - 4. Explain that the adapter complements the catalog and roadmap work rather than replacing it. -- **Patterns to follow:** Existing developer plugin guide terminology, CLI command tables, and public artifact style rules. -- **Test scenarios:** Test expectation: none - this unit documents behavior already enforced and tested by U1-U3. -- **Verification:** Docs build or targeted Docusaurus validation passes, command help renders the portable-package capability accurately, and no full-conformance claim exceeds tested behavior. - ---- - -## Verification Contract - -| Gate | Scope | Evidence | -|---|---|---| -| Focused validator tests | U1 | Manifest, version, fields, skills, MCP variants, placeholders, and containment cases pass. | -| Plugin loader and CLI tests | U2 | Isolated `HOME` and `HERMES_HOME` integration proves disabled/untrusted packages, valid loading, partial failures, CLI behavior, and native plugin regression. | -| MCP unit and integration tests | U3 | Native config plus portable translation, SDK `cwd`, safe environment, collision isolation, mixed entries, and connection failure isolation pass. | -| Existing regression suites | U2, U3 | Relevant plugin-skill, plugin-manager, plugin CLI, MCP startup, and MCP tool suites pass unchanged. | -| Static quality | U1-U4 | Repository lint or compile checks cover every changed Python file. | -| Documentation | U4 | Website docs build or the repository's targeted docs validation completes successfully. | -| Full repository confidence | All | `scripts/run_tests.sh` passes, or any infrastructure-only failure is isolated with exact evidence and the focused required suites remain green. | -| Public artifact audit | All | Diff, commit, and PR contain no secrets, assistant attribution, model/provider provenance, badges, or em/en dashes. | -| PR and CI | All | PR links #64182, explains non-conflict with #69446/#64181 and independence from #64436, and all actionable CI/review failures are resolved or precisely handed to a maintainer-only gate. | - ---- - -## Definition of Done - -- R1-R20 are implemented or explicitly evidenced as outside the supported subset. -- AE1-AE8 pass through automated tests matched to their full integration scope. -- Portable packages remain disabled until explicitly enabled, and disabled packages cannot register skills, servers, tools, or subprocesses. -- Manifest and component validation performs no schema network requests at load time. -- Filesystem and symlink escapes are rejected at the narrowest required failure boundary. -- PLUGIN variables, command token handling, `cwd`, and MCP failure isolation match the documented subset. -- Native Hermes plugins and MCP configuration retain existing behavior. -- Prompt and tool snapshots remain stable for the life of an active conversation except existing cache-safe turn-boundary mechanisms. -- Documentation states exact support and source-status discrepancy without an unproved conformance claim. -- No catalog, pack, manifest-v2, consent, lifecycle, gateway, Desktop, or core-tool scope appears in the diff. -- Focused verification, relevant regressions, docs checks, and CI are green or have a maintainer-only block documented exactly once. -- Abandoned experiments, debug output, temporary fixtures, and unrelated formatting changes are absent from the final diff. diff --git a/hermes_cli/agent_plugins.py b/hermes_cli/agent_plugins.py index dad1d9e9461b8..0ac1da7519592 100644 --- a/hermes_cli/agent_plugins.py +++ b/hermes_cli/agent_plugins.py @@ -362,7 +362,6 @@ def _translate_stdio( "args": [_expand(value, plugin_root, data_root) for value in args], "env": translated_env, "cwd": str(cwd_value), - "agent_plugin": True, } @@ -415,6 +414,17 @@ def _discover_mcp( translated_server = _translate_stdio(server, root, data_root) if create_data: data_root.mkdir(parents=True, exist_ok=True) + cwd_path = Path(translated_server["cwd"]) + try: + cwd_path.relative_to(data_root) + except ValueError: + pass + else: + # The MCP client starts stdio servers with this cwd. + # Create only data-root descendants; plugin-root paths + # remain package-owned and are never made writable as a + # side effect of discovery. + cwd_path.mkdir(parents=True, exist_ok=True) translated[name] = translated_server except (OSError, ValueError) as exc: diagnostics.append(AgentPluginDiagnostic(scope, str(exc))) @@ -476,81 +486,13 @@ def read_agent_plugin_manifest(plugin_root: Path) -> tuple[dict, tuple[AgentPlug def has_enabled_agent_plugin_mcp(raw_config: Mapping[str, Any]) -> bool: - """Cheaply detect an enabled portable package with a root ``mcp.json``. + """Compatibility wrapper for the shared PluginManager MCP probe. - This probe intentionally does not import or register native plugins. Full - validation remains in the background MCP discovery path. + Directory scanning belongs to :mod:`hermes_cli.plugins` so startup gating + and full plugin discovery cannot drift apart. Keep this import-compatible + entry point for callers that used the original helper. """ - plugins_config = raw_config.get("plugins") - if not isinstance(plugins_config, dict): - return False - enabled_value = plugins_config.get("enabled") - if not isinstance(enabled_value, list): - return False - enabled = {value for value in enabled_value if isinstance(value, str)} - disabled_value = plugins_config.get("disabled", []) - disabled = ( - {value for value in disabled_value if isinstance(value, str)} - if isinstance(disabled_value, list) - else set() - ) - if not enabled: - return False + from hermes_cli.plugins import has_enabled_agent_plugin_mcp as _probe - from hermes_constants import get_hermes_home - from utils import env_var_enabled - - if env_var_enabled("HERMES_SAFE_MODE"): - return False - - bundled = Path( - os.getenv("HERMES_BUNDLED_PLUGINS", Path(__file__).resolve().parent.parent / "plugins") - ) - search_roots: list[tuple[Path, set[str]]] = [ - ( - bundled, - {"memory", "context_engine", "platforms", "model-providers"}, - ), - (bundled / "platforms", set()), - (get_hermes_home() / "plugins", set()), - ] - if env_var_enabled("HERMES_ENABLE_PROJECT_PLUGINS"): - search_roots.append((Path.cwd() / ".hermes" / "plugins", set())) - - winners: dict[str, tuple[str, Path]] = {} - - def scan(directory: Path, *, prefix: str, depth: int, skip: set[str]) -> None: - try: - children = sorted(directory.iterdir(), key=lambda path: path.name) - except OSError: - return - for child in children: - if not child.is_dir() or (depth == 0 and child.name in skip): - continue - if (child / "plugin.yaml").exists() or (child / "plugin.yml").exists(): - continue - portable_file = child / "plugin.json" - if portable_file.exists() or portable_file.is_symlink(): - try: - manifest, _ = read_agent_plugin_manifest(child) - except (AgentPluginError, OSError, RuntimeError): - continue - key = f"{prefix}/{child.name}" if prefix else manifest["name"] - winners[key] = (manifest["name"], child) - continue - if depth == 0: - nested_prefix = f"{prefix}/{child.name}" if prefix else child.name - scan(child, prefix=nested_prefix, depth=1, skip=set()) - - for search_root, skip_names in search_roots: - scan(search_root, prefix="", depth=0, skip=skip_names) - - for key, (name, root) in winners.items(): - if key in disabled or name in disabled: - continue - if key not in enabled and name not in enabled: - continue - if _discover_mcp(root, get_hermes_home() / "plugin-data" / name, [], create_data=False): - return True - return False + return _probe(raw_config) diff --git a/hermes_cli/plugins.py b/hermes_cli/plugins.py index f00fb7d9298f5..c6ed42d726e73 100644 --- a/hermes_cli/plugins.py +++ b/hermes_cli/plugins.py @@ -293,6 +293,17 @@ def _portable_skill_namespace(key: str) -> str: return f"agent-plugin-{slug}-{digest}" +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 "" if value is None else str(value) + + @dataclass class PluginManifest: """Parsed representation of a plugin.yaml manifest.""" @@ -300,7 +311,7 @@ class PluginManifest: name: str version: str = "" description: str = "" - author: Any = "" + author: str = "" requires_env: List[Union[str, Dict[str, Any]]] = field(default_factory=list) provides_tools: List[str] = field(default_factory=list) provides_hooks: List[str] = field(default_factory=list) @@ -1368,56 +1379,11 @@ class PluginManager: def _discover_and_load_inner(self) -> None: """The actual discovery sweep — see :meth:`discover_and_load`.""" - manifests: List[PluginManifest] = [] + manifests: List[PluginManifest] = self._collect_directory_manifests() - # 1. Bundled plugins (/plugins//) - # - # Repo-shipped plugins live next to hermes_cli/. Two layouts are - # supported (see ``_scan_directory`` for details): - # - # - flat: ``plugins/disk-cleanup/plugin.yaml`` (standalone) - # - category: ``plugins/image_gen/openai/plugin.yaml`` (backend) - # - # ``memory/``, ``context_engine/``, and ``model-providers/`` are - # skipped at the top level — they have their own discovery systems - # (plugins/memory/__init__.py, providers/__init__.py). ``platforms/`` - # is a category holding platform adapters (scanned one level deeper - # below). - repo_plugins = get_bundled_plugins_dir() - logger.debug("Scanning bundled plugins: %s", repo_plugins) - bundled = self._scan_directory( - repo_plugins, - source="bundled", - skip_names={"memory", "context_engine", "platforms", "model-providers"}, - ) - logger.debug(" bundled (top-level): %d manifest(s)", len(bundled)) - manifests.extend(bundled) - bundled_platforms = self._scan_directory( - repo_plugins / "platforms", source="bundled" - ) - logger.debug(" bundled/platforms: %d manifest(s)", len(bundled_platforms)) - manifests.extend(bundled_platforms) - - # 2. User plugins (~/.hermes/plugins/) - user_dir = get_hermes_home() / "plugins" - logger.debug("Scanning user plugins: %s", user_dir) - user_manifests = self._scan_directory(user_dir, source="user") - logger.debug(" user: %d manifest(s)", len(user_manifests)) - manifests.extend(user_manifests) - - # 3. Project plugins (./.hermes/plugins/) - if _env_enabled("HERMES_ENABLE_PROJECT_PLUGINS"): - project_dir = Path.cwd() / ".hermes" / "plugins" - logger.debug("Scanning project plugins: %s", project_dir) - project_manifests = self._scan_directory(project_dir, source="project") - logger.debug(" project: %d manifest(s)", len(project_manifests)) - manifests.extend(project_manifests) - else: - logger.debug( - "Project plugins disabled (set HERMES_ENABLE_PROJECT_PLUGINS=1 to enable)" - ) - - # 4. Pip / entry-point plugins + # Directory plugins are collected above. Pip / entry-point plugins + # are intentionally separate: portable packages are directory-only + # and the startup MCP probe must not import or register entry points. ep_manifests = self._scan_entry_points() logger.debug(" entrypoints: %d manifest(s)", len(ep_manifests)) manifests.extend(ep_manifests) @@ -1527,6 +1493,113 @@ class PluginManager: sum(1 for p in self._plugins.values() if p.enabled), ) + def _collect_directory_manifests(self) -> List[PluginManifest]: + """Collect directory manifests in the same order as full discovery. + + This method only reads manifests. It does not load native plugin + modules, register deferred platforms, or otherwise mutate manager + registries. Keeping the source ordering and scanner calls here lets + startup probes share the exact precedence and containment rules used + by :meth:`_discover_and_load_inner`. + """ + manifests: List[PluginManifest] = [] + + # 1. Bundled plugins (/plugins//). The excluded top-level + # categories have their own discovery systems; bundled platforms are + # scanned explicitly one level below. + repo_plugins = get_bundled_plugins_dir() + logger.debug("Scanning bundled plugins: %s", repo_plugins) + bundled = self._scan_directory( + repo_plugins, + source="bundled", + skip_names={"memory", "context_engine", "platforms", "model-providers"}, + ) + logger.debug(" bundled (top-level): %d manifest(s)", len(bundled)) + manifests.extend(bundled) + bundled_platforms = self._scan_directory( + repo_plugins / "platforms", source="bundled" + ) + logger.debug(" bundled/platforms: %d manifest(s)", len(bundled_platforms)) + manifests.extend(bundled_platforms) + + # 2. User plugins (~/.hermes/plugins/) + user_dir = get_hermes_home() / "plugins" + logger.debug("Scanning user plugins: %s", user_dir) + user_manifests = self._scan_directory(user_dir, source="user") + logger.debug(" user: %d manifest(s)", len(user_manifests)) + manifests.extend(user_manifests) + + # 3. Project plugins (./.hermes/plugins/), only when explicitly opted + # in. This must match the full discovery gate exactly. + if _env_enabled("HERMES_ENABLE_PROJECT_PLUGINS"): + project_dir = Path.cwd() / ".hermes" / "plugins" + logger.debug("Scanning project plugins: %s", project_dir) + project_manifests = self._scan_directory(project_dir, source="project") + logger.debug(" project: %d manifest(s)", len(project_manifests)) + manifests.extend(project_manifests) + else: + logger.debug( + "Project plugins disabled (set HERMES_ENABLE_PROJECT_PLUGINS=1 to enable)" + ) + + return manifests + + def has_enabled_portable_mcp(self, raw_config: Mapping[str, Any]) -> bool: + """Probe enabled portable MCP packages without loading plugins. + + The directory manifest collection is shared with full discovery, so + native ``plugin.yaml`` precedence, source ordering, depth limits, and + project-plugin gating cannot diverge between startup and runtime. + """ + if _env_enabled("HERMES_SAFE_MODE"): + return False + + plugins_config = raw_config.get("plugins") + if not isinstance(plugins_config, dict): + return False + enabled_value = plugins_config.get("enabled") + if not isinstance(enabled_value, list): + return False + enabled = {value for value in enabled_value if isinstance(value, str)} + disabled_value = plugins_config.get("disabled", []) + disabled = ( + {value for value in disabled_value if isinstance(value, str)} + if isinstance(disabled_value, list) + else set() + ) + if not enabled: + return False + + winners: Dict[str, PluginManifest] = {} + for manifest in self._collect_directory_manifests(): + winners[manifest.key or manifest.name] = manifest + + for manifest in winners.values(): + if not manifest.portable: + continue + lookup_key = manifest.key or manifest.name + if lookup_key in disabled or manifest.name in disabled: + continue + if lookup_key not in enabled and manifest.name not in enabled: + continue + try: + from hermes_cli.agent_plugins import _discover_mcp + + if _discover_mcp( + Path(manifest.path), + get_hermes_home() + / "plugin-data" + / (manifest.skill_namespace or lookup_key), + [], + create_data=False, + ): + return True + except (OSError, RuntimeError, ValueError): + # Full discovery will report component diagnostics. Startup + # probing should fail closed for an unreadable package. + continue + return False + # ----------------------------------------------------------------------- # Directory scanning # ----------------------------------------------------------------------- @@ -1610,7 +1683,7 @@ class PluginManager: name=data["name"], version=data.get("version", ""), description=data.get("description", ""), - author=data.get("author", ""), + author=_display_author(data.get("author", "")), source=source, path=str(child), key=key, @@ -1718,7 +1791,7 @@ class PluginManager: name=name, version=str(data.get("version", "")), description=data.get("description", ""), - author=data.get("author", ""), + author=_display_author(data.get("author", "")), requires_env=data.get("requires_env", []), provides_tools=data.get("provides_tools", []), provides_hooks=data.get("provides_hooks", []), @@ -2199,6 +2272,15 @@ def get_plugin_manager() -> PluginManager: return _plugin_manager +def has_enabled_agent_plugin_mcp(raw_config: Mapping[str, Any]) -> bool: + """Return whether config enables a portable package with MCP servers. + + A fresh manager performs manifest-only scanning, so this startup gate does + not mutate the process-wide plugin registry or import native plugin code. + """ + return PluginManager().has_enabled_portable_mcp(raw_config) + + def discover_plugins(force: bool = False) -> None: """Discover and load all plugins. diff --git a/tests/hermes_cli/test_agent_plugins.py b/tests/hermes_cli/test_agent_plugins.py index 9dea011c271d9..d564b8d803852 100644 --- a/tests/hermes_cli/test_agent_plugins.py +++ b/tests/hermes_cli/test_agent_plugins.py @@ -186,11 +186,52 @@ def test_stdio_command_and_data_cwd_containment(tmp_path: Path) -> None: assert package.mcp_servers["valid"]["cwd"] == str( (tmp_path / "data" / "state").resolve() ) + assert (tmp_path / "data" / "state").is_dir() + assert "agent_plugin" not in package.mcp_servers["valid"] assert package.mcp_servers["opaque-command"]["command"] == str( (root / "${PLUGIN_ROOT}").resolve() ) +def test_stdio_cwd_directory_failure_isolated_to_server( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + _write_json(tmp_path / "plugin.json", _manifest()) + _write_json( + tmp_path / "mcp.json", + { + "$schema": MCP_SCHEMA_V1, + "mcpServers": { + "broken": { + "type": "stdio", + "command": "python", + "cwd": "${PLUGIN_DATA}/broken", + }, + "valid": { + "type": "stdio", + "command": "python", + "cwd": "${PLUGIN_DATA}/valid", + }, + }, + }, + ) + data_root = (tmp_path / "data").resolve() + original_mkdir = Path.mkdir + + def fail_broken(path: Path, *args: object, **kwargs: object) -> None: + if path == data_root / "broken": + raise PermissionError("broken cwd") + original_mkdir(path, *args, **kwargs) + + monkeypatch.setattr(Path, "mkdir", fail_broken) + + package = load_agent_plugin(tmp_path, data_root) + + assert set(package.mcp_servers) == {"valid"} + assert (data_root / "valid").is_dir() + assert any(d.scope == "mcp:broken" for d in package.diagnostics) + + def test_malformed_skill_yaml_is_skipped(tmp_path: Path) -> None: _write_json(tmp_path / "plugin.json", _manifest()) skill = tmp_path / "skills" / "broken" diff --git a/tests/hermes_cli/test_plugins.py b/tests/hermes_cli/test_plugins.py index 157e20c284791..bec12cb6f367d 100644 --- a/tests/hermes_cli/test_plugins.py +++ b/tests/hermes_cli/test_plugins.py @@ -190,6 +190,60 @@ class TestPluginDiscovery: assert manager.list_plugin_skill_metadata() == [] assert manager.get_portable_mcp_servers() == {} + def test_portable_author_object_is_normalized_to_stable_string( + self, tmp_path, monkeypatch + ): + from hermes_cli.agent_plugins import PLUGIN_SCHEMA_V1 + from hermes_cli import plugins as plugins_mod + + home = tmp_path / "home" + plugin = home / "plugins" / "portable" + plugin.mkdir(parents=True) + (plugin / "plugin.json").write_text( + json.dumps( + { + "$schema": PLUGIN_SCHEMA_V1, + "name": "portable.test", + "author": { + "url": "https://example.test", + "name": "Ada Lovelace", + "email": "ada@example.test", + }, + } + ) + ) + bundled = tmp_path / "bundled" + bundled.mkdir() + monkeypatch.setenv("HERMES_HOME", str(home)) + monkeypatch.setenv("HERMES_BUNDLED_PLUGINS", str(bundled)) + + manager = PluginManager() + manifests = manager._collect_directory_manifests() + + [manifest] = [item for item in manifests if item.portable] + assert manifest.author == ( + "Ada Lovelace, ada@example.test, https://example.test" + ) + assert isinstance(manifest.author, str) + + empty_author = home / "plugins" / "empty-author" + empty_author.mkdir() + (empty_author / "plugin.json").write_text( + json.dumps( + { + "$schema": PLUGIN_SCHEMA_V1, + "name": "empty-author", + "author": {}, + } + ) + ) + [empty] = [ + item + for item in manager._collect_directory_manifests() + if item.name == "empty-author" + ] + assert empty.author == "" + def test_plugin_can_register_and_invoke_middleware(self, tmp_path, monkeypatch): plugins_dir = tmp_path / "hermes_test" / "plugins" diff --git a/tests/tools/test_mcp_tool.py b/tests/tools/test_mcp_tool.py index e6532e5ee1d86..37349a658c35c 100644 --- a/tests/tools/test_mcp_tool.py +++ b/tests/tools/test_mcp_tool.py @@ -184,6 +184,7 @@ class TestLoadMCPConfig: assert server["cwd"] == str(plugin.resolve()) assert server["env"]["PLUGIN_ROOT"] == str(plugin.resolve()) assert server["env"]["PLUGIN_DATA"].startswith(str(home / "plugin-data")) + assert "agent_plugin" not in server class TestMCPParallelSafetyProvenance: