feat(agents): add native Hermes ACP runtime and model selection

[amanning3390]

Problem

Buzz supports ACP-compliant agent runtimes (Claude Code, Codex, Goose) but not Hermes Agent, which implements ACP natively via hermes acp. Users who prefer Hermes as their agent need a way to select it as a managed runtime.

What This Changes

Registers native Hermes ACP as a first-class managed agent runtime:

Rust (crates/buzz-acp + desktop/src-tauri)

• Runtime registration: Hermes added to KNOWN_ACP_RUNTIMES with install commands, auth probe, login hint, config file path
• Spawn supervisor: Hermes's bash launcher execs its Python entry point, which stops servicing stdio when made process-group leader. A non-execing /bin/sh supervisor keeps Hermes as a child so killpg still cleans up descendants
• Cold-start budget: 45-second model probe timeout for Hermes (vs 10s default) — Hermes performs substantially more Python/module initialization
• MCP isolation: Sets HERMES_ACP_SKIP_CONFIGURED_MCP=1 for Hermes probes and managed sessions (Buzz provides MCP servers via session/new)
• Config bridge: Reads ~/.hermes/config.yaml for model, provider, reasoning effort, and MCP server inventory
• Readiness: Unconfigured Hermes (no provider set) produces a clear CliLogin requirement with "run hermes model" hint

Frontend (desktop/src)

• Onboarding: Hermes appears in the runtime picker with official icon and label
• Doctor panel: Hermes shown in settings with logo and sort priority
• Grid layout: Expands to 3-column when 3+ runtimes detected

Assets

• desktop/public/runtime-icons/hermes.png — official Hermes Agent icon

Scope

15 files across Rust, TypeScript, and assets. No changes to existing runtime behavior.

Verification

cargo test -p buzz-acp --lib  →  597 passed
cargo test --manifest-path desktop/src-tauri/Cargo.toml hermes  →  5 passed
cargo fmt --all -- --check  →  clean

Dependencies

This PR is independently reviewable. It links two pending Hermes Agent PRs that enhance the ACP experience:

• Cross-provider model choices (Hermes exposes all authenticated providers)
• Skip configured MCP startup (generic ACP host mechanism)

Both are optional — this PR works against current Hermes main. The enhanced features activate when those PRs land.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[Bartok9]

Hey @amanning3390 — this is a thoughtful Hermes integration (catalog entry, YAML bridge with real parse tests, readiness/hermes model hint, onboarding + doctor UI). Excited about Buzz ↔ Hermes Agent; want to help get it mergeable and actually work on cold start.

One intentional correctness gap (high confidence):

Wire model_probe_timeout_for_agent + acp_env_for_agent into the real spawn/probe path

In crates/buzz-acp/src/lib.rs both helpers are marked #[allow(dead_code)] and are only exercised by their unit tests. On the PR head:

• Probe/initialize paths still use bare MODELS_TIMEOUT (10s) — see the tokio::time::timeout(MODELS_TIMEOUT, client.initialize()) call sites — even though the PR body notes live Hermes probes at ~13–21s and defines a 45s Hermes budget.
• HERMES_ACP_SKIP_CONFIGURED_MCP=1 is never applied on the Command in AcpClient spawn (acp.rs applies Codex env / process_group(0), but never acp_env_for_agent(...)).

Impact: Hermes can show up in onboarding, then model discovery times out; and the paired Hermes change (NousResearch/hermes-agent#70405) stays inert from Buzz’s side until the env is set.

Minimal fix sketch:

1. At spawn (after Command::from(build_agent_spawn_command(...))):
   for (k, v) in acp_env_for_agent(command) { cmd.env(k, v); }
2. Replace probe MODELS_TIMEOUT usages with model_probe_timeout_for_agent(command) (or thread the duration into those helpers).
3. Drop both #[allow(dead_code)] so the compiler keeps you honest.
4. One unit test: hermes spawn env contains the skip marker; codex does not.

DCO

Also red on this PR — git commit --amend -s / rebase --signoff + force-with-lease.

Soft note on the /bin/sh supervisor

process_group(0) is still set on the tokio Command after the wrapper (good for killpg). Worth a quick manual kill test that Hermes + tool children die with the session; if anything leaks, a signal-forwarding trap in the shell snippet is the usual fix. Not blocking if killpg already covers it.

I can push a small “wire timeout + env” patch on top of your branch if you want the help — or you take it and I’ll re-review immediately. Either way, thank you for doing this integration properly (config bridge + readiness, not just a logo). Rooting for both projects.

[amanning3390]

Thanks @Bartok9 — you were right on the correctness gap, and it was the important one. Both items are addressed.

1) Helpers are now wired into the real spawn/probe path

I confirmed your read before changing anything: both helpers carried #[allow(dead_code)] and were only reachable from their own unit tests, while every real timeout site used the bare 10s MODELS_TIMEOUT and acp_env_for_agent(...) was never applied to the Command.

What changed in fix(agents): apply Hermes ACP env and probe timeout at spawn:

• Env at spawn — applied in AcpClient::spawn right after Command::from(build_agent_spawn_command(...)), so it covers every spawn path (probes and managed sessions) rather than only the call site I might have remembered to patch. Operator precedence is preserved: an already-exported value is never overwritten, matching the existing extra_env treatment directly above it.
• Probe timeout — resolved via model_probe_timeout_for_agent(command) at all three sites (run_auth_methods, run_authenticate, run_models). The timeout messages now print the budget actually used instead of a hardcoded 10s, so an operator sees 45s for Hermes rather than a number that no longer applies.
• #[allow(dead_code)] removed from both, so the compiler enforces the wiring from here on.
• Helpers moved next to the spawn they serve.

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

Tests (test(agents) content folded into the same commit):

test acp::tests::hermes_runtime_gets_the_configured_mcp_skip_marker ... ok
test acp::tests::non_hermes_runtimes_get_no_extra_acp_env ... ok
test acp::tests::hermes_probe_budget_exceeds_the_default_and_others_keep_it ... ok

The timeout test asserts the contract (hermes > default, others == MODELS_TIMEOUT) rather than freezing 45, so tuning the budget later won't turn it into a change-detector.

Gates on this branch:

• cargo test -p buzz-acp --lib → 600 passed, 0 failed
• cargo fmt --all -- --check → clean
• cargo clippy -p buzz-acp → clean

2) Process-group kill — verified, no leak, no trap added

I reproduced the exact supervisor shape (/bin/sh -c '"$@"; …' buzz-acp-hermes-supervisor <agent> acp with process_group(0)) using a stand-in agent that spawns its own grandchild, then observed the real process group before and after killpg:

BEFORE killpg: 4 member(s)
   pid=21598 stat=S /bin/sh -c "$@"; status=$?; exit "$status" buzz-ac
   pid=21599 stat=S /bin/sh /tmp/pgkill/fake_agent.sh acp
   pid=21600 stat=S sleep 300
   pid=21601 stat=S sleep 300

AFTER killpg (before parent reap): 1 member(s)
   pid=21598 stat=Z <defunct>

AFTER parent reap (p.wait()): 0 member(s)
live_survivors=0  zombies=0

killpg reaps the agent and both grandchildren. The single remaining entry is a zombie, not a live process, and it disappears once the owner reaps it — which the real code already does via kill_on_drop(true) + shutdown(). So killpg does cover it and I have not added a signal-forwarding trap; that would be speculative code for a leak that doesn't reproduce.

Worth noting: my first version of that probe printed "NO LEAK" while its ps filter matched zero group members — a false pass. I fixed the probe to assert the group was actually observed populated before trusting the result, which is where the stat= column and the zombie distinction came from.

3) DCO

Green on both this PR and #2655. Signoff added via --amend per the DCO bot's own instructions, plus a Co-authored-by trailer. That was the one force-push; every fix above landed as an additive commit so the review diff stays readable, per CONTRIBUTING.

One scope note: the paired Hermes side is #70405, where I also documented the marker for host authors (ACP guide + env reference) so the contract isn't only discoverable by reading the adapter source. With this PR applying it on spawn, both halves are now live.

Happy to split the env-at-spawn change into its own commit if you'd prefer a finer-grained review. Thanks for catching this — a runtime that appears in onboarding and then times out would have been a rough first impression.

[Bartok9]

Verified the diff — this is exactly right on all three:

1) Wiring (the important one). acp_env_for_agent applied in AcpClient::spawn right after Command::from(build_agent_spawn_command(...)) covers every spawn path (probes + managed sessions), and operator precedence is preserved. model_probe_timeout_for_agent now resolves at all three probe sites (run_auth_methods/run_authenticate/run_models) with the message printing the real budget, and both #[allow(dead_code)] are gone so the compiler enforces it. Non-Hermes stays on the 10s fast-fail. 600 passed / fmt / clippy clean.

2) Process-group kill. The before/after killpg probe (with the false-pass catch on the zero-member filter) is a cleaner verification than I asked for. Agreed: killpg + kill_on_drop + shutdown() covers the grandchildren; a signal-forwarding trap would be speculative code for a non-reproducing leak. Right call not to add it.

3) DCO green on both this and #2655, additive commits keeping the diff readable, and documenting the marker on the paired hermes side (#70405) so the contract is discoverable — appreciate that both halves are live.

No need to split env-at-spawn into its own commit for me; the diff reads fine. LGTM. Thanks for taking this seriously — nice integration.

[Bartok9]

This is a complete fix — thank you for taking the correctness gap seriously.

Wiring acp_env_for_agent in AcpClient::spawn (so it covers every spawn path, probes and managed sessions, not just one remembered call site) and resolving model_probe_timeout_for_agent at all three probe sites is exactly right, and preserving operator precedence (never overwriting an exported value) matches the existing extra_env treatment. Dropping both #[allow(dead_code)] so the compiler enforces the wiring is the detail that keeps this from silently regressing. The contract-style timeout test (hermes > default, others == MODELS_TIMEOUT) is the right assertion — freezing 45 would have made it a change-detector.

The process-group verification is excellent and above the bar I asked for: reproducing the real supervisor shape, observing the group populated before killpg, then showing 0 live survivors (the lone entry a zombie that the existing kill_on_drop + shutdown() reaps). Correctly declining to add a speculative signal-forwarding trap for a leak that doesn't reproduce — and catching your own probe's false "NO LEAK" (zero group members matched) is the kind of self-check that makes the result trustworthy.

DCO green on both, paired Hermes side (#70405) documenting the marker for host authors so the contract isn't source-only. Both halves live. Ready from my side; deferring to maintainers on merge.

[Bartok9]

This is the correctness gap closed properly — thank you. Wiring the env in AcpClient::spawn (so it covers probes and managed sessions, with operator precedence preserved) and resolving the probe timeout at all three sites is exactly right, and removing the #[allow(dead_code)] means the compiler now enforces the wiring. The reproduced killpg trace answering the leak question empirically — zombie-only remainder reaped by the existing kill_on_drop(true) + shutdown(), no speculative signal trap — is the correct call. 600 passed, fmt/clippy clean. Good to go pending maintainer review.

[joelbrilliant]

Flagging an overlap before a maintainer has to untangle it: I've got #2633 open on the same three files in buzz-acp (acp.rs, config.rs, lib.rs) plus discovery.rs. Came at it from a different angle though, and having read both I think they're complementary rather than competing.

Yours does runtime registration, the config bridge, readiness probing and the model-selection UX, and in acp.rs the Hermes cold-start budget plus the HERMES_ACP_SKIP_CONFIGURED_MCP env. Mine does durable session load, session_store.rs and pool.rs, so an ACP session survives a Buzz restart instead of living in an in-memory channel map that resets. Your diff doesn't go near that, and it's the part I actually care about.

So the proposal: land yours first, then I rebase #2633 down to just the session-store and pool work on top of it. Everything in mine that duplicates yours gets dropped. That way both halves land and neither of us is sitting on a branch that conflicts.

Happy the other way round too if you'd prefer, or whatever the maintainers would rather review first.

On the Hermes side your #70404 and #70405 are already in and they don't collide with my #70326 - tool policy is a different concern to model choice, and I re-checked that it still merges clean on current main.

Also worth saying the packaged release and the install scripts are the bit I hadn't gone near. Good to see someone else landed on the same idea for getting Hermes into Buzz natively.

[Bartok9]

Thanks for the heads-up @joelbrilliant — and for reading both diffs before flagging it. Agreed the two are complementary: mine is runtime registration + config bridge + readiness probing + model-selection UX (plus the Hermes cold-start budget and HERMES_ACP_SKIP_CONFIGURED_MCP in acp.rs), and nothing here touches durable session load, session_store.rs, or pool.rs — which is the part you actually care about.

Your proposed order works well for me: land this first, then you rebase #2633 down to just the session-store + pool work on top, dropping anything that overlaps. That keeps both halves landing cleanly with no branch sitting in conflict. I'm equally happy the other way round if maintainers prefer to review yours first — whatever's easiest for them.

Good confirmation too that the Hermes side is clean: #70404/#70405 in and not colliding with your #70326 (tool policy vs model choice are separate concerns). Deferring to maintainers on final merge + order.

[tlongwell-block]

We're going to feature Hermes in Buzz by name and with it's icon. We'll have it set atop generic machinery that any agent harness can use, though. It'll probably look like #2773 Hermes will be in tier 2 there, hard coded into Buzz and with art featured.

Thanks for the contributions here! I think we can close this out and use it as a reference for the wider-reaching implementation

[Bartok9]

Sounds great @tlongwell-block — a generic seam with Hermes featured in tier 2 is the right shape, and I'm glad this was useful as a reference. Happy to have it closed in favor of the wider-reaching #2773.

One small ask: if any of the runtime-registration / config-bridge / readiness-probing / model-selection approach here carries over into #2773, a person-level nod (a @Bartok9 mention or Co-authored-by) would be appreciated. Totally your call on the mechanics. Glad to help review #2773 or port over the Hermes cold-start budget / HERMES_ACP_SKIP_CONFIGURED_MCP handling if that's useful there. Thanks for the thoughtful direction!