feat(hermes): add hermes-acp streaming harness (ACP transport)

[dosenr]

Related issue

Closes #2151

Summary

The current hermes harness waits for a batch turn to finish. This adds a separate hermes-acp
harness that carries Hermes' ACP stream through the shared AcpExecutor, without changing what
hermes means.

hermes acp --accept-hooks -> shared AcpExecutor -> Omnigent events
             |
             +-> per-session HERMES_HOME -> model and pre_tool_call hook
managed configured MCP          -> suppressed by Hermes 0.19.1 guard
session/new                     -> MCP servers through shared AcpExecutor

• Register Hermes as one declarative builtin ACP row. The shared executor owns transport, session
  lifecycle, streaming, and MCP delivery.
• Build the per-session HERMES_HOME through the existing Hermes bridge. It carries the selected
  vendor model and the existing Omnigent policy hook, but no configured MCP. Require Hermes 0.19.1
  and suppress managed configured MCP startup so the shared executor remains the sole owner of MCP
  delivery through session/new.
• Keep the hook authoritative for Hermes tool policy. Once it allows a call, the later Hermes ACP
  permission request continues without evaluating policy again or creating a second card.
• Reject restrictive skill selection because current Hermes ACP does not apply it. A requested
  restriction therefore cannot silently widen.
• Fail closed when the shared Hermes hook lacks policy context, receives malformed input or response,
  or cannot evaluate policy. This correction also applies to the existing batch and native Hermes modes
  that use the same hook.

Generic native-tool classification and dispatch correlation are provided by #2387 on main.

Test Plan

• pytest -q tests/test_acp_cli_harnesses.py tests/inner/test_acp_executor.py tests/inner/test_hermes_policy_hook.py tests/test_hermes_native_bridge.py tests/cli/test_configure_models.py: 312 passed.
• The opt-in tests/e2e/test_hermes_acp_e2e.py drives the catalog builder and generic ACP wrapper
  against a real configured Hermes CLI. It skipped because the live opt-in flag was absent.
• Ruff check and format check passed on all changed files.
• Targeted Pyrefly reported 0 errors.
• Applicable changed-file pre-commit hooks passed.

Demo

The surface remains the normal Omnigent streaming transcript and tool cards. These existing captures
show the Hermes ACP stream and the same cards after reload:

The restack changes the shared ACP integration point, not this rendered surface.

Type of change

• Bug fix
• Feature
• UI / frontend change
• Refactor / chore
• Docs
• Test / CI
• Breaking change

Test coverage

• Unit tests added / updated
• Integration tests added / updated
• E2E tests added / updated
• Manual verification completed
• Existing tests cover this change
• Not applicable

Coverage notes

Unit tests cover catalog registration, the Hermes 0.19.1 minimum, per-session configuration,
user-config filtering, restrictive-skill rejection, managed environment filtering, MCP precedence, one
policy evaluation per Hermes tool call, and fail-closed hook boundaries. The opt-in E2E covers one real
streaming turn through the builtin row and shared executor.

Manual verification confirmed that the one-commit restack retains current ACP prompt injection,
permission scopes, permission mode, extension handling, and #2387 correlation behavior.

Changelog

Hermes sessions can use the shared streaming ACP harness without replacing the existing batch harness.

[dosenr]

#2152 (generic ACP harness) landed after this was opened and covers most of the client plumbing here. Reworking this PR to build on that instead: the hermes executor becomes a thin layer over the generic AcpExecutor, keeping only what hermes needs on top - the per-session HERMES_HOME policy-hook wiring (hermes raises session/request_permission for only a subset of its tools, so the pre_tool_call hook stays the complete gate) plus the adapter persistence fix for self-executed tool cards. Diff will shrink substantially. Updated branch shortly - hold review until then.

[dosenr]

Rework done - this is now a thin layer over #2152's generic AcpExecutor, as promised above. Delta vs the original diff:

• Dropped: the standalone ACP client (transport, session lifecycle, streaming loop) - all inherited from AcpExecutor now. Net diff went from +1736 to +1064, most of which is tests.
• Kept: per-session HERMES_HOME policy-hook wiring (with a small _spawn_env() seam in acp_executor.py), the self-executed tool-card handling in the adapter, Hermes usage normalization, and session/new extras.
• New since the original: spec spawn-env builder + dispatcher branch (model/os_env/skills now flow from the spec; a rejected restrictive skills filter fails closed), missing-CLI preflight mapping, exact bridge-vs-self-executed classification, and permission fallback to the generic policy route when the hook is not wired.
• Demo re-captured on this exact branch: one turn exercising both a self-executed terminal card and an Omnigent web_fetch card through feat(acp): generic ACP harness + Omnigent-tool MCP bridge for all ACP harnesses #2152's MCP bridge; both persist across reload. Updated recording coming in the Demo section shortly.

Ready for review.

[TomeHirata]

/review

[TomeHirata]

We hope that AcpExecutor can cover all ACP compatible agents. For the extra logic in this PR, is this something we can generalize into AcpExecutor?

[dosenr]

Yes - most of it generalizes, and one piece arguably should regardless of hermes:

1. Tool-card classification (self_executed): every ACP agent reports tool_call updates for tools it runs itself. Today the generic executor stamps a bare call_id, which the adapter enqueues for dispatch correlation - for agent-native tools nothing ever drains that queue, so the next MCP-bridge dispatch pops a stale id and mis-pairs its card (and native tool cards don't survive reload: the item store persists only completed function_calls). This PR fixes that for hermes, but the gap applies to goose/qwen/generic acp too. I can move the classification + marker into AcpExecutor._handle_session_update in this PR.
2. Usage mapping: the cache-aware mapping is a tolerant superset (cachedReadTokens split out when present) - can fold into the generic _usage_from_result.
3. session/new extras with reject-retry and permission auto-allow: can become AcpAgentConfig knobs (session_new_extras, permission_mode) instead of subclass overrides.

What stays vendor-specific is the per-session HERMES_HOME prep - merging the user's auth and wiring the pre_tool_call policy hook is Hermes' own config mechanism, the same reason hermes_executor.py exists for the batch path. With 1-3 generalized the subclass shrinks to roughly that plus the command preset.

Happy to do 1-3 here - or the classification piece as its own PR first if you'd prefer, since it changes behavior for goose/qwen too and may be cleaner to review alone. Will start on the generalization so either way it lands quickly.

(Coordination note: #2216, same author, refactors the hermes home plumbing this PR imports - whichever lands second I rebase promptly.)

[dosenr]

Split the generic half out as #2387: the tool-call classification and card persistence now live in AcpExecutor/ExecutorAdapter. Native calls stamp self_executed and carry rawInput into the completion; unknown shapes default to self-executed, which degrades to a duplicate card rather than a mis-paired dispatch queue. Usage normalization, session extras, and permission behavior stayed out.

I rebased this branch onto that change and removed the generic HermesAcpExecutor override. The remaining layer is Hermes-specific: hermes acp --accept-hooks, per-session HERMES_HOME, the pre_tool_call policy hook, registration/spec/tests/demo. Until the generic PR lands, GitHub's main-based diff includes both commits; the Hermes-only delta is here: dosenr/omnigent@contrib/acp-native-tool-call-correlation...contrib/hermes-acp-upstream. If you'd prefer a different split, I can adjust.

[dosenr]

Post-restack validation on the final branch (d7ecfb0, stacked on #2387): one live turn through hermes acp containing a Hermes-native terminal command and a bridged Omnigent tool, SSE captured from the server.

function_call        | in_progress | terminal: cat /tmp/<nonce-file>     | tc-18a34ce4ef4f
function_call        | completed   | terminal: cat /tmp/<nonce-file>     | tc-18a34ce4ef4f
function_call_output |             |                                     | tc-18a34ce4ef4f
function_call        | in_progress | mcp_omnigent_sys_session_get_info   | tc-d62a58c779e6
function_call        | completed   | sys_session_get_info                | tc-d62a58c779e6
function_call_output | completed   |                                     | tc-d62a58c779e6

The native call renders, completes durably, and pairs under its own id; the bridged dispatch reuses the observed event's id (the correlation queue paired correctly with a native call in the same turn). The opt-in e2e (OMNIGENT_E2E_HERMES_ACP=1, real hermes binary) passes on this branch: 2 passed, including the self-executed-pair case now running through the generic classification.

The rerun caught one restack gap, fixed in d7ecfb0: this executor's overridden _ensure_session did not capture the sent mcpServers names the way the base method now does, so #2387's no-bridge-advertised guard classified bridge calls as self-executed (duplicate card). The override now performs the same capture; screenshots/video in the PR body remain from pre-restack c3e7b40 (UI unchanged).

[dosenr]

Rebased onto the updated base branch (#2387 @ 19f61f24, the session-snapshot classification fix). The Hermes delta itself is unchanged: d7ecfb06 → 4624c564 is the same single commit replayed; the full suite over the ACP + Hermes-ACP executors passes (74 tests).

[dosenr]

Status update:

• Propagated the formatter-only fix from fix(acp): keep agent-native tool calls out of dispatch correlation #2387; pre-commit checks now pass on head cf529b06.
• The Hermes-specific delta is unchanged.
• Local targeted verification passes: 143 ACP, Hermes ACP, executor-adapter, spawn-env, and harness-readiness tests.
• This PR carries fix(acp): keep agent-native tool calls out of dispatch correlation #2387 and remains ordered after it.

[TomeHirata]

Also how does hermes-acp harness compare to hermes harness? I wonder if we should replace herness harness with this if hermes-acp works better

[dosenr]

Yes, I think hermes-acp should eventually replace the existing Hermes harness. It provides structured streaming, native tool progress, cancellation, usage, and the generic ACP integration instead of parsing final subprocess output. It also works headlessly, so I would not keep batch as a permanent headless architecture.

I would retain the current harness temporarily while proving a short parity checklist: process restart and Hermes session resume, cancellation during native and bridged tools, policy coverage, usage, text/thinking/tool-card persistence, and failure cleanup.

Exercising Hermes across native and bridged tools has already surfaced integration bugs in both the policy path (#2220, merged) and generic ACP correlation (#2387, in review; this PR stacks on it), so I'd prove parity rather than assume it.

I'm happy to own that follow-up, then make ACP the implementation behind hermes and retain the old executor briefly as hermes-batch for rollback.

One process note: I'm rebasing and tightening #2387 against current main with a smaller focused test set, then I'll restack this PR and re-run the Goose and Hermes live validations. I'll post the refreshed evidence here once the stack is stable.

[dosenr]

Following up on the ACP generalization and harness-replacement questions: I rebased both branches onto current main and reduced the stack to one generic commit (ee1bf0d6) plus one Hermes-specific commit (20d6753a). #2387 now owns the immutable session bridge snapshot, native-tool persistence, and dispatch-correlation boundary; this PR inherits that behavior and keeps only Hermes configuration, policy, registration, and streaming integration.

The restack also fixed an integration gap in the previous receipt: the Hermes session/new override advertised MCP tools but copied only part of the base classification state. The miss was contained by the fail-safe: affected calls degraded to duplicate self-executed cards without touching dispatch correlation. The override now calls the shared base helper, and an integration test covers native and bridged classification after the real override runs. On the final commits, the selected regression suite passes all 128 tests, including existing adjacent coverage, and both all-files pre-commit runs pass. Fresh live validation produced a Hermes native pair on tc-bd86f23d2afd, a separate MCP bridge pair on tc-4bc5636c5ff1, and a Goose bridge pair on call_3BQVnaIZznhpzmv7DTeHcYLr. These receipts supersede the earlier live proof, which predates the complete session snapshot.

[dosenr]

Marking this draft while the generic ACP prerequisite #2387 awaits merge. The Hermes-specific design and previous validation evidence remain documented here, but main has since changed runner initialization and harness readiness substantially. This branch therefore requires a fresh restack and renewed live validation before further review. I will avoid producing another moving target until the prerequisite lands or maintainers request a different direction.

[dosenr]

Fresh restack is up on current main, using the shared ACP catalog and executor. #2387 remains separate. I’ve updated the description and test evidence.

And yes, I think ACP should eventually replace the batch harness: better streaming and live-tool UX, less custom code to maintain.

[dosenr]

Rebased onto current main now that #2387 has landed. Generic native-tool classification and dispatch correlation now come from the base; this PR remains the Hermes ACP integration.