Skip to content

feat: add Hermes Agent install target - #778

Open
sykuang wants to merge 3 commits into
tirth8205:mainfrom
sykuang:feat/hermes-install-config
Open

feat: add Hermes Agent install target#778
sykuang wants to merge 3 commits into
tirth8205:mainfrom
sykuang:feat/hermes-install-config

Conversation

@sykuang

@sykuang sykuang commented Jul 28, 2026

Copy link
Copy Markdown

Closes #588.

Adds --platform hermes for Hermes Agent, the first YAML-configured platform.

Why this needed new install machinery

Hermes reads <HERMES_HOME>/config.yaml (default ~/.hermes/config.yaml) under a top-level mcp_servers key. The installer previously handled object, array, and toml — this adds yaml alongside them, driven by the same PLATFORMS table so install and uninstall stay in sync automatically.

That config is hand-edited and full of comments, ordering, and settings CRG knows nothing about. Neither install nor uninstall round-trips it through a YAML dumper, which would reformat the entire file. Both edit it as text; the parsed document is used only to decide whether an edit is needed and to validate the result before writing. Uninstall additionally diffs the reparsed document against the expected one and refuses to write if any unrelated setting moved.

When the file cannot be edited safely (unparseable YAML, mcp_servers in flow style, mcp_servers not a mapping), the installer refuses and says why rather than guessing. Refusal is distinct from "already configured" internally, so the CLI never reports a platform as configured when it declined to touch it.

No --repo in the entry — deliberate

The first draft modelled the Hermes entry on OpenCode and appended --repo <root> to args. That analogy is wrong, and testing on a real install showed why.

An IDE window is scoped to one project, so a baked-in repo matches what the user is looking at. Hermes Agent is a long-lived assistant that moves between projects, and its stdio schema has no cwd, so the pinned repo is whichever one happened to run install.

The result was a silent wrong answer. Asked about the codebase in one project, the agent called get_minimal_context_tool without repo_root — the tool docs say it is auto-detected — and --repo resolved the call against a completely different repo. Stats, risk score, and review priorities all came back for the wrong project, with nothing to indicate it.

All 30 tools take an explicit repo_root, so dropping the default costs no capability. Without it the same call fails with repo_root does not look like a project root, which the agent sees and corrects by passing the path. A loud failure beats a confident wrong answer.

Skills, instructions, hooks

Skills install to <HERMES_HOME>/skills/code-review-graph/<name>/SKILL.md, reusing generate_skills since Hermes uses the same frontmatter as Claude Code. Hermes also reads project AGENTS.md, so it joins that file's owner set.

Hooks are not wired up. Hermes' hook model differs from the settings.json shape used by Claude and Qoder, and MCP plus skills already cover proactive graph use. Happy to follow up separately if you want it.

Test isolation fix (please read)

test_uninstall_removes_mcp_entry_for_every_current_platform_spec walks every PLATFORMS entry and calls its config_path. For every other platform that lands under a patched Path.home(). Hermes resolves an absolute path from HERMES_HOME, which ignores Path.home() patching — so on a machine where a contributor actually uses Hermes, the suite would have overwritten their real config.yaml. It did exactly that to mine while developing this.

tests/conftest.py now pins HERMES_HOME to a temp directory for the whole suite. Pinning rather than clearing is deliberate: cleared, the fallback still reaches the real home in any test that forgets to patch it. TestSuiteSafety asserts the pin holds so nobody can quietly remove it.

Verification

Verified end to end against a real Hermes Agent install, not just fixtures:

  • hermes mcp test code-review-graphConnected, 30 tools discovered
  • get_minimal_context_tool invoked through Hermes, returned real graph data
  • install → uninstall restored a 763-line config byte for byte
  • the wrong-repo scenario above reproduced before the fix and confirmed to fail loudly after

Full suite: 2324 passed, 5 skipped, 2 xpassed. ruff clean on changed files.

Round-tripping caught two real bugs that unit tests alone missed: removal ate the blank line separating the block from the next section, and _scope_for_config rejected a relocated HERMES_HOME as outside the home/repo boundary — meaning install honoured the variable while uninstall refused to clean up the file it had written. Both fixed, both covered.

Commits

Kept as two commits so the reasoning is visible: the feature, then the --repo correction with its rationale. Squash if you prefer.

sykuang added 2 commits July 28, 2026 21:25
Adds `--platform hermes`, the first YAML-configured platform. Hermes Agent
reads `<HERMES_HOME>/config.yaml` (default `~/.hermes/config.yaml`) under
`mcp_servers`, so the installer gains a `yaml` format alongside `object`,
`array`, and `toml`.

That config is hand-edited and full of comments, ordering, and settings CRG
knows nothing about, so both install and uninstall edit it as text rather
than round-tripping it through a YAML dumper, which would reformat the whole
file. The parsed document is used only to decide whether an edit is needed
and to validate the result before writing; uninstall additionally diffs the
reparsed document against the expected one and refuses to write if any
unrelated setting moved.

Hermes' `mcp_servers` schema has no `cwd`, so the repo root is passed to the
server as `--repo <root>` in `args`, as is already done for OpenCode.

Skills are installed to `<HERMES_HOME>/skills/code-review-graph/<name>/SKILL.md`,
reusing `generate_skills` since Hermes uses the same frontmatter as Claude
Code. Hooks are not wired up: Hermes' hook model differs from the
settings.json shape used by Claude and Qoder, and MCP plus skills already
cover proactive graph use.

`_scope_for_config` now recognises a relocated `HERMES_HOME` as its own
user-scope boundary. Without this, install honoured the variable while
uninstall refused the resulting path as outside home/repo, writing a file it
could not clean up.

The `PLATFORMS`-driven uninstall test reaches whatever path each spec
resolves to. For every other platform that lands under a patched
`Path.home()`, but Hermes resolves an absolute path from the environment, so
conftest pins `HERMES_HOME` to a temp directory for the whole suite. Pinning
rather than clearing is deliberate: cleared, any test that forgets to patch
the home would silently rewrite the developer's real config.

Verified end to end against a real Hermes Agent install: `hermes mcp test`
connects and discovers 30 tools, `get_minimal_context_tool` returns graph
data through Hermes, and install followed by uninstall restores a 763-line
config byte for byte.
The Hermes entry was modelled on OpenCode and appended `--repo <root>` to
args. That analogy is wrong. An IDE window is scoped to one project, so a
baked-in repo matches what the user is looking at. Hermes Agent is a
long-lived assistant that moves between projects, and its stdio schema has
no `cwd`, so the pinned repo is whichever one happened to run `install`.

The result was a silent wrong answer rather than an error. Asked about the
codebase in one project, the agent called `get_minimal_context_tool` without
`repo_root` — the tool docs say it is auto-detected — and `--repo` resolved
the call against a completely different repo. Stats, risk score, and review
priorities all came back for the wrong project, with nothing to indicate it.

Every one of the 30 tools takes an explicit `repo_root`, so dropping the
default costs no capability. Without it the same call fails with "repo_root
does not look like a project root", which the agent can see and correct by
passing the path. A loud failure beats a confident wrong answer.

Verified against a real Hermes Agent install: from an unrelated project, the
same question that previously reported another repo's graph now surfaces the
error and the agent asks which repository to use.
@tirth8205

Copy link
Copy Markdown
Owner

Verified this end to end: Hermes Agent is real (NousResearch/hermes-agent), the generated config matches its documented ~/.hermes/config.yaml mcp_servers schema, _hermes_home() matches upstream hermes_constants including the Windows branch, and the full suite passes locally (2324 passed, 5 skipped, 2 xpassed). Ruff is clean. The YAML preservation tests genuinely cover the behavior (a deliberate local mutation of the blank-line handling made two round-trip tests fail). One blocker before merge: mypy. Main is clean, but the two new bare import yaml statements (skills.py:526, uninstall.py:542) produce Library stubs not installed for "yaml" [import-untyped] under the CI invocation, since CI does not install types-PyYAML. parser.py and eval/runner.py already annotate their yaml imports with # type: ignore[import-untyped] for this reason. Please add the same comment to both new imports (or add types-PyYAML to the CI type-check step), and this is good to merge.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@sykuang

sykuang commented Aug 2, 2026

Copy link
Copy Markdown
Author

Addressed in 6e45468: both new import yaml statements now use # type: ignore[import-untyped], matching the existing PyYAML import pattern in the codebase. Thanks for catching this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: code-review-graph for Hermes install config

2 participants