Skip to content

fix(hermes): register the Omnigent MCP server for the headless harness - #2216

Merged
dhruv0811 merged 1 commit into
omnigent-ai:mainfrom
dosenr:contrib/hermes-headless-mcp-upstream
Jul 13, 2026
Merged

fix(hermes): register the Omnigent MCP server for the headless harness#2216
dhruv0811 merged 1 commit into
omnigent-ai:mainfrom
dosenr:contrib/hermes-headless-mcp-upstream

Conversation

@dosenr

@dosenr dosenr commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Related issue

Closes #1926

Summary

The headless hermes harness writes a per-session HERMES_HOME with only the policy-hook
config, so a headless Hermes agent has zero Omnigent builtin tools (sys_*, web_*,
load_skill). The native twin hermes-native already registers an mcp_servers.omnigent
entry via hermes_native_bridge.write_policy_hook_config; the headless side never did.

_setup_hermes_home now writes an mcp_servers.omnigent (serve-mcp --bridge-dir <dir>)
entry into config.yaml and starts the runner-hosted tool relay for hermes turns
(runner/app.py, alongside the existing native branches) so serve-mcp can dispatch the
builtin tools via tool_relay.json. The executor-local _populate_hermes_home duplicate is
removed in favor of the shared write_policy_hook_config.

write_policy_hook_config gains an optional hermes_home parameter. The headless path passes
a private tempfile.mkdtemp() (0700) so the copied .env / auth.json credentials stay off
the predictable bridge path; only the runner<->serve-mcp coordination files (bridge.json,
tool_relay.json) live in the deterministic bridge dir. hermes-native keeps its existing
default (bridge_dir/hermes_home), so that path is unchanged.

No new auth or server infrastructure: serve-mcp, the relay, and the bridge-dir layout all
exist for hermes-native already.

Test plan

tests/inner/test_hermes_executor.py::TestSetupHermesHome builds the headless executor with a
fake session and asserts the generated config.yaml registers mcp_servers.omnigent pointed
at the bridge dir, that the credential-bearing HERMES_HOME is a private tempdir (not under
the bridge dir), and that bridge.json lands in the bridge dir. The mcp_servers assertion
fails before the change (KeyError — no mcp_servers key) and passes after. Full
tests/inner/test_hermes_executor.py + tests/test_hermes_native_bridge.py +
tests/runner/test_comment_relay.py green; scripts/backend-smoke.sh green.

Demo

Live web UI run of the headless hermes harness with the Omnigent MCP server
registered. The agent calls sys_session_get_info (an Omnigent-only tool -
before this change the headless harness had no Omnigent tools at all); a demo
ASK policy gates it, one approval card parks, Approve runs it:

approval card

approved, tool ran

(The single approval card also depends on #2220 - without the hook skip the
same call would be policy-evaluated twice and park two cards. Captured with
both PRs applied.)

Terminal transcript (tool list, original demo)

A headless hermes turn (no UI — the demo is the transcript). Before this change the
agent had no Omnigent tools; here it lists them and calls one:

Omnigent MCP tool names available to me:
- mcp_omnigent_list_comments
- mcp_omnigent_sys_add_policy
- mcp_omnigent_sys_agent_download
- mcp_omnigent_sys_agent_get
- mcp_omnigent_sys_agent_list
- mcp_omnigent_sys_call_async
- mcp_omnigent_sys_cancel_async
- mcp_omnigent_sys_cancel_task
- mcp_omnigent_sys_os_edit
- mcp_omnigent_sys_os_read
- mcp_omnigent_sys_os_shell
- mcp_omnigent_sys_os_write
- mcp_omnigent_sys_policy_registry
- mcp_omnigent_sys_read_inbox
- mcp_omnigent_sys_session_get_history
- mcp_omnigent_sys_session_get_info
- mcp_omnigent_sys_session_list
- mcp_omnigent_update_comment
I called the session-status tool:
- mcp_omnigent_sys_session_get_info
It returned:
{
  "session_id": "conv_ID",
  "status": "running",
  "title": "List the tool names available to you from the omnigent MCP…",
  "agent_id": "ag_ID",
  "agent_name": "hermes-headless-test",
  "runner_id": "runner_ID",
  "runner_online": true,
  "host_id": "host_ID",
  "parent_session_id": null,
  "sub_agent_name": null,
  "reasoning_effort": null,
  "model": null,
  "workspace": "<workspace>
  "git_branch": null,
  "pending_elicitations": [],
  "pending_elicitation_count": 0
}

Type of change

  • Bug fix

Test coverage

  • Unit test added

Coverage notes

Live integration (a headless hermes turn actually invoking an Omnigent tool) needs a running
runner, the hermes CLI, and provider credentials; verified manually against a live deployment.
The added unit tests cover the config-generation and bridge-dir wiring that was the defect.

Changelog

Fixed the headless Hermes harness registering no Omnigent MCP server (agent had no builtin tools).

@github-actions
github-actions Bot requested a review from dhruv0811 July 8, 2026 17:57
@github-actions github-actions Bot added the size/L Pull request size: L label Jul 8, 2026
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

@dosenr This PR is a Bug fix, Feature, or UI / frontend change but the Demo section is missing or only contains a placeholder.

These change types require a screenshot or screen recording so reviewers can see the new behaviour without checking out the branch. Please update the Demo section with:

  • A screenshot or screen recording of the change, or
  • A link to a hosted video or GIF showing the new behaviour.

Use N/A only when the change has no user-visible effect whatsoever (e.g. a pure refactor or test-only change). If that's the case, uncheck the relevant type box and check Refactor / chore or Test / CI instead.

@github-actions github-actions Bot added the needs-demo PR needs a demo screenshot or recording label Jul 8, 2026
The headless hermes harness populated a private tempdir HERMES_HOME with only
the policy hook config, so a headless Hermes agent had zero Omnigent builtin
tools (sys_*, web_*, load_skill). The native twin already writes an
mcp_servers.omnigent entry via write_policy_hook_config.

Point the executor's HERMES_HOME at the session's deterministic bridge dir and
reuse write_policy_hook_config, which writes the hook config, bridge.json, and
the mcp_servers.omnigent (serve-mcp) entry together. Start the runner-hosted
tool relay for hermes turns alongside the existing native branches so
tool_relay.json lands in the same dir and serve-mcp can dispatch the builtin
tools. The executor-local _populate_hermes_home duplicate becomes dead and is
removed.

Signed-off-by: rdosen <robert.dosen@gmail.com>
@dosenr

dosenr commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Headless/server-side change with no UI surface; the terminal transcript in the Demo section is the demonstration.

@dosenr

dosenr commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Coordination note (same author): #2211 layers hermes-acp on the same per-session home plumbing this PR refactors - it currently imports _populate_hermes_home from hermes_executor.py, which this PR moves into hermes_native_bridge. Whichever lands second I will rebase promptly. One specific for that rebase: #2211's ACP harness must NOT get the mcp_servers.omnigent config entry this PR writes - it already receives Omnigent tools via #2152's session/new.mcpServers bridge, and both at once would register the tool set twice.

(#2220, now merged - thanks! - is what keeps ASK policies from double-carding once a hermes home carries relay tools.)

@dhruv0811

Copy link
Copy Markdown
Member

/review

@omnigent-ci

omnigent-ci Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Polly AI Review

Blocking issues

None. The refactor is sound and the coupling holds up:

  • import json removal is safe. The only json. references in hermes_executor.py were inside the deleted _populate_hermes_home (config/allowlist writes at old lines 267/289). No remaining code in the file uses json, so removing the import introduces no NameError.
  • Bridge-dir rendezvous is consistent. The headless executor keys its bridge dir on bridge_dir_for_session_id(_get_conversation_id()) and the runner relay branch keys on bridge_dir_for_session_id(conv) — both derive from the same --conversation-id, so bridge.json (executor), tool_relay.json (runner relay), and serve-mcp --bridge-dir <dir> (spawned by Hermes) all meet in the same directory.
  • Ordering is correct. The relay is started in _run_turn_bg_setup_and_stream before _stream_message_to_harness, so tool_relay.json is on disk before Hermes lazily boots serve-mcp. await_notify=False is appropriate here (lazy MCP client, same rationale as the codex/antigravity branches).
  • No credential regression. The headless HERMES_HOME was already a private mkdtemp before this PR; passing hermes_home=self._hermes_home into the shared helper preserves that, keeping .env/auth.json/the token-bearing wrapper off the predictable bridge path while only bridge.json/tool_relay.json live there. _delete_native_bridge_dirs already includes hermes_bridge_dir(session_id), and close() still rmtrees the private home.

Security vulnerabilities

None introduced. This change improves the isolation posture (credentials moved from the deterministic bridge dir to an owner-only tempdir). The serve-mcp invocation is built as an argv list (sys.executable -m omnigent.claude_native_bridge serve-mcp --bridge-dir <path>) with no shell, so no injection surface. bridge.json is written 0600 in a 0700 dir. test_home_is_owner_only / test_credentials_stay_off_the_predictable_bridge_path lock these in.

Non-blocking notes

  • Stale docstring reference (not in diff). omnigent/inner/hermes_policy_hook.py:4 still points at :func:_populate_hermes_home``, which this PR deletes. Worth redirecting to hermes_native_bridge.write_policy_hook_config in a follow-up so the cross-reference doesn't dangle. (The docstring inside `write_policy_hook_config` that previously said "Mirrors `_populate_hermes_home`" is fixed by this diff.)
  • Best-effort tempdir cleanup. The credential-bearing mkdtemp is only reclaimed via close(); a hard-killed executor process leaks it under TMPDIR. This matches pre-PR behavior (it was already a mkdtemp), so no regression — just noting the accumulation path isn't covered by the session-delete bridge cleanup.
  • Relay bind on the hermes bridge root. The relay's _ensure_secure_dir trusted-parent check must accept /tmp/omnigent-<uid>/hermes-native/.... This path is already exercised by the hermes-native comment relay, and relay-start failures are logged/swallowed rather than breaking the turn, so risk is low — but it's the one path that only surfaces at runtime rather than in the unit tests.

Summary

A clean, well-scoped parity fix: it de-dupes the headless HERMES_HOME setup onto the shared write_policy_hook_config, registers the Omnigent MCP server so headless Hermes finally gets the sys_*/web_*/load_skill surface, and starts the runner relay in the same bridge dir so serve-mcp can dispatch those tools — while tightening credential isolation via a private tempdir. The bridge-dir coupling, startup ordering, and import removal all check out, and the tests assert the previously-missing mcp_servers.omnigent key plus the credential-isolation invariants. No blocking or security issues; only a couple of stale doc references and lifecycle caveats worth a follow-up.


Automated review by Polly · workflow run

@dhruv0811 dhruv0811 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!! Thanks for this :)

@dhruv0811
dhruv0811 enabled auto-merge (squash) July 13, 2026 23:33
@dhruv0811
dhruv0811 merged commit e5102de into omnigent-ai:main Jul 13, 2026
48 of 49 checks passed
@github-actions github-actions Bot added the no-doc-update Merged PR does not need a docs update label Jul 13, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🏷️ Doc impact: no-doc-update

Internal refactor of the headless Hermes executor's per-session HERMES_HOME/bridge-dir plumbing (moving credentials to a private tempdir and sharing serve-mcp config); no user-facing setup, CLI flag, integration, or documented default changed.

Auto-classified on merge. Set the label manually before merging to override. · run

@omnigent-ci

omnigent-ci Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Polly AI Review

Blocking issues

None. The core claims verify against the source:

  • write_policy_hook_config fully retains the removed _populate_hermes_home behavior — it merges the same user ~/.hermes/config.yaml keys (model, providers, fallback_providers, credential_pool_strategies), copies .env and auth.json, writes the hook wrapper + allowlist + pre_tool_call hook, and now additionally registers mcp_servers.omnigent. So headless Hermes can still resolve a model and authenticate — no auth regression.
  • The headless executor and the runner both key off the same deterministic bridge dir via hermes_native_bridge.bridge_dir_for_session_id(conv_id), so the executor-written bridge.json/mcp_servers and the runner-started relay's tool_relay.json land in the same rendezvous. Tool dispatch is wired correctly.
  • The generated MCP command is python -m omnigent.claude_native_bridge serve-mcp --bridge-dir <bridge_dir>, matching both the test assertion and the real serve-mcp entrypoint.
  • Cleanup separation is coherent: close() removes only the private temp HERMES_HOME; the shared bridge dir (holding bridge.json/tool_relay.json) is reclaimed by session-delete (_delete_native_bridge_dirs), so there's no double-free and nothing serve-mcp still needs is prematurely removed.

Security vulnerabilities

None. The credential-bearing HERMES_HOME is a private tempfile.mkdtemp() (0700, mkdtemp default) that keeps the copied .env, auth.json, and the token-bearing hook wrapper off the predictable bridge path; only non-secret relay/MCP coordination files live in the deterministic bridge dir. This is a net improvement over placing credentials under the predictable bridge_dir/hermes_home path a co-located local user could pre-create. The hermes-native default path is unchanged.

Non-blocking notes

  • write_policy_hook_config's docstring/reference still points at _populate_hermes_home (the removed function) in one spot (hermes_native_bridge.py ~line 318); minor stale cross-reference, not functional.

Summary

Clean, well-scoped parity fix. It consolidates the two divergent HERMES_HOME setups onto the shared write_policy_hook_config, closing the gap where headless Hermes had zero Omnigent builtin tools, while preserving user auth config and actually tightening the credential-placement story via a private 0700 home. The relay is started against the same bridge dir the executor writes to, and the MCP command targets the existing shared serve-mcp. Test coverage asserts the new mcp_servers.omnigent registration (which fails pre-change) and the credential/bridge-dir separation. No blocking correctness or security issues found; the change is backend plumbing and the PR includes the relevant demo screenshots. Ready to merge.


Automated review by Polly · workflow run

yours-aditya pushed a commit to yours-aditya/omnigent that referenced this pull request Jul 16, 2026
omnigent-ai#2216)

The headless hermes harness populated a private tempdir HERMES_HOME with only
the policy hook config, so a headless Hermes agent had zero Omnigent builtin
tools (sys_*, web_*, load_skill). The native twin already writes an
mcp_servers.omnigent entry via write_policy_hook_config.

Point the executor's HERMES_HOME at the session's deterministic bridge dir and
reuse write_policy_hook_config, which writes the hook config, bridge.json, and
the mcp_servers.omnigent (serve-mcp) entry together. Start the runner-hosted
tool relay for hermes turns alongside the existing native branches so
tool_relay.json lands in the same dir and serve-mcp can dispatch the builtin
tools. The executor-local _populate_hermes_home duplicate becomes dead and is
removed.

Signed-off-by: rdosen <robert.dosen@gmail.com>
Signed-off-by: Aditya Devarapalli <adityareddyd2@gmail.com>
@PattaraS PattaraS mentioned this pull request Jul 17, 2026
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-demo PR needs a demo screenshot or recording no-doc-update Merged PR does not need a docs update size/L Pull request size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hermes: headless harness registers no Omnigent MCP server (agent has zero sys_*/web_*/load_skill tools)

2 participants