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 index b270010c84c40..e72591811fa3b 100644 --- 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 @@ -118,7 +118,7 @@ The official rendered specification page labels v1.0.0 a Working Draft, while th - 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 the canonical discovered plugin key as their namespace and enter the same registry as `ctx.register_skill` output. Registration reports and skips a duplicate qualified name rather than overwriting a native or portable skill, and progressive disclosure does not join the editable flat skill tree or force a system-prompt rebuild. +- 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. diff --git a/hermes_cli/agent_plugins.py b/hermes_cli/agent_plugins.py index f59f6d8c39baf..ba6e846e2fc10 100644 --- a/hermes_cli/agent_plugins.py +++ b/hermes_cli/agent_plugins.py @@ -15,7 +15,7 @@ from dataclasses import dataclass from pathlib import Path from typing import Any, Dict, Mapping, Tuple -from agent.skill_utils import parse_frontmatter +from agent.skill_utils import yaml_load PLUGIN_SCHEMA_V1 = "https://agent-plugins.org/schemas/1.0.0/plugin.schema.json" @@ -52,7 +52,6 @@ class AgentPluginError(ValueError): class AgentPluginDiagnostic: scope: str message: str - fatal: bool = False @dataclass(frozen=True) @@ -174,8 +173,8 @@ def _valid_skill_frontmatter( return "license must be a string" if "compatibility" in frontmatter: compatibility = frontmatter["compatibility"] - if not isinstance(compatibility, str) or len(compatibility) > 500: - return "compatibility must be a string of at most 500 characters" + if not isinstance(compatibility, str) or not 1 <= len(compatibility) <= 500: + return "compatibility must be a string of 1 to 500 characters" if "metadata" in frontmatter: metadata = frontmatter["metadata"] if not isinstance(metadata, dict) or any( @@ -221,9 +220,19 @@ def _discover_skills( continue try: content = skill_md.read_text(encoding="utf-8") - if not content.lstrip("\ufeff").startswith("---"): + content = content.lstrip("\ufeff") + if not content.startswith("---"): raise ValueError("missing YAML frontmatter") - frontmatter, _ = parse_frontmatter(content) + end_match = re.search(r"\n---\s*\n", content[3:]) + if end_match is None: + raise ValueError("unterminated YAML frontmatter") + try: + parsed = yaml_load(content[3 : end_match.start() + 3]) + except Exception as exc: + raise ValueError(f"invalid YAML frontmatter: {exc}") from exc + if not isinstance(parsed, dict): + raise ValueError("YAML frontmatter must be an object") + frontmatter = parsed except (OSError, UnicodeError, ValueError) as exc: diagnostics.append(AgentPluginDiagnostic(scope, f"invalid SKILL.md: {exc}")) continue @@ -252,22 +261,21 @@ def _expand(value: str, plugin_root: Path, data_root: Path) -> str: def _resolve_scoped_path(value: str, plugin_root: Path, data_root: Path) -> Path: + expanded = _expand(value, plugin_root, data_root) if value.startswith("./"): base = plugin_root - candidate = base / value[2:] + candidate = base / expanded[2:] elif value == "${PLUGIN_ROOT}" or value.startswith("${PLUGIN_ROOT}/"): base = plugin_root - suffix = value[len("${PLUGIN_ROOT}") :].lstrip("/") - candidate = base / suffix + candidate = Path(expanded) elif value == "${PLUGIN_DATA}" or value.startswith("${PLUGIN_DATA}/"): base = data_root - suffix = value[len("${PLUGIN_DATA}") :].lstrip("/") - candidate = base / suffix + candidate = Path(expanded) else: raise ValueError("path must start with ./, ${PLUGIN_ROOT}, or ${PLUGIN_DATA}") resolved = candidate.resolve(strict=False) try: - resolved.relative_to(base.resolve(strict=True)) + resolved.relative_to(base.resolve(strict=False)) except (OSError, RuntimeError, ValueError) as exc: raise ValueError("path escapes its resolved root") from exc return resolved @@ -303,6 +311,8 @@ def _translate_stdio( raise ValueError("command must be a non-empty executable token") if command.startswith("./"): command_value = str(_resolve_scoped_path(command, plugin_root, data_root)) + elif any(character.isspace() for character in command): + raise ValueError("command must contain one executable token") elif "/" in command or "\\" in command or command in {".", ".."}: raise ValueError("command must be a bare executable or begin with ./") else: @@ -347,6 +357,8 @@ def _discover_mcp( root: Path, data_root: Path, diagnostics: list[AgentPluginDiagnostic], + *, + create_data: bool = True, ) -> Dict[str, Dict[str, Any]]: mcp_path = root / "mcp.json" if not mcp_path.exists() and not mcp_path.is_symlink(): @@ -387,7 +399,10 @@ def _discover_mcp( server_type = server.get("type") if server_type == "stdio": try: - translated[name] = _translate_stdio(server, root, data_root) + translated_server = _translate_stdio(server, root, data_root) + if create_data: + data_root.mkdir(parents=True, exist_ok=True) + translated[name] = translated_server except (OSError, ValueError) as exc: diagnostics.append(AgentPluginDiagnostic(scope, str(exc))) elif server_type in {"streamable-http", "sse"}: @@ -422,8 +437,6 @@ def load_agent_plugin(plugin_root: Path, data_root: Path) -> AgentPluginPackage: raise AgentPluginError("plugin root must be a directory") manifest, diagnostics = _validate_manifest(root) resolved_data = Path(data_root).resolve(strict=False) - resolved_data.mkdir(parents=True, exist_ok=True) - resolved_data = resolved_data.resolve(strict=True) skills = _discover_skills(root, diagnostics) mcp_servers = _discover_mcp(root, resolved_data, diagnostics) return AgentPluginPackage( @@ -447,3 +460,84 @@ def read_agent_plugin_manifest(plugin_root: Path) -> tuple[dict, tuple[AgentPlug raise AgentPluginError("plugin root must be a directory") manifest, diagnostics = _validate_manifest(root) return manifest, tuple(diagnostics) + + +def has_enabled_agent_plugin_mcp(raw_config: Mapping[str, Any]) -> bool: + """Cheaply detect an enabled portable package with a root ``mcp.json``. + + This probe intentionally does not import or register native plugins. Full + validation remains in the background MCP discovery path. + """ + + 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_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 diff --git a/hermes_cli/mcp_startup.py b/hermes_cli/mcp_startup.py index 1b9646b6d2eed..c3688054055a2 100644 --- a/hermes_cli/mcp_startup.py +++ b/hermes_cli/mcp_startup.py @@ -16,13 +16,13 @@ def _has_configured_mcp_servers() -> bool: try: from hermes_cli.config import read_raw_config - mcp_servers = (read_raw_config() or {}).get("mcp_servers") + raw_config = read_raw_config() or {} + mcp_servers = raw_config.get("mcp_servers") if isinstance(mcp_servers, dict) and len(mcp_servers) > 0: return True - from hermes_cli.plugins import discover_plugins, get_plugin_manager + from hermes_cli.agent_plugins import has_enabled_agent_plugin_mcp - discover_plugins() - return get_plugin_manager().has_portable_mcp_servers() + return has_enabled_agent_plugin_mcp(raw_config) except Exception: # Be conservative: if config probing fails, try discovery in the # background so startup still can't block. diff --git a/hermes_cli/plugins.py b/hermes_cli/plugins.py index bc0b76f89b026..f00fb7d9298f5 100644 --- a/hermes_cli/plugins.py +++ b/hermes_cli/plugins.py @@ -45,7 +45,7 @@ import threading import types from dataclasses import dataclass, field from pathlib import Path -from typing import Any, Callable, Dict, List, Optional, Set, Union +from typing import Any, Callable, Dict, List, Mapping, Optional, Set, Union from hermes_constants import get_hermes_home from utils import env_var_enabled, fast_safe_load @@ -284,7 +284,10 @@ _VALID_PLUGIN_KINDS: Set[str] = {"standalone", "backend", "exclusive", "platform def _portable_skill_namespace(key: str) -> str: """Return a readable, collision-resistant namespace for a portable plugin.""" - slug = "".join(ch if ch.isalnum() or ch in "_-" else "-" for ch in key.lower()) + slug = "".join( + ch if ch.isascii() and (ch.isalnum() or ch in "_-") else "-" + for ch in key.lower() + ) slug = slug.strip("-_") or "plugin" digest = hashlib.sha256(key.encode("utf-8")).hexdigest()[:8] return f"agent-plugin-{slug}-{digest}" @@ -1242,6 +1245,7 @@ class PluginContext: name: str, path: Path, description: str = "", + frontmatter: Optional[Mapping[str, Any]] = None, ) -> None: """Register a read-only skill provided by this plugin. @@ -1272,13 +1276,14 @@ class PluginContext: namespace = self.manifest.skill_namespace or self.manifest.name qualified = f"{namespace}:{name}" - if qualified in self._manager._plugin_skills: + if self.manifest.portable and qualified in self._manager._plugin_skills: raise ValueError(f"Plugin skill '{qualified}' is already registered") self._manager._plugin_skills[qualified] = { "path": path, "plugin": namespace, "bare_name": name, "description": description, + "frontmatter": dict(frontmatter or {}), } logger.debug( "Plugin %s registered skill: %s", @@ -1933,7 +1938,12 @@ class PluginManager: ) for skill in package.skills: try: - ctx.register_skill(skill.name, skill.skill_md, skill.description) + ctx.register_skill( + skill.name, + skill.skill_md, + skill.description, + skill.frontmatter, + ) except Exception as exc: logger.warning( "Agent Plugin '%s' skill '%s' skipped: %s", @@ -2145,7 +2155,7 @@ class PluginManager: if qn.startswith(prefix) ) - def list_plugin_skill_metadata(self) -> List[Dict[str, str]]: + def list_plugin_skill_metadata(self) -> List[Dict[str, Any]]: """Return progressive-disclosure metadata for registered plugin skills.""" return [ @@ -2153,6 +2163,7 @@ class PluginManager: "name": qualified, "description": str(entry.get("description", "")), "category": "plugin", + "frontmatter": dict(entry.get("frontmatter", {})), } for qualified, entry in sorted(self._plugin_skills.items()) ] diff --git a/tests/hermes_cli/test_agent_plugins.py b/tests/hermes_cli/test_agent_plugins.py index 4dc6f97c89fc8..55c8a6a3e5e4a 100644 --- a/tests/hermes_cli/test_agent_plugins.py +++ b/tests/hermes_cli/test_agent_plugins.py @@ -11,6 +11,7 @@ from hermes_cli.agent_plugins import ( MCP_SCHEMA_V1, PLUGIN_SCHEMA_V1, AgentPluginError, + has_enabled_agent_plugin_mcp, load_agent_plugin, ) @@ -100,7 +101,6 @@ def test_unknown_fields_and_non_object_extensions_are_nonfatal(tmp_path: Path) - ) package = load_agent_plugin(tmp_path, tmp_path / "data") assert len(package.diagnostics) == 2 - assert all(d.fatal is False for d in package.diagnostics) def test_invalid_skill_does_not_hide_valid_sibling(tmp_path: Path) -> None: @@ -118,6 +118,7 @@ def test_invalid_skill_does_not_hide_valid_sibling(tmp_path: Path) -> None: ("field", "value"), [ ("license", ["MIT"]), + ("compatibility", ""), ("compatibility", 1), ("metadata", []), ("allowed-tools", ["terminal"]), @@ -168,6 +169,11 @@ def test_stdio_command_and_data_cwd_containment(tmp_path: Path) -> None: "type": "stdio", "command": "./../outside", }, + "mixed-root": { + "type": "stdio", + "command": "python", + "cwd": "./${PLUGIN_DATA}/state", + }, }, }, ) @@ -178,6 +184,50 @@ def test_stdio_command_and_data_cwd_containment(tmp_path: Path) -> None: ) +def test_malformed_skill_yaml_is_skipped(tmp_path: Path) -> None: + _write_json(tmp_path / "plugin.json", _manifest()) + skill = tmp_path / "skills" / "broken" + skill.mkdir(parents=True) + (skill / "SKILL.md").write_text( + "---\nname: broken\ndescription: [unterminated\n---\nBody.\n", + encoding="utf-8", + ) + + package = load_agent_plugin(tmp_path, tmp_path / "data") + + assert package.skills == () + assert any(d.scope == "skill:broken" for d in package.diagnostics) + + +def test_data_directory_failure_preserves_valid_skills( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + _write_json(tmp_path / "plugin.json", _manifest()) + _write_skill(tmp_path) + _write_json( + tmp_path / "mcp.json", + { + "$schema": MCP_SCHEMA_V1, + "mcpServers": {"worker": {"type": "stdio", "command": "python"}}, + }, + ) + data_root = tmp_path / "data" + original_mkdir = Path.mkdir + + def fail_data_mkdir(path: Path, *args: object, **kwargs: object) -> None: + if path == data_root: + raise PermissionError("read-only profile") + original_mkdir(path, *args, **kwargs) + + monkeypatch.setattr(Path, "mkdir", fail_data_mkdir) + + package = load_agent_plugin(tmp_path, data_root) + + assert len(package.skills) == 1 + assert package.mcp_servers == {} + assert any(d.scope == "mcp:worker" for d in package.diagnostics) + + def test_invalid_entries_and_unsupported_remote_preserve_valid_stdio( tmp_path: Path, ) -> None: @@ -189,6 +239,17 @@ def test_invalid_entries_and_unsupported_remote_preserve_valid_stdio( "mcpServers": { "valid": {"type": "stdio", "command": "python"}, "invalid": {"type": "stdio", "command": "python", "extra": True}, + "multi-token": {"type": "stdio", "command": "python -m worker"}, + "reserved-root": { + "type": "stdio", + "command": "python", + "env": {"PLUGIN_ROOT": "override"}, + }, + "reserved-data": { + "type": "stdio", + "command": "python", + "env": {"PLUGIN_DATA": "override"}, + }, "remote": { "type": "streamable-http", "url": "https://example.test/mcp", @@ -200,6 +261,9 @@ def test_invalid_entries_and_unsupported_remote_preserve_valid_stdio( assert set(package.mcp_servers) == {"valid"} assert {d.scope for d in package.diagnostics} >= { "mcp:invalid", + "mcp:multi-token", + "mcp:reserved-root", + "mcp:reserved-data", "mcp:remote", } @@ -214,3 +278,86 @@ def test_invalid_mcp_top_level_preserves_skills(tmp_path: Path) -> None: package = load_agent_plugin(tmp_path, tmp_path / "data") assert len(package.skills) == 1 assert package.mcp_servers == {} + + +def test_enabled_portable_mcp_probe_does_not_load_plugins( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + home = tmp_path / "home" + plugin = home / "plugins" / "portable" + plugin.mkdir(parents=True) + _write_json(plugin / "plugin.json", _manifest()) + _write_json( + plugin / "mcp.json", + { + "$schema": MCP_SCHEMA_V1, + "mcpServers": {"worker": {"type": "stdio", "command": "python"}}, + }, + ) + bundled = tmp_path / "bundled" + bundled.mkdir() + monkeypatch.setenv("HERMES_HOME", str(home)) + monkeypatch.setenv("HERMES_BUNDLED_PLUGINS", str(bundled)) + + assert has_enabled_agent_plugin_mcp( + {"plugins": {"enabled": ["portable.test"]}} + ) + assert not has_enabled_agent_plugin_mcp( + { + "plugins": { + "enabled": ["portable.test"], + "disabled": ["portable.test"], + } + } + ) + + +def test_portable_mcp_probe_ignores_unsupported_only_config( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + home = tmp_path / "home" + plugin = home / "plugins" / "portable" + plugin.mkdir(parents=True) + _write_json(plugin / "plugin.json", _manifest()) + _write_json( + plugin / "mcp.json", + { + "$schema": MCP_SCHEMA_V1, + "mcpServers": { + "remote": { + "type": "streamable-http", + "url": "https://example.test/mcp", + } + }, + }, + ) + bundled = tmp_path / "bundled" + bundled.mkdir() + monkeypatch.setenv("HERMES_HOME", str(home)) + monkeypatch.setenv("HERMES_BUNDLED_PLUGINS", str(bundled)) + + assert not has_enabled_agent_plugin_mcp( + {"plugins": {"enabled": ["portable.test"]}} + ) + + +def test_portable_mcp_probe_honors_native_precedence( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + home = tmp_path / "home" + plugin = home / "plugins" / "portable" + plugin.mkdir(parents=True) + _write_json(plugin / "plugin.json", _manifest()) + (plugin / "plugin.yaml").write_text("name: native\n", encoding="utf-8") + _write_json( + plugin / "mcp.json", + {"$schema": MCP_SCHEMA_V1, "mcpServers": {}}, + ) + bundled = tmp_path / "bundled" + bundled.mkdir() + monkeypatch.setenv("HERMES_HOME", str(home)) + monkeypatch.setenv("HERMES_BUNDLED_PLUGINS", str(bundled)) + + assert not has_enabled_agent_plugin_mcp( + {"plugins": {"enabled": ["portable.test"]}} + ) diff --git a/tests/hermes_cli/test_mcp_startup.py b/tests/hermes_cli/test_mcp_startup.py index ffcdf14c0a0b2..9c4b94a182669 100644 --- a/tests/hermes_cli/test_mcp_startup.py +++ b/tests/hermes_cli/test_mcp_startup.py @@ -147,7 +147,6 @@ def test_background_mcp_discovery_suppresses_interactive_oauth(monkeypatch): def test_portable_only_mcp_configuration_opens_startup_gate(monkeypatch): - manager = types.SimpleNamespace(has_portable_mcp_servers=lambda: True) monkeypatch.setitem( sys.modules, "hermes_cli.config", @@ -155,10 +154,9 @@ def test_portable_only_mcp_configuration_opens_startup_gate(monkeypatch): ) monkeypatch.setitem( sys.modules, - "hermes_cli.plugins", + "hermes_cli.agent_plugins", types.SimpleNamespace( - discover_plugins=lambda: None, - get_plugin_manager=lambda: manager, + has_enabled_agent_plugin_mcp=lambda _config: True, ), ) @@ -201,4 +199,3 @@ def _install_retry_stubs(monkeypatch, *, connected: bool, calls: dict): ) - diff --git a/tests/hermes_cli/test_plugins.py b/tests/hermes_cli/test_plugins.py index 8a7dd84e1ae69..157e20c284791 100644 --- a/tests/hermes_cli/test_plugins.py +++ b/tests/hermes_cli/test_plugins.py @@ -22,6 +22,7 @@ from hermes_cli.plugins import ( get_pre_verify_continue_message, has_middleware, resolve_plugin_command_result, + _portable_skill_namespace, ) from hermes_cli.middleware import ( VALID_MIDDLEWARE, @@ -34,6 +35,15 @@ from hermes_cli.middleware import ( # ── Helpers ──────────────────────────────────────────────────────────────── +def test_portable_skill_namespace_is_ascii_safe(): + from agent.skill_utils import is_valid_namespace + + namespace = _portable_skill_namespace("café/portable") + + assert namespace.isascii() + assert is_valid_namespace(namespace) + + def _make_plugin_dir(base: Path, name: str, *, register_body: str = "pass", manifest_extra: dict | None = None, auto_enable: bool = True) -> Path: diff --git a/tests/hermes_cli/test_plugins_cmd.py b/tests/hermes_cli/test_plugins_cmd.py index 0e43232875097..1f38fca04749f 100644 --- a/tests/hermes_cli/test_plugins_cmd.py +++ b/tests/hermes_cli/test_plugins_cmd.py @@ -617,6 +617,43 @@ class TestSubdirInstallE2E: with pytest.raises(PluginOperationError, match="does not exist"): pc._install_plugin_core(identifier, force=False) + def test_installs_portable_root_package_disabled(self, tmp_path, monkeypatch): + if shutil.which("git") is None: + pytest.skip("git not available") + + import json + import subprocess as sp + from hermes_cli import plugins_cmd as pc + from hermes_cli.agent_plugins import PLUGIN_SCHEMA_V1 + + repo_root = tmp_path / "portable-repo" + repo_root.mkdir() + (repo_root / "plugin.json").write_text( + json.dumps({"$schema": PLUGIN_SCHEMA_V1, "name": "portable.test"}) + ) + env = { + **os.environ, + "GIT_AUTHOR_NAME": "t", + "GIT_AUTHOR_EMAIL": "t@t", + "GIT_COMMITTER_NAME": "t", + "GIT_COMMITTER_EMAIL": "t@t", + } + sp.run(["git", "init", "-q"], cwd=repo_root, check=True, env=env) + sp.run(["git", "add", "-A"], cwd=repo_root, check=True, env=env) + sp.run(["git", "commit", "-q", "-m", "init"], cwd=repo_root, check=True, env=env) + plugins_dir = tmp_path / "installed" + plugins_dir.mkdir() + monkeypatch.setattr(pc, "_plugins_dir", lambda: plugins_dir) + + target, manifest, name = pc._install_plugin_core( + f"file://{repo_root}", force=False + ) + + assert name == "portable.test" + assert manifest["name"] == "portable.test" + assert target == (plugins_dir / "portable.test").resolve() + assert pc._resolve_plugin_key("portable.test") == "portable.test" + def test_portable_manifest_is_visible_to_plugin_cli(tmp_path): import json diff --git a/tests/test_plugin_skills.py b/tests/test_plugin_skills.py index 46cc68f35334e..72aead9acecc0 100644 --- a/tests/test_plugin_skills.py +++ b/tests/test_plugin_skills.py @@ -140,6 +140,7 @@ class TestPluginContextRegisterSkill: ctx.register_skill("foo", tmp_path / "nonexistent.md") def test_duplicate_qualified_name_is_rejected(self, ctx, tmp_path): + ctx.manifest.portable = True first = tmp_path / "first" / "SKILL.md" second = tmp_path / "second" / "SKILL.md" first.parent.mkdir() @@ -150,6 +151,19 @@ class TestPluginContextRegisterSkill: with pytest.raises(ValueError, match="already registered"): ctx.register_skill("foo", second) + def test_native_duplicate_preserves_overwrite_semantics(self, ctx, tmp_path): + first = tmp_path / "first" / "SKILL.md" + second = tmp_path / "second" / "SKILL.md" + first.parent.mkdir() + second.parent.mkdir() + first.write_text("first") + second.write_text("second") + + ctx.register_skill("foo", first) + ctx.register_skill("foo", second) + + assert ctx._manager.find_plugin_skill("testplugin:foo") == second + # ── skill_view qualified name dispatch ──────────────────────────────────── @@ -205,6 +219,27 @@ class TestSkillViewQualifiedName: assert result["success"] is True assert result["content"] == "API details." + def test_platform_gate_applies_before_supporting_file(self, tmp_path): + from tools.skills_tool import skill_view + + md = self._register_skill( + tmp_path, + content=( + "---\nname: writing-plans\ndescription: desc\n" + "platforms: [windows]\n---\nBody.\n" + ), + ) + reference = md.parent / "references" / "guide.md" + reference.parent.mkdir() + reference.write_text("Windows only.") + + result = json.loads( + skill_view("superpowers:writing-plans", file_path="references/guide.md") + ) + + assert result["success"] is False + assert result["readiness_status"] == "unsupported" + def test_rejects_supporting_file_escape(self, tmp_path): from tools.skills_tool import skill_view diff --git a/tests/test_tui_entry_mcp_owner.py b/tests/test_tui_entry_mcp_owner.py index cf5b8c86bf187..78d2c9bcd43e8 100644 --- a/tests/test_tui_entry_mcp_owner.py +++ b/tests/test_tui_entry_mcp_owner.py @@ -14,6 +14,12 @@ from hermes_cli import mcp_startup from tui_gateway import entry +def test_tui_uses_shared_portable_mcp_gate(monkeypatch): + monkeypatch.setattr(mcp_startup, "_has_configured_mcp_servers", lambda: True) + + assert entry._has_configured_mcp_servers() is True + + def test_wait_falls_through_to_shared_owner(monkeypatch): monkeypatch.setattr(entry, "_mcp_discovery_thread", None) # The fall-through to the shared owner only exists for the stdio TUI, diff --git a/tests/tools/test_mcp_tool.py b/tests/tools/test_mcp_tool.py index 6e876b637c48a..e6532e5ee1d86 100644 --- a/tests/tools/test_mcp_tool.py +++ b/tests/tools/test_mcp_tool.py @@ -141,6 +141,50 @@ class TestLoadMCPConfig: assert result["native"]["args"] == ["3000"] assert result["agent-plugin-demo__worker"]["args"] == ["${UNKNOWN}"] + def test_portable_server_resolves_through_real_plugin_discovery( + self, tmp_path, monkeypatch + ): + import json + import yaml + from hermes_cli.agent_plugins import MCP_SCHEMA_V1, 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"}) + ) + (plugin / "mcp.json").write_text( + json.dumps( + { + "$schema": MCP_SCHEMA_V1, + "mcpServers": { + "worker": {"type": "stdio", "command": "python"} + }, + } + ) + ) + home.mkdir(exist_ok=True) + (home / "config.yaml").write_text( + yaml.safe_dump({"plugins": {"enabled": ["portable.test"]}}) + ) + bundled = tmp_path / "bundled" + bundled.mkdir() + monkeypatch.setenv("HERMES_HOME", str(home)) + monkeypatch.setenv("HERMES_BUNDLED_PLUGINS", str(bundled)) + monkeypatch.setattr(plugins_mod, "_plugin_manager", None) + + from tools.mcp_tool import _load_mcp_config + + result = _load_mcp_config() + + [server] = result.values() + assert server["command"] == "python" + assert server["cwd"] == str(plugin.resolve()) + assert server["env"]["PLUGIN_ROOT"] == str(plugin.resolve()) + assert server["env"]["PLUGIN_DATA"].startswith(str(home / "plugin-data")) + class TestMCPParallelSafetyProvenance: def test_parallel_safe_servers_keep_exact_raw_names(self, monkeypatch): @@ -779,6 +823,23 @@ class TestMCPServerTask: asyncio.run(_test()) + def test_start_preserves_native_default_cwd(self): + from tools.mcp_tool import MCPServerTask + + mock_session = MagicMock() + mock_session.initialize = AsyncMock() + mock_session.list_tools = AsyncMock(return_value=SimpleNamespace(tools=[])) + p_stdio, p_cs, _, _ = self._mock_stdio_and_session(mock_session) + + async def _test(): + with patch("tools.mcp_tool.StdioServerParameters") as params, p_stdio, p_cs: + server = MCPServerTask("native") + await server.start({"command": "npx", "args": ["-y", "test"]}) + assert params.call_args.kwargs["cwd"] is None + await server.shutdown() + + asyncio.run(_test()) + def test_stdio_recycle_deadline_pauses_while_rpc_active(self): from tools.mcp_tool import MCPServerTask diff --git a/tools/skills_tool.py b/tools/skills_tool.py index 0835d6d7e0089..e939224e347b9 100644 --- a/tools/skills_tool.py +++ b/tools/skills_tool.py @@ -808,7 +808,13 @@ def skills_list(category: str = None, task_id: str = None) -> str: from hermes_cli.plugins import discover_plugins, get_plugin_manager discover_plugins() - all_skills.extend(get_plugin_manager().list_plugin_skill_metadata()) + for plugin_skill in get_plugin_manager().list_plugin_skill_metadata(): + frontmatter = plugin_skill.pop("frontmatter", {}) + if not skill_matches_platform(frontmatter): + continue + if _is_skill_disabled(plugin_skill["name"]): + continue + all_skills.append(plugin_skill) except Exception: logger.debug("Plugin skill listing failed", exc_info=True) @@ -877,6 +883,40 @@ def _serve_plugin_skill( ensure_ascii=False, ) + try: + content = skill_md.read_text(encoding="utf-8") + except Exception as e: + return json.dumps( + {"success": False, "error": f"Failed to read skill '{namespace}:{bare}': {e}"}, + ensure_ascii=False, + ) + + parsed_frontmatter: Dict[str, Any] = {} + try: + parsed_frontmatter, _ = _parse_frontmatter(content) + except Exception: + pass + + qualified_name = f"{namespace}:{bare}" + if _is_skill_disabled(qualified_name): + return json.dumps( + { + "success": False, + "error": f"Skill '{qualified_name}' is disabled.", + }, + ensure_ascii=False, + ) + + if not skill_matches_platform(parsed_frontmatter): + return json.dumps( + { + "success": False, + "error": f"Skill '{qualified_name}' is not supported on this platform.", + "readiness_status": SkillReadinessStatus.UNSUPPORTED.value, + }, + ensure_ascii=False, + ) + if file_path: from tools.path_security import has_traversal_component, validate_within_dir @@ -930,30 +970,6 @@ def _serve_plugin_skill( ensure_ascii=False, ) - try: - content = skill_md.read_text(encoding="utf-8") - except Exception as e: - return json.dumps( - {"success": False, "error": f"Failed to read skill '{namespace}:{bare}': {e}"}, - ensure_ascii=False, - ) - - parsed_frontmatter: Dict[str, Any] = {} - try: - parsed_frontmatter, _ = _parse_frontmatter(content) - except Exception: - pass - - if not skill_matches_platform(parsed_frontmatter): - return json.dumps( - { - "success": False, - "error": f"Skill '{namespace}:{bare}' is not supported on this platform.", - "readiness_status": SkillReadinessStatus.UNSUPPORTED.value, - }, - ensure_ascii=False, - ) - # Injection scan — log but still serve (matches local-skill behaviour) if any(p in content.lower() for p in _INJECTION_PATTERNS): logger.warning( diff --git a/tui_gateway/entry.py b/tui_gateway/entry.py index 0a44c32c6d21e..00b801011b306 100644 --- a/tui_gateway/entry.py +++ b/tui_gateway/entry.py @@ -376,19 +376,10 @@ _recovery_times: list[float] = [] def _has_configured_mcp_servers() -> bool: - """Return whether startup should attempt MCP discovery. + """Delegate to the shared native and portable MCP startup gate.""" + from hermes_cli.mcp_startup import _has_configured_mcp_servers as configured - Keep this cheap so non-MCP users do not pay the MCP SDK import cost. - """ - try: - from hermes_cli.config import read_raw_config - - mcp_servers = (read_raw_config() or {}).get("mcp_servers") - return isinstance(mcp_servers, dict) and len(mcp_servers) > 0 - except Exception: - # Be conservative: if we can't decide, fall back to attempting - # discovery. The caller starts it in the background. - return True + return configured() def ensure_mcp_discovery_started() -> None: diff --git a/website/docs/developer-guide/plugins/index.md b/website/docs/developer-guide/plugins/index.md index 480e8b164d5ba..9749c7b46b262 100644 --- a/website/docs/developer-guide/plugins/index.md +++ b/website/docs/developer-guide/plugins/index.md @@ -72,6 +72,9 @@ them. An enabled package may provide immediate `skills/*/SKILL.md` directories and stdio MCP servers from root `mcp.json`. Skills are read-only, namespaced, and loaded through `skills_list` plus `skill_view`. MCP commands are passed as one executable token with a separate argument list, never through a shell. +Use `skills_list` to discover the full qualified skill name. Portable skill +namespaces have the deterministic form `agent-plugin--`, derived +from the discovered plugin key so sanitized names cannot collide. Hermes validates `plugin.json`, Agent Skills frontmatter, fixed component locations, `mcp.json`, resolved paths, and symlink containment locally. It does