feat: add Hermes Agent integration (with review fixes)#2651
Open
majordave wants to merge 6 commits into
Open
Conversation
- 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>
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>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a new built-in Hermes Agent integration to Spec Kit’s integration system, enabling specify init --integration hermes / --ai hermes to install Spec Kit command skills under Hermes’ expected skills directory and wire up project context via AGENTS.md.
Changes:
- Introduces
HermesIntegration(aSkillsIntegrationsubclass) and registers it as a built-in integration. - Updates
specify integration uninstallto callintegration.teardown()(enabling integrations to perform custom uninstall logic). - Adds Hermes to the integration catalog and adds a Hermes integration test suite based on shared
SkillsIntegrationTests.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/integrations/hermes/__init__.py |
New Hermes integration implementation (global skills install + custom teardown + CLI dispatch args). |
src/specify_cli/integrations/__init__.py |
Registers Hermes as a built-in integration. |
src/specify_cli/__init__.py |
Switches uninstall path to use integration.teardown() and adds an error when integration is missing from registry. |
integrations/catalog.json |
Adds hermes entry to the integration catalog. |
tests/integrations/test_integration_hermes.py |
Adds Hermes-specific integration tests, overriding shared assumptions about project-local skills. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 5/5 changed files
- Comments generated: 6
Comment on lines
+1982
to
+1986
| 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) |
Comment on lines
+30
to
+34
| 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) |
Comment on lines
+185
to
+191
| def teardown( | ||
| self, | ||
| project_root: Path, | ||
| manifest: IntegrationManifest, | ||
| *, | ||
| force: bool = False, | ||
| ) -> tuple[list[Path], list[Path]]: |
Comment on lines
+213
to
+217
| 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) |
Comment on lines
+153
to
+160
| 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) |
Comment on lines
+239
to
+258
| def test_ai_hermes_without_ai_skills_auto_promotes(self, tmp_path): | ||
| """--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 | ||
|
|
||
| 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}" | ||
| # Skills should be in global ~/.hermes/skills/ | ||
| assert (Path.home() / ".hermes" / "skills" / "speckit-plan" / "SKILL.md").exists() | ||
| # Local marker should exist |
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).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds a new built-in integration for Hermes Agent (by Nous Research) to Spec Kit's integration system, enabling
specify init --integration hermes/--ai hermesto scaffold Spec Kit commands as skills under.hermes/skills/with anAGENTS.mdcontext file.Based on the original work by @Zhaoxiaoguang001 in PR #2547, with the following fixes and improvements:
Changes from original PR #2547
src/specify_cli/integrations/hermes/__init__.py— Full Hermes integration:SkillsIntegration(skills-based, same as Claude/Codex).hermes/skills/(manifest-tracked) and global~/.hermes/skills/(where Hermes discovers them)setup()delegates tosuper().setup()+ post-processing (same pattern as Claude)teardown()cleans up both locations-Q(quiet) and-q(query) flags/alone no longer passes an empty skill namesrc/specify_cli/__init__.py— CLIintegration_uninstallnow callsintegration.teardown()instead of callingmanifest.uninstall()directly, allowing custom teardown logic in integrations to run properlyintegrations/catalog.json— Addedhermescatalog entrytests/integrations/test_integration_hermes.py— Uses sharedSkillsIntegrationTests(28 tests, all passing)Why global skills?
Unlike Claude Code (
.claude/skills/) or Codex (.agents/skills/), Hermes loads skills from~/.hermes/skills/$HERMES_HOME/skills/(user home directory). Skills are installed to both locations:.hermes/skills/— tracked by the manifest for clean uninstall~/.hermes/skills/— where Hermes discovers them at runtimeBoth locations are cleaned up on
specify integration uninstall.Co-authored-by: Zhaoxiaoguang001 3357983213@qq.com