Skip to content

fix(codex): preserve dynamic Hermes MCP context - #66170

Closed
WaterR47 wants to merge 1 commit into
NousResearch:mainfrom
WaterR47:fix/codex-mcp-dynamic-context
Closed

fix(codex): preserve dynamic Hermes MCP context#66170
WaterR47 wants to merge 1 commit into
NousResearch:mainfrom
WaterR47:fix/codex-mcp-dynamic-context

Conversation

@WaterR47

Copy link
Copy Markdown

What does this PR do?

Preserves Hermes' dynamic Kanban/runtime context when hermes codex creates or repairs the managed hermes-tools MCP entry.

The migration previously generated the managed MCP command and arguments but omitted the environment variables used to identify the active task, worker, profile, project, and launch context. Codex workers could therefore connect to hermes-tools while losing the runtime context that makes native Kanban operations act on the correct task.

This change makes those variables product-generated configuration. It preserves existing user-provided env_vars, refreshes Hermes-managed dynamic values from the current process environment, and removes stale managed values when they are no longer present.

Related Issue

None. Reproduced on v2026.7.7.2 and current main; no matching issue or PR was found.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • Tests (adding or improving test coverage)

Changes Made

  • Add the managed dynamic variable allowlist and merge behavior in hermes_cli/codex_runtime_plugin_migration.py.
  • Add migration regressions covering generation, refresh, preservation of user values, removal of stale managed values, and idempotence.

How to Test

  1. Create the isolated test environment expected by scripts/run_tests.sh.
  2. Run bash scripts/run_tests.sh tests/hermes_cli/test_codex_runtime_plugin_migration.py.
  3. Confirm all 67 tests pass.

Also exercised on Ubuntu under WSL2 with a real Codex app-server worker: the generated MCP configuration carried the active Hermes context and native Kanban operations resolved the intended task without a shell fallback.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass (focused mandatory-wrapper suite run: 67 passed)
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Windows 11 host, Ubuntu under WSL2

Documentation & Housekeeping

  • I've updated relevant documentation (N/A; behavior is internal migration repair)
  • I've updated cli-config.yaml.example if I added/changed config keys (N/A; no user config key added)
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows (N/A)
  • I've considered cross-platform impact (environment merge uses platform-neutral Python mappings)
  • I've updated tool descriptions/schemas if I changed tool behavior (N/A)

Screenshots / Logs

Focused validation: 67 passed, 0 failed.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard tool/mcp MCP client and OAuth codex sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Jul 17, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: Approved

Fix preserves dynamic Hermes MCP context (HERMES_KANBAN_*) when Codex starts stdio MCP servers as child processes. The HERMES_TOOLS_DYNAMIC_ENV_VARS allowlist explicitly excludes credential-adjacent variables (API_KEY, AUTH_TOKEN, BASE_URL suffixes), preventing cross-process credential leakage. Good security boundary design.


Reviewed by Hermes Agent

@WaterR47
WaterR47 marked this pull request as ready for review July 17, 2026 10:22

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for addressing the dynamic MCP environment handoff. The underlying issue is present on current main: hermes_cli/codex_runtime_plugin_migration.py:596-601 emits the managed stdio entry without Codex env_vars, so Codex has no configured forwarding allowlist.

Problems

  • HERMES_TOOLS_DYNAMIC_ENV_VARS omits HERMES_KANBAN_CLAIM_LOCK. The dispatcher injects that value for each worker at hermes_cli/kanban_db.py:8238-8239; kanban_heartbeat passes it to heartbeat_claim at tools/kanban_tools.py:776-782. heartbeat_claim only renews when the supplied value matches the task's stored lock (hermes_cli/kanban_db.py:3694-3709), so the MCP child cannot extend the dispatched claim without this variable.

Suggested changes

  • Add HERMES_KANBAN_CLAIM_LOCK to the managed allowlist and cover it in tests/hermes_cli/test_codex_runtime_plugin_migration.py.

Automated hermes-sweeper review.

out: dict[str, Any] = {
"command": sys.executable,
"args": ["-m", "agent.transports.hermes_tools_mcp_server"],
"env_vars": list(HERMES_TOOLS_DYNAMIC_ENV_VARS),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please also allowlist HERMES_KANBAN_CLAIM_LOCK. The dispatcher injects it for each worker, and kanban_heartbeat needs the identical lock to renew the claim; without it the MCP child falls back to its own host:pid and cannot update the dispatcher's claim.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 18, 2026
@WaterR47 WaterR47 closed this by deleting the head repository Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

codex comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state tool/mcp MCP client and OAuth type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants