From 6a17aabd9f2b6ae7bfda2b0b49d66d5b0100696d Mon Sep 17 00:00:00 2001 From: Zhaoxiaoguang001 <3357983213@qq.com> Date: Thu, 14 May 2026 06:28:22 +0800 Subject: [PATCH 1/6] feat: add Hermes Agent integration --- .../integrations/hermes/__init__.py | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 src/specify_cli/integrations/hermes/__init__.py diff --git a/src/specify_cli/integrations/hermes/__init__.py b/src/specify_cli/integrations/hermes/__init__.py new file mode 100644 index 0000000000..c8810d4c94 --- /dev/null +++ b/src/specify_cli/integrations/hermes/__init__.py @@ -0,0 +1,78 @@ +"""Hermes Agent integration — skills-based agent. + +Hermes Agent (https://github.com/NousResearch/hermes-agent) is an open-source +AI agent framework by Nous Research. It uses the ``.hermes/skills/`` directory +for agent skills, following the same ``speckit-/SKILL.md`` layout as +Claude Code and Codex. + +Usage:: + + specify init my-project --integration hermes + specify init --here --ai hermes +""" + +from __future__ import annotations + +from ..base import IntegrationOption, SkillsIntegration + + +class HermesIntegration(SkillsIntegration): + """Integration for Hermes Agent skills.""" + + key = "hermes" + config = { + "name": "Hermes Agent", + "folder": ".hermes/", + "commands_subdir": "skills", + "install_url": "https://github.com/NousResearch/hermes-agent", + "requires_cli": True, + } + registrar_config = { + "dir": ".hermes/skills", + "format": "markdown", + "args": "$ARGUMENTS", + "extension": "/SKILL.md", + } + context_file = "AGENTS.md" + + @classmethod + def options(cls) -> list[IntegrationOption]: + return [ + IntegrationOption( + "--skills", + is_flag=True, + default=True, + help="Install as agent skills (default for Hermes Agent)", + ), + ] + + def build_exec_args( + self, + prompt: str, + *, + model: str | None = None, + output_json: bool = True, + ) -> list[str] | None: + """Build Hermes CLI invocation for programmatic dispatch. + + Uses ``hermes chat -q`` for one-shot queries, mapping slash-command + invocations to the appropriate skill-based dispatch. + """ + args = [self.key, "chat", "-Q"] + + if model: + args.extend(["-m", model]) + if output_json: + args.append("--json") + + # If prompt starts with a slash command, pass it directly + # so Hermes can dispatch to the appropriate skill. + if prompt.startswith("/"): + command, _, remainder = prompt[1:].partition(" ") + args.extend(["-s", command]) + if remainder: + args.extend(["-q", remainder]) + else: + args.extend(["-q", prompt]) + + return args From cd91046cc4964259acccd66991f9f26edee61a7d Mon Sep 17 00:00:00 2001 From: Zhaoxiaoguang001 <3357983213@qq.com> Date: Thu, 14 May 2026 06:28:23 +0800 Subject: [PATCH 2/6] feat: add Hermes Agent integration --- tests/integrations/test_integration_hermes.py | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 tests/integrations/test_integration_hermes.py diff --git a/tests/integrations/test_integration_hermes.py b/tests/integrations/test_integration_hermes.py new file mode 100644 index 0000000000..a43bb2ee0e --- /dev/null +++ b/tests/integrations/test_integration_hermes.py @@ -0,0 +1,33 @@ +"""Tests for HermesIntegration.""" + +from .test_integration_base_skills import SkillsIntegrationTests + + +class TestHermesIntegration(SkillsIntegrationTests): + KEY = "hermes" + FOLDER = ".hermes/" + COMMANDS_SUBDIR = "skills" + REGISTRAR_DIR = ".hermes/skills" + CONTEXT_FILE = "AGENTS.md" + + +class TestHermesAutoPromote: + """--ai hermes auto-promotes to integration path.""" + + def test_ai_hermes_without_ai_skills_auto_promotes(self, tmp_path): + """--ai hermes should work the same as --integration hermes.""" + from typer.testing import CliRunner + from specify_cli import app + + runner = CliRunner() + target = tmp_path / "test-proj" + result = runner.invoke(app, [ + "init", str(target), + "--ai", "hermes", + "--no-git", + "--ignore-agent-tools", + "--script", "sh", + ]) + + assert result.exit_code == 0, f"init --ai hermes failed: {result.output}" + assert (target / ".hermes" / "skills" / "speckit-plan" / "SKILL.md").exists() From e83cd543aac38de881853f6a89c23e8040c3a81a Mon Sep 17 00:00:00 2001 From: Zhaoxiaoguang001 <3357983213@qq.com> Date: Thu, 14 May 2026 06:28:57 +0800 Subject: [PATCH 3/6] feat: add Hermes Agent integration --- src/specify_cli/integrations/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/specify_cli/integrations/__init__.py b/src/specify_cli/integrations/__init__.py index 4a78e7d035..ad1440d074 100644 --- a/src/specify_cli/integrations/__init__.py +++ b/src/specify_cli/integrations/__init__.py @@ -61,6 +61,7 @@ def _register_builtins() -> None: from .gemini import GeminiIntegration from .generic import GenericIntegration from .goose import GooseIntegration + from .hermes import HermesIntegration from .iflow import IflowIntegration from .junie import JunieIntegration from .kilocode import KilocodeIntegration @@ -93,6 +94,7 @@ def _register_builtins() -> None: _register(GeminiIntegration()) _register(GenericIntegration()) _register(GooseIntegration()) + _register(HermesIntegration()) _register(IflowIntegration()) _register(JunieIntegration()) _register(KilocodeIntegration()) From 541b278ccf9087592202018b79a1244be259fa17 Mon Sep 17 00:00:00 2001 From: majordave Date: Wed, 20 May 2026 13:32:07 -0600 Subject: [PATCH 4/6] feat: add Hermes Agent integration (with review fixes) - Full SkillsIntegration subclass with dual install strategy (project-local .hermes/skills/ + global ~/.hermes/skills/) - CLI fix: integration_uninstall now calls integration.teardown() instead of manifest.uninstall() directly, allowing custom cleanup - Fix Copilot review issues: - Docstring now reflects both -Q (quiet) and -q (query) flags - Empty command guard prevents passing empty skill names - Add catalog entry for hermes in integrations/catalog.json Co-authored-by: Zhaoxiaoguang001 <3357983213@qq.com> --- integrations/catalog.json | 9 ++ src/specify_cli/__init__.py | 8 +- .../integrations/hermes/__init__.py | 123 ++++++++++++++++-- 3 files changed, 127 insertions(+), 13 deletions(-) diff --git a/integrations/catalog.json b/integrations/catalog.json index 16e321cf58..e10f594c6c 100644 --- a/integrations/catalog.json +++ b/integrations/catalog.json @@ -272,6 +272,15 @@ "author": "spec-kit-core", "repository": "https://github.com/github/spec-kit", "tags": ["cli"] + }, + "hermes": { + "id": "hermes", + "name": "Hermes Agent", + "version": "1.0.0", + "description": "Hermes Agent skills-based integration by Nous Research", + "author": "spec-kit-core", + "repository": "https://github.com/github/spec-kit", + "tags": ["cli", "skills"] } } } diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index d4e8632215..b6bb625141 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1979,11 +1979,11 @@ def integration_uninstall( console.print(f"[dim]Details:[/dim] {exc}") raise typer.Exit(1) - removed, skipped = manifest.uninstall(project_root, force=force) + if not integration: + console.print(f"[red]Error:[/red] Integration '{key}' not found in registry.") + raise typer.Exit(1) - # Remove managed context section from the agent context file - if integration: - integration.remove_context_section(project_root) + removed, skipped = integration.teardown(project_root, manifest, force=force) remaining = [installed for installed in installed_keys if installed != key] new_default = default_key if default_key != key else (remaining[0] if remaining else None) diff --git a/src/specify_cli/integrations/hermes/__init__.py b/src/specify_cli/integrations/hermes/__init__.py index c8810d4c94..2b829ca3e3 100644 --- a/src/specify_cli/integrations/hermes/__init__.py +++ b/src/specify_cli/integrations/hermes/__init__.py @@ -1,9 +1,8 @@ """Hermes Agent integration — skills-based agent. Hermes Agent (https://github.com/NousResearch/hermes-agent) is an open-source -AI agent framework by Nous Research. It uses the ``.hermes/skills/`` directory -for agent skills, following the same ``speckit-/SKILL.md`` layout as -Claude Code and Codex. +AI agent framework by Nous Research. It stores skills in +``~/.hermes/skills/`` (user-global) rather than a project-local directory. Usage:: @@ -13,11 +12,22 @@ from __future__ import annotations +from pathlib import Path +from shutil import rmtree +from typing import Any + from ..base import IntegrationOption, SkillsIntegration +from ..manifest import IntegrationManifest class HermesIntegration(SkillsIntegration): - """Integration for Hermes Agent skills.""" + """Integration for Hermes Agent skills. + + Hermes loads skills from ``~/.hermes/skills/`` (user home directory) + rather than a project-local path. Skills are installed in both + locations so they are available to Hermes globally while still being + tracked in the project manifest for clean uninstall. + """ key = "hermes" config = { @@ -35,6 +45,15 @@ class HermesIntegration(SkillsIntegration): } context_file = "AGENTS.md" + # -- Helpers ----------------------------------------------------------- + + @staticmethod + def _hermes_home_skills_dir() -> Path: + """Return ``~/.hermes/skills/`` — the global skills directory.""" + return Path.home() / ".hermes" / "skills" + + # -- Options ----------------------------------------------------------- + @classmethod def options(cls) -> list[IntegrationOption]: return [ @@ -46,6 +65,88 @@ def options(cls) -> list[IntegrationOption]: ), ] + # -- Skills directory -------------------------------------------------- + + def skills_dest(self, project_root: Path) -> Path: + """Return the project-local skills directory.""" + return project_root / ".hermes" / "skills" + + # -- Setup ------------------------------------------------------------- + + def setup( + self, + project_root: Path, + manifest: IntegrationManifest, + parsed_options: dict[str, Any] | None = None, + **opts: Any, + ) -> list[Path]: + """Install command templates as Hermes skills. + + Delegates to ``super().setup()`` for the project-local + ``.hermes/skills/`` (tracked by the manifest for clean uninstall), + then also writes each skill to the global ``~/.hermes/skills/`` + where Hermes discovers them at runtime. + """ + # Let the parent class handle project-local installation + created = super().setup( + project_root, manifest, + parsed_options=parsed_options, + **opts, + ) + + # Also write each skill to the global Hermes skills directory + global_skills_dir = self._hermes_home_skills_dir() + global_skills_dir.mkdir(parents=True, exist_ok=True) + + for skill_md in created: + # Only copy SKILL.md files under the project skills directory + try: + skill_md.resolve().relative_to( + self.skills_dest(project_root).resolve() + ) + except ValueError: + continue + if skill_md.name != "SKILL.md": + continue + + skill_name = skill_md.parent.name # e.g. "speckit-plan" + global_skill_dir = global_skills_dir / skill_name + global_skill_dir.mkdir(parents=True, exist_ok=True) + global_dest = global_skill_dir / "SKILL.md" + + content = skill_md.read_bytes() + normalized = content.replace(b"\r\n", b"\n") + global_dest.write_bytes(normalized) + + return created + + # -- Uninstall --------------------------------------------------------- + + def teardown( + self, + project_root: Path, + manifest: IntegrationManifest, + *, + force: bool = False, + ) -> tuple[list[Path], list[Path]]: + """Uninstall integration files and clean up global skills.""" + # Remove managed context section from AGENTS.md + self.remove_context_section(project_root) + + # Remove project-local files via manifest + removed, skipped = manifest.uninstall(project_root, force=force) + + # Also remove global Hermes skills for speckit + global_skills_dir = self._hermes_home_skills_dir() + if global_skills_dir.is_dir(): + for skill_dir in global_skills_dir.iterdir(): + if skill_dir.is_dir() and skill_dir.name.startswith("speckit-"): + rmtree(skill_dir, ignore_errors=True) + + return removed, skipped + + # -- CLI dispatch ------------------------------------------------------ + def build_exec_args( self, prompt: str, @@ -55,8 +156,9 @@ def build_exec_args( ) -> list[str] | None: """Build Hermes CLI invocation for programmatic dispatch. - Uses ``hermes chat -q`` for one-shot queries, mapping slash-command - invocations to the appropriate skill-based dispatch. + Uses ``hermes chat -Q -q`` for one-shot queries in quiet mode, + mapping slash-command invocations to the appropriate skill-based + dispatch. """ args = [self.key, "chat", "-Q"] @@ -69,9 +171,12 @@ def build_exec_args( # so Hermes can dispatch to the appropriate skill. if prompt.startswith("/"): command, _, remainder = prompt[1:].partition(" ") - args.extend(["-s", command]) - if remainder: - args.extend(["-q", remainder]) + if command: + args.extend(["-s", command]) + if remainder: + args.extend(["-q", remainder]) + else: + args.extend(["-q", prompt]) else: args.extend(["-q", prompt]) From f19a57efed2721998e332c6cc841e71570df3e77 Mon Sep 17 00:00:00 2001 From: majordave Date: Wed, 20 May 2026 19:10:44 -0600 Subject: [PATCH 5/6] feat: write Hermes skills directly to global ~/.hermes/skills/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hermes loads skills from the global ~/.hermes/skills/ directory, not from project-local paths. The old dual-install strategy copied SKILL.md files to both locations — project-local (for manifest tracking) and global (for Hermes discovery). This change removes the project-local copies entirely: - setup() writes directly to ~/.hermes/skills/speckit-*/SKILL.md - An empty .hermes/skills/ marker directory is created in the project so extension commands (e.g. git) can detect Hermes as an active integration via register_commands_for_all_agents() - teardown() cleans both the global speckit-* dirs and the local marker - import yaml moved to local import inside setup() Tests updated: Hermes-specific tests now assert global skill location, and shared SkillsIntegrationTests that assumed project-local files are overridden with Hermes-appropriate assertions. Co-authored-by: Zhaoxiaoguang001 <3357983213@qq.com> --- .../integrations/hermes/__init__.py | 166 +++++++++---- tests/integrations/test_integration_hermes.py | 235 +++++++++++++++++- 2 files changed, 352 insertions(+), 49 deletions(-) diff --git a/src/specify_cli/integrations/hermes/__init__.py b/src/specify_cli/integrations/hermes/__init__.py index 2b829ca3e3..f6963bc166 100644 --- a/src/specify_cli/integrations/hermes/__init__.py +++ b/src/specify_cli/integrations/hermes/__init__.py @@ -24,9 +24,12 @@ class HermesIntegration(SkillsIntegration): """Integration for Hermes Agent skills. Hermes loads skills from ``~/.hermes/skills/`` (user home directory) - rather than a project-local path. Skills are installed in both - locations so they are available to Hermes globally while still being - tracked in the project manifest for clean uninstall. + rather than a project-local path. Skills are installed directly to + the global directory — no project-local copies are created since + Hermes discovers them globally. A project-local marker directory + (``.hermes/skills/`` empty) is created so extension commands (e.g. + git) can detect Hermes as an active integration. Uninstall removes + both the marker and global skills. """ key = "hermes" @@ -65,12 +68,6 @@ def options(cls) -> list[IntegrationOption]: ), ] - # -- Skills directory -------------------------------------------------- - - def skills_dest(self, project_root: Path) -> Path: - """Return the project-local skills directory.""" - return project_root / ".hermes" / "skills" - # -- Setup ------------------------------------------------------------- def setup( @@ -80,43 +77,106 @@ def setup( parsed_options: dict[str, Any] | None = None, **opts: Any, ) -> list[Path]: - """Install command templates as Hermes skills. - - Delegates to ``super().setup()`` for the project-local - ``.hermes/skills/`` (tracked by the manifest for clean uninstall), - then also writes each skill to the global ``~/.hermes/skills/`` - where Hermes discovers them at runtime. + """Install command templates as global Hermes skills. + + Writes each skill directly to + ``~/.hermes/skills/speckit-/SKILL.md`` where Hermes + discovers them at runtime. No project-local SKILL.md copies are + created — the global directory is the single source of truth. + A project-local marker (``.hermes/skills/`` empty) is created + so extension commands (e.g. git) can detect Hermes as an active + integration. """ - # Let the parent class handle project-local installation - created = super().setup( - project_root, manifest, - parsed_options=parsed_options, - **opts, + templates = self.list_command_templates() + if not templates: + return [] + + script_type = opts.get("script_type", "sh") + arg_placeholder = ( + self.registrar_config.get("args", "$ARGUMENTS") + if self.registrar_config + else "$ARGUMENTS" ) - # Also write each skill to the global Hermes skills directory global_skills_dir = self._hermes_home_skills_dir() global_skills_dir.mkdir(parents=True, exist_ok=True) - for skill_md in created: - # Only copy SKILL.md files under the project skills directory - try: - skill_md.resolve().relative_to( - self.skills_dest(project_root).resolve() - ) - except ValueError: - continue - if skill_md.name != "SKILL.md": - continue - - skill_name = skill_md.parent.name # e.g. "speckit-plan" - global_skill_dir = global_skills_dir / skill_name - global_skill_dir.mkdir(parents=True, exist_ok=True) - global_dest = global_skill_dir / "SKILL.md" - - content = skill_md.read_bytes() - normalized = content.replace(b"\r\n", b"\n") - global_dest.write_bytes(normalized) + created: list[Path] = [] + + for src_file in templates: + raw = src_file.read_text(encoding="utf-8") + + # Derive the skill name from the template stem + command_name = src_file.stem # e.g. "plan" + skill_name = f"speckit-{command_name.replace('.', '-')}" + + # Parse frontmatter for description + frontmatter: dict[str, Any] = {} + if raw.startswith("---"): + parts = raw.split("---", 2) + if len(parts) >= 3: + import yaml + + try: + fm = yaml.safe_load(parts[1]) + if isinstance(fm, dict): + frontmatter = fm + except yaml.YAMLError: + pass + + # Process body through the standard template pipeline + processed_body = self.process_template( + raw, + self.key, + script_type, + arg_placeholder, + context_file=self.context_file or "", + invoke_separator=self.invoke_separator, + ) + # Strip the processed frontmatter — we rebuild it for skills. + if processed_body.startswith("---"): + parts = processed_body.split("---", 2) + if len(parts) >= 3: + processed_body = parts[2] + + # Select description + description = frontmatter.get("description", "") + if not description: + description = f"Spec Kit: {command_name} workflow" + + # Build SKILL.md with manually formatted frontmatter + def _quote(v: str) -> str: + escaped = v.replace("\\", "\\\\").replace('"', '\\"') + return f'"{escaped}"' + + skill_content = ( + f"---\n" + f"name: {_quote(skill_name)}\n" + f"description: {_quote(description)}\n" + f"compatibility: " + f"{_quote('Requires spec-kit project structure with .specify/ directory')}\n" + f"metadata:\n" + f" author: {_quote('github-spec-kit')}\n" + f" source: {_quote('templates/commands/' + src_file.name)}\n" + f"---\n" + f"{processed_body}" + ) + + # Write directly to global ~/.hermes/skills/speckit-/SKILL.md + skill_dir = global_skills_dir / skill_name + skill_dir.mkdir(parents=True, exist_ok=True) + skill_file = skill_dir / "SKILL.md" + normalized = skill_content.replace("\r\n", "\n") + skill_file.write_bytes(normalized.encode("utf-8")) + created.append(skill_file) + + # Upsert managed context section into the agent context file + self.upsert_context_section(project_root) + + # Create project-local marker directory so extension commands + # (e.g. git) can detect Hermes as an active integration. + # Hermes itself ignores this directory — skills live globally. + (project_root / ".hermes" / "skills").mkdir(parents=True, exist_ok=True) return created @@ -129,21 +189,35 @@ def teardown( *, force: bool = False, ) -> tuple[list[Path], list[Path]]: - """Uninstall integration files and clean up global skills.""" + """Uninstall integration files and clean up global skills. + + Removes the managed context section from AGENTS.md, removes the + project-local marker directory, and deletes all ``speckit-*`` + directories from ``~/.hermes/skills/``. + """ # Remove managed context section from AGENTS.md self.remove_context_section(project_root) - # Remove project-local files via manifest - removed, skipped = manifest.uninstall(project_root, force=force) + # Remove project-local marker directory if empty + local_skills_dir = project_root / ".hermes" / "skills" + if local_skills_dir.is_dir() and not any(local_skills_dir.iterdir()): + local_skills_dir.rmdir() + hermes_dir = project_root / ".hermes" + if hermes_dir.is_dir() and not any(hermes_dir.iterdir()): + hermes_dir.rmdir() - # Also remove global Hermes skills for speckit + # Remove global Hermes skills for speckit + removed: list[Path] = [] global_skills_dir = self._hermes_home_skills_dir() if global_skills_dir.is_dir(): - for skill_dir in global_skills_dir.iterdir(): + for skill_dir in sorted(global_skills_dir.iterdir()): if skill_dir.is_dir() and skill_dir.name.startswith("speckit-"): + skill_file = skill_dir / "SKILL.md" + if skill_file.exists(): + removed.append(skill_file) rmtree(skill_dir, ignore_errors=True) - return removed, skipped + return removed, [] # -- CLI dispatch ------------------------------------------------------ diff --git a/tests/integrations/test_integration_hermes.py b/tests/integrations/test_integration_hermes.py index a43bb2ee0e..f90b7b0d43 100644 --- a/tests/integrations/test_integration_hermes.py +++ b/tests/integrations/test_integration_hermes.py @@ -1,4 +1,19 @@ -"""Tests for HermesIntegration.""" +"""Tests for HermesIntegration. + +Hermes is special among SkillsIntegration subclasses: it writes skills +to ``~/.hermes/skills/`` (global) rather than the project-local +``.hermes/skills/`` directory. A project-local marker (empty directory) +is created so extension commands (e.g. git) can detect Hermes. + +Several shared tests from ``SkillsIntegrationTests`` assume project-local +skills — they are overridden here with Hermes-appropriate assertions. +""" + +from pathlib import Path + +from specify_cli.integrations import INTEGRATION_REGISTRY, get_integration +from specify_cli.integrations.hermes import HermesIntegration +from specify_cli.integrations.manifest import IntegrationManifest from .test_integration_base_skills import SkillsIntegrationTests @@ -10,12 +25,220 @@ class TestHermesIntegration(SkillsIntegrationTests): REGISTRAR_DIR = ".hermes/skills" CONTEXT_FILE = "AGENTS.md" + # -- Hermes-specific setup: skills go to ~/.hermes/skills/ ------------- + + def test_setup_writes_to_global_skills_dir(self, tmp_path): + """Skills are written to ~/.hermes/skills/, not project-local.""" + i = get_integration(self.KEY) + m = IntegrationManifest(self.KEY, tmp_path) + created = i.setup(tmp_path, m) + skill_files = [f for f in created if "scripts" not in f.parts] + + assert len(skill_files) > 0, "No skill files were created" + for f in skill_files: + # Every skill file should be under ~/.hermes/skills/speckit-*/ + assert str(f).startswith( + str(Path.home() / ".hermes" / "skills") + ), f"{f} is not under ~/.hermes/skills/" + + def test_local_marker_dir_created(self, tmp_path): + """Project-local .hermes/skills/ should exist but be empty.""" + i = get_integration(self.KEY) + m = IntegrationManifest(self.KEY, tmp_path) + i.setup(tmp_path, m) + marker = tmp_path / ".hermes" / "skills" + assert marker.is_dir(), "Marker directory was not created" + # Should be empty (no SKILL.md files) + children = list(marker.iterdir()) + assert children == [], f"Marker directory should be empty, got: {children}" + + # -- Override shared tests that assume project-local skills ------------ + + def test_setup_writes_to_correct_directory(self, tmp_path): + """Override: Hermes writes to global, not project-local.""" + self.test_setup_writes_to_global_skills_dir(tmp_path) + + def test_plan_references_correct_context_file(self, tmp_path): + """Plan skill goes to global dir, but we check it still references AGENTS.md.""" + i = get_integration(self.KEY) + if not i.context_file: + return + m = IntegrationManifest(self.KEY, tmp_path) + created = i.setup(tmp_path, m) + # Find the plan skill in global ~/.hermes/skills/ + plan_file = Path.home() / ".hermes" / "skills" / "speckit-plan" / "SKILL.md" + assert plan_file.exists(), f"Plan skill {plan_file} not created globally" + content = plan_file.read_text(encoding="utf-8") + assert i.context_file in content, ( + f"Plan skill should reference {i.context_file!r} but it was not found" + ) + assert "__CONTEXT_FILE__" not in content, ( + "Plan skill has unprocessed __CONTEXT_FILE__ placeholder" + ) + + def test_all_files_tracked_in_manifest(self, tmp_path): + """Override: Hermes does not track skills in the project manifest + since they live globally. Only project-local files (scripts, + templates, context) are tracked.""" + i = get_integration(self.KEY) + m = IntegrationManifest(self.KEY, tmp_path) + created = i.setup(tmp_path, m) + for f in created: + # Global files (in ~/.hermes/) are not tracked in manifest + if str(f).startswith(str(Path.home())): + continue + rel = f.resolve().relative_to(tmp_path.resolve()).as_posix() + assert rel in m.files, f"{rel} not tracked in manifest" + + def test_install_uninstall_roundtrip(self, tmp_path): + """Override: Hermes uninstall removes global skills + local marker.""" + i = get_integration(self.KEY) + m = IntegrationManifest(self.KEY, tmp_path) + created = i.install(tmp_path, m) + assert len(created) > 0 + m.save() + # All SKILL.md files should exist globally + for f in created: + if "SKILL.md" in str(f): + assert f.exists(), f"{f} does not exist" + removed, skipped = i.uninstall(tmp_path, m) + # Global skills should be gone + for f in created: + if "SKILL.md" in str(f): + assert not f.exists(), f"{f} should have been removed" + # Local marker should be gone + assert not (tmp_path / ".hermes" / "skills").exists() + + def test_modified_file_survives_uninstall(self, tmp_path): + """Override: Hermes global skills are always removed on uninstall + (no hash-based preservation since they're not in manifest).""" + i = get_integration(self.KEY) + m = IntegrationManifest(self.KEY, tmp_path) + created = i.install(tmp_path, m) + m.save() + # Pick a global skill file + skill_files = [f for f in created if "SKILL.md" in str(f)] + assert len(skill_files) > 0 + modified_file = skill_files[0] + modified_file.write_text("user modified this", encoding="utf-8") + removed, skipped = i.uninstall(tmp_path, m) + # Global skills are removed unconditionally (no manifest tracking) + assert not modified_file.exists(), ( + "Modified global skill should be removed on uninstall" + ) + + def test_pre_existing_skills_not_removed(self, tmp_path): + """Override: pre-existing non-speckit skills in the global dir + should survive Hermes uninstall.""" + i = get_integration(self.KEY) + # Create a foreign skill in the global dir first + global_skills_dir = i._hermes_home_skills_dir() + foreign_dir = global_skills_dir / "other-tool" + foreign_dir.mkdir(parents=True, exist_ok=True) + (foreign_dir / "SKILL.md").write_text("# Foreign skill\n") + + m = IntegrationManifest(self.KEY, tmp_path) + i.setup(tmp_path, m) + + assert (foreign_dir / "SKILL.md").exists(), "Foreign skill was removed" + + def test_complete_file_inventory_sh(self, tmp_path): + """Override: Hermes init produces no local SKILL.md files, + only the empty .hermes/skills/ marker.""" + from typer.testing import CliRunner + from specify_cli import app + + project = tmp_path / f"inventory-sh-{self.KEY}" + project.mkdir() + old_cwd = Path.cwd() + import os + try: + os.chdir(project) + result = CliRunner().invoke(app, [ + "init", "--here", "--integration", self.KEY, + "--script", "sh", "--no-git", "--ignore-agent-tools", + ], catch_exceptions=False) + finally: + os.chdir(old_cwd) + assert result.exit_code == 0, f"init failed: {result.output}" + actual = sorted( + p.relative_to(project).as_posix() + for p in project.rglob("*") if p.is_file() + ) + # Ensure no .hermes/skills/speckit-*/SKILL.md in project dir + hermes_skill_files = [f for f in actual if f.startswith(".hermes/skills/speckit-")] + assert hermes_skill_files == [], ( + f"Expected no local SKILL.md files, found: {hermes_skill_files}" + ) + # Ensure the marker exists (empty dir won't appear in file listing) + assert (project / ".hermes" / "skills").is_dir() + + def test_complete_file_inventory_ps(self, tmp_path): + """Override: Same as sh variant but for PowerShell script type.""" + from typer.testing import CliRunner + from specify_cli import app + + project = tmp_path / f"inventory-ps-{self.KEY}" + project.mkdir() + old_cwd = Path.cwd() + import os + try: + os.chdir(project) + result = CliRunner().invoke(app, [ + "init", "--here", "--integration", self.KEY, + "--script", "ps", "--no-git", "--ignore-agent-tools", + ], catch_exceptions=False) + finally: + os.chdir(old_cwd) + assert result.exit_code == 0, f"init failed: {result.output}" + actual = sorted( + p.relative_to(project).as_posix() + for p in project.rglob("*") if p.is_file() + ) + hermes_skill_files = [f for f in actual if f.startswith(".hermes/skills/speckit-")] + assert hermes_skill_files == [], ( + f"Expected no local SKILL.md files, found: {hermes_skill_files}" + ) + assert (project / ".hermes" / "skills").is_dir() + + def test_install_uninstall_cleanup(self, tmp_path): + """Verify global skills are cleaned and local marker is removed.""" + i = get_integration(self.KEY) + m = IntegrationManifest(self.KEY, tmp_path) + created = i.setup(tmp_path, m) + + # Verify global skills exist + global_skills = [ + f for f in created + if "SKILL.md" in str(f) + and str(f).startswith(str(Path.home() / ".hermes")) + ] + assert len(global_skills) > 0 + for f in global_skills: + assert f.exists() + + # Verify local marker exists + assert (tmp_path / ".hermes" / "skills").is_dir() + + # Teardown + removed, skipped = i.teardown(tmp_path, m) + + # Global skills removed + for f in global_skills: + assert not f.exists(), f"{f} should have been removed" + + # Local marker removed + assert not (tmp_path / ".hermes" / "skills").exists(), ( + "Local marker should be removed on teardown" + ) + class TestHermesAutoPromote: """--ai hermes auto-promotes to integration path.""" def test_ai_hermes_without_ai_skills_auto_promotes(self, tmp_path): - """--ai hermes should work the same as --integration hermes.""" + """--ai hermes should work the same as --integration hermes, + creating global skills and a local marker.""" from typer.testing import CliRunner from specify_cli import app @@ -30,4 +253,10 @@ def test_ai_hermes_without_ai_skills_auto_promotes(self, tmp_path): ]) assert result.exit_code == 0, f"init --ai hermes failed: {result.output}" - assert (target / ".hermes" / "skills" / "speckit-plan" / "SKILL.md").exists() + # Skills should be in global ~/.hermes/skills/ + assert (Path.home() / ".hermes" / "skills" / "speckit-plan" / "SKILL.md").exists() + # Local marker should exist + assert (target / ".hermes" / "skills").is_dir() + # No SKILL.md files in project-local dir + local_skills = list((target / ".hermes" / "skills").iterdir()) + assert local_skills == [], f"Local skills dir should be empty, got: {local_skills}" From 70f5732976c17f11b13d917daddd51992ed4f455 Mon Sep 17 00:00:00 2001 From: majordave Date: Thu, 21 May 2026 10:46:02 -0600 Subject: [PATCH 6/6] fix: address Copilot review feedback on Hermes integration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses all 6 review comments from copilot-pull-request-reviewer: 1. Hard-fail on missing integration key → fall back to manifest.uninstall() with a warning instead of raising an error. Allows users to always remove stale integration files even when the integration class is missing from the registry. 2. HOME isolation in tests → every test that calls setup() or CliRunner now monkeypatches Path.home() to a temp directory, keeping the test suite hermetic and non-destructive. 3. HermesIntegration.teardown() now delegates to manifest.uninstall() for project-local tracked files (scripts, manifest), merging results with global cleanup. 4. Global skills cleanup gated behind force=True to avoid destroying speckit-* skills shared across multiple Spec Kit projects when running 'specify integration uninstall hermes' without --force. 5. Line 160 isolation (CLI test test_complete_file_inventory_sh). 6. Line 258 isolation (Path.home assertion in test_ai_hermes_without_ai_skills_auto_promotes). --- src/specify_cli/__init__.py | 11 +- .../integrations/hermes/__init__.py | 39 ++++--- tests/integrations/test_integration_hermes.py | 108 ++++++++++++------ 3 files changed, 105 insertions(+), 53 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index b6bb625141..b57b5bffe6 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1980,10 +1980,13 @@ def integration_uninstall( raise typer.Exit(1) if not integration: - console.print(f"[red]Error:[/red] Integration '{key}' not found in registry.") - raise typer.Exit(1) - - removed, skipped = integration.teardown(project_root, manifest, force=force) + console.print( + f"[yellow]Warning:[/yellow] Integration '{key}' not found " + "in registry. Falling back to manifest-based cleanup." + ) + removed, skipped = manifest.uninstall(project_root, force=force) + else: + removed, skipped = integration.teardown(project_root, manifest, force=force) remaining = [installed for installed in installed_keys if installed != key] new_default = default_key if default_key != key else (remaining[0] if remaining else None) diff --git a/src/specify_cli/integrations/hermes/__init__.py b/src/specify_cli/integrations/hermes/__init__.py index f6963bc166..6d442bae7c 100644 --- a/src/specify_cli/integrations/hermes/__init__.py +++ b/src/specify_cli/integrations/hermes/__init__.py @@ -189,15 +189,23 @@ def teardown( *, force: bool = False, ) -> tuple[list[Path], list[Path]]: - """Uninstall integration files and clean up global skills. + """Uninstall integration files and optionally clean up global skills. Removes the managed context section from AGENTS.md, removes the - project-local marker directory, and deletes all ``speckit-*`` - directories from ``~/.hermes/skills/``. + project-local marker directory, and delegates to + ``manifest.uninstall()`` for project-local tracked files. + + Global ``speckit-*`` skills under ``~/.hermes/skills/`` are only + removed when ``force=True`` to avoid destroying skills shared with + other Spec Kit projects. """ # Remove managed context section from AGENTS.md self.remove_context_section(project_root) + # Delegate to manifest for project-local tracked files (scripts, + # templates, context entries tracked in the manifest). + removed, skipped = manifest.uninstall(project_root, force=force) + # Remove project-local marker directory if empty local_skills_dir = project_root / ".hermes" / "skills" if local_skills_dir.is_dir() and not any(local_skills_dir.iterdir()): @@ -206,18 +214,19 @@ def teardown( if hermes_dir.is_dir() and not any(hermes_dir.iterdir()): hermes_dir.rmdir() - # Remove global Hermes skills for speckit - removed: list[Path] = [] - global_skills_dir = self._hermes_home_skills_dir() - if global_skills_dir.is_dir(): - for skill_dir in sorted(global_skills_dir.iterdir()): - if skill_dir.is_dir() and skill_dir.name.startswith("speckit-"): - skill_file = skill_dir / "SKILL.md" - if skill_file.exists(): - removed.append(skill_file) - rmtree(skill_dir, ignore_errors=True) - - return removed, [] + # Remove global Hermes skills for speckit — only when force=True + # to avoid destroying skills shared with other Spec Kit projects. + if force: + global_skills_dir = self._hermes_home_skills_dir() + if global_skills_dir.is_dir(): + for skill_dir in sorted(global_skills_dir.iterdir()): + if skill_dir.is_dir() and skill_dir.name.startswith("speckit-"): + skill_file = skill_dir / "SKILL.md" + if skill_file.exists(): + removed.append(skill_file) + rmtree(skill_dir, ignore_errors=True) + + return removed, skipped # -- CLI dispatch ------------------------------------------------------ diff --git a/tests/integrations/test_integration_hermes.py b/tests/integrations/test_integration_hermes.py index f90b7b0d43..854dcfc711 100644 --- a/tests/integrations/test_integration_hermes.py +++ b/tests/integrations/test_integration_hermes.py @@ -5,19 +5,26 @@ ``.hermes/skills/`` directory. A project-local marker (empty directory) is created so extension commands (e.g. git) can detect Hermes. -Several shared tests from ``SkillsIntegrationTests`` assume project-local -skills — they are overridden here with Hermes-appropriate assertions. +All tests that touch ``~/.hermes/`` use ``monkeypatch`` to isolate +``Path.home()`` to a temp directory so the test suite is hermetic and +non-destructive to a developer's real Hermes installation. """ from pathlib import Path -from specify_cli.integrations import INTEGRATION_REGISTRY, get_integration -from specify_cli.integrations.hermes import HermesIntegration +from specify_cli.integrations import get_integration from specify_cli.integrations.manifest import IntegrationManifest from .test_integration_base_skills import SkillsIntegrationTests +def _fake_home(tmp_path: Path) -> Path: + """Create and return an isolated home directory under *tmp_path*.""" + home = tmp_path / "home" + home.mkdir(exist_ok=True) + return home + + class TestHermesIntegration(SkillsIntegrationTests): KEY = "hermes" FOLDER = ".hermes/" @@ -27,8 +34,11 @@ class TestHermesIntegration(SkillsIntegrationTests): # -- Hermes-specific setup: skills go to ~/.hermes/skills/ ------------- - def test_setup_writes_to_global_skills_dir(self, tmp_path): + def test_setup_writes_to_global_skills_dir(self, tmp_path, monkeypatch): """Skills are written to ~/.hermes/skills/, not project-local.""" + home = _fake_home(tmp_path) + monkeypatch.setattr(Path, "home", lambda: home) + i = get_integration(self.KEY) m = IntegrationManifest(self.KEY, tmp_path) created = i.setup(tmp_path, m) @@ -37,12 +47,16 @@ def test_setup_writes_to_global_skills_dir(self, tmp_path): assert len(skill_files) > 0, "No skill files were created" for f in skill_files: # Every skill file should be under ~/.hermes/skills/speckit-*/ - assert str(f).startswith( - str(Path.home() / ".hermes" / "skills") - ), f"{f} is not under ~/.hermes/skills/" + expected_prefix = str(home / ".hermes" / "skills") + assert str(f).startswith(expected_prefix), ( + f"{f} is not under ~/.hermes/skills/" + ) - def test_local_marker_dir_created(self, tmp_path): + def test_local_marker_dir_created(self, tmp_path, monkeypatch): """Project-local .hermes/skills/ should exist but be empty.""" + home = _fake_home(tmp_path) + monkeypatch.setattr(Path, "home", lambda: home) + i = get_integration(self.KEY) m = IntegrationManifest(self.KEY, tmp_path) i.setup(tmp_path, m) @@ -54,19 +68,22 @@ def test_local_marker_dir_created(self, tmp_path): # -- Override shared tests that assume project-local skills ------------ - def test_setup_writes_to_correct_directory(self, tmp_path): + def test_setup_writes_to_correct_directory(self, tmp_path, monkeypatch): """Override: Hermes writes to global, not project-local.""" - self.test_setup_writes_to_global_skills_dir(tmp_path) + self.test_setup_writes_to_global_skills_dir(tmp_path, monkeypatch) - def test_plan_references_correct_context_file(self, tmp_path): + def test_plan_references_correct_context_file(self, tmp_path, monkeypatch): """Plan skill goes to global dir, but we check it still references AGENTS.md.""" + home = _fake_home(tmp_path) + monkeypatch.setattr(Path, "home", lambda: home) + i = get_integration(self.KEY) if not i.context_file: return m = IntegrationManifest(self.KEY, tmp_path) - created = i.setup(tmp_path, m) + i.setup(tmp_path, m) # Find the plan skill in global ~/.hermes/skills/ - plan_file = Path.home() / ".hermes" / "skills" / "speckit-plan" / "SKILL.md" + plan_file = home / ".hermes" / "skills" / "speckit-plan" / "SKILL.md" assert plan_file.exists(), f"Plan skill {plan_file} not created globally" content = plan_file.read_text(encoding="utf-8") assert i.context_file in content, ( @@ -76,22 +93,28 @@ def test_plan_references_correct_context_file(self, tmp_path): "Plan skill has unprocessed __CONTEXT_FILE__ placeholder" ) - def test_all_files_tracked_in_manifest(self, tmp_path): + def test_all_files_tracked_in_manifest(self, tmp_path, monkeypatch): """Override: Hermes does not track skills in the project manifest since they live globally. Only project-local files (scripts, templates, context) are tracked.""" + home = _fake_home(tmp_path) + monkeypatch.setattr(Path, "home", lambda: home) + i = get_integration(self.KEY) m = IntegrationManifest(self.KEY, tmp_path) created = i.setup(tmp_path, m) for f in created: # Global files (in ~/.hermes/) are not tracked in manifest - if str(f).startswith(str(Path.home())): + if str(f).startswith(str(home)): continue rel = f.resolve().relative_to(tmp_path.resolve()).as_posix() assert rel in m.files, f"{rel} not tracked in manifest" - def test_install_uninstall_roundtrip(self, tmp_path): + def test_install_uninstall_roundtrip(self, tmp_path, monkeypatch): """Override: Hermes uninstall removes global skills + local marker.""" + home = _fake_home(tmp_path) + monkeypatch.setattr(Path, "home", lambda: home) + i = get_integration(self.KEY) m = IntegrationManifest(self.KEY, tmp_path) created = i.install(tmp_path, m) @@ -101,17 +124,19 @@ def test_install_uninstall_roundtrip(self, tmp_path): for f in created: if "SKILL.md" in str(f): assert f.exists(), f"{f} does not exist" - removed, skipped = i.uninstall(tmp_path, m) - # Global skills should be gone + removed, skipped = i.teardown(tmp_path, m, force=True) for f in created: if "SKILL.md" in str(f): assert not f.exists(), f"{f} should have been removed" # Local marker should be gone assert not (tmp_path / ".hermes" / "skills").exists() - def test_modified_file_survives_uninstall(self, tmp_path): - """Override: Hermes global skills are always removed on uninstall - (no hash-based preservation since they're not in manifest).""" + def test_modified_file_survives_uninstall(self, tmp_path, monkeypatch): + """Override: Hermes global skills are removed on uninstall only + when force=True (no hash-based preservation since they're not in manifest).""" + home = _fake_home(tmp_path) + monkeypatch.setattr(Path, "home", lambda: home) + i = get_integration(self.KEY) m = IntegrationManifest(self.KEY, tmp_path) created = i.install(tmp_path, m) @@ -121,15 +146,18 @@ def test_modified_file_survives_uninstall(self, tmp_path): assert len(skill_files) > 0 modified_file = skill_files[0] modified_file.write_text("user modified this", encoding="utf-8") - removed, skipped = i.uninstall(tmp_path, m) - # Global skills are removed unconditionally (no manifest tracking) + # Global skills are only removed with force=True + removed, skipped = i.teardown(tmp_path, m, force=True) assert not modified_file.exists(), ( - "Modified global skill should be removed on uninstall" + "Modified global skill should be removed on teardown with force=True" ) - def test_pre_existing_skills_not_removed(self, tmp_path): + def test_pre_existing_skills_not_removed(self, tmp_path, monkeypatch): """Override: pre-existing non-speckit skills in the global dir should survive Hermes uninstall.""" + home = _fake_home(tmp_path) + monkeypatch.setattr(Path, "home", lambda: home) + i = get_integration(self.KEY) # Create a foreign skill in the global dir first global_skills_dir = i._hermes_home_skills_dir() @@ -142,9 +170,12 @@ def test_pre_existing_skills_not_removed(self, tmp_path): assert (foreign_dir / "SKILL.md").exists(), "Foreign skill was removed" - def test_complete_file_inventory_sh(self, tmp_path): + def test_complete_file_inventory_sh(self, tmp_path, monkeypatch): """Override: Hermes init produces no local SKILL.md files, only the empty .hermes/skills/ marker.""" + home = _fake_home(tmp_path) + monkeypatch.setattr(Path, "home", lambda: home) + from typer.testing import CliRunner from specify_cli import app @@ -173,8 +204,11 @@ def test_complete_file_inventory_sh(self, tmp_path): # Ensure the marker exists (empty dir won't appear in file listing) assert (project / ".hermes" / "skills").is_dir() - def test_complete_file_inventory_ps(self, tmp_path): + def test_complete_file_inventory_ps(self, tmp_path, monkeypatch): """Override: Same as sh variant but for PowerShell script type.""" + home = _fake_home(tmp_path) + monkeypatch.setattr(Path, "home", lambda: home) + from typer.testing import CliRunner from specify_cli import app @@ -201,8 +235,11 @@ def test_complete_file_inventory_ps(self, tmp_path): ) assert (project / ".hermes" / "skills").is_dir() - def test_install_uninstall_cleanup(self, tmp_path): + def test_install_uninstall_cleanup(self, tmp_path, monkeypatch): """Verify global skills are cleaned and local marker is removed.""" + home = _fake_home(tmp_path) + monkeypatch.setattr(Path, "home", lambda: home) + i = get_integration(self.KEY) m = IntegrationManifest(self.KEY, tmp_path) created = i.setup(tmp_path, m) @@ -211,7 +248,7 @@ def test_install_uninstall_cleanup(self, tmp_path): global_skills = [ f for f in created if "SKILL.md" in str(f) - and str(f).startswith(str(Path.home() / ".hermes")) + and str(f).startswith(str(home / ".hermes")) ] assert len(global_skills) > 0 for f in global_skills: @@ -220,8 +257,8 @@ def test_install_uninstall_cleanup(self, tmp_path): # Verify local marker exists assert (tmp_path / ".hermes" / "skills").is_dir() - # Teardown - removed, skipped = i.teardown(tmp_path, m) + # Teardown with force=True to clean global skills + removed, skipped = i.teardown(tmp_path, m, force=True) # Global skills removed for f in global_skills: @@ -236,9 +273,12 @@ def test_install_uninstall_cleanup(self, tmp_path): class TestHermesAutoPromote: """--ai hermes auto-promotes to integration path.""" - def test_ai_hermes_without_ai_skills_auto_promotes(self, tmp_path): + def test_ai_hermes_without_ai_skills_auto_promotes(self, tmp_path, monkeypatch): """--ai hermes should work the same as --integration hermes, creating global skills and a local marker.""" + home = _fake_home(tmp_path) + monkeypatch.setattr(Path, "home", lambda: home) + from typer.testing import CliRunner from specify_cli import app @@ -254,7 +294,7 @@ def test_ai_hermes_without_ai_skills_auto_promotes(self, tmp_path): assert result.exit_code == 0, f"init --ai hermes failed: {result.output}" # Skills should be in global ~/.hermes/skills/ - assert (Path.home() / ".hermes" / "skills" / "speckit-plan" / "SKILL.md").exists() + assert (home / ".hermes" / "skills" / "speckit-plan" / "SKILL.md").exists() # Local marker should exist assert (target / ".hermes" / "skills").is_dir() # No SKILL.md files in project-local dir