fix(installer): use Hermes hook event names (pre_tool_call/post_tool_call/on_session_end)#193
Conversation
…call/on_session_end) The Hermes hook adapter was writing entries under the Claude-Code event key 'pre_tool_use'. Hermes only recognises 'pre_tool_call', 'post_tool_call', and 'on_session_end' — it logs a warning and silently ignores everything else. Discovered on a real fleet on 2026-05-13: 2538 SessionStart events but zero PostToolUse events for 5+ days, which left the cloud lift report empty. Fix: - Rename the event key written by gradata.hooks.adapters.hermes from 'pre_tool_use' to 'pre_tool_call'. - Add HERMES_EVENT_NAMES mapping that pins the runtime contract for pre_tool_use / post_tool_use / session_end. - Add a migration helper so an old, broken install (entries under pre_tool_use) is folded into pre_tool_call on the next 'gradata install --agent hermes' run. This handles the symmetric uninstall concern without leaking stale entries. - Regression tests covering: correct key on fresh install, idempotency, legacy-key migration, and contract pinning. The claude-code adapter (which uses CamelCase 'PreToolUse' as Claude Code expects natively) is left untouched. Refs #190 (install UX epic).
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughSummary
WalkthroughThis PR adds legacy event migration support to the Hermes hook adapter. A new event-name mapping constant and migration helper function enable safe transition from Claude-Code event key names ( ChangesHermes legacy event migration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.20.0)OpenGrep fatal error (exit code 2): �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@Gradata/src/gradata/hooks/adapters/hermes.py`:
- Around line 165-169: The code calls _migrate_legacy_event which mutates hooks
in memory but may return InstallResult("already_present") before those changes
are saved; update the flow so that after calling _migrate_legacy_event (and
before returning from the already_present branch) you persist the migrated hooks
to disk using the existing agent config persistence routine used elsewhere (the
same function that writes agent_config_path / agent config), then return
InstallResult; reference _migrate_legacy_event, pre_tool_call, and
agent_config_path so the save is performed immediately after migration and
before returning.
In `@Gradata/tests/test_hermes_hook_event_names.py`:
- Around line 51-77: Add a regression case to cover the "already_present" path
by creating a test (or extending
test_install_migrates_legacy_pre_tool_use_entry) that writes a legacy config
with pre_tool_use containing the current signature (the same sig created from
brain used by hermes.install), then call hermes.install(brain, config) and
assert result.action == "already_present"; also verify the config text no longer
contains "pre_tool_use:" and does contain "pre_tool_call:" and the legacy entry
and the current signature were persisted. Ensure you reference the same
variables used in the test (brain, config, hermes.install) so the migration
logic is exercised for the already_present branch.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 23f74258-be0f-4369-98c3-fa0ef90f8ed3
📒 Files selected for processing (2)
Gradata/src/gradata/hooks/adapters/hermes.pyGradata/tests/test_hermes_hook_event_names.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: pytest ubuntu-latest / py3.12
- GitHub Check: pytest macos-latest / py3.11
- GitHub Check: pytest macos-latest / py3.12
- GitHub Check: pytest windows-latest / py3.11
- GitHub Check: pytest windows-latest / py3.12
- GitHub Check: pytest ubuntu-latest / py3.11
- GitHub Check: pytest (py3.11)
- GitHub Check: pytest (py3.12)
🧰 Additional context used
📓 Path-based instructions (2)
Gradata/tests/**/*.py
📄 CodeRabbit inference engine (Gradata/AGENTS.md)
Gradata/tests/**/*.py: SetBRAIN_DIRenvironment variable viatmp_pathin conftest.py for test isolation — ensure_paths.pymodule cache refreshes when callingBrain.init()directly inside tests
Add unit tests intests/test_*.pyfor every CI push without LLM calls (deterministic); mark integration tests with@pytest.mark.integrationand skip them by default (they hit real LLM APIs)
Files:
Gradata/tests/test_hermes_hook_event_names.py
Gradata/src/**/*.py
📄 CodeRabbit inference engine (Gradata/AGENTS.md)
Gradata/src/**/*.py: Prefersentence-transformersfor local embeddings,google-genaifor Gemini embeddings,cryptographyfor AES-GCM encrypted system.db,bm25sfor BM25 rule ranking, andmem0aifor external memory adapters — guard all optional dependency imports withtry / except ImportErrorat the call site, never at module level
Maintain strict layering: Layer 0 (Primitives: _types.py, _db.py, _events.py, _paths.py, _file_lock.py; Patterns: contrib/patterns/) must never import from Layer 1 (Enhancements: enhancements/, rules/) or Layer 2 (Public API: brain.py, cli.py, daemon.py, mcp_server.py)
Never use bareexcept: pass— use typed exceptions or at minimumlogger.warning(...)withexc_info=Trueto avoid silent failure in a memory product
Never import from out-of-scope sibling directories../Sprites/or../Hausgem/withingradata/*code — that is a layering bug
Never leak private-sibling paths into public docs/code — no references to../Sprites/,../Hausgem/, email addresses, OneDrive paths, or Sprites-specific examples from insidegradata/*
Use atomic-write helper when writing JSON files to prevent corruption from mid-write crashes
Files:
Gradata/src/gradata/hooks/adapters/hermes.py
🔇 Additional comments (2)
Gradata/src/gradata/hooks/adapters/hermes.py (1)
16-48: LGTM!Also applies to: 170-172
Gradata/tests/test_hermes_hook_event_names.py (1)
20-49: LGTM!Also applies to: 79-87
| pre_tool_call = _migrate_legacy_event(hooks, "pre_tool_use", "pre_tool_call") | ||
| if any(isinstance(entry, dict) and entry.get("id") == sig for entry in pre_tool_call): | ||
| return InstallResult( | ||
| AGENT, agent_config_path, "already_present", "hook already present" | ||
| ) |
There was a problem hiding this comment.
Persist migrated config before returning already_present.
Line 165 mutates hooks in memory, but Line 166-Line 169 may return before any write. If the legacy entry already has the same id, migration is effectively dropped and pre_tool_use remains on disk.
Suggested fix
-def _migrate_legacy_event(hooks: dict, legacy_key: str, current_key: str) -> list:
+def _migrate_legacy_event(
+ hooks: dict, legacy_key: str, current_key: str
+) -> tuple[list, bool]:
@@
- current = hooks.setdefault(current_key, [])
+ migrated = False
+ current = hooks.setdefault(current_key, [])
@@
if isinstance(legacy, list):
for entry in legacy:
if entry not in current:
current.append(entry)
del hooks[legacy_key]
+ migrated = True
elif legacy_key in hooks:
@@
del hooks[legacy_key]
- return current
+ migrated = True
+ return current, migrated
@@
- pre_tool_call = _migrate_legacy_event(hooks, "pre_tool_use", "pre_tool_call")
+ pre_tool_call, migrated = _migrate_legacy_event(
+ hooks, "pre_tool_use", "pre_tool_call"
+ )
if any(isinstance(entry, dict) and entry.get("id") == sig for entry in pre_tool_call):
+ if migrated:
+ atomic_write_text(agent_config_path, _dump_simple_yaml(data))
return InstallResult(
AGENT, agent_config_path, "already_present", "hook already present"
)🤖 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 `@Gradata/src/gradata/hooks/adapters/hermes.py` around lines 165 - 169, The
code calls _migrate_legacy_event which mutates hooks in memory but may return
InstallResult("already_present") before those changes are saved; update the flow
so that after calling _migrate_legacy_event (and before returning from the
already_present branch) you persist the migrated hooks to disk using the
existing agent config persistence routine used elsewhere (the same function that
writes agent_config_path / agent config), then return InstallResult; reference
_migrate_legacy_event, pre_tool_call, and agent_config_path so the save is
performed immediately after migration and before returning.
| def test_install_migrates_legacy_pre_tool_use_entry(tmp_path: Path) -> None: | ||
| """An old broken install (entries under ``pre_tool_use``) must be migrated | ||
| to ``pre_tool_call`` on the next ``gradata install --agent hermes`` run. | ||
| """ | ||
| brain = tmp_path / "brain" | ||
| brain.mkdir() | ||
| config = tmp_path / ".hermes" / "config.yaml" | ||
| config.parent.mkdir() | ||
| legacy = ( | ||
| "hooks:\n" | ||
| " pre_tool_use:\n" | ||
| " - id: gradata:hermes:/some/other/brain\n" | ||
| " command: legacy-command\n" | ||
| ) | ||
| config.write_text(legacy, encoding="utf-8") | ||
|
|
||
| result = hermes.install(brain, config) | ||
|
|
||
| assert result.action == "added", result.message | ||
| text = config.read_text(encoding="utf-8") | ||
| # Legacy key has been removed. | ||
| assert "pre_tool_use:" not in text, text | ||
| # New key holds both the migrated entry and the freshly-installed one. | ||
| assert "pre_tool_call:" in text, text | ||
| assert "legacy-command" in text, text | ||
| assert f"gradata:hermes:{brain.resolve().as_posix()}" in text, text | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add a regression case for legacy migration on the already_present path.
Line 67 currently validates only the added flow. Please add a case where pre_tool_use already contains the current sig; this catches migration bugs where install returns already_present without persisting the key rewrite.
Suggested test addition
+def test_install_migrates_legacy_key_even_when_signature_already_present(tmp_path: Path) -> None:
+ brain = tmp_path / "brain"
+ brain.mkdir()
+ config = tmp_path / ".hermes" / "config.yaml"
+ config.parent.mkdir()
+ sig = f"gradata:hermes:{brain.resolve().as_posix()}"
+ legacy = (
+ "hooks:\n"
+ " pre_tool_use:\n"
+ f" - id: {sig}\n"
+ " command: legacy-command\n"
+ )
+ config.write_text(legacy, encoding="utf-8")
+
+ result = hermes.install(brain, config)
+
+ assert result.action == "already_present"
+ text = config.read_text(encoding="utf-8")
+ assert "pre_tool_use:" not in text, text
+ assert "pre_tool_call:" in text, text📝 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.
| def test_install_migrates_legacy_pre_tool_use_entry(tmp_path: Path) -> None: | |
| """An old broken install (entries under ``pre_tool_use``) must be migrated | |
| to ``pre_tool_call`` on the next ``gradata install --agent hermes`` run. | |
| """ | |
| brain = tmp_path / "brain" | |
| brain.mkdir() | |
| config = tmp_path / ".hermes" / "config.yaml" | |
| config.parent.mkdir() | |
| legacy = ( | |
| "hooks:\n" | |
| " pre_tool_use:\n" | |
| " - id: gradata:hermes:/some/other/brain\n" | |
| " command: legacy-command\n" | |
| ) | |
| config.write_text(legacy, encoding="utf-8") | |
| result = hermes.install(brain, config) | |
| assert result.action == "added", result.message | |
| text = config.read_text(encoding="utf-8") | |
| # Legacy key has been removed. | |
| assert "pre_tool_use:" not in text, text | |
| # New key holds both the migrated entry and the freshly-installed one. | |
| assert "pre_tool_call:" in text, text | |
| assert "legacy-command" in text, text | |
| assert f"gradata:hermes:{brain.resolve().as_posix()}" in text, text | |
| def test_install_migrates_legacy_key_even_when_signature_already_present(tmp_path: Path) -> None: | |
| brain = tmp_path / "brain" | |
| brain.mkdir() | |
| config = tmp_path / ".hermes" / "config.yaml" | |
| config.parent.mkdir() | |
| sig = f"gradata:hermes:{brain.resolve().as_posix()}" | |
| legacy = ( | |
| "hooks:\n" | |
| " pre_tool_use:\n" | |
| f" - id: {sig}\n" | |
| " command: legacy-command\n" | |
| ) | |
| config.write_text(legacy, encoding="utf-8") | |
| result = hermes.install(brain, config) | |
| assert result.action == "already_present" | |
| text = config.read_text(encoding="utf-8") | |
| assert "pre_tool_use:" not in text, text | |
| assert "pre_tool_call:" in text, text |
🤖 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 `@Gradata/tests/test_hermes_hook_event_names.py` around lines 51 - 77, Add a
regression case to cover the "already_present" path by creating a test (or
extending test_install_migrates_legacy_pre_tool_use_entry) that writes a legacy
config with pre_tool_use containing the current signature (the same sig created
from brain used by hermes.install), then call hermes.install(brain, config) and
assert result.action == "already_present"; also verify the config text no longer
contains "pre_tool_use:" and does contain "pre_tool_call:" and the legacy entry
and the current signature were persisted. Ensure you reference the same
variables used in the test (brain, config, hermes.install) so the migration
logic is exercised for the already_present branch.
1 participant
Problem
gradata install --agent hermeswrites hook entries using Claude-Code event names (pre_tool_use,post_tool_use,session_end) into~/.hermes/config.yaml. Hermes only recognizespre_tool_call,post_tool_call,on_session_endand SILENTLY ignores the others (warning only, no failure).Impact
Discovered 2026-05-13 on a real fleet: 2538 SessionStart events in telemetry but ZERO PostToolUse events for 5+ days. No corrections captured, no lessons crystallized, cloud dashboard lift report empty. The hooks looked installed and the warnings are easy to miss in noisy logs.
Fix
Map per-agent event names in the installer:
Update both install AND uninstall paths so users on the old broken config can be migrated cleanly.
The claude-code adapter (which writes CamelCase
PreToolUseinto~/.claude/settings.json, a different config format that Claude Code expects natively) is left untouched.What changed
Gradata/src/gradata/hooks/adapters/hermes.pypre_tool_use→pre_tool_call.HERMES_EVENT_NAMESmapping that pins the runtime contract for all three events._migrate_legacy_eventhelper: when an existing~/.hermes/config.yamlstill has entries under the broken legacy key, they are folded into the correct key on the nextgradata install --agent hermesrun. This is the symmetric uninstall fix — old broken entries no longer leak.Gradata/tests/test_hermes_hook_event_names.py(new)Test plan
The 4 unrelated failures in
tests/test_rule_to_hook.py(em-dash rule lessons.md handling) were verified to be pre-existing onmainand are not touched by this PR.Layering / risk
Refs #190 (install UX epic).