feat: add Hermes Agent (NousResearch) integration - #1110
Conversation
Revisits PR getkimchi#454 and fixes the issues that made it fail CI: - preserves the ToolDefinition interface in types.ts (was accidentally deleted) - wires Hermes into setup-wizard and setup-tools loaders - adds kimchi hermes CLI command Mirrors the existing OpenClaw integration pattern: - detect via ~/.hermes dir or hermes on PATH - CLI-first write via hermes config set with merge fallbacks - direct YAML write fallback - env file management for KIMCHI_API_KEY Includes 29 unit tests covering provider block, models catalog, env writer, direct and CLI write paths, and registration.
|
Documentation Changes Added
Actions
If neither actions are selected, on PR close/merge the docs branch in ReadMe will remain open. |
Kimchi Code Review
Summary📊 Review Score: 80/100 (overall code quality — 0 lowest, 100 highest) 🧪 Tests: yes — A thorough new test file ( 📝 Found 6 issue(s). See inline comments for details. What to expectKimchi will analyze the changes in this pull request and post:
The review typically completes within a few minutes. This comment will be updated once the review is ready. Interact with Kimchi
ConfigurationReviews are configured by your organization admin. Powered by Kimchi — AI-powered code review by CAST AI |
![kimchi-review[bot]](https://avatars.githubusercontent.com/in/3223788?s=60&v=4){kind=small}
There was a problem hiding this comment.
📊 Review Score: 80/100 (overall code quality — 0 lowest, 100 highest)
⏱️ Estimated effort to review: 3/5 (1 = trivial, 5 = very complex)
🧪 Tests: yes — A thorough new test file (src/integrations/hermes.test.ts) covers provider block construction, model catalog generation, .env writing/merging, helper utilities (asObject, mergeFallbacks, mergeModelsCatalog), version constants, and both the direct-YAML and CLI-driven integration paths. The command runner itself (src/commands/hermes.ts) is not unit-tested.
📝 Found 6 issue(s). See inline comments for details.
|
|
||
| const providerBlock = buildHermesProviderBlock(models) | ||
| const modelsCatalog = buildHermesModelsCatalog(models) | ||
|
|
There was a problem hiding this comment.
writeHermesViaCLI does not accept or respect the scope parameter from writeHermes, so when the hermes binary is on PATH the configuration is always written to Hermes' default/global location. In contrast, writeHermesDirect uses resolveScopePath(scope, HERMES_CONFIG_PATH). This means kimchi hermes --scope project behaves differently depending on whether hermes is installed, surprising users and making scoped configuration unreliable.
💡 Suggestion: Either pass the scope through to the Hermes CLI if it supports scoped config, or validate/reject non-global scopes early in writeHermes so the CLI path is only used when scope === "global".
| /** Minimum Hermes version we know the config layout works against. Exported for tests and version gating. */ | ||
| export const HERMES_VERSION_MIN = "2026.1.0" | ||
|
|
||
| /** Matches `hermes --version` output: "Hermes 2026.1.2". Exported for tests and version gating. */ |
There was a problem hiding this comment.
HERMES_VERSION_MIN and HERMES_VERSION_REGEX are documented as being for "version gating" but are never consulted at runtime. Writing the new models.providers.kimchi block to an older Hermes version could produce an incompatible or broken configuration without any diagnostic.
💡 Suggestion: Add a runtime check in writeHermesViaCLI (and optionally writeHermesDirect) that runs hermes --version, parses the version with HERMES_VERSION_REGEX, compares it to HERMES_VERSION_MIN, and throws a clear error when the installed version is unsupported.
| const raw = readFileSync(path, "utf-8") | ||
| const trimmed = raw.trim() | ||
| if (trimmed !== "") { | ||
| const parsed = parseYaml(trimmed) |
There was a problem hiding this comment.
When the Hermes gateway is not running, writeHermesViaCLI silently runs hermes onboard --install-daemon .... Installing a system daemon is a significant side effect for a command whose summary is "Configure Hermes to use Kimchi", and may surprise users who only expected a config file update.
💡 Suggestion: Log that the daemon will be installed before invoking onboard, or split daemon installation into an explicit opt-in step (e.g., kimchi hermes --install-daemon) rather than performing it automatically during configuration.
| const mainSlug = resolved.main?.slug ?? models[0].slug | ||
| const fallbacks = ([resolved.coding, resolved.sub] as Array<ModelMetadata | undefined>) | ||
| .filter((m): m is ModelMetadata => m !== undefined) | ||
| .map((m) => `${PROVIDER_NAME}/${m.slug}`) |
There was a problem hiding this comment.
ℹ️
In writeHermesDirect, the try/catch only handles readFileSync failures. If the existing config.yaml is malformed, parseYaml(trimmed) throws an unhandled exception that bubbles up as a raw YAML parse error rather than a clean, actionable Kimchi error message.
💡 Suggestion: Extend the try/catch to wrap parseYaml(trimmed) and throw a descriptive error such as throw new Error(\Existing Hermes config at ${path} is not valid YAML`)`.
| if (result.status !== 0) { | ||
| const detail = (result.stderr || result.stdout || "").trim() | ||
| throw new Error(`hermes ${args.slice(0, 2).join(" ")} failed: ${detail || `exit ${result.status}`}`) | ||
| } |
There was a problem hiding this comment.
ℹ️
runHermesCmd reports exit ${result.status} when spawnSync returns status: null because the child was killed by a signal. The resulting message exit null is unhelpful for diagnosing the real cause.
💡 Suggestion: Check for result.signal when result.status === null and include it in the error: const reason = result.status === null && result.signal ? \signal ${result.signal}` : `exit ${result.status}`; throw new Error(`hermes ${args.slice(0, 2).join(" ")} failed: ${detail || reason}`)`.
| return 1 | ||
| } | ||
| await tool.write(scope, prepped.apiKey, prepped.models) | ||
| console.log("kimchi hermes: configuration written.") |
There was a problem hiding this comment.
ℹ️
runHermes casts the caught value to Error and reads .message. If the integration throws a string, number, or other non-Error value, the cast will succeed at compile time but the runtime access to .message will produce undefined, and console.error will print an unhelpful message.
💡 Suggestion: Use a safe message extraction such as console.error(\kimchi hermes: ${err instanceof Error ? err.message : String(err)}`)`.
…mands - Replace invalid --no-prompt/--no-onboard install args with --skip-setup --non-interactive --skip-browser --no-skills - Remove hermes onboard call that hangs in container/headless installs - Remove hermes gateway restart to avoid blocking setup-tools when daemon management is unavailable
|
2 participants
feat: add Hermes Agent (NousResearch) integration
Re-introduces the Hermes Agent integration attempted in PR #454, with the fixes required to pass CI and work against the real Hermes CLI.
What changed since PR #454
ToolDefinitioninterface — the original PR deleted it and only extendedToolId, which broketscfor every other integration.--no-prompt/--no-onboard. Uses supported flags:--skip-setup --non-interactive --skip-browser --no-skills.model:block (provider: custom,base_url,api_key,default), not undermodels.providers. The integration writes that block plus afallback_providerschain.hermes onboard --install-daemonandhermes gateway restart, both of which hang in container/headless environments.ToolId, imported in the setup wizard andsetup-tools, and exposed askimchi hermes.Files changed
src/integrations/hermes.ts— Hermes integrationsrc/integrations/hermes.test.ts— unit testssrc/integrations/types.ts— added"hermes"toToolIdsrc/setup-wizard/index.ts— imported Hermes integrationsrc/commands/setup-tools.ts— imported Hermes integrationsrc/commands/hermes.ts— newkimchi hermessubcommandsrc/commands/registry.ts— registeredkimchi hermesVerification
pnpm run check— lint + typecheck cleanpnpm vitest run src/integrations/hermes.test.ts— 33/33 passingnode scripts/build-binary.js --target linux-arm64— builds Linux ARM64 binary