Skip to content

fix(acp): allow hosts to skip configured MCP startup#70405

Merged
teknium1 merged 2 commits into
NousResearch:mainfrom
amanning3390:fix/acp-skip-configured-mcp
Jul 24, 2026
Merged

fix(acp): allow hosts to skip configured MCP startup#70405
teknium1 merged 2 commits into
NousResearch:mainfrom
amanning3390:fix/acp-skip-configured-mcp

Conversation

@amanning3390

@amanning3390 amanning3390 commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Problem

When Hermes runs as an ACP subprocess under a host like Buzz, it starts all globally configured MCP servers before entering its JSON-RPC loop. The host already provides MCP servers through the ACP session/new handshake (mcpServers field), so this global startup is redundant and can cause:

  • Slow cold starts (each MCP server adds latency)
  • Port/resource conflicts when the host and Hermes both try to start the same server
  • Timeout failures in the host's probe timeout window

What This Changes

Adds HERMES_ACP_SKIP_CONFIGURED_MCP=1 environment variable behavior. When set (by the ACP host), Hermes skips global configured-MCP discovery before entering its serve loop. When unset/empty/false, existing behavior is fully retained.

ACP session-supplied MCP handling (session/new mcpServers) is completely unaffected — per-session MCP servers still work normally.

Scope

  • acp_adapter/entry.py — env var check before global MCP discovery
  • tests/acp/test_entry.py — 3 new tests covering skip-enabled, skip-disabled (None/empty/0/false), and session-MCP-unchanged

Verification

pytest tests/acp/ -q  →  315 passed
ruff check acp_adapter/entry.py tests/acp/test_entry.py  →  All checks passed!

Usage

Hosts set the env var when spawning hermes acp:

HERMES_ACP_SKIP_CONFIGURED_MCP=1 hermes acp

This is a generic mechanism — any ACP host can use it, not just Buzz.

Infographic

acp-skip-configured-mcp

@alt-glitch alt-glitch added type/perf Performance improvement or optimization P4 Best-effort: we will get to it when we get to it (no commitment) comp/acp Agent Communication Protocol adapter tool/mcp MCP client and OAuth area/config Config system, migrations, profiles labels Jul 24, 2026
@Bartok9

Bartok9 commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Hey @amanning3390 — clean, minimal host escape hatch. Exact "1" match + tests for None/""/"0"/"false" is the right conservative default so random truthy strings don’t silently strip MCP. This pairs with Buzz’s intended HERMES_ACP_SKIP_CONFIGURED_MCP=1 when the host owns MCP via session/new.

Two intentional notes so it lands smoothly:

1) Desktop E2E failure looks unrelated to this diff

CI is red on Desktop E2E / Playwright (session-compression-and-queue-stop.spec.ts expecting aria-label="Queue message"). This PR only touches acp_adapter/entry.py + tests/acp/test_entry.py, so that’s almost certainly main-branch flake / ambient desktop flake, not your change. Worth a re-run of the failed job (or a maintainer “unrelated E2E” look) so required checks go green.

2) Document the env var for ACP host authors

One short note in ACP docs / env reference (name, exact value 1, and that session/new MCP still applies) will save the next host implementer. Buzz #2656 defines the same marker but still needs to apply it on spawn (called out on that PR) — once both sides wire it, cold start gets much healthier.

I can help with a tiny docs blurb or a CI re-run comment if you want — otherwise no pressure. Thanks for making Hermes a good citizen under external ACP hosts. Rooting for Hermes + Buzz.

Adds a 'Host integration' section to the ACP guide and a row in the
environment variable reference so the next ACP host implementer does not
have to read the adapter source.

Documents the exact contract the tests already pin: the value must be
exactly `1`; unset/empty/`0`/`false` keep the default behavior; only
globally configured config.yaml MCP discovery is skipped, and servers
supplied by the ACP session through session/new are still registered.

Framed as a host-set process marker rather than user configuration - the
same shape as the existing HERMES_KANBAN_TASK entry - so it does not read
as a behavioral setting that belongs in config.yaml.

Co-authored-by: amanning3390 <adam.manning@pro-serveinc.com>
Signed-off-by: amanning3390 <adam.manning@pro-serveinc.com>
amanning3390 added a commit to amanning3390/buzz that referenced this pull request Jul 24, 2026
The Hermes runtime helpers were defined but never reached a real code
path: both carried #[allow(dead_code)] and were only exercised by their
own unit tests. As a result Hermes could appear in onboarding and then
fail model discovery on the shared 10s budget, and the paired Hermes
change (NousResearch/hermes-agent#70405) stayed inert because Buzz never
set the marker it reads.

- Apply acp_env_for_agent() on the AcpClient::spawn command so every
  spawn path (probes and managed sessions) gets runtime-specific env.
  Operator precedence is preserved: an exported value is not overwritten.
- Resolve the probe budget with model_probe_timeout_for_agent() at all
  three initialize/session sites, and report the budget actually used in
  the timeout messages instead of a hardcoded 10s.
- Drop both #[allow(dead_code)] attributes so the compiler enforces the
  wiring.
- Move the helpers next to the spawn they serve.

Non-Hermes runtimes are unchanged: no extra env and the existing
MODELS_TIMEOUT fast-fail.

Co-authored-by: amanning3390 <adam.manning@pro-serveinc.com>
Signed-off-by: amanning3390 <adam.manning@pro-serveinc.com>
@amanning3390

Copy link
Copy Markdown
Contributor Author

Thanks @Bartok9 — both items addressed.

1) Desktop E2E failure — evidence it's unrelated

Your read is correct, and here's the specific evidence rather than just agreement.

What failed:

✘ 34 e2e/session-compression-and-queue-stop.spec.ts:118:3 › session compression in progress ›
     queues an Enter-submitted draft instead of steering while compaction is active

Error: expect(locator).toHaveAttribute(expected) failed
Expected: "Queue message"
> 134 |  await expect(primary).toHaveAttribute('aria-label', 'Queue message')

That's the desktop queue/compaction UI. This PR's entire diff is two files: acp_adapter/entry.py and tests/acp/test_entry.py. Zero overlap with the desktop renderer, the compaction path, or that spec.

The decisive part: the identical job passed on #70404 in the same CI sweep:

PR Desktop E2E / Playwright E2E (Linux)
#70404 pass (5m9s)
#70405 (this) fail (5m5s)

Same base, same sweep, and the failing spec is untouched by either branch — so it's ambient desktop flake, not a regression from this change. It also failed on retry #1, which is consistent with a genuinely flaky assertion rather than a transient timeout.

Could a maintainer re-run the failed e2e-desktop job so required checks can go green? I've deliberately not touched that spec from this PR — it's unrelated to this diff, and both Buzz and Nous guidance call for one logical change per PR.

2) Marker documented for ACP host authors

New commit: docs(acp): document HERMES_ACP_SKIP_CONFIGURED_MCP for ACP hosts — added in two places:

  • website/docs/user-guide/features/acp.md — a new "Host integration" section: the exact value, that only globally configured config.yaml discovery is skipped, and that MCP servers supplied via session/new are still registered (so a host loses no capability it asked for).
  • website/docs/reference/environment-variables.md — a row in the Agent Behavior table, cross-linked to that section.

On framing: AGENTS.md rejects new HERMES_* env vars for non-secret config, but explicitly allows "bridge to an internal env var if the mechanism needs one." This is a host→subprocess process marker, not a user-facing setting, so I documented it in the same shape as the existing HERMES_KANBAN_TASK entry ("Set by the … when spawning … Don't set manually") rather than as something a user should put in config.yaml. Happy to reword if you'd rather it read differently.

I checked the docs against the implementation instead of describing intent — the wording matches what entry.py does (.strip() != "1") and what the tests already pin:

test_main_skips_configured_mcp_discovery_when_requested PASSED
test_main_discovers_configured_mcp_when_skip_is_not_enabled[None] PASSED
test_main_discovers_configured_mcp_when_skip_is_not_enabled[] PASSED
test_main_discovers_configured_mcp_when_skip_is_not_enabled[0] PASSED
test_main_discovers_configured_mcp_when_skip_is_not_enabled[false] PASSED

pytest tests/acp/test_entry.py → 15 passed.

Both halves are now live

You noted Buzz #2656 defined the marker but never applied it on spawn — correct, and that's fixed: block/buzz#2656 now applies acp_env_for_agent(...) on the AcpClient::spawn command (covering probes and managed sessions), removes the #[allow(dead_code)] so the compiler enforces it, and threads the 45s Hermes probe budget into the three real timeout sites. With this PR's Hermes-side handling plus that, cold start is healthy end to end.

Thanks for pushing on the docs — you're right that the next host implementer shouldn't have to read the adapter source to find this.

@teknium1
teknium1 merged commit 939670c into NousResearch:main Jul 24, 2026
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/acp Agent Communication Protocol adapter P4 Best-effort: we will get to it when we get to it (no commitment) tool/mcp MCP client and OAuth type/perf Performance improvement or optimization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants