feat(backend): add Hermes ACP agent support (#1090)

[jcfs]

Adds Nous Research Hermes as a selectable ACP agent so users can pick it from the agents list — fulfils #1090. Hermes ships an ACP stdio adapter (hermes acp), so it drops into the existing native-binary scaffold alongside Kimi/Cursor/Kiro/Trae/Omp without needing a new protocol path.

Important Changes

• InstallScript() returns a docs URL instead of curl | bash. Hermes uses a multi-step Python/uv installer that may need system packages and a follow-up hermes setup — not a clean one-shot like npm install -g, so a runnable install button would be misleading. Matches the Kimi/Cursor pattern for native-binary agents that aren't on npm.
• Added \"hermes\" to allowedProbeCommands in acp_executor.go — required so the host-utility probe's CodeQL-safe exec.Command allowlist accepts the new binary.

Validation

• make fmt
• go build ./...
• make test (all packages green, including TestNewACPAgents_* and TestProbeAllowlist_CoversEveryEnabledInferenceAgent)
• make lint (0 issues)

Checklist

• Tests added/updated where relevant
• Docs updated where relevant
• Changes are user-visible enough to mention in release notes

Closes #1090

Preview Environment

URL	https://kandev-pr-1117-bwo7.sprites.app
Commit	133b0d8
Agent	Mock agent

Updates automatically on each push. Destroyed when the PR is closed.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a Hermes ACP passthrough agent that integrates the Hermes CLI via ACP (stdin/stdout), a spec entry validating commands, registry registration so Hermes is available by default, and permits the hermes binary in the ACP executor allow-list.

Changes

Hermes ACP Agent Support

Layer / File(s)	Summary
HermesACP implementation and interface wiring apps/backend/internal/agent/agents/hermes_acp.go	Defines HermesACP with embedded logos and constants; implements NewHermesACP, identity/display methods, IsInstalled (detection, MCP and session-resume flags), BuildCommand (hermes acp), Runtime (ACP runtime config with session resume and recoverability), RemoteAuth, InstallScript (links upstream docs), PermissionSettings, InferenceConfig, and BillingType.
Spec, registry, and executor allow-list apps/backend/internal/agent/agents/new_acp_agents_test.go, apps/backend/internal/agent/registry/registry.go, apps/backend/internal/agentctl/server/utility/acp_executor.go	Adds a NewHermesACP() spec entry validating detection binary hermes, expected argv for BuildCommand/Runtime/Inference, registers NewHermesACP() in LoadDefaults so Hermes is available by default, and adds "hermes" to allowedProbeCommands.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• kdlbs/kandev#807: Introduced ACP agent framework, registry wiring, and contract test scaffold that this change extends.
• kdlbs/kandev#971: Extends ACP native-binary agent specs and updates acp_executor.go allow-list entries for probe commands.

Poem

🐰 Hermes hops with CLI in paw,

ACP hums, stdout in a draw,
Sessions resume when errors pry,
Hermes helps the tasks zip by,
A rabbit cheers — new agent, hi!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title 'feat(backend): add Hermes ACP agent support' clearly and concisely describes the main change: adding support for the Hermes ACP agent to the backend.
Linked Issues check	✅ Passed	The PR successfully meets all coding requirements from #1090: Hermes is added as a new agent definition, made selectable/installable, integrated into the native-binary scaffold, and included in the probe allowlist for parity with other agents.
Out of Scope Changes check	✅ Passed	All code changes are directly scoped to the requirements in #1090: adding the Hermes agent implementation, integration into the registry, test coverage, and security allowlist updates.
Description check	✅ Passed	PR description is well-structured, comprehensive, and follows the template with all required sections completed.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/implement-feature-he-y12

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[claude]

Claude finished @jcfs's task in 3m 2s —— View job

---

Code Review

• Gather context and read skill
• Read changed files
• Analyze for issues
• Post findings

---

Findings

Suggestion (recommended, doesn't block)

1. Inconsistent installer description in doc comment — apps/backend/internal/agent/agents/hermes_acp.go:29
   • Issue: The struct-level doc comment calls it an "upstream curl|bash script" while the inline comment inside InstallScript() (line 103) calls it a "multi-step Python/uv installer". These describe different things — one is a delivery mechanism, the other the actual toolchain.
   • Fix: Align the struct doc comment with the more accurate description from the inline comment.

Summary

Severity	Count
Blocker	0
Suggestion	1

Verdict: Ready to merge

The implementation is clean and follows the established native-binary ACP agent scaffold exactly. All six wiring points are correct: the agent file, SVG logos, registry entry, executor allowlist, and test coverage. The TestProbeAllowlist_CoversEveryEnabledInferenceAgent and TestNewACPAgents_* suite thoroughly guards the contracts that matter most (binary detection, command surface consistency, logo non-emptiness, display order uniqueness). No logic, security, or correctness issues found.

[greptile-apps]

Greptile Summary

This PR adds Nous Research Hermes as a new selectable ACP agent, dropping into the existing native-binary scaffold alongside Kimi, Cursor, Kiro, Trae, and Omp. The implementation is clean and addresses the previously flagged binary name collision with Meta's Hermes JS engine by using WithCommandOutput with a version-banner regex rather than a bare LookPath.

• hermes_acp.go: Follows the established scaffold exactly; IsInstalled uses WithCommandOutput("(?i)hermes\s+agent", "hermes", "--version") to distinguish Nous Hermes from any stray hermes-engine npm binary, InstallScript returns a docs URL (consistent with Kimi's docs-pointer pattern), and SessionDirTemplate is {home}/.hermes.
• acp_executor.go: "hermes" is correctly added to allowedProbeCommands, satisfying the TestProbeAllowlist_CoversEveryEnabledInferenceAgent guard.
• Tests: Hermes entry in newACPAgentSpecs pins all command surfaces (BuildCommand, Runtime.Cmd, InferenceConfig.Command, PassthroughCmd) and participates in the full suite including detection, logo non-emptiness, and display-order uniqueness checks.

Confidence Score: 5/5

Safe to merge — additive, self-contained new agent with no changes to existing code paths.

All five touch points of the native-binary scaffold are correctly filled, the binary collision fix with WithCommandOutput is in place, and the comprehensive shared test suite validates every contract the new agent must honor.

No files require special attention.

Important Files Changed

Filename	Overview
apps/backend/internal/agent/agents/hermes_acp.go	New ACP agent for Nous Research Hermes; correctly uses WithCommandOutput regex to disambiguate from Meta's Hermes JS engine, follows scaffold pattern of peer agents.
apps/backend/internal/agent/agents/new_acp_agents_test.go	Adds Hermes to the shared spec table covering ID, display, all command surfaces, install script, logo non-emptiness, and display-order uniqueness.
apps/backend/internal/agent/registry/registry.go	Registers NewHermesACP() in LoadDefaults(); placement is consistent with the existing ordered list.
apps/backend/internal/agentctl/server/utility/acp_executor.go	Adds 'hermes' to allowedProbeCommands so CodeQL-safe exec.Command allowlist accepts the new binary for probe/inference.
apps/backend/internal/agent/agents/logos/hermes_acp_light.svg	SVG logo for light-mode display (dark card, light stroke); correctly paired with _dark.svg variant.
apps/backend/internal/agent/agents/logos/hermes_acp_dark.svg	SVG logo for dark-mode display (light card, dark stroke); correctly paired with _light.svg variant.

_{Reviews (3): Last reviewed commit: "fix(backend): tighten Hermes ACP detecti..." | Re-trigger Greptile}

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@apps/backend/internal/agent/agents/hermes_acp.go`:
- Around line 28-29: Update the top-of-file installer comment that currently
states "hermes is installed via the upstream curl|bash script which puts the
`hermes` Python entry point on PATH" to reflect the current install flow
described later in the file (the multi-step Python/uv setup and docs-based
guidance around the `hermes` entry point). Replace the stale curl|bash
instruction with a brief, accurate summary of the documented installation steps
and point readers to the project docs/README for the full multi-step
Python/virtualenv installation instructions; ensure the comment mentions the
`hermes` Python entry point and the docs link so future maintainers follow the
current flow.

🪄 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: eb4116ce-5577-4c12-9433-bbb9266dd3df

📥 Commits

Reviewing files that changed from the base of the PR and between 3b1b347 and 8447e29.

⛔ Files ignored due to path filters (2)

• apps/backend/internal/agent/agents/logos/hermes_acp_dark.svg is excluded by !**/*.svg
• apps/backend/internal/agent/agents/logos/hermes_acp_light.svg is excluded by !**/*.svg

📒 Files selected for processing (4)

• apps/backend/internal/agent/agents/hermes_acp.go
• apps/backend/internal/agent/agents/new_acp_agents_test.go
• apps/backend/internal/agent/registry/registry.go
• apps/backend/internal/agentctl/server/utility/acp_executor.go

[cubic-dev-ai]

1 issue found across 6 files

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[claude]

Claude finished @jcfs's task in 2m 55s —— View job

---

Code Review

• Read skill and gather context
• Read changed files (hermes_acp.go, test, registry, executor)
• Verify binary-collision fix (WithCommandOutput regex)
• Verify doc comment alignment fix
• Check for any new issues introduced by the fix commit
• Post findings

---

Findings

All prior review comments confirmed addressed in b9935e7:

• Stale struct comment (curl|bash script → multi-step Python/uv flow) — fixed.
• Binary collision with Meta's Hermes JS engine — IsInstalled now uses WithCommandOutput((?i)hermes\s+agent, "hermes", "--version") instead of a bare WithCommand. Meta's engine emits Hermes JavaScript Engine v… / Hermes release … in --version, which doesn't match hermes\s+agent, so it falls through to Available=false correctly.

One observation (non-blocking):

WithCommandOutput uses exec.Cmd.Output(), which captures stdout only. If a future Hermes release shifts its --version banner to stderr, detection would silently return false. This is a framework-level concern shared by every WithCommandOutput caller in the codebase — not introduced by this PR — but worth keeping in mind if Hermes detection starts flaking in the future.

Test coverage note (non-blocking):

TestNewACPAgents_DetectionRequiresGlobalBinary skips when exec.LookPath("hermes") finds any binary. With the now-regex-based detection, even if Meta's hermes-engine binary is on PATH, the test would skip rather than verify the regex correctly rejects it. The PR's manual verification ("Meta's engine prints Hermes JavaScript Engine / Hermes release, so it falls through correctly") is sound reasoning, but the automated test doesn't cover that specific scenario. Not a blocker — adding it would require a mock CLI or a dedicated test table.

Summary

Severity	Count
Blocker	0
Observation	2 (non-blocking, pre-existing framework patterns)

Verdict: Ready to merge. The scaffold is correct and consistent with peers. Both previously flagged issues are cleanly addressed. No new logic, security, or correctness problems introduced.

[coderabbitai]

Actionable comments posted: 0

[claude]

Claude finished @jcfs's task in 2m 15s —— View job

---

Code Review

• Gather context and understand request
• Read changed files
• Read skill guidance
• Analyze for issues
• Post findings

---

Findings

No new blockers or actionable issues found.

Both previously flagged items from the first review cycle are confirmed fixed:

• Binary collision (hermes-engine): IsInstalled now uses WithCommandOutput((?i)hermes\s+agent, "hermes", "--version") — Meta's engine emits Hermes JavaScript Engine / Hermes release, neither of which matches the pattern. Correct.
• Stale struct comment: Updated to describe the Python/uv multi-step install flow and docs-URL approach. Consistent with InstallScript().

All six wiring points are correct and consistent: BuildCommand, Runtime.Cmd, InferenceConfig.Command, PassthroughCmd, allowedProbeCommands entry, and registry LoadDefaults. DisplayOrder 19 is unique. SVG logos are non-empty (427 bytes each). TestProbeAllowlist_CoversEveryEnabledInferenceAgent and the newACPAgentSpecs table collectively guard the contracts that matter most.

Two pre-existing, non-blocking observations (flagged in prior round, no change needed):

1. WithCommandOutput captures stdout only (cmd.Output()). If a future Hermes release moves its version banner to stderr, detection silently returns Available=false. Framework-level limitation shared by all WithCommandOutput callers — not introduced here.

2. TestNewACPAgents_DetectionRequiresGlobalBinary skips when LookPath("hermes") finds any binary, including Meta's. The regex discrimination against the Meta engine is sound but not covered by the automated test — a mock-CLI test would be required to close this gap.

Summary

Severity	Count
Blocker	0
Suggestion	0

Verdict: Ready to merge.