fix(hermes): declare all plugin hooks#486
Conversation
|
@honor2030 is attempting to deploy a commit to the rohitg00's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughUpdated Hermes plugin manifest to declare all six lifecycle hooks ( ChangesHermes Plugin Hook Declarations
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 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 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. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
![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)
test/hermes-plugin.test.ts (2)
13-32: ⚖️ Poor tradeoffConsider using a YAML parser library.
The manual line-by-line parsing works for the simple
plugin.yamlstructure but is fragile to YAML features like comments, multi-line values, or anchors. While acceptable for this focused test, a proper YAML parser (e.g.,js-yaml) would be more robust.📦 Example using js-yaml
+import yaml from "js-yaml"; + function readHermesPluginHooks(): string[] { - const manifest = readFileSync("integrations/hermes/plugin.yaml", "utf8"); - const hooks: string[] = []; - let inHooks = false; - - for (const line of manifest.split(/\r?\n/)) { - if (line.trim() === "hooks:") { - inHooks = true; - continue; - } - if (!inHooks) continue; - if (line.trim() === "") continue; - if (!line.startsWith(" ")) break; - - const match = line.match(/^\s*-\s*([A-Za-z_][A-Za-z0-9_]*)\s*$/); - if (match) hooks.push(match[1]); - } - - return hooks; + const content = readFileSync("integrations/hermes/plugin.yaml", "utf8"); + const manifest = yaml.load(content) as { hooks?: string[] }; + return manifest.hooks || []; }Requires adding
js-yamland@types/js-yamlto dev dependencies.🤖 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 `@test/hermes-plugin.test.ts` around lines 13 - 32, Replace the fragile manual parser in readHermesPluginHooks with a proper YAML parser: import and use js-yaml to read and parse "integrations/hermes/plugin.yaml", then extract the "hooks" key from the parsed object, validate it's an array of strings and return it; ensure the new implementation handles missing/invalid hooks by returning an empty array or throwing a clear error and update any types accordingly (reference function name readHermesPluginHooks and the "hooks" field in plugin.yaml).
38-38: ⚡ Quick winRegex fragile to indentation changes.
The pattern
/^ def .../assumes exactly 4 spaces. If the Python file is reformatted with different indentation (tabs, 2 spaces, etc.), the regex breaks.♻️ More flexible indentation pattern
- const hookMethodPattern = - /^ def (prefetch|sync_turn|on_session_end|on_pre_compress|on_memory_write|system_prompt_block)\(/gm; + const hookMethodPattern = + /^\s+def (prefetch|sync_turn|on_session_end|on_pre_compress|on_memory_write|system_prompt_block)\(/gm;Using
\s+matches any indentation (spaces or tabs).🤖 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 `@test/hermes-plugin.test.ts` at line 38, The regex /^ def (prefetch|sync_turn|on_session_end|on_pre_compress|on_memory_write|system_prompt_block)\(/gm is brittle because it requires exactly four spaces; update it to allow any whitespace indentation by using /^\s+def (prefetch|sync_turn|on_session_end|on_pre_compress|on_memory_write|system_prompt_block)\(/gm (keep the same flags), then run tests to ensure matches for the listed function names still work across tabs/2-space/4-space formatting.
🤖 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 `@test/hermes-plugin.test.ts`:
- Around line 34-54: The test currently limits discovery to six hardcoded names
via hookMethodPattern in readAgentMemoryProviderHookMethods which allows YAML
drift; change readAgentMemoryProviderHookMethods to use a general method regex
(e.g., /^ def (\w+)\(/gm) to collect all method names defined in
integrations/hermes/__init__.py (specifically methods on the AgentMemoryProvider
implementation), then filter those names for valid Hermes hook patterns (e.g.,
names that match the Hermes hook naming convention or a small predicate), and
finally replace the current partial assertions so the test asserts exact
equality between the discovered Python hooks and the YAML-declared hooks from
readHermesPluginHooks (instead of comparing against expectedHermesHooks) to
ensure no drift.
---
Nitpick comments:
In `@test/hermes-plugin.test.ts`:
- Around line 13-32: Replace the fragile manual parser in readHermesPluginHooks
with a proper YAML parser: import and use js-yaml to read and parse
"integrations/hermes/plugin.yaml", then extract the "hooks" key from the parsed
object, validate it's an array of strings and return it; ensure the new
implementation handles missing/invalid hooks by returning an empty array or
throwing a clear error and update any types accordingly (reference function name
readHermesPluginHooks and the "hooks" field in plugin.yaml).
- Line 38: The regex /^ def
(prefetch|sync_turn|on_session_end|on_pre_compress|on_memory_write|system_prompt_block)\(/gm
is brittle because it requires exactly four spaces; update it to allow any
whitespace indentation by using /^\s+def
(prefetch|sync_turn|on_session_end|on_pre_compress|on_memory_write|system_prompt_block)\(/gm
(keep the same flags), then run tests to ensure matches for the listed function
names still work across tabs/2-space/4-space formatting.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6accfccd-e7e1-497f-8cc5-e5c86367b978
📒 Files selected for processing (2)
integrations/hermes/plugin.yamltest/hermes-plugin.test.ts
|
Merged + shipping in next release. Thanks @honor2030 — Hermes plugin manifest now declares all 6 hooks (prefetch, sync_turn, on_session_end, on_pre_compress, on_memory_write, system_prompt_block) instead of just 3. |
Quality + integration wave. Bundles 11 PRs since v0.9.20: Contributor feature: - #237 OpenCode plugin with 22 auto-capture hooks (@cl0ckt0wer) Bug fixes (9): - #516 memory_recall endpoint + format/token_budget (@serhiizghama, closes #507/#440) - #461 env-file AGENTMEMORY_DROP_STALE_INDEX flag honored (@honor2030, closes #456) - #487 Windows hook path quoting (@honor2030, closes #477) - #517 viewer IME composition guard (@jonathanzhan1975) - #472 chunk large sessions for LLM context window (@efenex) - #473 surface lessons in smart-search + diagnose tally (@efenex) - #486 declare all Hermes plugin hooks (@honor2030) - #500 rebuildIndex non-blocking on boot (@efenex) - #504 batched embed in rebuildIndex (25h -> 3h) (@efenex) - #491 cli skip onboarding without tty (@honor2030) Upstream-installer revert: - #546 drop --next workaround now that iii-hq/iii#1660 shipped 1067/1067 tests pass across 95 files.
2 participants
Summary
AgentMemoryProviderinintegrations/hermes/plugin.yaml.Why
AgentMemoryProvideralready implementsprefetch,sync_turn, andsystem_prompt_block, and the Hermes README documents them. The plugin manifest only declaredon_session_end,on_pre_compress, andon_memory_write, so Hermes may never invoke the missing lifecycle hooks. That can silently disable context prefetching, turn capture, and system-prompt context injection for Hermes users.Closes #478.
Conflict / duplicate check
integrations/hermes/plugin.yaml,integrations/hermes/__init__.py, ortest/hermes-plugin.test.ts: none found.Test plan
PATH=/opt/homebrew/bin:$PATH npm test -- test/hermes-plugin.test.ts --reporter=verboseon_session_end,on_pre_compress, andon_memory_write.PATH=/opt/homebrew/bin:$PATH npm test -- test/hermes-plugin.test.ts --reporter=verbose1 passedPATH=/opt/homebrew/bin:$PATH npm test -- test/hermes-plugin.test.ts test/integration-plaintext-http.test.ts --reporter=verbose14 passedPATH=/opt/homebrew/bin:$PATH npm test -- --reporter=dot92 passed,1008 passedPATH=/opt/homebrew/bin:$PATH npm run buildgit diff --checkNote: local shell defaulted to Node 16, so I used Homebrew Node
v23.11.0on PATH for the repo's Node >=20 toolchain.Summary by CodeRabbit
New Features
Tests