feat: add Hermes agent adapter (Native Hermes Support)#114
feat: add Hermes agent adapter (Native Hermes Support)#114rohitg00 merged 11 commits intorohitg00:mainfrom
Conversation
|
@RUFFY-369 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 (5)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds first-class Hermes agent support: new HermesAdapter (generate/parse/detect/invoke), AgentType/config/discovery mappings, CLI sync marker support, translator/generator wiring, skill discovery paths, tests, and README update across monorepo and legacy src trees. Changes
Sequence DiagramsequenceDiagram
participant CLI as CLI
participant Detect as detectAgent()
participant Hermes as HermesAdapter
participant FS as FileSystem
participant Sync as updateConfigContent()
participant Gen as generateConfig()
CLI->>Detect: request agent detection
Detect->>Hermes: isDetected()
Hermes->>FS: check .hermes (cwd / home) and AGENTS.md
FS-->>Hermes: exists? (true/false)
Hermes-->>Detect: detection result
Detect-->>CLI: selected agent (hermes)
CLI->>Sync: updateConfigContent(skills, hermes markers)
Sync->>Gen: generateConfig(enabledSkills)
Gen->>Gen: filter enabled skills & createSkillXml blocks
Gen-->>Sync: return <skills_system> XML with <!-- SKILLS_TABLE_START/END -->
Sync->>FS: write/update AGENTS.md between markers
FS-->>CLI: sync complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
🐛 1 issue in files not directly in the diff
🐛 Legacy sync command missing hermes markers causes config duplication on re-sync (src/commands/sync.ts:111-124)
The PR adds the hermes agent to the legacy codebase (src/agents/hermes.ts, src/agents/index.ts) but does not add hermes-specific markers to the legacy src/commands/sync.ts:111-124 updateConfigContent function. The packages version at packages/cli/src/commands/sync.ts:217-220 was correctly updated with hermes markers (SKILLS_TABLE_START/SKILLS_TABLE_END), but the legacy version was not. When the legacy sync path is used for hermes, markers[agentType] is undefined, so it falls back to markers.universal which uses SKILLKIT_SKILLS_START/SKILLKIT_SKILLS_END. These don't match the SKILLS_TABLE_START/SKILLS_TABLE_END markers generated by the hermes adapter's generateConfig() (src/agents/hermes.ts:30,50). On re-sync, the function won't find existing markers in the file and will append a second copy of the config instead of replacing the existing one.
View 10 additional findings in Devin Review.
rohitg00
left a comment
rohitg00
left a comment
There was a problem hiding this comment.
Review
Verified locally at commit 27afc45. Build + typecheck clean, all targeted tests pass.
Validation
pnpm -F @skillkit/core build→ okpnpm -F @skillkit/agents build→ okpnpm -F @skillkit/core typecheck→ okpnpm -F @skillkit/agents typecheck→ ok@skillkit/agents test→ 192/192 pass (3 new Hermes cases inadapters.test.ts)@skillkit/core test→ 1035/1035 passtests/agents.test.ts→ 28/28 pass
Good calls
parseConfigscopes the<name>regex toSKILLS_TABLE_START/ENDmarkers — strictly better than the Clawdbot parser which greedy-matches any<name>tag in the whole file. Test caseshould ignore <name> tags outside of sync markersverifies this.- Sync-marker wiring in
packages/cli/src/commands/sync.tsgives idempotent updates. - 17 touchpoints wired (vs OpenClaw's 8) — types, agent-config, discovery paths, format map, translator map, primer template, command generator, commands/sync, legacy
src/mirror. - Windsurf configFile correction (
AGENTS.md→.windsurf/rules/skills.md) aligns the legacysrc/tree withpackages/core/src/agent-config.tswhich already had the modern path.
Nits (non-blocking)
-
MCP runtime discovery gap —
packages/mcp/src/tools.tsAGENT_DIR_MAP(line ~184) lists every other agent but nothermes. MCP server'sgetLocalSkillDirswon't pick up.hermes/skillsat runtime. Please add:'hermes': ['.hermes/skills'],
-
README count drift —
README.md:221adds Hermes row. Collapsed line still readsPlus 34 more. Adapter count moved 45→46 in tests; consider reconciling the README blurb or dropping the fixed number. -
XML format assumption — upstream Hermes Agent (NousResearch/hermes-agent v0.10.0) uses Markdown + YAML for SOUL.md/AGENTS.md/MEMORY.md. XML-wrapped blocks inside
AGENTS.mdare tolerated by Claude-family parsers but not an upstream convention. Matches Clawdbot precedent, so fine to ship — just worth flagging in case the Hermes maintainers prefer a Markdown-only variant later.
Approve once (1) is addressed.
#117) * fix(openclaw): align paths with upstream openclaw/openclaw conventions - skillsDir: 'skills' -> '.openclaw/skills' (match project-scoped convention) - configFile: 'CLAUDE.md' -> 'AGENTS.md' (upstream uses AGENTS.md/SOUL.md) - globalSkillsDir: '~/.openclaw/skills' -> '~/.openclaw/workspace/skills' - altSkillsDirs: keep 'skills' for back-compat, add workspace variant - isDetected: also check project .openclaw/ and global workspace/ - MCP AGENT_DIR_MAP: .openclaw/skills for runtime discovery - README: reflect real paths Refs NousResearch upstream layout; pairs with #114 Hermes support. * fix(openclaw): address review findings for discovery + detection - skills.ts: add '.openclaw/skills' to SKILL_DISCOVERY_PATHS (alphabetical) - config.ts: getSearchDirs prefers globalSkillsDir for home path (was deriving from skillsDir, producing ~/.openclaw/skills instead of ~/.openclaw/workspace/skills) - agent-config.ts: add altConfigFiles to AgentDirectoryConfig schema, set openclaw altConfigFiles: ['openclaw.json'] - context/sync.ts: detectAgents also iterates altConfigFiles so projects with only openclaw.json still detected - mcp/tools.ts: AGENT_DIR_MAP includes '.openclaw/workspace/skills' for home-rooted global discovery
# Conflicts: # README.md
|
Pushed to your branch (maintainer edit) to keep attribution: Merged main — resolved README conflict. Main restructured the agent table into "Top 11" + "Plus 34 more" collapsed list. Hermes added to the "Plus 34 more" list, count bumped to 35. Review-driven additions:
Validation: #117 already landed the OpenClaw upstream path alignment with the same pattern, so the infra this PR touches is now consistent across both agents. Ready to merge. |
There was a problem hiding this comment.
🟡 Missing export * from './hermes.js' in src/agents/index.ts
The HermesAdapter is imported at line 20 and added to the adapters record at line 59, but export * from './hermes.js' is not added to the re-exports (lines 22-39). Every other adapter in this file has a corresponding export * statement. This means consumers who import from src/agents/index.ts cannot access HermesAdapter directly. The equivalent file packages/agents/src/index.ts:45 correctly includes the re-export.
(Refers to line 39)
Was this helpful? React with 👍 or 👎 to provide feedback.
| hermes: { | ||
| start: '<!-- SKILLS_TABLE_START -->', | ||
| end: '<!-- SKILLS_TABLE_END -->', | ||
| }, |
There was a problem hiding this comment.
🔴 Repeated skillkit sync --agent hermes duplicates </skills_system> closing tag each time
The new hermes entry in updateConfigContent at packages/cli/src/commands/sync.ts:217-220 uses <!-- SKILLS_TABLE_START/END --> markers. When existing content already contains these markers (i.e., any re-sync), the update logic at lines 228-233 concatenates: (1) existing content before start marker, (2) newConfig.slice(newConfig.indexOf(startMarker)) which includes everything after the end marker (\n\n</skills_system>), and (3) existing.slice(endIdx + endMarker.length) which also captures \n\n</skills_system>. This produces a duplicate </skills_system> tag, and the duplication accumulates with every subsequent sync, progressively corrupting the config file.
Trace of the duplication
The HermesAdapter.generateConfig() output (packages/agents/src/hermes.ts:32-58) has content after the end marker:
<!-- SKILLS_TABLE_END -->
</skills_system>
So newConfig.slice(newConfig.indexOf('<!-- SKILLS_TABLE_START -->')) includes </skills_system> at the end, and existing.slice(endIdx + endMarker.length) also yields \n\n</skills_system>. Both get concatenated.
Prompt for agents
The updateConfigContent function in packages/cli/src/commands/sync.ts has a bug where the agent-specific marker replacement path (lines 228-233) duplicates content that appears after the end marker. When newConfig contains content after the end marker (like </skills_system> for hermes and claude-code), both newConfig.slice(newConfig.indexOf(startMarker)) and existing.slice(endIdx + endMarker.length) include that trailing content, causing duplication.
The fix should ensure that when slicing from newConfig, only the content between (and including) the start and end markers is taken, OR the existing content after the end marker is not appended. One approach: find the end marker in newConfig and use newConfig.slice(newConfig.indexOf(startMarker), newConfig.indexOf(endMarker) + endMarker.length) instead of slicing to the end. This would preserve the existing file's structure outside the markers while replacing only the content within them.
This affects both the new hermes entry and the pre-existing claude-code entry which share the same marker pattern.
Was this helpful? React with 👍 or 👎 to provide feedback.
Includes: - v1.24.0 across all publishable packages - Add packages/mcp + packages/api to bump-version.sh (were drifting) - Regenerate assets/tags/v1.24.0.svg, remove stale v1.23.0.svg - Bump agent count references from 45 to 46 (adds Hermes) - Update gen-tags.mjs (version + agent count) - Sync onboarding logo test expectation Release covers rohitg00#114 (Hermes native support) + rohitg00#117 (OpenClaw upstream paths).
2 participants
Summary
Hermes Agent (by Nous Research) is a powerful AI agent optimized for tool-calling and agentic reasoning. Previously, using SkillKit with Hermes required manual configuration or "hacking" the Claude adapter due to the shared XML roots.
This PR adds first-class support for the Hermes agent with correct paths, native XML translation markers, and dual-source parity for legacy environments.
Changes
hermesagent type toAgentTypeenum and all relatedRecordtypes.HermesAdapter(packages/agents/src/hermes.ts) with detection for.hermes/projects and~/.hermes/global environments.agent-config.tsspecifying.hermes/skills/as the primary directory andAGENTS.mdas the instruction file.<!-- SKILLS_TABLE_START -->markers ingenerateConfigto enable idempotent updates without overwriting custom system instructions.AGENT_INSTRUCTION_TEMPLATESwith a specific section order (overview,stack,commands,conventions,structure,guidelines).CommandGeneratorwith support for.hermes/commandsand slash commands.hermes → xmlin the translator format map and updatedtranslateSkillToAllto include Hermes as a target.src/tree (src/agents/hermes.ts) to maintain 100% parity with legacy research environments.AGENTS.mdinstead of.windsurfrules.Build Verification
Test Plan
Related Issues
Fixes #113
Summary by CodeRabbit