feat: add Hermes Agent adapter#226
Conversation
WalkthroughThis PR adds an interactive Hermes adapter, Hermes-specific profile validation and configuration, user-scoped hook installation, canonical event relaying, Hermes-aware probing, and CLI adapter dispatch. ChangesHermes integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant HermesAdapter
participant tmux
participant Hermes
participant bmad_loop_relay
participant RunEvents
HermesAdapter->>tmux: launch interactive Hermes session
tmux->>Hermes: start CLI process
HermesAdapter->>tmux: verify pane and inject prompt
Hermes->>bmad_loop_relay: invoke Stop hook with payload
bmad_loop_relay->>RunEvents: write canonical event file
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR adds first-class support for the Hermes Agent by introducing a dedicated tmux-based adapter, a bundled Hermes profile, and a user-scoped hook relay that converts Hermes post_llm_call hooks into canonical BMAD Stop events.
Changes:
- Add
HermesAdapter/HermesDevAdapterand route adapter selection via profile metadata. - Implement user-scoped Hermes hook registration/validation and a
bmad-loop relay <Event>command for hook payload ingestion. - Update probing behavior to avoid live probing for user-scoped Hermes hooks (falls back to scan with guidance), with comprehensive new tests.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_profile.py | Verifies bundled Hermes profile fields and rejects invalid adapter/hook_scope. |
| tests/test_probe.py | Ensures user-scoped Hermes probing falls back to scan and avoids JSON probe hook merging. |
| tests/test_install.py | Validates Hermes user-scoped relay registration idempotency and detection. |
| tests/test_hermes_hooks.py | Unit tests for Hermes YAML hook merge/remove and event relay emission. |
| tests/test_hermes_adapter.py | Tests Hermes tmux startup/prompt injection behavior and an optional tmux E2E relay path. |
| tests/test_cli.py | Confirms _make_adapters dispatches Hermes profiles to Hermes adapters. |
| src/bmad_loop/probe.py | Uses centralized hooks_registered and adds scan fallback for user-scoped Hermes hooks during probe. |
| src/bmad_loop/install.py | Adds Hermes YAML hook detection/registration and makes hook script copying conditional on project-scoped profiles. |
| src/bmad_loop/hermes_hooks.py | Implements Hermes config path resolution, YAML hook merging/removal, and canonical event relay writer. |
| src/bmad_loop/data/profiles/hermes.toml | Introduces bundled Hermes profile configured for user-scoped hooks and interactive tmux usage. |
| src/bmad_loop/cli.py | Adds relay subcommand and validates user-scoped hook registration via shared install logic. |
| src/bmad_loop/adapters/profile.py | Adds adapter + hook_scope to profile schema with validation and Hermes dialect support. |
| src/bmad_loop/adapters/hermes.py | New interactive Hermes adapter that injects the initial prompt after session startup. |
| src/bmad_loop/adapters/generic.py | Refactors session startup into _prepare_session/_launch_session to support Hermes startup injection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if dialect == "hermes-config-yaml": | ||
| return any( | ||
| any( | ||
| isinstance(entry, dict) and entry.get("command") == HERMES_RELAY_COMMAND | ||
| for entry in container.get(event, []) | ||
| ) | ||
| for event in events | ||
| ) |
| try: | ||
| loaded = yaml.safe_load(config_path.read_text(encoding="utf-8")) | ||
| except yaml.YAMLError: | ||
| print(f"FAIL: {config_path} is not valid YAML; fix it and re-run init") | ||
| return 1 | ||
| if loaded is not None: | ||
| if not isinstance(loaded, dict): | ||
| print(f"FAIL: {config_path} must contain a YAML mapping; fix it and re-run init") | ||
| return 1 | ||
| config = loaded | ||
| try: | ||
| config, changed = merge_hermes_stop_hook(config) | ||
| except ValueError as exc: | ||
| print(f"FAIL: {config_path}: {exc}") | ||
| return 1 | ||
| if changed: | ||
| config_path.parent.mkdir(parents=True, exist_ok=True) | ||
| config_path.write_text(yaml.safe_dump(config, sort_keys=False), encoding="utf-8") | ||
| print(f" hooks registered ({profile.name}): {config_path}") |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/bmad_loop/cli.py (1)
461-466: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHook-config path resolution duplicated with
install.hooks_registered.The
hook_configcomputed here re-derives the samehook_scope == "user"vs project-scoped branching thatinstall.hooks_registeredperforms internally (seesrc/bmad_loop/install.pylines 277-293). It's harmless today since both sides compute the identical path, but it's two independent copies of one decision — a future change to the scoping rule in one spot risks the reportedconfig_pathsilently diverging from the path actually checked for registration.Consider having
install.hooks_registered(or a small shared helper) expose the resolved path alongside the boolean, socmd_validatedoesn't need to reimplement the branch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/bmad_loop/cli.py` around lines 461 - 466, Eliminate the duplicated hook-scope path branching between cmd_validate and install.hooks_registered. Update hooks_registered or introduce a shared resolver to return the resolved hook configuration path together with the registration result, then have cmd_validate reuse that path for hook_config and preserve the existing reported config_path behavior.src/bmad_loop/install.py (1)
418-446: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftRewriting the user's live Hermes
config.yamlviayaml.safe_load/yaml.safe_dumpsilently drops comments and formatting.PyYAML's
safe_load/safe_dumpdo not preserve comments, anchors, or the original block/flow style — every field survives, but any hand-written comments in the user's~/.hermes/config.yamlare gone after the firstbmad-loop init --cli hermesthat needs to add the relay hook. Since this file is owned by the user (not by bmad-loop), that's a real, if narrow, loss of their content, unlike the JSON hook configs (which the CLIs there don't typically comment).Consider using
ruamel.yaml's round-trip loader/dumper here to preserve comments/formatting when mutating a config bmad-loop doesn't own.♻️ Sketch using ruamel.yaml for round-trip preservation
-import yaml +from ruamel.yaml import YAML + +_yaml = YAML() +_yaml.preserve_quotes = True ... - try: - loaded = yaml.safe_load(config_path.read_text(encoding="utf-8")) - except yaml.YAMLError: + try: + loaded = _yaml.load(config_path.read_text(encoding="utf-8")) + except Exception: print(f"FAIL: {config_path} is not valid YAML; fix it and re-run init") return 1 ... - config_path.write_text(yaml.safe_dump(config, sort_keys=False), encoding="utf-8") + with config_path.open("w", encoding="utf-8") as f: + _yaml.dump(config, f)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/bmad_loop/install.py` around lines 418 - 446, Update _register_user_scoped_hooks to use ruamel.yaml’s round-trip loader and dumper when reading and writing the user-owned Hermes config, preserving comments, anchors, and block/flow formatting while applying merge_hermes_stop_hook. Retain the existing YAML mapping validation, error handling, and registration behavior, and add or reuse the project’s ruamel.yaml dependency.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/bmad_loop/adapters/profile.py`:
- Around line 151-157: Update the validation around hook_scope and dialect in
the profile adapter validation flow so their correlation is enforced for every
dialect, including "none": require user-scoped hooks to use "hermes-config-yaml"
with "config.yaml", and reject "hermes-config-yaml" unless hook_scope is "user".
Keep the existing config_path safety checks for non-user scopes in their
separate conditional path.
---
Nitpick comments:
In `@src/bmad_loop/cli.py`:
- Around line 461-466: Eliminate the duplicated hook-scope path branching
between cmd_validate and install.hooks_registered. Update hooks_registered or
introduce a shared resolver to return the resolved hook configuration path
together with the registration result, then have cmd_validate reuse that path
for hook_config and preserve the existing reported config_path behavior.
In `@src/bmad_loop/install.py`:
- Around line 418-446: Update _register_user_scoped_hooks to use ruamel.yaml’s
round-trip loader and dumper when reading and writing the user-owned Hermes
config, preserving comments, anchors, and block/flow formatting while applying
merge_hermes_stop_hook. Retain the existing YAML mapping validation, error
handling, and registration behavior, and add or reuse the project’s ruamel.yaml
dependency.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 11d112e2-0391-45ac-8c25-432b00d0ffcd
📒 Files selected for processing (14)
src/bmad_loop/adapters/generic.pysrc/bmad_loop/adapters/hermes.pysrc/bmad_loop/adapters/profile.pysrc/bmad_loop/cli.pysrc/bmad_loop/data/profiles/hermes.tomlsrc/bmad_loop/hermes_hooks.pysrc/bmad_loop/install.pysrc/bmad_loop/probe.pytests/test_cli.pytests/test_hermes_adapter.pytests/test_hermes_hooks.pytests/test_install.pytests/test_probe.pytests/test_profile.py
| if hook_scope == "user": | ||
| if dialect != "hermes-config-yaml" or config_path != "config.yaml": | ||
| raise fail( | ||
| 'user-scoped hooks require dialect = "hermes-config-yaml" ' | ||
| 'and config_path = "config.yaml"' | ||
| ) | ||
| elif not config_path or is_absolute_path(config_path) or has_parent_ref(config_path): |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Prevent dialect == "none" from bypassing hook_scope == "user" validation.
Currently, if dialect == "none", the validation for user-scoped hooks is skipped entirely because it is inside the else: block. This allows an invalid state where a profile defines hook_scope = "user" but has dialect = "none" (which would erroneously cause it to be handled as a hookless HTTP adapter).
Additionally, this block does not explicitly restrict dialect = "hermes-config-yaml" to hook_scope = "user", which could cause hooks_registered to attempt to parse a project-scoped YAML configuration as JSON.
Lift the scope and dialect correlation checks out of the else block to enforce them universally.
🛠️ Proposed fix
Replace lines 142-158 with the following:
- if dialect == "none":
- # hookless: nothing is ever registered, so a config_path or events map
- # is a contradiction — reject rather than silently ignore.
- if hooks_d.get("config_path") or hooks_d.get("events"):
- raise fail('hookless profiles (dialect = "none") must not set hooks.config_path/events')
- config_path = ""
- events: dict[str, str] = {}
- else:
- config_path = str(hooks_d.get("config_path", ""))
- if hook_scope == "user":
- if dialect != "hermes-config-yaml" or config_path != "config.yaml":
- raise fail(
- 'user-scoped hooks require dialect = "hermes-config-yaml" '
- 'and config_path = "config.yaml"'
- )
- elif not config_path or is_absolute_path(config_path) or has_parent_ref(config_path):
- raise fail("hooks.config_path must be a project-relative path")
+ config_path = str(hooks_d.get("config_path", ""))
+
+ if hook_scope == "user":
+ if dialect != "hermes-config-yaml" or config_path != "config.yaml":
+ raise fail(
+ 'user-scoped hooks require dialect = "hermes-config-yaml" '
+ 'and config_path = "config.yaml"'
+ )
+ elif dialect == "hermes-config-yaml":
+ raise fail('dialect = "hermes-config-yaml" requires hook_scope = "user"')
+
+ if dialect == "none":
+ # hookless: nothing is ever registered, so a config_path or events map
+ # is a contradiction — reject rather than silently ignore.
+ if hooks_d.get("config_path") or hooks_d.get("events"):
+ raise fail('hookless profiles (dialect = "none") must not set hooks.config_path/events')
+ config_path = ""
+ events: dict[str, str] = {}
+ else:
+ if not config_path or is_absolute_path(config_path) or has_parent_ref(config_path):
+ raise fail("hooks.config_path must be a project-relative path")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if hook_scope == "user": | |
| if dialect != "hermes-config-yaml" or config_path != "config.yaml": | |
| raise fail( | |
| 'user-scoped hooks require dialect = "hermes-config-yaml" ' | |
| 'and config_path = "config.yaml"' | |
| ) | |
| elif not config_path or is_absolute_path(config_path) or has_parent_ref(config_path): | |
| config_path = str(hooks_d.get("config_path", "")) | |
| if hook_scope == "user": | |
| if dialect != "hermes-config-yaml" or config_path != "config.yaml": | |
| raise fail( | |
| 'user-scoped hooks require dialect = "hermes-config-yaml" ' | |
| 'and config_path = "config.yaml"' | |
| ) | |
| elif dialect == "hermes-config-yaml": | |
| raise fail('dialect = "hermes-config-yaml" requires hook_scope = "user"') | |
| if dialect == "none": | |
| # hookless: nothing is ever registered, so a config_path or events map | |
| # is a contradiction — reject rather than silently ignore. | |
| if hooks_d.get("config_path") or hooks_d.get("events"): | |
| raise fail('hookless profiles (dialect = "none") must not set hooks.config_path/events') | |
| config_path = "" | |
| events: dict[str, str] = {} | |
| else: | |
| if not config_path or is_absolute_path(config_path) or has_parent_ref(config_path): | |
| raise fail("hooks.config_path must be a project-relative path") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/bmad_loop/adapters/profile.py` around lines 151 - 157, Update the
validation around hook_scope and dialect in the profile adapter validation flow
so their correlation is enforced for every dialect, including "none": require
user-scoped hooks to use "hermes-config-yaml" with "config.yaml", and reject
"hermes-config-yaml" unless hook_scope is "user". Keep the existing config_path
safety checks for non-user scopes in their separate conditional path.
1442172 to
66c9a92
Compare
Summary
post_llm_callrelay and translate it into BMADStopeventsconfig.yamlfiles in isolated worktrees; Hermes hooks remain user-scopedVerification
GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null uv run pytest -q(2698 passed, 18 skipped)uv run ruff check src testsuv buildhermes hooks test post_llm_callrelay checkNote: the default environment has a global
core.hooksPaththat bypasses two test-local rejecting hooks; the full suite is run with global Git config isolated so those tests exercise their intended local hooks.Summary by CodeRabbit
bmad-loop relaycommand for forwarding Hermes events.