feat: add Hermes Agent harness adapter - #322
#322feat: add Hermes Agent harness adapter#322chrisl10 wants to merge 5 commits intolegioncodeinc:mainlegioncodeinc/honeycomb:mainfrom chrisl10:feat/hermes-harnesschrisl10/honeycomb:feat/hermes-harnessCopy head branch name to clipboard
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughHermes receives a production connector with native YAML hooks, MCP registration, capture and recall lifecycle handling, ownership-safe installation, CLI wiring, packed conformance checks, and updated support documentation. ChangesHermes production integration
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
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 |
6ca6bb2 to
adb5e4d
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/connectors/hermes.ts (2)
382-389: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRedundant
toConfigEntryoverride — identical to the base default.Since
HERMES_HANDLERSnever setshandler.async, this override produces the exact same shape asHarnessConnector.toConfigEntry(type,command, optionaltimeout,HONEYCOMB_ENTRY_KEY). Removing it and relying on the inherited default reduces duplication and keeps Hermes automatically in sync if the base entry shape ever changes.♻️ Simplify by dropping the override
- protected toConfigEntry(handler: HookHandlerEntry): ConfigHookEntry { - return { - type: "command", - command: handler.command, - ...(handler.timeout === undefined ? {} : { timeout: handler.timeout }), - [HONEYCOMB_ENTRY_KEY]: true, - }; - } -🤖 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/connectors/hermes.ts` around lines 382 - 389, Remove the redundant toConfigEntry override from the Hermes connector and rely on the inherited HarnessConnector.toConfigEntry implementation. Keep the existing Hermes handler configuration and behavior unchanged.Source: Coding guidelines
275-282: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the base class's
dirOfhelper instead of manual slicing.
file.targetPath.slice(0, file.targetPath.lastIndexOf("/"))reimplements dirname logic the baseHarnessConnector.install()/.uninstall()already gets viadirOf(...). It's also fragile: iftargetPathever lacked a/(e.g. a future refactor ofpluginRoot/handler paths),lastIndexOfreturns-1and.slice(0, -1)silently truncates the last character instead of erroring.Consider exporting
dirOffromcontracts.tsand reusing it here for consistency with the base class.Also applies to: 337-339
🤖 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/connectors/hermes.ts` around lines 275 - 282, Export the existing dirOf helper from contracts.ts and update the file-writing path in the managed-files loop to call dirOf(file.targetPath) when ensuring the parent directory. Apply the same replacement to the corresponding path-handling logic around the second referenced location, removing the manual lastIndexOf/slice dirname implementation.Source: Coding guidelines
🤖 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 @.github/workflows/ci.yaml:
- Around line 142-144: Rename the affected workflow steps in quality-gate,
windows-smoke, and macos-service-smoke from Hermes conformance to CLI-only
conformance, since they run only npm run test:packed-cli; leave native Hermes
coverage associated with minimum-node-packed-hermes unless explicitly adding the
Hermes test command to those jobs.
In `@src/daemon/runtime/dashboard/harness-detect.ts`:
- Around line 86-88: Update detectInstalledHarnesses() and its Hermes marker
path configuration to resolve the effective Hermes home from HERMES_HOME,
matching the connector’s existing home-resolution behavior, before probing
capture.js. Ensure the default path remains homedir() when no override is set
and non-default profiles probe their resolved profile home, including through
the dashboard’s default uninstall flow.
- Around line 86-88: Update the Hermes detection path in the paths handler to
target the packaged bundle marker capture.mjs instead of capture.js, preferably
by reusing the shared Hermes handler or bundle constant so detection stays
aligned with shipped artifacts and conformance checks.
In `@src/hooks/CONVENTIONS.md`:
- Line 75: Update the Hermes guidance in the ContextEnvelope and harness
sections to match the production Hermes adapter: treat native `{ context }`
output as model-only rather than user-visible, and remove the claim that no
harness is live-wired. Keep the existing Hermes model-only contract and no
pre-tool interception behavior consistent throughout the document, including the
remaining stale rows.
In `@tests/daemon/runtime/dashboard/harness-detect.test.ts`:
- Around line 80-81: Update the Hermes install fixtures to use capture.mjs
instead of capture.js at
tests/daemon/runtime/dashboard/harness-detect.test.ts:80-81,
tests/daemon/runtime/dashboard/harness-detect.test.ts:137-137, and
tests/daemon/runtime/dashboard/harness-installed-wiring.test.ts:168-168,
preserving the existing test behavior while matching the packaged
harnesses/hermes/bundle/capture.mjs contract.
---
Nitpick comments:
In `@src/connectors/hermes.ts`:
- Around line 382-389: Remove the redundant toConfigEntry override from the
Hermes connector and rely on the inherited HarnessConnector.toConfigEntry
implementation. Keep the existing Hermes handler configuration and behavior
unchanged.
- Around line 275-282: Export the existing dirOf helper from contracts.ts and
update the file-writing path in the managed-files loop to call
dirOf(file.targetPath) when ensuring the parent directory. Apply the same
replacement to the corresponding path-handling logic around the second
referenced location, removing the manual lastIndexOf/slice dirname
implementation.
🪄 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 Plus
Run ID: c1fd0817-cd14-48f9-9ca7-ccedcb445a69
📒 Files selected for processing (42)
.github/workflows/ci.yaml.github/workflows/release.yamlREADME.mdesbuild.config.mjsharnesses/hermes/.mcp.jsonharnesses/hermes/src/index.tslibrary/knowledge/private/integrations/harness-integration.mdlibrary/knowledge/private/integrations/hook-lifecycle.mdlibrary/knowledge/private/overview.mdlibrary/knowledge/public/faqs/faq.mdlibrary/knowledge/public/overview/glossary.mdlibrary/knowledge/public/overview/what-is-honeycomb.mdpackage.jsonreferences/hermes/hooks-schema.tsscripts/pack-check.mjsscripts/packed-cli-conformance.mjsscripts/packed-hermes-conformance.mjssrc/cli/connector-runner.tssrc/commands/contracts.tssrc/connectors/contracts.tssrc/connectors/hermes.tssrc/connectors/index.tssrc/connectors/node-fs.tssrc/daemon/runtime/dashboard/harness-detect.tssrc/daemon/runtime/dashboard/harness-registry.tssrc/hooks/CONVENTIONS.mdsrc/hooks/hermes/shim.tssrc/hooks/index.tstests/cli/connector-runner.test.tstests/cli/harness-reconcile.test.tstests/commands/lifecycle-verbs.test.tstests/conformance/hermes-hooks-conformance.test.tstests/connectors/hermes.test.tstests/daemon/runtime/dashboard/harness-api.test.tstests/daemon/runtime/dashboard/harness-detect.test.tstests/daemon/runtime/dashboard/harness-installed-wiring.test.tstests/hooks/claude-code/shim.test.tstests/hooks/harness-identity-stamp.test.tstests/hooks/hermes/binary-e2e.test.tstests/hooks/hermes/shim.test.tstests/hooks/shims-channel.test.tstests/mcp/registration.test.ts
💤 Files with no reviewable changes (1)
- harnesses/hermes/.mcp.json
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@library/knowledge/private/overview.md`:
- Line 25: Update the compound modifiers in the overview sentence to use hyphens
where required, including “live-tested,” “end-to-end,” “cross-device,” and any
similarly formed modifiers, while preserving the sentence’s meaning and wording.
In `@src/daemon/runtime/dashboard/harness-detect.ts`:
- Around line 56-57: Update the marker validation flow around
detectInstalledHarnesses to support a per-marker validator: use
fileMarkerExists, backed by statSync(...).isFile(), for Hermes marker paths,
while retaining existsSync for Pi/OpenClaw directory markers. Pass the Hermes
validator through the existing marker definition or selection path without
changing the shared directory-marker behavior.
In `@tests/connectors/hermes.test.ts`:
- Around line 68-82: Strengthen the assertions for config.hooks.post_tool_call
in the Hermes connector test: require exactly one handler and verify its command
is the completed-tool capture command, rather than relying only on every(...).
Keep the existing matcher assertion after confirming the handler exists.
🪄 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 Plus
Run ID: fddd5e2e-b135-4ea3-98cc-15f812710672
📒 Files selected for processing (44)
.github/workflows/ci.yaml.github/workflows/release.yamlREADME.mdesbuild.config.mjsharnesses/hermes/.mcp.jsonharnesses/hermes/src/index.tslibrary/knowledge/private/integrations/harness-integration.mdlibrary/knowledge/private/integrations/hook-lifecycle.mdlibrary/knowledge/private/overview.mdlibrary/knowledge/public/faqs/faq.mdlibrary/knowledge/public/overview/glossary.mdlibrary/knowledge/public/overview/what-is-honeycomb.mdpackage.jsonreferences/hermes/hooks-schema.tsscripts/pack-check.mjsscripts/packed-cli-conformance.mjsscripts/packed-hermes-conformance.mjssrc/cli/connector-runner.tssrc/commands/contracts.tssrc/connectors/contracts.tssrc/connectors/hermes.tssrc/connectors/index.tssrc/connectors/node-fs.tssrc/daemon/runtime/dashboard/harness-detect.tssrc/daemon/runtime/dashboard/harness-registry.tssrc/hooks/CONVENTIONS.mdsrc/hooks/hermes/shim.tssrc/hooks/index.tstests/cli/connector-runner.test.tstests/cli/daemon-service.test.tstests/cli/harness-reconcile.test.tstests/commands/lifecycle-verbs.test.tstests/conformance/hermes-hooks-conformance.test.tstests/connectors/hermes.test.tstests/daemon/runtime/dashboard/harness-api.test.tstests/daemon/runtime/dashboard/harness-detect.test.tstests/daemon/runtime/dashboard/harness-installed-wiring.test.tstests/daemon/runtime/logs/log-store.test.tstests/hooks/claude-code/shim.test.tstests/hooks/harness-identity-stamp.test.tstests/hooks/hermes/binary-e2e.test.tstests/hooks/hermes/shim.test.tstests/hooks/shims-channel.test.tstests/mcp/registration.test.ts
💤 Files with no reviewable changes (1)
- harnesses/hermes/.mcp.json
🚧 Files skipped from review as they are similar to previous changes (33)
- library/knowledge/public/overview/what-is-honeycomb.md
- tests/cli/connector-runner.test.ts
- library/knowledge/public/faqs/faq.md
- library/knowledge/public/overview/glossary.md
- tests/daemon/runtime/dashboard/harness-installed-wiring.test.ts
- src/connectors/index.ts
- src/commands/contracts.ts
- scripts/packed-cli-conformance.mjs
- README.md
- harnesses/hermes/src/index.ts
- tests/commands/lifecycle-verbs.test.ts
- tests/hooks/hermes/binary-e2e.test.ts
- library/knowledge/private/integrations/hook-lifecycle.md
- src/connectors/node-fs.ts
- references/hermes/hooks-schema.ts
- package.json
- tests/conformance/hermes-hooks-conformance.test.ts
- tests/cli/harness-reconcile.test.ts
- src/hooks/CONVENTIONS.md
- tests/hooks/harness-identity-stamp.test.ts
- tests/mcp/registration.test.ts
- src/daemon/runtime/dashboard/harness-registry.ts
- library/knowledge/private/integrations/harness-integration.md
- src/connectors/contracts.ts
- tests/hooks/hermes/shim.test.ts
- esbuild.config.mjs
- scripts/pack-check.mjs
- src/connectors/hermes.ts
- tests/hooks/shims-channel.test.ts
- scripts/packed-hermes-conformance.mjs
- src/hooks/hermes/shim.ts
- src/hooks/index.ts
- tests/hooks/claude-code/shim.test.ts
|
@thenotoriousllama The code/review matrix is green apart from the repository release gate. This PR comes from a fork, so both jobs in |
What this does
Adds a production Hermes Agent 0.19 harness adapter so Honeycomb can be installed with
honeycomb connect hermes.The adapter:
$HERMES_HOME, validates explicit profile roots, and defaults to~/.hermes;on_session_start,pre_llm_call,post_tool_call,post_llm_call, andon_session_finalizeonto Honeycomb's shared runtime;{ context }response;pre_tool_call, whose response contract cannot safely support Honeycomb interception;on_session_endas a per-turn event andon_session_finalizeas final termination;.mjshook/MCP bundles with an absolute Node executable, including Node 22.5 compatibility;mcp_servers.honeycombis foreign-owned;The implementation and protocol oracle are grounded in Hermes Agent 0.19 source and its native shell-hook parser.
Related issue
Closes #319
How it was tested
npm run typechecknpm run buildnpm run pack:checknpm run test:packed-clinpm run test:packed-hermesunder the current runtime and exact minimum-supported Node 22.5.0git diff --checkHOME/HERMES_HOMEprofiles.Local
npm run cireaches 472/479 test files and 5210/5230 tests on macOS. Four persistent failures reproduce unchanged on cleanupstream/main(onboarding-api,onboarding-api-extended,log-store, andcli-invocation); three timeout-sensitive failures pass when rerun serially. The focused adapter, build, package, formatting, and type gates are green.Checklist
npm run cipasses locally (the four persistent failures reproduce on cleanupstream/main; see testing notes)Summary by CodeRabbit
connect <harness>command for wiring supported assistants.