Skip to content

fix: expose Hermes MCP tools to Codex workers - #31279

Open
stefan-wisbric wants to merge 3 commits into
NousResearch:mainfrom
stefan-wisbric:fix/codex-kanban-mcp-bridge
Open

fix: expose Hermes MCP tools to Codex workers#31279
stefan-wisbric wants to merge 3 commits into
NousResearch:mainfrom
stefan-wisbric:fix/codex-kanban-mcp-bridge

Conversation

@stefan-wisbric

Copy link
Copy Markdown

Summary

  • Inject a dynamic hermes-tools MCP server config into Codex app-server Kanban worker sessions.
  • Pass the live Kanban task environment (HERMES_KANBAN_TASK, board, DB, workspace, profile, etc.) into that MCP subprocess so kanban_show, kanban_complete, and kanban_block are actually exposed.
  • Preserve Hermes tool JSON schemas when registering dynamic FastMCP handlers so tools like skill_view(name=...) and Kanban tools receive named arguments instead of an empty kwargs shape.

Why

Codex app-server workers could perform shell/file work but finish with prose because Hermes' Kanban lifecycle tools were missing or malformed in the MCP tool surface. This made the dispatcher block tasks with external-runtime-prose even when the worker had done useful work.

Verification

  • git diff --cached --check
  • Static scan of added lines for hardcoded secrets, shell injection, eval/exec, pickle, and obvious SQL formatting patterns
  • /Users/stefans/git/hermes-agent/venv/bin/python -m pytest tests/agent/transports/test_codex_app_server_session.py tests/agent/transports/test_hermes_tools_mcp_server.py tests/run_agent/test_codex_app_server_integration.py -q -o addopts=''
  • Direct MCP smoke confirmed kanban_show, kanban_complete, kanban_block, and skill_view's name schema are present
  • Live Kanban smoke completed: a Codex app-server worker called kanban_block
  • Retried the two blocked wisbric-core parent tasks; both completed with kanban_complete

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/mcp MCP client and OAuth codex labels May 24, 2026
@stefan-wisbric
stefan-wisbric force-pushed the fix/codex-kanban-mcp-bridge branch from bc2eaaf to 722e2ac Compare May 29, 2026 05:19
@stefan-wisbric

Copy link
Copy Markdown
Author

Rebased onto the latest main (was ~540 commits behind) and re-ran the affected tests — green (83 passed across test_codex_app_server_session, test_hermes_tools_mcp_server, and the codex app-server integration tests).

This one is generic and not fork-specific: it exposes Hermes' own MCP tools (kanban_*) to Codex app-server workers, so the dispatcher observes real block/complete transitions instead of recording a clean prose-only exit. Would appreciate a review when you have a moment.

@stefan-wisbric

Copy link
Copy Markdown
Author

Pushed a follow-up hardening commit:

  • Runtime escape hatch: HERMES_KANBAN_WORKER_MCP=0 disables the injected bridge without a code change.
  • Private-API use made explicit: documented why the guarded _tool_manager access is needed (FastMCP has no public explicit-schema registration in the 1.x line), replaced the silent schema-pin except with debug logging, and log the public-add_tool fallback.
  • Test coverage: the bridge now returns no args without HERMES_KANBAN_TASK and when disabled; the env allowlist is proven to drop non-allowlisted secrets while setting HERMES_REDACT_SECRETS=true/HERMES_QUIET=1/PYTHONPATH; and _signature_from_json_schema (the fix for Codex sending empty arg dicts) has a direct unit test incl. the non-identifier edge case.
  • Docs updated for the flag + allowlist.

89 tests pass across the affected files. Note the _add_schema_backed_tool change is a general FastMCP-compat fix (correct arg names for any MCP client), independent of the Codex-worker env injection.

Stefan Scharlott and others added 3 commits May 29, 2026 17:34
…, tests

Addresses review feedback on the bridge:
- Add HERMES_KANBAN_WORKER_MCP runtime escape hatch (default on) to disable the
  injected hermes-tools MCP override without a code change.
- Document the intentional, guarded FastMCP _tool_manager access and replace the
  silent schema-pin swallow with debug logging; log the public-add_tool fallback.
- Tests: gate returns [] without HERMES_KANBAN_TASK and when disabled; env
  allowlist does not forward non-allowlisted secrets and sets the safety
  defaults; direct unit test of _signature_from_json_schema (the mechanism that
  fixes Codex sending empty arg dicts), incl. the non-identifier edge case.
- Docs: note the disable flag and the fixed env allowlist.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@herbalizer404

Copy link
Copy Markdown
Contributor

Local validation from another Hermes Kanban/Codex app-server setup:\n\n- Checked this PR against the exact issue we hit with Codex app-server Kanban workers lacking reliable native lifecycle tools.\n- Checked out the PR branch locally and ran the targeted suites:\n - tests/agent/transports/test_codex_app_server_session.py\n - tests/agent/transports/test_hermes_tools_mcp_server.py\n - tests/run_agent/test_codex_app_server_integration.py\n- Result: 89 passed, 1 warning on Python 3.11.\n\nThis matches the right fix shape for us: dynamic hermes-tools MCP injection with live Kanban env forwarding, rather than asking Codex workers to shell out or touch SQLite directly.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tracing the missing Codex-worker lifecycle path and adding focused schema coverage. Current main still constructs the Codex client without per-task MCP overrides (agent/transports/codex_app_server_session.py:247-249) and still registers MCP handlers as generic **kwargs (agent/transports/hermes_tools_mcp_server.py:155-185), so the core fix remains relevant.

Problems

  • The new HERMES_KANBAN_WORKER_MCP toggle is a user-facing non-secret behavior setting in the PR diff (agent/transports/codex_app_server_session.py, _kanban_worker_mcp_enabled). AGENTS.md requires such settings to be configured through config.yaml, not a new HERMES_* environment variable.
  • The added smoke instructions use direct python -m pytest (website/docs/user-guide/features/codex-app-server-runtime.md, added “Run the focused regression tests” block), while AGENTS.md requires scripts/run_tests.sh for CI-parity execution.

Suggested changes

  • Move the MCP-bridge enable/disable control to config.yaml and retain an internal environment bridge only if the subprocess mechanism requires it.
  • Update the documentation command to use scripts/run_tests.sh.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 13, 2026
@alt-glitch alt-glitch added needs-decision Awaiting maintainer decision before any implementation sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state and removed sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Jul 18, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #52493 and #66170, not a duplicate: this branch injects per-worker MCP configuration and uses synthetic callable signatures for dynamic schemas, whereas those PRs address the persisted migration context and private schema-override mechanisms respectively.

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

Labels

codex comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users 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