From a1f39a4de82f9962737e10084fb10cc746492f18 Mon Sep 17 00:00:00 2001 From: Juan Miret Date: Sat, 1 Aug 2026 17:23:47 -0300 Subject: [PATCH 1/3] Hermes harness: full Hermes Agent support over ACP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds Hermes Agent (NousResearch/hermes-agent) as a first-class harness, driven through `hermes acp` — the Agent Client Protocol stdio server Hermes also serves to Zed/VS Code/JetBrains. Because ACP runs the same agent core as the CLI and the Electron desktop app, Comet sessions inherit the device's ~/.hermes config, skills, memory, and session history. Every wire shape here was captured from a live `hermes acp` 0.19.1 session rather than inferred, including the fixtures the tests replay. - crates/harness/src/hermes/: initialize handshake (no client-side fs/terminal capability — Hermes owns its tools), session/new + session/load, prompt, cancel, and the session/update stream. session/load replays the whole prior transcript before responding, so replays are drained concurrently and dropped: Comet's doc already holds those parts, and a long history would otherwise fill the incoming channel and deadlock the response. - Live model catalog: Hermes's models are whatever providers are authenticated on the device, so they come from session/new (cached 5min) instead of a curated snapshot. No per-turn effort knob exists over ACP, so the reasoning ladder is deliberately empty. - Steering: a mid-turn session/prompt is redirected into the running turn by Hermes's core (verified live). Its ack resolves immediately and is NOT a turn end — in-flight state tells them apart — and the "Redirected the active turn…" text Hermes streams as assistant output is swallowed so it can't land in the transcript. - Tool normalization: Hermes omits rawInput for its "polished" tools, so operands come from its deterministic title prefixes, locations[], and content blocks (the `$ cmd` block wins over the title, which truncates at 80 chars). - Permissions bridge session/request_permission to RunControls::request_input; sandbox level + auto_approve map onto Hermes's edit-approval session modes. - codex/rpc.rs → jsonrpc.rs: the stdio JSON-RPC client is now shared by both the Codex app-server and Hermes ACP adapters. - Registry slot, harness picker mark, COMET_HARNESS=hermes, docs. Tests: 12 integration tests against a fake ACP server (tests/fixtures/ fake-hermes.sh) plus unit tests, and an #[ignore]d end-to-end test against a real installed hermes. Co-Authored-By: Claude Opus 5 (1M context) --- ARCHITECTURE.md | 10 +- README.md | 2 +- apps/comet/src/main.rs | 1 + crates/engine/src/agent_accounts.rs | 1 + crates/engine/src/registry.rs | 61 +- crates/harness/src/claude/mod.rs | 5 +- crates/harness/src/codex/mod.rs | 3 +- crates/harness/src/hermes/catalog.rs | 150 +++ crates/harness/src/hermes/mod.rs | 1190 +++++++++++++++++ crates/harness/src/hermes/normalize.rs | 515 +++++++ .../harness/src/{codex/rpc.rs => jsonrpc.rs} | 24 +- crates/harness/src/lib.rs | 3 + crates/harness/tests/fixtures/fake-hermes.sh | 179 +++ crates/harness/tests/hermes.rs | 588 ++++++++ crates/proto/src/agent.rs | 2 + crates/ui/assets/icons/hermes-mark.svg | 9 + crates/ui/src/icons.rs | 5 + crates/ui/src/pickers.rs | 1 + crates/ui/src/settings/accounts.rs | 1 + 19 files changed, 2712 insertions(+), 38 deletions(-) create mode 100644 crates/harness/src/hermes/catalog.rs create mode 100644 crates/harness/src/hermes/mod.rs create mode 100644 crates/harness/src/hermes/normalize.rs rename crates/harness/src/{codex/rpc.rs => jsonrpc.rs} (88%) create mode 100755 crates/harness/tests/fixtures/fake-hermes.sh create mode 100644 crates/harness/tests/hermes.rs create mode 100644 crates/ui/assets/icons/hermes-mark.svg diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 06c6a9c3..3d828ee5 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -1,7 +1,8 @@ # comet-native — Architecture A ground-up native rewrite of [comet](../comet) — a multi-device controller for coding agents -(Claude Code / Codex) — in Rust, with a gpui UI. Fresh app; no backwards compatibility required. +(Claude Code / Codex / Hermes) — in Rust, with a gpui UI. Fresh app; no backwards compatibility +required. **Pillars (from the goal):** - Sync is Loro CRDT docs (loro-mirror model) through Cloudflare Durable Objects. @@ -138,7 +139,8 @@ comet-native/ # ephemeral presence, DocsStore (SQLite snapshots + # processed-command ledger) harness/ comet-harness # Harness trait + claude-code (stream-json subprocess), - # codex (app-server JSON-RPC), mock; steering mailbox, + # codex (app-server JSON-RPC), hermes (ACP over stdio), + # mock; shared jsonrpc client, steering mailbox, # requestInput, models/reasoning/options catalogs engine/ comet-engine # sessions engine (pub/sub, run journal, recovery, stall # watchdog), doc host + command executor, repos/worktrees, @@ -225,7 +227,9 @@ Direct ports of comet behaviors (spec: feature-inventory §3): - **Harness** (research pending — `docs/research/harness.md`): trait mirroring comet's `HarnessShape`; Claude Code via `claude` CLI stream-json in/out (control protocol for permissions/AskUserQuestion→requestInput, resume, steering); Codex via app-server JSON-RPC or - `codex exec --json`; model/reasoning/option catalogs ported from `packages/harness`. + `codex exec --json`; Hermes via `hermes acp` (Agent Client Protocol — session/new+load, + session/prompt, session/update stream, session/request_permission, live model catalog); + model/reasoning/option catalogs ported from `packages/harness`. - **Repos/diffs**: git2 or `git` subprocess (subprocess — matches comet, avoids libgit2 edge cases); worktrees under `~/.comet-native/worktrees`; fs watchers (`notify`) + 2min repair; diff capture (patch + numstat + untracked, 3MiB cap, sha256) → workspace doc summary + DO diff diff --git a/README.md b/README.md index 18758dfc..6a51cbf1 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # Comet -Control your coding agents (Claude Code, Codex) from any of your devices. +Control your coding agents (Claude Code, Codex, Hermes) from any of your devices. ![Comet running a Claude Code session](docs/screenshot.png) diff --git a/apps/comet/src/main.rs b/apps/comet/src/main.rs index 9a212f38..89885c8a 100644 --- a/apps/comet/src/main.rs +++ b/apps/comet/src/main.rs @@ -199,6 +199,7 @@ fn harness_from_env() -> comet_engine::HarnessId { Ok("mock") => comet_engine::HarnessId::Mock, Ok("codex") => comet_engine::HarnessId::Codex, Ok("cursor") => comet_engine::HarnessId::Cursor, + Ok("hermes") => comet_engine::HarnessId::Hermes, _ => comet_engine::HarnessId::ClaudeCode, } } diff --git a/crates/engine/src/agent_accounts.rs b/crates/engine/src/agent_accounts.rs index 75189fd8..673c636e 100644 --- a/crates/engine/src/agent_accounts.rs +++ b/crates/engine/src/agent_accounts.rs @@ -1295,6 +1295,7 @@ fn harness_slug(harness: HarnessId) -> &'static str { HarnessId::ClaudeCode => "claude-code", HarnessId::Codex => "codex", HarnessId::Cursor => "cursor", + HarnessId::Hermes => "hermes", HarnessId::Mock => "mock", } } diff --git a/crates/engine/src/registry.rs b/crates/engine/src/registry.rs index 6af8aa64..83ca8f1f 100644 --- a/crates/engine/src/registry.rs +++ b/crates/engine/src/registry.rs @@ -210,6 +210,21 @@ pub fn default_registry() -> HarnessRegistry { }, Box::new(|| Ok(Arc::new(comet_harness::CodexHarness::new()) as Arc)), ); + // Hermes (NousResearch/hermes-agent) over ACP, same lazy pattern. The + // static descriptor mirrors HermesHarness exactly: "Hermes", StepBoundary + // steering (a mid-turn prompt is redirected into the running turn), and an + // EMPTY reasoning ladder — effort is a property of the provider/model + // picked in `hermes model`, not a per-turn ACP knob. + registry.register_lazy( + HarnessDescriptor { + id: HarnessId::Hermes, + name: "Hermes".into(), + supports_steering: true, + steering_mode: SteeringMode::StepBoundary, + reasoning_levels: vec![], + }, + Box::new(|| Ok(Arc::new(comet_harness::HermesHarness::new()) as Arc)), + ); registry } @@ -249,12 +264,17 @@ mod tests { } #[test] - fn default_registry_lists_mock_claude_and_codex_slots() { + fn default_registry_lists_mock_claude_codex_and_hermes_slots() { let registry = default_registry(); let ids: Vec = registry.descriptors().iter().map(|d| d.id).collect(); assert_eq!( ids, - vec![HarnessId::Mock, HarnessId::ClaudeCode, HarnessId::Codex] + vec![ + HarnessId::Mock, + HarnessId::ClaudeCode, + HarnessId::Codex, + HarnessId::Hermes + ] ); assert!(registry.resolve(HarnessId::Mock).is_ok()); assert!(registry.resolve(HarnessId::ClaudeCode).is_ok()); @@ -262,6 +282,8 @@ mod tests { // cheap; CLI discovery is deferred to models()/run()). let codex = registry.resolve(HarnessId::Codex).unwrap(); assert_eq!(codex.id(), HarnessId::Codex); + let hermes = registry.resolve(HarnessId::Hermes).unwrap(); + assert_eq!(hermes.id(), HarnessId::Hermes); } /// The Codex lazy descriptor must be indistinguishable from `describe()` @@ -271,22 +293,23 @@ mod tests { /// `[Ultrathink]` while the resolved adapter reports `[Low..Max]` — left /// as-is here; flagged for its own pass.) #[test] - fn codex_lazy_descriptor_matches_resolved_harness() { - let registry = default_registry(); - let before = registry - .descriptors() - .into_iter() - .find(|d| d.id == HarnessId::Codex) - .unwrap(); - registry.resolve(HarnessId::Codex).unwrap(); - let after = registry - .descriptors() - .into_iter() - .find(|d| d.id == HarnessId::Codex) - .unwrap(); - assert_eq!(before.name, after.name); - assert_eq!(before.supports_steering, after.supports_steering); - assert_eq!(before.steering_mode, after.steering_mode); - assert_eq!(before.reasoning_levels, after.reasoning_levels); + fn codex_and_hermes_lazy_descriptors_match_resolved_harnesses() { + for id in [HarnessId::Codex, HarnessId::Hermes] { + let registry = default_registry(); + let find = |registry: &HarnessRegistry| { + registry + .descriptors() + .into_iter() + .find(|d| d.id == id) + .unwrap() + }; + let before = find(®istry); + registry.resolve(id).unwrap(); + let after = find(®istry); + assert_eq!(before.name, after.name, "{id:?}"); + assert_eq!(before.supports_steering, after.supports_steering, "{id:?}"); + assert_eq!(before.steering_mode, after.steering_mode, "{id:?}"); + assert_eq!(before.reasoning_levels, after.reasoning_levels, "{id:?}"); + } } } diff --git a/crates/harness/src/claude/mod.rs b/crates/harness/src/claude/mod.rs index 18a8050b..9eb818b3 100644 --- a/crates/harness/src/claude/mod.rs +++ b/crates/harness/src/claude/mod.rs @@ -329,12 +329,13 @@ enum StdinMsg { /// Anthropic's API caps inline images at 5MB of raw bytes; larger files stay /// path refs only. -const MAX_INLINE_IMAGE_BYTES: u64 = 5 * 1024 * 1024; +pub(crate) const MAX_INLINE_IMAGE_BYTES: u64 = 5 * 1024 * 1024; /// Media type for an inline image block — extension first, magic bytes as the /// fallback (pasted screenshots may carry odd names). Only the API-supported /// inline types map; anything else (svg/bmp/tiff/…) returns `None`. -fn image_media_type(path: &std::path::Path, bytes: &[u8]) -> Option<&'static str> { +/// Shared with the Hermes ACP adapter, which inlines the same staged uploads. +pub(crate) fn image_media_type(path: &std::path::Path, bytes: &[u8]) -> Option<&'static str> { let by_ext = match path .extension() .and_then(|e| e.to_str()) diff --git a/crates/harness/src/codex/mod.rs b/crates/harness/src/codex/mod.rs index 81842577..3e6a85dc 100644 --- a/crates/harness/src/codex/mod.rs +++ b/crates/harness/src/codex/mod.rs @@ -26,7 +26,6 @@ mod catalog; mod normalize; -mod rpc; use std::collections::{HashSet, VecDeque}; use std::path::PathBuf; @@ -47,12 +46,12 @@ use comet_proto::{ UserInputAnswer, UserInputQuestion, }; +use crate::jsonrpc::{Incoming, RpcClient}; use crate::{Harness, HarnessError, RunControls}; use catalog::{REASONING_LEVELS, sandbox_mode, sandbox_policy_value, static_models, to_effort}; use normalize::{ Phase, delta_text, item_id, item_type, map_item, turn_error_message, turn_id, usage_event, }; -use rpc::{Incoming, RpcClient}; /// Locate the device's installed Codex CLI: `CODEX_EXECUTABLE`, then our own /// PATH, then the login-shell PATH snapshot (the user's shell init shapes diff --git a/crates/harness/src/hermes/catalog.rs b/crates/harness/src/hermes/catalog.rs new file mode 100644 index 00000000..62f165fb --- /dev/null +++ b/crates/harness/src/hermes/catalog.rs @@ -0,0 +1,150 @@ +//! Model catalog + session-mode mapping for Hermes. +//! +//! Unlike Codex (curated snapshot), Hermes's model list is LIVE: it is whatever +//! providers the user has authenticated on this device, so the catalog is +//! discovered from `session/new`'s `models.availableModels` rather than +//! hardcoded. Model ids are provider-qualified (`xai-oauth:grok-4.5`) and are +//! passed back verbatim to `session/set_model`. + +use comet_proto::{Model, ReasoningLevel, SandboxLevel}; +use serde_json::Value; + +/// Hermes exposes no reasoning-effort control over ACP — effort is a property +/// of the selected provider/model, chosen in `hermes model`, not a per-turn +/// knob. Advertising an empty ladder keeps the composer from offering a +/// setting the harness would silently drop. +pub(crate) const REASONING_LEVELS: &[ReasoningLevel] = &[]; + +/// Hermes's ACP session modes are its edit-approval policy (`_MODE_*` in +/// `acp_adapter/server.py`). Comet's sandbox level plus `auto_approve` pick one: +/// +/// - `default` — ask before edits (read-only runs never write anyway) +/// - `accept_edits` — auto-allow workspace and /tmp edits, still ask for +/// sensitive paths (the workspace-write default) +/// - `dont_ask` — auto-allow every file edit except sensitive paths +pub(crate) fn session_mode(sandbox: SandboxLevel, auto_approve: bool) -> &'static str { + if auto_approve { + return "dont_ask"; + } + match sandbox { + SandboxLevel::ReadOnly => "default", + SandboxLevel::WorkspaceWrite => "accept_edits", + SandboxLevel::DangerFullAccess => "dont_ask", + } +} + +/// Parse `models.availableModels` from a `session/new` / `session/load` +/// response into Comet's catalog shape. Entries without a `modelId` are +/// dropped; `name`/`description` degrade to the id when absent. +pub(crate) fn models_from_session(result: &Value) -> Vec { + let Some(available) = result + .get("models") + .and_then(|m| m.get("availableModels")) + .and_then(Value::as_array) + else { + return Vec::new(); + }; + available + .iter() + .filter_map(|m| { + let id = m.get("modelId").and_then(Value::as_str)?; + if id.is_empty() { + return None; + } + let label = m + .get("name") + .and_then(Value::as_str) + .filter(|s| !s.is_empty()) + .unwrap_or(id); + let description = m + .get("description") + .and_then(Value::as_str) + .filter(|s| !s.is_empty()) + .map(str::to_owned); + Some(Model { + id: id.to_owned(), + label: label.to_owned(), + description, + reasoning_levels: Vec::new(), + options: Vec::new(), + }) + }) + .collect() +} + +/// `models.currentModelId` — the provider/model pair Hermes booted with, used +/// to skip a redundant `session/set_model`. +pub(crate) fn current_model(result: &Value) -> Option { + result + .get("models") + .and_then(|m| m.get("currentModelId")) + .and_then(Value::as_str) + .filter(|s| !s.is_empty()) + .map(str::to_owned) +} + +/// ACP `stopReason` → whether the turn ran to completion. `cancelled` is the +/// only interrupted outcome; `refusal` and the `max_*` limits are ordinary +/// turn ends whose explanation already streamed as assistant text. +pub(crate) fn stop_reason_interrupted(stop_reason: &str) -> bool { + stop_reason == "cancelled" +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + #[test] + fn modes_follow_sandbox_and_auto_approve() { + assert_eq!(session_mode(SandboxLevel::ReadOnly, false), "default"); + assert_eq!( + session_mode(SandboxLevel::WorkspaceWrite, false), + "accept_edits" + ); + assert_eq!( + session_mode(SandboxLevel::DangerFullAccess, false), + "dont_ask" + ); + // auto_approve overrides every sandbox level. + assert_eq!(session_mode(SandboxLevel::ReadOnly, true), "dont_ask"); + } + + #[test] + fn models_parse_from_live_session_payload() { + let result = json!({ + "sessionId": "s1", + "models": { + "currentModelId": "xai-oauth:grok-4.5", + "availableModels": [ + {"modelId": "xai-oauth:grok-4.5", "name": "xAI · grok-4.5", + "description": "Provider: xAI"}, + {"modelId": "openai-codex:gpt-5.5", "name": "OpenAI Codex · gpt-5.5"}, + {"name": "no id — dropped"}, + ] + } + }); + let models = models_from_session(&result); + assert_eq!(models.len(), 2); + assert_eq!(models[0].id, "xai-oauth:grok-4.5"); + assert_eq!(models[0].label, "xAI · grok-4.5"); + assert_eq!(models[0].description.as_deref(), Some("Provider: xAI")); + // A missing description stays None rather than echoing the label. + assert_eq!(models[1].description, None); + // Hermes has no per-turn effort control. + assert!(models[0].reasoning_levels.is_empty()); + assert_eq!( + current_model(&result).as_deref(), + Some("xai-oauth:grok-4.5") + ); + assert_eq!(current_model(&json!({})), None); + } + + #[test] + fn only_cancelled_reads_as_interrupted() { + assert!(stop_reason_interrupted("cancelled")); + assert!(!stop_reason_interrupted("end_turn")); + assert!(!stop_reason_interrupted("refusal")); + assert!(!stop_reason_interrupted("max_tokens")); + } +} diff --git a/crates/harness/src/hermes/mod.rs b/crates/harness/src/hermes/mod.rs new file mode 100644 index 00000000..6cd81e9f --- /dev/null +++ b/crates/harness/src/hermes/mod.rs @@ -0,0 +1,1190 @@ +//! Hermes harness: spawns the installed `hermes` CLI as `hermes acp` and speaks +//! the Agent Client Protocol (JSON-RPC 2.0 over stdio) — the same interface +//! Hermes exposes to Zed / VS Code / JetBrains, and the one its own desktop app +//! shares a core with (NousResearch/hermes-agent, `acp_adapter/`). +//! +//! Verified against `hermes acp` from Hermes Agent 0.19.1; every wire shape +//! below (and the fixtures in `normalize`'s tests) was captured from a live +//! session rather than inferred. +//! +//! - `initialize` handshake advertising no client-side fs/terminal capability +//! (Hermes runs its own file and shell tools; it never calls back), then +//! `session/new { cwd, mcpServers }` — or `session/load` when resuming. +//! - The session response carries the LIVE model catalog +//! (`models.availableModels`, provider-qualified ids) and the mode list; +//! `session/set_model` and `session/set_mode` apply Comet's picks. +//! - `session/prompt` runs one turn and RESOLVES when the turn (plus anything +//! Hermes queued behind it) is finished — that response, not a notification, +//! is the authoritative turn end and carries the token usage. +//! - `session/update` notifications map to [`AgentEvent`]s: message/thought +//! chunks → Text/Reasoning deltas, `tool_call`/`tool_call_update` → typed +//! ToolCall/ToolResult, `plan` → a Todo call. +//! - Steering: a second `session/prompt` sent while a turn is in flight is +//! absorbed by Hermes's active-turn redirect (confirmed live: the running +//! turn changes course mid-stream). Its ack response returns immediately and +//! is NOT a turn end — the harness tracks in-flight state to tell them apart. +//! - Approvals: `session/request_permission` round-trips through +//! [`RunControls::request_input`], or is auto-allowed under `auto_approve`. +//! - Interrupt: `session/cancel` (a notification), escalating to SIGTERM → +//! SIGKILL; the pending prompt resolves with `stopReason: "cancelled"` and +//! the stream always ends with `Done { status: Interrupted }`. + +mod catalog; +mod normalize; + +use std::path::PathBuf; +use std::process::Stdio; +use std::sync::{Arc, Mutex}; +use std::time::{Duration, Instant}; + +use async_trait::async_trait; +use futures::StreamExt; +use futures::stream::BoxStream; +use serde_json::{Value, json}; +use tokio::io::AsyncBufReadExt; +use tokio::process::{Child, Command}; +use tokio::sync::mpsc; + +use comet_proto::{ + AgentEvent, DoneStatus, HarnessId, Model, ReasoningLevel, RunRequest, SteeringMode, ToolCall, + UserInputAnswer, UserInputQuestion, +}; + +use crate::jsonrpc::{Incoming, RpcClient}; +use crate::{Harness, HarnessError, RunControls}; +use catalog::{ + REASONING_LEVELS, current_model, models_from_session, session_mode, stop_reason_interrupted, +}; + +/// How long a discovered model catalog stays fresh. Discovery costs a full +/// `hermes acp` spawn plus a throwaway session, so it is worth caching; a few +/// minutes still picks up a `hermes login` in another window without a restart. +const MODEL_CACHE_TTL: Duration = Duration::from_secs(300); + +/// Locate the device's installed Hermes CLI: `HERMES_EXECUTABLE`, then PATH, +/// then the locations `setup-hermes.sh` installs into. Resolved per call — +/// cheap, and PATH may be adopted from the login shell after startup. +fn resolve_hermes_executable() -> Option { + if let Some(p) = std::env::var_os("HERMES_EXECUTABLE") + && !p.is_empty() + { + return Some(PathBuf::from(p)); + } + let exe = if cfg!(windows) { + "hermes.exe" + } else { + "hermes" + }; + let mut candidates: Vec = std::env::var_os("PATH") + .map(|path| { + std::env::split_paths(&path) + .filter(|d| !d.as_os_str().is_empty()) + .map(|d| d.join(exe)) + .collect() + }) + .unwrap_or_default(); + if let Some(home) = std::env::var_os("HOME").map(PathBuf::from) { + // The installer's launcher shim, then the venv console script inside + // HERMES_HOME (`~/.hermes/bin/hermes`, `~/.hermes/hermes-agent/venv/bin`). + candidates.push(home.join(".local").join("bin").join("hermes")); + candidates.push(home.join(".hermes").join("bin").join("hermes")); + candidates.push( + home.join(".hermes") + .join("hermes-agent") + .join("venv") + .join("bin") + .join("hermes"), + ); + } + candidates.push(PathBuf::from("/opt/homebrew/bin/hermes")); + candidates.push(PathBuf::from("/usr/local/bin/hermes")); + candidates.into_iter().find(|p| p.exists()) +} + +/// The Hermes harness. Construct with [`HermesHarness::new`]; tests point it at +/// a fake ACP server with [`HermesHarness::with_executable`]. +pub struct HermesHarness { + executable: Option, + /// Grace between `session/cancel` and SIGTERM. + interrupt_grace: Duration, + /// Grace between SIGTERM and SIGKILL. + kill_grace: Duration, + models: Mutex)>>, +} + +impl Default for HermesHarness { + fn default() -> Self { + Self { + executable: None, + interrupt_grace: Duration::from_secs(2), + kill_grace: Duration::from_secs(3), + models: Mutex::new(None), + } + } +} + +impl HermesHarness { + pub fn new() -> Self { + Self::default() + } + + /// Use a fixed CLI binary instead of PATH/known-location resolution. + pub fn with_executable(mut self, path: impl Into) -> Self { + self.executable = Some(path.into()); + self + } + + /// Tune the interrupt→SIGTERM→SIGKILL escalation timing. + pub fn with_graces(mut self, interrupt_grace: Duration, kill_grace: Duration) -> Self { + self.interrupt_grace = interrupt_grace; + self.kill_grace = kill_grace; + self + } + + fn resolve_executable(&self) -> Result { + if let Some(p) = &self.executable { + return Ok(p.clone()); + } + resolve_hermes_executable().ok_or_else(|| { + HarnessError::NotInstalled( + "hermes (searched PATH, ~/.local/bin, ~/.hermes/bin, \ + ~/.hermes/hermes-agent/venv/bin, /opt/homebrew/bin, and /usr/local/bin; \ + set HERMES_EXECUTABLE to override)" + .into(), + ) + }) + } + + /// Spawn `hermes acp` with piped stdio and a stderr tail reader. + fn spawn(&self, cwd: Option<&str>) -> Result<(Child, crate::StderrTail), HarnessError> { + let exe = self.resolve_executable()?; + let mut cmd = Command::new(&exe); + cmd.arg("acp"); + crate::prepend_exe_dir_to_path(&mut cmd, &exe); + if let Some(cwd) = cwd.filter(|c| !c.is_empty()) { + cmd.current_dir(cwd); + } + cmd.stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .kill_on_drop(true); + let mut child = cmd.spawn().map_err(|e| { + if e.kind() == std::io::ErrorKind::NotFound { + HarnessError::NotInstalled(exe.display().to_string()) + } else { + HarnessError::Io(e) + } + })?; + let stderr_tail = crate::StderrTail::default(); + if let Some(stderr) = child.stderr.take() { + let tail = stderr_tail.clone(); + tokio::spawn(async move { + let mut lines = tokio::io::BufReader::new(stderr).lines(); + while let Ok(Some(line)) = lines.next_line().await { + tracing::debug!(target: "comet_harness::hermes", "stderr: {line}"); + tail.push(&line); + } + }); + } + Ok((child, stderr_tail)) + } + + fn cached_models(&self) -> Option> { + let cache = self + .models + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + cache + .as_ref() + .filter(|(at, _)| at.elapsed() < MODEL_CACHE_TTL) + .map(|(_, models)| models.clone()) + } + + fn store_models(&self, models: &[Model]) { + let mut cache = self + .models + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + *cache = Some((Instant::now(), models.to_vec())); + } +} + +/// The `initialize` params every connection sends. Hermes never calls back into +/// the client (it owns its file/shell tools), so no fs or terminal capability +/// is advertised — claiming one we don't serve would wedge a turn if a future +/// Hermes started using it. +fn initialize_params() -> Value { + json!({ + "protocolVersion": 1, + "clientCapabilities": { + "fs": { "readTextFile": false, "writeTextFile": false }, + "terminal": false, + }, + "clientInfo": { + "name": "comet-native", + "title": "Comet", + "version": env!("CARGO_PKG_VERSION"), + }, + }) +} + +#[async_trait] +impl Harness for HermesHarness { + fn id(&self) -> HarnessId { + HarnessId::Hermes + } + fn display_name(&self) -> &str { + // Must match the registry's lazy descriptor so the catalog entry + // doesn't change after the first resolve. + "Hermes" + } + fn supports_steering(&self) -> bool { + true + } + /// A prompt sent mid-turn is redirected into the running turn by Hermes's + /// core (`agent._supports_active_turn_redirect`), landing at the next step + /// boundary; anything it can't absorb it queues for the next turn. + fn steering_mode(&self) -> SteeringMode { + SteeringMode::StepBoundary + } + fn reasoning_levels(&self) -> &[ReasoningLevel] { + REASONING_LEVELS + } + + /// Live catalog: Hermes's models are whatever providers are authenticated + /// on this device, so a short-lived `hermes acp` reports them via + /// `session/new`. Cached for [`MODEL_CACHE_TTL`]. + async fn models(&self) -> Result, HarnessError> { + if let Some(models) = self.cached_models() { + return Ok(models); + } + let (mut child, stderr_tail) = self.spawn(None)?; + let stdin = child + .stdin + .take() + .ok_or_else(|| HarnessError::Protocol("hermes acp child has no stdin".into()))?; + let stdout = child + .stdout + .take() + .ok_or_else(|| HarnessError::Protocol("hermes acp child has no stdout".into()))?; + let (client, _incoming) = RpcClient::new(stdin, stdout); + + let discover = async { + client.request("initialize", initialize_params()).await?; + let session = client + .request("session/new", json!({ "cwd": ".", "mcpServers": [] })) + .await?; + Ok::, HarnessError>(models_from_session(&session)) + }; + let discovered = discover.await; + shutdown_child(&mut child, self.kill_grace).await; + + match discovered { + Ok(models) if !models.is_empty() => { + self.store_models(&models); + Ok(models) + } + // An authenticated-provider-less Hermes reports an empty catalog; + // surface that as a protocol error with the stderr tail rather than + // an empty picker with no explanation. + Ok(_) => Err(HarnessError::Protocol(match stderr_tail.snapshot() { + Some(tail) => format!("hermes acp reported no models: {tail}"), + None => { + "hermes acp reported no models (run `hermes model` to configure a provider)" + .into() + } + })), + Err(e) => Err(e), + } + } + + async fn run( + &self, + request: RunRequest, + controls: RunControls, + ) -> Result>, HarnessError> { + let (mut child, stderr_tail) = self.spawn(Some(&request.cwd))?; + let stdin = child + .stdin + .take() + .ok_or_else(|| HarnessError::Protocol("hermes acp child has no stdin".into()))?; + let stdout = child + .stdout + .take() + .ok_or_else(|| HarnessError::Protocol("hermes acp child has no stdout".into()))?; + + let (client, incoming) = RpcClient::new(stdin, stdout); + let (event_tx, event_rx) = mpsc::channel::>(256); + tokio::spawn(run_session(Session { + child, + client, + incoming, + event_tx, + controls, + request, + interrupt_grace: self.interrupt_grace, + kill_grace: self.kill_grace, + stderr_tail, + })); + + Ok(futures::stream::unfold(event_rx, |mut rx| async move { + rx.recv().await.map(|ev| (ev, rx)) + }) + .boxed()) + } +} + +// --------------------------------------------------------------------------- +// Session +// --------------------------------------------------------------------------- + +struct Session { + child: Child, + client: RpcClient, + incoming: mpsc::Receiver, + event_tx: mpsc::Sender>, + controls: RunControls, + request: RunRequest, + interrupt_grace: Duration, + kill_grace: Duration, + stderr_tail: crate::StderrTail, +} + +/// A resolved `session/prompt`. Only [`Prompt::Turn`] ends a turn: a steer sent +/// into a live turn resolves as [`Prompt::Ack`] the moment Hermes absorbs it, +/// while the turn keeps running. +enum Prompt { + Turn(Result), + Ack(Result), +} + +fn new_message_id() -> String { + uuid::Uuid::new_v4().to_string() +} + +/// Rotate the assistant message id; returns (previous, next). +fn rotate(id: &mut String) -> (String, String) { + let prev = std::mem::replace(id, new_message_id()); + (prev, id.clone()) +} + +async fn send(tx: &mpsc::Sender>, ev: AgentEvent) -> bool { + tx.send(Ok(ev)).await.is_ok() +} + +/// Fire a `session/prompt` without blocking the message loop; the outcome +/// arrives on `tx` tagged by whether it opens a turn or merely acks a steer. +fn spawn_prompt(client: &RpcClient, params: Value, tx: &mpsc::Sender, authoritative: bool) { + let client = client.clone(); + let tx = tx.clone(); + tokio::spawn(async move { + let outcome = client.request("session/prompt", params).await; + let _ = tx + .send(if authoritative { + Prompt::Turn(outcome) + } else { + Prompt::Ack(outcome) + }) + .await; + }); +} + +/// Build the `prompt` content blocks: the text turn plus any staged image +/// attachments inlined as base64 (Hermes advertises `promptCapabilities.image`). +async fn prompt_blocks(text: &str, attachments: &[String]) -> Value { + use base64::Engine as _; + let mut blocks = vec![json!({ "type": "text", "text": text })]; + for path in attachments { + let bytes = match tokio::fs::read(path).await { + Ok(bytes) => bytes, + Err(err) => { + tracing::warn!(target: "comet_harness::hermes", %path, error = %err, "attachment unreadable; path ref only"); + continue; + } + }; + if bytes.len() as u64 > crate::claude::MAX_INLINE_IMAGE_BYTES { + tracing::debug!(target: "comet_harness::hermes", %path, "attachment over inline cap; path ref only"); + continue; + } + let Some(mime) = crate::claude::image_media_type(std::path::Path::new(path), &bytes) else { + tracing::debug!(target: "comet_harness::hermes", %path, "attachment not an inline-supported image; path ref only"); + continue; + }; + blocks.push(json!({ + "type": "image", + "mimeType": mime, + "data": base64::engine::general_purpose::STANDARD.encode(&bytes), + })); + } + Value::Array(blocks) +} + +/// The per-run event loop: one task multiplexing ACP messages, the steering +/// mailbox, the interrupt token, and consumer liveness. +async fn run_session(session: Session) { + let Session { + mut child, + client, + mut incoming, + event_tx, + controls, + request, + interrupt_grace, + kill_grace, + stderr_tail, + } = session; + let RunControls { + request_input, + mut steering, + interrupt, + } = controls; + let request_input = Arc::new(request_input); + + // ---- handshake + session (interruptible) ------------------------------ + let setup = async { + client.request("initialize", initialize_params()).await?; + + let session_params = json!({ + "cwd": request.cwd, + "mcpServers": [], + }); + let (session_id, result) = match &request.resume { + Some(resume) => { + // `session/load` REPLAYS the whole prior transcript as + // session/update notifications before it responds. Comet's doc + // already holds those parts, so they are drained and dropped + // here — concurrently, because a transcript longer than the + // incoming channel would otherwise block the reader and + // deadlock the response. + let load = client.request( + "session/load", + json!({ + "sessionId": resume, + "cwd": request.cwd, + "mcpServers": [], + }), + ); + tokio::pin!(load); + let loaded = loop { + tokio::select! { + res = &mut load => break res, + inc = incoming.recv() => match inc { + // Replay chatter: dropped on the floor. + Some(Incoming::Notification { .. }) => continue, + // Nothing should ASK us anything mid-replay; refuse + // rather than leave the agent waiting forever. + Some(Incoming::Request { id, method, .. }) => { + client.respond_error( + &id, + -32601, + &format!("unsupported during session/load: {method}"), + ); + continue; + } + Some(Incoming::Eof) | None => break Err(HarnessError::Protocol( + "hermes acp exited during session/load".into(), + )), + }, + } + }; + match loaded { + // A `null` result means Hermes has no such session. + Ok(result) if !result.is_null() => (resume.clone(), result), + other => { + if let Err(e) = other { + tracing::debug!( + target: "comet_harness::hermes", + "session/load failed (starting fresh): {e}" + ); + } + let result = client + .request("session/new", session_params.clone()) + .await?; + let id = result + .get("sessionId") + .and_then(Value::as_str) + .unwrap_or_default() + .to_owned(); + (id, result) + } + } + } + None => { + let result = client.request("session/new", session_params).await?; + let id = result + .get("sessionId") + .and_then(Value::as_str) + .unwrap_or_default() + .to_owned(); + (id, result) + } + }; + if session_id.is_empty() { + return Err(HarnessError::Protocol( + "hermes acp returned no sessionId".into(), + )); + } + + // Model + edit-approval mode. A rejected set is logged, not fatal: + // the session still runs on Hermes's configured defaults. + if let Some(model) = request.model.as_ref().filter(|m| !m.is_empty()) + && current_model(&result).as_ref() != Some(model) + && let Err(e) = client + .request( + "session/set_model", + json!({ "sessionId": session_id, "modelId": model }), + ) + .await + { + tracing::warn!( + target: "comet_harness::hermes", + "session/set_model({model}) rejected; using the session default: {e}" + ); + } + let mode = session_mode(request.sandbox, request.auto_approve); + if let Err(e) = client + .request( + "session/set_mode", + json!({ "sessionId": session_id, "modeId": mode }), + ) + .await + { + tracing::debug!( + target: "comet_harness::hermes", + "session/set_mode({mode}) rejected: {e}" + ); + } + Ok::<(String, Value), HarnessError>((session_id, result)) + }; + + let (session_id, session_result) = tokio::select! { + res = setup => match res { + Ok(pair) => pair, + Err(e) => { + // A handshake that dies because the child did should say so + // (missing provider config, a broken venv) rather than shrug. + let error = match child.try_wait() { + Ok(Some(status)) => { + crate::crash_message("hermes acp", Some(status), &stderr_tail) + } + _ => e.to_string(), + }; + let _ = event_tx + .send(Ok(AgentEvent::Done { + status: DoneStatus::Errored, + result: None, + error: Some(error), + session_id: None, + })) + .await; + shutdown_child(&mut child, kill_grace).await; + return; + } + }, + _ = interrupt.cancelled() => { + let _ = event_tx + .send(Ok(AgentEvent::Done { + status: DoneStatus::Interrupted, + result: None, + error: None, + session_id: None, + })) + .await; + shutdown_child(&mut child, kill_grace).await; + return; + } + }; + + let mut assistant_message_id = new_message_id(); + if !send( + &event_tx, + AgentEvent::SessionStarted { + harness: HarnessId::Hermes, + model: request + .model + .clone() + .or_else(|| current_model(&session_result)) + .unwrap_or_default(), + tools: Vec::new(), + cwd: request.cwd.clone(), + session_id: session_id.clone(), + assistant_message_id: assistant_message_id.clone(), + }, + ) + .await + { + shutdown_child(&mut child, kill_grace).await; + return; + } + + // ---- first turn ------------------------------------------------------- + let (prompt_tx, mut prompt_rx) = mpsc::channel::(16); + let turn_params = + |blocks: Value| -> Value { json!({ "sessionId": session_id, "prompt": blocks }) }; + spawn_prompt( + &client, + turn_params(prompt_blocks(&request.prompt, &request.attachments).await), + &prompt_tx, + true, + ); + + // ---- main loop -------------------------------------------------------- + let mut turn_in_flight = true; + // Steer acks Hermes streams as assistant text, awaiting suppression. + let mut pending_steer_acks: usize = 0; + let mut steering_open = true; + let mut interrupted = false; + let mut interrupt_sent = false; + // A Done has been emitted for the turn currently/last in flight. + let mut done_current = false; + let mut done_after_interrupt = false; + let mut escalation: Option> = None; + + 'main: loop { + tokio::select! { + inc = incoming.recv() => match inc { + Some(Incoming::Notification { method, params }) => { + if method != "session/update" { + // Unknown notification methods are tolerated by design. + continue; + } + let update = normalize::update(¶ms).clone(); + match normalize::update_kind(¶ms) { + "agent_message_chunk" => { + if let Some(text) = normalize::chunk_text(&update) { + // Drop Hermes's "Redirected the active turn…" / + // "Queued for the next turn…" acknowledgement of + // OUR steer: Comet renders steering itself. + if pending_steer_acks > 0 && normalize::is_steer_ack(&text) { + pending_steer_acks -= 1; + continue; + } + if !send(&event_tx, AgentEvent::TextDelta { text }).await { + break 'main; + } + } + } + + "agent_thought_chunk" => { + if let Some(text) = normalize::chunk_text(&update) + && !send(&event_tx, AgentEvent::ReasoningDelta { text }).await + { + break 'main; + } + } + + "tool_call" => { + let id = normalize::tool_call_id(&update); + if !send( + &event_tx, + AgentEvent::ToolCall { + id, + call: normalize::tool_call(&update), + }, + ) + .await + { + break 'main; + } + } + + "tool_call_update" => { + // A terminal update refreshes the call's metadata + // and resolves it; progress-only updates are noise. + let Some(is_error) = normalize::tool_status(&update) else { + continue; + }; + let id = normalize::tool_call_id(&update); + if !send( + &event_tx, + AgentEvent::ToolCall { + id: id.clone(), + call: normalize::tool_call(&update), + }, + ) + .await + || !send(&event_tx, AgentEvent::ToolResult { id, is_error }).await + { + break 'main; + } + } + + "plan" => { + // The plan update IS the todo list; give it a stable + // id so successive revisions replace one part. + let items = normalize::plan_items(&update); + if !send( + &event_tx, + AgentEvent::ToolCall { + id: format!("hermes-plan-{session_id}"), + call: ToolCall::Todo { items }, + }, + ) + .await + { + break 'main; + } + } + + // user_message_chunk (queued-prompt echo), + // available_commands_update, usage_update (a context + // window gauge, not token counts — the prompt response + // carries those), session_info_update, current_mode_update. + _ => {} + } + } + + Some(Incoming::Request { id, method, params }) => { + handle_server_request( + &client, + id, + &method, + ¶ms, + request.auto_approve, + &request_input, + ); + } + + // stdout EOF or reader gone: hermes acp exited. + Some(Incoming::Eof) | None => break 'main, + }, + + prompt = prompt_rx.recv() => match prompt { + Some(Prompt::Turn(outcome)) => { + turn_in_flight = false; + match outcome { + Ok(result) => { + if let Some(usage) = normalize::usage_event(&result) + && !send(&event_tx, usage).await + { + break 'main; + } + let stop = result + .get("stopReason") + .and_then(Value::as_str) + .unwrap_or("end_turn"); + let status = if interrupted || stop_reason_interrupted(stop) { + DoneStatus::Interrupted + } else { + DoneStatus::Completed + }; + done_current = true; + if !send( + &event_tx, + AgentEvent::Done { + status, + result: None, + error: None, + session_id: Some(session_id.clone()), + }, + ) + .await + { + break 'main; + } + if interrupted { + done_after_interrupt = true; + break 'main; + } + // Persistent session: stay alive for the steering + // mailbox — the caller owns teardown. + if !steering_open { + break 'main; + } + } + Err(e) => { + done_current = true; + if interrupted { + done_after_interrupt = true; + } + let _ = send( + &event_tx, + AgentEvent::Done { + status: if interrupted { + DoneStatus::Interrupted + } else { + DoneStatus::Errored + }, + result: None, + error: Some(e.to_string()), + session_id: Some(session_id.clone()), + }, + ) + .await; + break 'main; + } + } + } + // A steer absorbed by the live turn: not a turn end. Only a + // failure is worth surfacing. + Some(Prompt::Ack(outcome)) => { + if let Err(e) = outcome { + pending_steer_acks = pending_steer_acks.saturating_sub(1); + if !send( + &event_tx, + AgentEvent::Error { + message: format!("Steering failed: {e}"), + }, + ) + .await + { + break 'main; + } + } + } + None => break 'main, + }, + + steer = steering.recv(), if steering_open && !interrupted => match steer { + Some(msg) => { + let blocks = prompt_blocks(&msg.prompt, &[]).await; + // In flight → Hermes redirects it into the running turn and + // acks immediately. Idle → this prompt IS the next turn. + if turn_in_flight { + pending_steer_acks += 1; + spawn_prompt(&client, turn_params(blocks), &prompt_tx, false); + } else { + turn_in_flight = true; + done_current = false; + spawn_prompt(&client, turn_params(blocks), &prompt_tx, true); + } + let (prev, next) = rotate(&mut assistant_message_id); + if !send( + &event_tx, + AgentEvent::Steered { + assistant_message_id: Some(prev), + next_assistant_message_id: Some(next), + }, + ) + .await + { + break 'main; + } + } + None => { + // Mailbox closed (the caller's graceful idle-reap): finish + // once nothing is in flight. + steering_open = false; + if !turn_in_flight { + break 'main; + } + } + }, + + _ = interrupt.cancelled(), if !interrupt_sent => { + interrupt_sent = true; + interrupted = true; + if turn_in_flight { + // `session/cancel` is an ACP notification; the pending + // session/prompt then resolves with stopReason "cancelled". + client.notify( + "session/cancel", + Some(json!({ "sessionId": session_id })), + ); + // Escalate if the agent doesn't wind down within the graces. + if let Some(pid) = child.id() { + escalation = Some(tokio::spawn(async move { + tokio::time::sleep(interrupt_grace).await; + send_signal(pid, Signal::Term); + tokio::time::sleep(kill_grace).await; + send_signal(pid, Signal::Kill); + })); + } + } else { + // Idle between turns: nothing to interrupt — the terminal + // bookkeeping below still guarantees Done { Interrupted }. + break 'main; + } + }, + + _ = event_tx.closed() => break 'main, + } + } + + // Terminal bookkeeping: never end the stream without a Done unless the + // consumer already hung up. + if !event_tx.is_closed() { + if interrupted && !done_after_interrupt { + let _ = event_tx + .send(Ok(AgentEvent::Done { + status: DoneStatus::Interrupted, + result: None, + error: None, + session_id: Some(session_id.clone()), + })) + .await; + } else if !interrupted && !done_current { + // A child killed mid-turn (OS memory pressure, `killall hermes`) + // must not read as a silent success. + let status = child.try_wait().ok().flatten(); + let _ = event_tx + .send(Ok(AgentEvent::Done { + status: DoneStatus::Errored, + result: None, + error: Some(crate::crash_message("hermes acp", status, &stderr_tail)), + session_id: Some(session_id.clone()), + })) + .await; + } + } + + shutdown_child(&mut child, kill_grace).await; + if let Some(handle) = escalation { + handle.abort(); + } +} + +// --------------------------------------------------------------------------- +// Permissions (approval-as-input parity with comet's UX) +// --------------------------------------------------------------------------- + +type RequestInputFn = Box< + dyn Fn(Vec) -> tokio::sync::oneshot::Receiver> + + Send + + Sync, +>; + +/// Pick the option id matching an outcome. ACP option `kind`s are +/// `allow_once` / `allow_always` / `reject_once` / `reject_always`; the id is +/// free-form, so selection goes by kind with an id-prefix fallback. +fn option_id(params: &Value, allow: bool) -> Option { + let options = params.get("options").and_then(Value::as_array)?; + let wanted = if allow { "allow" } else { "reject" }; + options + .iter() + .find(|o| { + o.get("kind") + .and_then(Value::as_str) + .is_some_and(|k| k.starts_with(wanted)) + }) + .or_else(|| { + options.iter().find(|o| { + o.get("optionId") + .and_then(Value::as_str) + .is_some_and(|id| id.starts_with(wanted) || (!allow && id.starts_with("deny"))) + }) + }) + .and_then(|o| o.get("optionId").and_then(Value::as_str)) + .map(str::to_owned) +} + +/// Serve one server→client request. `session/request_permission` round-trips +/// through `request_input` as a synthesized yes/no question (in a subtask so +/// the message loop keeps flowing); with `auto_approve` it is allowed outright. +/// Anything else is rejected as unsupported so the agent never wedges awaiting +/// a reply. +fn handle_server_request( + client: &RpcClient, + id: Value, + method: &str, + params: &Value, + auto_approve: bool, + request_input: &Arc, +) { + if method != "session/request_permission" { + tracing::debug!( + target: "comet_harness::hermes", + "unhandled server request: {method}" + ); + client.respond_error(&id, -32601, &format!("unsupported method: {method}")); + return; + } + + let respond = |client: &RpcClient, id: &Value, allow: bool| match option_id(params, allow) { + Some(option) => client.respond( + &id.clone(), + json!({ "outcome": { "outcome": "selected", "optionId": option } }), + ), + // No option of the wanted polarity: cancel rather than guess. + None => client.respond( + &id.clone(), + json!({ "outcome": { "outcome": "cancelled" } }), + ), + }; + + if auto_approve { + respond(client, &id, true); + return; + } + + let question = permission_question(params); + let client = client.clone(); + let params = params.clone(); + let request_input = Arc::clone(request_input); + tokio::spawn(async move { + // The engine's input bridge owns the InputRequested/InputResolved + // lifecycle; a dropped sender (caller went away) degrades to a decline + // so the agent is unblocked — never silently allowed. + let answers = (request_input)(vec![question.clone()]) + .await + .unwrap_or_default(); + let accept = answers.iter().any(|a| { + a.question_id == question.id && a.labels.iter().any(|l| l.eq_ignore_ascii_case("yes")) + }); + match option_id(¶ms, accept) { + Some(option) => client.respond( + &id, + json!({ "outcome": { "outcome": "selected", "optionId": option } }), + ), + None => client.respond(&id, json!({ "outcome": { "outcome": "cancelled" } })), + } + }); +} + +/// Synthesize the yes/no question a permission request surfaces to the user. +/// Hermes puts the pending call under `toolCall` (title, kind, and — uniquely +/// for permission prompts — a `rawInput` naming the tool and its arguments). +fn permission_question(params: &Value) -> UserInputQuestion { + let tool_call = params.get("toolCall").unwrap_or(&Value::Null); + let title = tool_call + .get("title") + .and_then(Value::as_str) + .unwrap_or("") + .trim(); + let kind = tool_call.get("kind").and_then(Value::as_str).unwrap_or(""); + let header = match kind { + "execute" => "Approve command", + "edit" => "Approve file change", + "fetch" => "Approve network access", + _ => "Approve tool call", + }; + let question = if title.is_empty() { + "Hermes wants to run a tool. Allow it?".to_owned() + } else { + format!("Hermes wants to `{title}`. Allow it?") + }; + UserInputQuestion { + id: new_message_id(), + header: header.to_owned(), + question, + options: vec!["Yes".into(), "No".into()], + multi_select: false, + } +} + +// --------------------------------------------------------------------------- +// Child lifecycle +// --------------------------------------------------------------------------- + +/// Reap the child: graceful SIGTERM first, SIGKILL after `kill_grace`. +/// (`kill_on_drop` remains the last-resort backstop.) +async fn shutdown_child(child: &mut Child, kill_grace: Duration) { + if matches!(child.try_wait(), Ok(Some(_))) { + return; + } + if let Some(pid) = child.id() { + send_signal(pid, Signal::Term); + if tokio::time::timeout(kill_grace, child.wait()).await.is_ok() { + return; + } + } + let _ = child.start_kill(); + let _ = child.wait().await; +} + +#[derive(Clone, Copy)] +enum Signal { + Term, + Kill, +} + +#[cfg(unix)] +fn send_signal(pid: u32, signal: Signal) { + let sig = match signal { + Signal::Term => libc::SIGTERM, + Signal::Kill => libc::SIGKILL, + }; + // SAFETY: plain kill(2) on a pid we spawned and have not yet reaped. + unsafe { + libc::kill(pid as libc::pid_t, sig); + } +} + +#[cfg(not(unix))] +fn send_signal(_pid: u32, _signal: Signal) { + // No SIGTERM off unix; `start_kill`/`kill_on_drop` handle termination. +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + /// Options captured from a live `session/request_permission`. + #[test] + fn permission_options_select_by_kind() { + let params = json!({ + "options": [ + {"kind": "allow_once", "name": "Allow edit", "optionId": "allow_once"}, + {"kind": "reject_once", "name": "Deny", "optionId": "deny"}, + ], + }); + assert_eq!(option_id(¶ms, true).as_deref(), Some("allow_once")); + assert_eq!(option_id(¶ms, false).as_deref(), Some("deny")); + } + + /// Kinds are the contract, but a server that omits them still resolves via + /// the id prefix (including Hermes's "deny"). + #[test] + fn permission_options_fall_back_to_id_prefix() { + let params = json!({ + "options": [ + {"name": "Allow", "optionId": "allow_always"}, + {"name": "Deny", "optionId": "deny"}, + ], + }); + assert_eq!(option_id(¶ms, true).as_deref(), Some("allow_always")); + assert_eq!(option_id(¶ms, false).as_deref(), Some("deny")); + // Nothing matching either polarity → cancel, never a guess. + assert_eq!(option_id(&json!({"options": []}), true), None); + assert_eq!(option_id(&json!({}), false), None); + } + + #[test] + fn permission_questions_are_yes_no_and_name_the_call() { + let q = permission_question(&json!({ + "toolCall": {"kind": "edit", "title": "Approve edit: notes.txt"}, + })); + assert_eq!(q.header, "Approve file change"); + assert!(q.question.contains("Approve edit: notes.txt")); + assert_eq!(q.options, vec!["Yes".to_string(), "No".to_string()]); + assert!(!q.multi_select); + + let q = permission_question(&json!({"toolCall": {"kind": "execute", "title": "rm -rf /"}})); + assert_eq!(q.header, "Approve command"); + + // A titleless call still asks something answerable. + let q = permission_question(&json!({})); + assert_eq!(q.header, "Approve tool call"); + assert!(q.question.contains("Allow it?")); + } + + #[tokio::test] + async fn prompt_blocks_carry_text_and_skip_unreadable_attachments() { + let blocks = prompt_blocks("hello", &["/nonexistent/x.png".into()]).await; + let blocks = blocks.as_array().unwrap(); + assert_eq!(blocks.len(), 1, "unreadable attachments are skipped"); + assert_eq!(blocks[0]["type"], "text"); + assert_eq!(blocks[0]["text"], "hello"); + + // A real PNG is inlined as an ACP image block. + let dir = tempfile::tempdir().unwrap(); + let png = dir.path().join("shot.png"); + std::fs::write(&png, [0x89, b'P', b'N', b'G', 0x0D, 0x0A, 0x1A, 0x0A]).unwrap(); + let blocks = prompt_blocks("look", &[png.display().to_string()]).await; + let blocks = blocks.as_array().unwrap(); + assert_eq!(blocks.len(), 2); + assert_eq!(blocks[1]["type"], "image"); + assert_eq!(blocks[1]["mimeType"], "image/png"); + assert!(blocks[1]["data"].as_str().is_some_and(|d| !d.is_empty())); + } + + #[test] + fn initialize_advertises_no_client_side_filesystem() { + let params = initialize_params(); + assert_eq!(params["protocolVersion"], 1); + assert_eq!(params["clientCapabilities"]["fs"]["readTextFile"], false); + assert_eq!(params["clientCapabilities"]["fs"]["writeTextFile"], false); + assert_eq!(params["clientInfo"]["name"], "comet-native"); + } +} diff --git a/crates/harness/src/hermes/normalize.rs b/crates/harness/src/hermes/normalize.rs new file mode 100644 index 00000000..8b9dda89 --- /dev/null +++ b/crates/harness/src/hermes/normalize.rs @@ -0,0 +1,515 @@ +//! ACP `session/update` → [`AgentEvent`] mapping for Hermes. +//! +//! Hermes renders its own tool calls before they reach the wire +//! (`acp_adapter/tools.py`): every ACP `tool_call` carries a `kind`, a +//! human-readable `title`, `locations`, and — for tools NOT in its "polished" +//! set — a `rawInput` copy of the arguments. Polished tools (terminal, +//! read_file, write_file, patch, search_files, web_search, …) deliberately omit +//! `rawInput`, so their operands are recovered from the deterministic title +//! prefixes that `build_tool_title` emits (`terminal: `, `read: `, `write: `, +//! `patch (mode): `, `search: `, `web search: `, `extract: `) and from +//! `locations[].path`. The `execute` kind additionally carries the FULL command +//! in its content block (`$ `), which the title truncates at 80 chars — +//! so content wins for commands. +//! +//! Anything unrecognized degrades to [`ToolCall::Unknown`] carrying the title +//! and whatever `rawInput` was published, never to a dropped tool call. + +use comet_proto::{AgentEvent, TodoItem, ToolCall}; +use serde_json::Value; + +/// The `sessionUpdate` discriminant of a `session/update` notification. +pub(crate) fn update_kind(params: &Value) -> &str { + params + .get("update") + .and_then(|u| u.get("sessionUpdate")) + .and_then(Value::as_str) + .unwrap_or("") +} + +pub(crate) fn update(params: &Value) -> &Value { + params.get("update").unwrap_or(&Value::Null) +} + +fn str_at<'a>(v: &'a Value, key: &str) -> &'a str { + v.get(key).and_then(Value::as_str).unwrap_or("") +} + +/// `update.content.text` — the chunk payload on message/thought updates. +/// Tolerates both a bare content object and the array form. +pub(crate) fn chunk_text(update: &Value) -> Option { + let content = update.get("content")?; + let text = match content { + Value::Array(items) => items + .iter() + .filter_map(|c| c.get("text").and_then(Value::as_str)) + .collect::(), + other => other.get("text").and_then(Value::as_str)?.to_owned(), + }; + (!text.is_empty()).then_some(text) +} + +/// The first `locations[].path`, which Hermes fills for every file-shaped tool. +fn first_location(update: &Value) -> Option { + update + .get("locations") + .and_then(Value::as_array)? + .iter() + .find_map(|l| l.get("path").and_then(Value::as_str)) + .filter(|p| !p.is_empty()) + .map(str::to_owned) +} + +/// Concatenated text of the tool call's content blocks. Hermes wraps each block +/// as `{"type": "content", "content": {"type": "text", "text": …}}`. +fn content_text(update: &Value) -> String { + let Some(items) = update.get("content").and_then(Value::as_array) else { + return String::new(); + }; + items + .iter() + .filter_map(|block| { + block + .get("content") + .and_then(|c| c.get("text")) + .or_else(|| block.get("text")) + .and_then(Value::as_str) + }) + .collect::>() + .join("\n") +} + +/// `rawInput`, published for every tool outside Hermes's polished set. +fn raw_input(update: &Value) -> Option { + update.get("rawInput").filter(|v| !v.is_null()).cloned() +} + +fn raw_str(raw: Option<&Value>, key: &str) -> Option { + raw?.get(key) + .and_then(Value::as_str) + .filter(|s| !s.is_empty()) + .map(str::to_owned) +} + +/// Split an `mcp____` name (Hermes's native MCP prefix, see +/// `tools/mcp_tool.py::mcp_prefixed_tool_name`) into its parts. +fn mcp_parts(name: &str) -> Option<(String, String)> { + let rest = name.strip_prefix("mcp__")?; + let (server, tool) = rest.split_once("__")?; + (!server.is_empty() && !tool.is_empty()).then(|| (server.to_owned(), tool.to_owned())) +} + +/// Map one ACP `tool_call` / `tool_call_update` payload to a typed [`ToolCall`]. +pub(crate) fn tool_call(update: &Value) -> ToolCall { + let kind = str_at(update, "kind"); + let title = str_at(update, "title"); + let raw = raw_input(update); + let raw_ref = raw.as_ref(); + + // MCP tools keep their prefixed name as the title (build_tool_title falls + // through to `return tool_name`) and always publish rawInput. + if let Some((server, tool)) = mcp_parts(title) { + return ToolCall::Mcp { + server, + tool, + input: raw, + }; + } + + match kind { + "execute" => { + // Prefer the content block: `$ ` carries the untruncated + // command, while the title clips at 80 chars. + let content = content_text(update); + let command = content + .strip_prefix("$ ") + .map(str::to_owned) + .or_else(|| raw_str(raw_ref, "command")) + .or_else(|| title.strip_prefix("terminal: ").map(str::to_owned)) + .unwrap_or_else(|| { + // execute_code ("python: …"), process, browser_* and any + // other execute-shaped tool: keep the rendered title. + raw_str(raw_ref, "code").unwrap_or_else(|| title.to_owned()) + }); + ToolCall::Exec { command } + } + + "read" => match first_location(update) + .or_else(|| raw_str(raw_ref, "path")) + .or_else(|| title.strip_prefix("read: ").map(str::to_owned)) + { + Some(path) => ToolCall::ReadFile { path }, + // skill_view / skills_list / browser_snapshot are read-kind but + // pathless — they are not file reads. + None => ToolCall::Unknown { + name: title.to_owned(), + input: raw, + }, + }, + + "edit" => { + let path = first_location(update) + .or_else(|| raw_str(raw_ref, "path")) + .or_else(|| title.strip_prefix("write: ").map(str::to_owned)); + // `patch (): ` is an in-place edit; `write: ` a + // whole-file write. Hermes attaches the diff as a content block, + // which the render-parts policy strips anyway, so old/new stay None. + if title.starts_with("patch") { + return match path { + Some(path) => ToolCall::EditFile { + path, + old_string: None, + new_string: None, + }, + None => ToolCall::ApplyPatch { path: None }, + }; + } + match path { + Some(path) => ToolCall::WriteFile { + path, + content: None, + }, + None => ToolCall::Unknown { + name: title.to_owned(), + input: raw, + }, + } + } + + "search" => { + let pattern = raw_str(raw_ref, "pattern") + .or_else(|| title.strip_prefix("search: ").map(str::to_owned)) + .unwrap_or_else(|| title.to_owned()); + ToolCall::Search { + pattern, + path: first_location(update).or_else(|| raw_str(raw_ref, "path")), + } + } + + "fetch" => { + if let Some(query) = raw_str(raw_ref, "query") + .or_else(|| title.strip_prefix("web search: ").map(str::to_owned)) + { + return ToolCall::WebSearch { query }; + } + // web_extract renders `extract: (+N)`; browser_navigate + // renders `navigate: `. + let url = raw_str(raw_ref, "url") + .or_else(|| { + title + .strip_prefix("extract: ") + .map(|u| u.split(" (+").next().unwrap_or(u).to_owned()) + }) + .or_else(|| title.strip_prefix("navigate: ").map(str::to_owned)) + .unwrap_or_else(|| title.to_owned()); + ToolCall::WebFetch { url, prompt: None } + } + + // "other" / "think" / anything new: keep the rendered title and the + // arguments Hermes published. + _ => ToolCall::Unknown { + name: title.to_owned(), + input: raw, + }, + } +} + +/// A `tool_call_update` reports terminal status as `completed` / `failed`; +/// `pending` / `in_progress` are progress-only and resolve nothing. +pub(crate) fn tool_status(update: &Value) -> Option { + match str_at(update, "status") { + "completed" => Some(false), + "failed" => Some(true), + _ => None, + } +} + +pub(crate) fn tool_call_id(update: &Value) -> String { + str_at(update, "toolCallId").to_owned() +} + +/// ACP `plan` update → a Todo tool call. Entry status is one of +/// `pending` / `in_progress` / `completed`. +pub(crate) fn plan_items(update: &Value) -> Vec { + update + .get("entries") + .and_then(Value::as_array) + .map(|a| a.as_slice()) + .unwrap_or_default() + .iter() + .map(|e| TodoItem { + text: str_at(e, "content").to_owned(), + done: str_at(e, "status") == "completed", + }) + .collect() +} + +/// `session/prompt`'s response `usage` → a [`AgentEvent::Usage`] snapshot. +pub(crate) fn usage_event(result: &Value) -> Option { + let usage = result.get("usage")?; + let count = |key: &str| usage.get(key).and_then(Value::as_u64).unwrap_or_default(); + Some(AgentEvent::Usage { + input_tokens: count("inputTokens"), + output_tokens: count("outputTokens"), + }) +} + +/// The acknowledgements Hermes streams as ordinary assistant text when a +/// mid-turn prompt is absorbed by the running turn (`server.py`'s redirect and +/// queue branches). They are protocol chatter about OUR steer, not model +/// output: Comet already renders steering via [`AgentEvent::Steered`], so +/// echoing them would plant a stray line in the transcript. +pub(crate) fn is_steer_ack(text: &str) -> bool { + let text = text.trim(); + text == "Redirected the active turn with your correction." + || (text.starts_with("Queued for the next turn. (") && text.ends_with("queued)")) +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + /// Frames captured verbatim from a live `hermes acp` 0.19.1 run. + #[test] + fn live_terminal_frame_maps_to_exec() { + let update = json!({ + "content": [{"content": {"text": "$ ls -la", "type": "text"}, "type": "content"}], + "kind": "execute", + "locations": [], + "title": "terminal: ls -la", + "toolCallId": "tc-503475423a98", + "sessionUpdate": "tool_call" + }); + assert_eq!( + tool_call(&update), + ToolCall::Exec { + command: "ls -la".into() + } + ); + assert_eq!(tool_call_id(&update), "tc-503475423a98"); + } + + /// The title truncates at 80 chars; the `$ ` content block does not. + #[test] + fn long_command_comes_from_content_not_truncated_title() { + let long = "echo ".to_owned() + &"x".repeat(200); + let update = json!({ + "content": [{"content": {"text": format!("$ {long}"), "type": "text"}, "type": "content"}], + "kind": "execute", + "title": "terminal: echo xxxxxxx...", + }); + assert_eq!(tool_call(&update), ToolCall::Exec { command: long }); + } + + #[test] + fn live_read_and_write_frames_map_to_file_calls() { + let read = json!({ + "kind": "read", + "locations": [{"path": "notes.txt"}], + "title": "read: notes.txt", + "toolCallId": "tc-f707daf50a8e", + "sessionUpdate": "tool_call" + }); + assert_eq!( + tool_call(&read), + ToolCall::ReadFile { + path: "notes.txt".into() + } + ); + + let write = json!({ + "content": [{"content": {"text": "Preparing write to notes.txt.", "type": "text"}, + "type": "content"}], + "kind": "edit", + "locations": [{"path": "notes.txt"}], + "title": "write: notes.txt", + "toolCallId": "tc-1c849d4ec18e", + "sessionUpdate": "tool_call" + }); + assert_eq!( + tool_call(&write), + ToolCall::WriteFile { + path: "notes.txt".into(), + content: None + } + ); + } + + #[test] + fn patch_maps_to_edit_and_pathless_patch_to_apply_patch() { + let patch = json!({ + "kind": "edit", + "locations": [{"path": "src/main.rs"}], + "title": "patch (replace): src/main.rs", + }); + assert_eq!( + tool_call(&patch), + ToolCall::EditFile { + path: "src/main.rs".into(), + old_string: None, + new_string: None + } + ); + let pathless = json!({"kind": "edit", "title": "patch (replace): patch input"}); + assert_eq!(tool_call(&pathless), ToolCall::ApplyPatch { path: None }); + } + + #[test] + fn search_and_web_kinds_map_to_typed_calls() { + assert_eq!( + tool_call(&json!({"kind": "search", "title": "search: TODO\\("})), + ToolCall::Search { + pattern: "TODO\\(".into(), + path: None + } + ); + assert_eq!( + tool_call(&json!({"kind": "fetch", "title": "web search: rust async"})), + ToolCall::WebSearch { + query: "rust async".into() + } + ); + assert_eq!( + tool_call(&json!({"kind": "fetch", "title": "extract: https://a.dev/x (+2)"})), + ToolCall::WebFetch { + url: "https://a.dev/x".into(), + prompt: None + } + ); + assert_eq!( + tool_call(&json!({"kind": "fetch", "title": "navigate: https://b.dev"})), + ToolCall::WebFetch { + url: "https://b.dev".into(), + prompt: None + } + ); + } + + /// Non-polished tools publish rawInput; MCP keeps its `mcp__server__tool` + /// name as the title. + #[test] + fn mcp_tools_split_server_and_tool() { + let update = json!({ + "kind": "other", + "title": "mcp__github__create_issue", + "rawInput": {"repo": "a/b", "title": "bug"}, + }); + assert_eq!( + tool_call(&update), + ToolCall::Mcp { + server: "github".into(), + tool: "create_issue".into(), + input: Some(json!({"repo": "a/b", "title": "bug"})), + } + ); + // A malformed prefix is not an MCP call. + assert!(matches!( + tool_call(&json!({"kind": "other", "title": "mcp__nosep"})), + ToolCall::Unknown { .. } + )); + } + + #[test] + fn unknown_kinds_keep_title_and_raw_input() { + let update = json!({ + "kind": "other", + "title": "memory search: rust", + "rawInput": {"action": "search"}, + }); + assert_eq!( + tool_call(&update), + ToolCall::Unknown { + name: "memory search: rust".into(), + input: Some(json!({"action": "search"})), + } + ); + } + + /// A read-kind tool with no path at all (skills_list) is not a file read. + #[test] + fn pathless_read_degrades_to_unknown() { + assert!(matches!( + tool_call(&json!({"kind": "read", "title": "skills list"})), + ToolCall::Unknown { .. } + )); + } + + #[test] + fn statuses_resolve_only_on_terminal_values() { + assert_eq!(tool_status(&json!({"status": "completed"})), Some(false)); + assert_eq!(tool_status(&json!({"status": "failed"})), Some(true)); + assert_eq!(tool_status(&json!({"status": "pending"})), None); + assert_eq!(tool_status(&json!({"status": "in_progress"})), None); + assert_eq!(tool_status(&json!({})), None); + } + + #[test] + fn chunk_text_reads_object_and_array_content() { + assert_eq!( + chunk_text(&json!({"content": {"text": "hi", "type": "text"}})), + Some("hi".into()) + ); + assert_eq!( + chunk_text(&json!({"content": [{"text": "a"}, {"text": "b"}]})), + Some("ab".into()) + ); + assert_eq!(chunk_text(&json!({"content": {"text": ""}})), None); + assert_eq!(chunk_text(&json!({})), None); + } + + #[test] + fn plan_entries_map_to_todo_items() { + let update = json!({"entries": [ + {"content": "Read the code", "status": "completed", "priority": "high"}, + {"content": "Write the fix", "status": "in_progress"}, + ]}); + assert_eq!( + plan_items(&update), + vec![ + TodoItem { + text: "Read the code".into(), + done: true + }, + TodoItem { + text: "Write the fix".into(), + done: false + }, + ] + ); + } + + #[test] + fn usage_reads_prompt_response_totals() { + // Shape captured from a live session/prompt response. + let result = json!({"stopReason": "end_turn", "usage": { + "cachedReadTokens": 2432, "inputTokens": 16165, "outputTokens": 20, + "thoughtTokens": 14, "totalTokens": 16185}}); + assert_eq!( + usage_event(&result), + Some(AgentEvent::Usage { + input_tokens: 16165, + output_tokens: 20 + }) + ); + // A steer ack carries no usage at all. + assert_eq!(usage_event(&json!({"stopReason": "end_turn"})), None); + } + + #[test] + fn steer_acks_are_recognized_but_real_text_is_not() { + assert!(is_steer_ack( + "Redirected the active turn with your correction." + )); + assert!(is_steer_ack("Queued for the next turn. (2 queued)")); + assert!(!is_steer_ack("Redirecting stdout to a file is easy.")); + assert!(!is_steer_ack("Done.")); + } + + #[test] + fn update_kind_reads_the_discriminant() { + let params = json!({"sessionId": "s", "update": {"sessionUpdate": "agent_message_chunk"}}); + assert_eq!(update_kind(¶ms), "agent_message_chunk"); + assert_eq!(update_kind(&json!({})), ""); + } +} diff --git a/crates/harness/src/codex/rpc.rs b/crates/harness/src/jsonrpc.rs similarity index 88% rename from crates/harness/src/codex/rpc.rs rename to crates/harness/src/jsonrpc.rs index d9fd4856..e0f91cdd 100644 --- a/crates/harness/src/codex/rpc.rs +++ b/crates/harness/src/jsonrpc.rs @@ -1,11 +1,13 @@ -//! Minimal JSON-RPC 2.0 client over the app server's stdio (newline-delimited -//! frames, id-multiplexed), ported from codex.ts's `startAppServer`. +//! Minimal JSON-RPC 2.0 client over a child's stdio (newline-delimited frames, +//! id-multiplexed). Shared by every stdio-JSON-RPC harness: Codex's +//! `codex app-server` (where it was first ported from codex.ts's +//! `startAppServer`) and Hermes's `hermes acp`. //! //! - Responses are matched to callers by numeric id (a shared pending map the //! reader task resolves directly, so requests can be awaited from anywhere — //! including inside the session loop — without starving notifications). -//! - Notifications and server→client requests (approvals) are pumped into an -//! [`Incoming`] channel the session loop drains. +//! - Notifications and server→client requests (approvals, permission prompts) +//! are pumped into an [`Incoming`] channel the session loop drains. //! - Writes to a dead child's stdin (EPIPE) are tolerated and logged, matching //! the TS harness's swallowed-EPIPE behavior. @@ -20,21 +22,21 @@ use tokio::sync::{mpsc, oneshot}; use crate::HarnessError; -/// A non-response line from the app server, in stdout order. +/// A non-response line from the child, in stdout order. #[derive(Debug)] pub(crate) enum Incoming { Notification { method: String, params: Value, }, - /// Server→client request (approvals); must be answered via + /// Server→client request (approvals / permission prompts); must be answered via /// [`RpcClient::respond`] / [`RpcClient::respond_error`]. Request { id: Value, method: String, params: Value, }, - /// stdout EOF: the app server exited. All pending requests fail. + /// stdout EOF: the child exited. All pending requests fail. Eof, } @@ -75,7 +77,7 @@ impl RpcClient { if self.writer.send(line.to_string()).is_err() { self.pending.lock().expect("pending lock").remove(&id); return Err(HarnessError::Protocol(format!( - "{method}: app-server stdin closed" + "{method}: child stdin closed" ))); } match rx.await { @@ -83,7 +85,7 @@ impl RpcClient { Ok(Err(message)) => Err(HarnessError::Protocol(format!("{method}: {message}"))), // Sender dropped: the reader hit EOF and failed all pending. Err(_) => Err(HarnessError::Protocol(format!( - "{method}: app-server exited before responding" + "{method}: child exited before responding" ))), } } @@ -124,7 +126,7 @@ async fn write_loop(mut stdin: ChildStdin, mut rx: mpsc::UnboundedReceiver(line) else { - tracing::debug!(target: "comet_harness::codex", "non-JSON stdout line (skipped)"); + tracing::debug!(target: "comet_harness::jsonrpc", "non-JSON stdout line (skipped)"); continue; }; let method = msg.get("method").and_then(Value::as_str); diff --git a/crates/harness/src/lib.rs b/crates/harness/src/lib.rs index 17ce3557..38cfe7b3 100644 --- a/crates/harness/src/lib.rs +++ b/crates/harness/src/lib.rs @@ -66,6 +66,8 @@ pub trait Harness: Send + Sync { pub mod claude; pub mod codex; +pub mod hermes; +pub(crate) mod jsonrpc; pub mod mock; pub mod shell_env; @@ -208,3 +210,4 @@ pub(crate) fn crash_message( pub use claude::ClaudeHarness; pub use codex::CodexHarness; +pub use hermes::HermesHarness; diff --git a/crates/harness/tests/fixtures/fake-hermes.sh b/crates/harness/tests/fixtures/fake-hermes.sh new file mode 100755 index 00000000..45823d52 --- /dev/null +++ b/crates/harness/tests/fixtures/fake-hermes.sh @@ -0,0 +1,179 @@ +#!/bin/sh +# Fake `hermes acp` server for comet-harness tests. +# +# Speaks scripted ACP (JSON-RPC 2.0 over stdio): initialize handshake, +# session/new or session/load, the set_model/set_mode preamble, then a scenario +# picked from the session/prompt text. Every notification shape below is copied +# from a live `hermes acp` 0.19.1 capture. Driven by +# crates/harness/tests/hermes.rs. + +emit() { printf '%s\n' "$1"; } +rid() { printf '%s' "$1" | sed 's/.*"id":\([0-9]*\).*/\1/'; } +has() { case "$1" in *"$2"*) return 0 ;; *) return 1 ;; esac; } + +SID='s-live' + +# `update` notification for session $SID. +upd() { emit "{\"method\":\"session/update\",\"params\":{\"sessionId\":\"$SID\",\"update\":$1}}"; } + +msg() { upd "{\"sessionUpdate\":\"agent_message_chunk\",\"content\":{\"type\":\"text\",\"text\":$1}}"; } + +# ---- handshake ------------------------------------------------------------- +read -r line || exit 1 +has "$line" '"method":"initialize"' || exit 1 +has "$line" '"name":"comet-native"' || exit 1 +# Comet must NOT claim a client filesystem it does not serve. +has "$line" '"readTextFile":false' || exit 1 +emit "{\"id\":$(rid "$line"),\"result\":{\"protocolVersion\":1,\"agentInfo\":{\"name\":\"hermes-agent\",\"version\":\"0.19.1\"},\"agentCapabilities\":{\"loadSession\":true,\"promptCapabilities\":{\"image\":true}}}}" + +# The model catalog + modes both session/new and session/load return. +MODELS='"models":{"currentModelId":"xai-oauth:grok-4.5","availableModels":[{"modelId":"xai-oauth:grok-4.5","name":"xAI · grok-4.5","description":"Provider: xAI"},{"modelId":"openai-codex:gpt-5.5","name":"OpenAI Codex · gpt-5.5"}]}' +MODES='"modes":{"currentModeId":"default","availableModes":[{"id":"default","name":"Default"},{"id":"accept_edits","name":"Accept Edits"},{"id":"dont_ask","name":"Don'"'"'t Ask"}]}' + +# ---- session/new | session/load -------------------------------------------- +read -r line || exit 1 +if has "$line" '"method":"session/load"'; then + if has "$line" '"sessionId":"resume-fail"'; then + # Hermes returns a null result for a session it cannot find; the harness + # must fall back to session/new. + emit "{\"id\":$(rid "$line"),\"result\":null}" + read -r line || exit 1 + has "$line" '"method":"session/new"' || exit 1 + SID='s-fresh' + emit "{\"id\":$(rid "$line"),\"result\":{\"sessionId\":\"$SID\",$MODELS,$MODES}}" + else + # A real load REPLAYS the prior transcript as session/update notifications + # BEFORE responding. Comet's doc already holds these; none may reach the + # event stream. + SID='s-resumed' + upd '{"sessionUpdate":"user_message_chunk","content":{"type":"text","text":"replayed user turn"}}' + msg '"REPLAYED-ASSISTANT-TEXT"' + upd '{"sessionUpdate":"tool_call","toolCallId":"tc-replay","kind":"read","title":"read: old.txt","locations":[{"path":"old.txt"}]}' + emit "{\"id\":$(rid "$line"),\"result\":{$MODELS,$MODES}}" + fi +elif has "$line" '"method":"session/new"'; then + emit "{\"id\":$(rid "$line"),\"result\":{\"sessionId\":\"$SID\",$MODELS,$MODES}}" +else + exit 1 +fi + +# ---- set_model / set_mode preamble, then the turn -------------------------- +SAW_SET_MODEL=no +SAW_MODE='' +promptline='' +while read -r line; do + case "$line" in + *'"method":"session/set_model"'*) + SAW_SET_MODEL=yes + has "$line" '"modelId":"openai-codex:gpt-5.5"' || exit 1 + emit "{\"id\":$(rid "$line"),\"result\":{}}" + ;; + *'"method":"session/set_mode"'*) + SAW_MODE=$(printf '%s' "$line" | sed 's/.*"modeId":"\([a-z_]*\)".*/\1/') + emit "{\"id\":$(rid "$line"),\"result\":{}}" + ;; + *'"method":"session/prompt"'*) + promptline="$line" + break + ;; + *) : ;; + esac +done +[ -n "$promptline" ] || exit 1 +pid=$(rid "$promptline") + +USAGE='"usage":{"inputTokens":16165,"outputTokens":20,"thoughtTokens":14,"totalTokens":16185,"cachedReadTokens":2432}' + +case "$promptline" in + +*scenario:happy*) + # The preamble the harness must have sent before the turn. + [ "$SAW_SET_MODEL" = yes ] || exit 1 + # auto_approve=true must pick Hermes's most permissive edit policy. + [ "$SAW_MODE" = dont_ask ] || exit 1 + has "$promptline" '"type":"text"' || exit 1 + + upd '{"sessionUpdate":"available_commands_update","availableCommands":[{"name":"help"}]}' + upd '{"sessionUpdate":"usage_update","size":500000,"used":10972}' + upd '{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"thinking"}}' + msg '"Hello"' + msg '" world"' + # Live tool_call frames (terminal / read / write). + upd '{"sessionUpdate":"tool_call","toolCallId":"tc-1","kind":"execute","locations":[],"title":"terminal: ls -la","content":[{"type":"content","content":{"type":"text","text":"$ ls -la"}}]}' + upd '{"sessionUpdate":"tool_call_update","toolCallId":"tc-1","kind":"execute","title":"terminal: ls -la","status":"completed"}' + upd '{"sessionUpdate":"tool_call","toolCallId":"tc-2","kind":"read","locations":[{"path":"notes.txt"}],"title":"read: notes.txt"}' + upd '{"sessionUpdate":"tool_call_update","toolCallId":"tc-2","kind":"read","locations":[{"path":"notes.txt"}],"title":"read: notes.txt","status":"failed"}' + # A progress-only update resolves nothing. + upd '{"sessionUpdate":"tool_call_update","toolCallId":"tc-3","kind":"edit","title":"write: out.txt","locations":[{"path":"out.txt"}],"status":"in_progress"}' + # MCP call: prefixed name + rawInput. + upd '{"sessionUpdate":"tool_call","toolCallId":"tc-4","kind":"other","title":"mcp__linear__create_issue","rawInput":{"team":"eng"}}' + upd '{"sessionUpdate":"plan","entries":[{"content":"Read the code","status":"completed"},{"content":"Write the fix","status":"pending"}]}' + emit "{\"id\":$pid,\"result\":{\"stopReason\":\"end_turn\",$USAGE}}" + ;; + +*scenario:resumed*) + msg '"after resume"' + emit "{\"id\":$pid,\"result\":{\"stopReason\":\"end_turn\",$USAGE}}" + ;; + +*scenario:readonly*) + # Sandbox ReadOnly without auto_approve maps to Hermes's "default" mode. + [ "$SAW_MODE" = default ] || exit 1 + emit "{\"id\":$pid,\"result\":{\"stopReason\":\"end_turn\"}}" + ;; + +*scenario:permission*) + upd '{"sessionUpdate":"tool_call","toolCallId":"tc-p","kind":"edit","title":"write: notes.txt","locations":[{"path":"notes.txt"}]}' + # Server→client permission request, exactly as captured live. + emit "{\"id\":900,\"method\":\"session/request_permission\",\"params\":{\"sessionId\":\"$SID\",\"options\":[{\"kind\":\"allow_once\",\"name\":\"Allow edit\",\"optionId\":\"allow_once\"},{\"kind\":\"reject_once\",\"name\":\"Deny\",\"optionId\":\"deny\"}],\"toolCall\":{\"toolCallId\":\"edit-approval-1\",\"status\":\"pending\",\"kind\":\"edit\",\"title\":\"Approve edit: notes.txt\"}}}" + read -r reply || exit 1 + # Echo the option the harness picked so the test can assert on it. + picked=$(printf '%s' "$reply" | sed 's/.*"optionId":"\([a-z_]*\)".*/\1/') + msg "\"picked:$picked\"" + emit "{\"id\":$pid,\"result\":{\"stopReason\":\"end_turn\"}}" + ;; + +*scenario:steer*) + msg '"first"' + # Block until the harness forwards the steer as a second session/prompt. + read -r steerline || exit 1 + has "$steerline" '"method":"session/prompt"' || exit 1 + has "$steerline" 'steered text' || exit 1 + # Hermes acks the redirect immediately (no usage) and streams the ack as + # ordinary assistant text — which Comet must swallow. + msg '"Redirected the active turn with your correction."' + emit "{\"id\":$(rid "$steerline"),\"result\":{\"stopReason\":\"end_turn\"}}" + msg '"second"' + emit "{\"id\":$pid,\"result\":{\"stopReason\":\"end_turn\",$USAGE}}" + ;; + +*scenario:interrupt*) + msg '"working"' + # Wait for session/cancel (a notification — no response), then resolve the + # pending prompt as cancelled. + read -r cancelline || exit 1 + has "$cancelline" '"method":"session/cancel"' || exit 1 + emit "{\"id\":$pid,\"result\":{\"stopReason\":\"cancelled\"}}" + ;; + +*scenario:promptfail*) + emit "{\"id\":$pid,\"error\":{\"code\":-32603,\"message\":\"provider exploded\"}}" + ;; + +*) + emit "{\"id\":$pid,\"result\":{\"stopReason\":\"end_turn\"}}" + ;; +esac + +# Stay alive for the persistent-session steering mailbox until stdin closes. +while read -r line; do + case "$line" in + *'"method":"session/prompt"'*) + nid=$(rid "$line") + msg '"follow-up turn"' + emit "{\"id\":$nid,\"result\":{\"stopReason\":\"end_turn\"}}" + ;; + *) : ;; + esac +done +exit 0 diff --git a/crates/harness/tests/hermes.rs b/crates/harness/tests/hermes.rs new file mode 100644 index 00000000..f164639a --- /dev/null +++ b/crates/harness/tests/hermes.rs @@ -0,0 +1,588 @@ +//! HermesHarness integration tests against the fake ACP server in +//! `tests/fixtures/fake-hermes.sh` (no real `hermes` binary involved). + +use std::path::PathBuf; +use std::time::Duration; + +use futures::StreamExt; +use tokio::sync::{mpsc, oneshot}; + +use comet_harness::{CancellationToken, Harness, HermesHarness, RunControls, SteerMessage}; +use comet_proto::{ + AgentEvent, DoneStatus, HarnessId, RunRequest, SandboxLevel, TodoItem, ToolCall, + UserInputAnswer, +}; + +fn fixture_path() -> PathBuf { + let path = PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("tests") + .join("fixtures") + .join("fake-hermes.sh"); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let _ = std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o755)); + } + path +} + +fn harness() -> HermesHarness { + HermesHarness::new() + .with_executable(fixture_path()) + .with_graces(Duration::from_millis(200), Duration::from_millis(200)) +} + +fn request(prompt: &str) -> RunRequest { + RunRequest { + prompt: prompt.into(), + // Differs from the fixture's currentModelId, so session/set_model must fire. + model: Some("openai-codex:gpt-5.5".into()), + reasoning: None, + model_options: serde_json::Map::new(), + cwd: String::new(), + sandbox: SandboxLevel::WorkspaceWrite, + auto_approve: true, + attachments: Vec::new(), + resume: None, + } +} + +/// Controls whose `request_input` answers every question with `answer_label`. +fn controls( + answer_label: &'static str, +) -> (RunControls, mpsc::Sender, CancellationToken) { + let (steer_tx, steer_rx) = mpsc::channel(8); + let token = CancellationToken::new(); + let controls = RunControls { + request_input: Box::new(move |questions| { + let (tx, rx) = oneshot::channel(); + let answers: Vec = questions + .iter() + .map(|q| UserInputAnswer { + question_id: q.id.clone(), + labels: vec![answer_label.into()], + }) + .collect(); + let _ = tx.send(answers); + rx + }), + steering: steer_rx, + interrupt: token.clone(), + }; + (controls, steer_tx, token) +} + +async fn run_to_end( + harness: &HermesHarness, + req: RunRequest, + controls: RunControls, +) -> Vec { + let stream = harness.run(req, controls).await.expect("run starts"); + tokio::time::timeout( + Duration::from_secs(10), + stream.map(|r| r.expect("stream event")).collect::>(), + ) + .await + .expect("run finished in time") +} + +/// Drop the steering sender so the persistent session reaps itself once the +/// turn is done, then collect. +async fn run_once(harness: &HermesHarness, req: RunRequest) -> Vec { + let (controls, steer, _token) = controls("Yes"); + drop(steer); + run_to_end(harness, req, controls).await +} + +#[tokio::test] +async fn happy_path_maps_chunks_tool_calls_plan_usage_and_done() { + let mut req = request("scenario:happy"); + req.cwd = "/tmp".into(); + let events = run_once(&harness(), req).await; + + // SessionStarted carries the ACP session id and the requested model. + let starts: Vec<_> = events + .iter() + .filter_map(|e| match e { + AgentEvent::SessionStarted { + harness, + model, + cwd, + session_id, + .. + } => Some((harness, model, cwd, session_id)), + _ => None, + }) + .collect(); + assert_eq!(starts.len(), 1, "{events:?}"); + let (h, model, cwd, session_id) = starts[0]; + assert_eq!(*h, HarnessId::Hermes); + assert_eq!(model, "openai-codex:gpt-5.5"); + assert_eq!(cwd, "/tmp"); + assert_eq!(session_id, "s-live"); + + // Message vs thought chunks land on their own channels. + assert!(events.contains(&AgentEvent::TextDelta { + text: "Hello".into() + })); + assert!(events.contains(&AgentEvent::TextDelta { + text: " world".into() + })); + assert!(events.contains(&AgentEvent::ReasoningDelta { + text: "thinking".into() + })); + + // execute → Exec, resolved by its terminal update. + assert!(events.contains(&AgentEvent::ToolCall { + id: "tc-1".into(), + call: ToolCall::Exec { + command: "ls -la".into() + }, + })); + assert!(events.contains(&AgentEvent::ToolResult { + id: "tc-1".into(), + is_error: false + })); + + // read → ReadFile; a `failed` status is an error result. + assert!(events.contains(&AgentEvent::ToolCall { + id: "tc-2".into(), + call: ToolCall::ReadFile { + path: "notes.txt".into() + }, + })); + assert!(events.contains(&AgentEvent::ToolResult { + id: "tc-2".into(), + is_error: true + })); + + // A progress-only (`in_progress`) update resolves nothing at all. + assert!( + !events + .iter() + .any(|e| matches!(e, AgentEvent::ToolResult { id, .. } if id == "tc-3")), + "in_progress must not resolve: {events:?}" + ); + + // MCP prefix split. + assert!(events.contains(&AgentEvent::ToolCall { + id: "tc-4".into(), + call: ToolCall::Mcp { + server: "linear".into(), + tool: "create_issue".into(), + input: Some(serde_json::json!({"team": "eng"})), + }, + })); + + // plan → a Todo call under one stable id. + assert!(events.contains(&AgentEvent::ToolCall { + id: "hermes-plan-s-live".into(), + call: ToolCall::Todo { + items: vec![ + TodoItem { + text: "Read the code".into(), + done: true + }, + TodoItem { + text: "Write the fix".into(), + done: false + }, + ] + }, + })); + + // Usage comes from the prompt RESPONSE, not the context-window gauge. + assert!(events.contains(&AgentEvent::Usage { + input_tokens: 16165, + output_tokens: 20, + })); + + // Exactly one Done, and it closes the stream. + let dones: Vec<_> = events + .iter() + .filter(|e| matches!(e, AgentEvent::Done { .. })) + .collect(); + assert_eq!(dones.len(), 1, "{events:?}"); + assert!(matches!( + events.last(), + Some(AgentEvent::Done { + status: DoneStatus::Completed, + session_id: Some(id), + .. + }) if id == "s-live" + )); +} + +/// `session/load` replays the whole prior transcript before responding. Comet's +/// doc already holds those parts, so not one replayed event may escape. +#[tokio::test] +async fn resume_replays_are_swallowed() { + let mut req = request("scenario:resumed"); + req.resume = Some("s-resumed".into()); + let events = run_once(&harness(), req).await; + + assert!( + !events.contains(&AgentEvent::TextDelta { + text: "REPLAYED-ASSISTANT-TEXT".into() + }), + "replayed history leaked into the stream: {events:?}" + ); + assert!( + !events + .iter() + .any(|e| matches!(e, AgentEvent::ToolCall { id, .. } if id == "tc-replay")), + "replayed tool call leaked: {events:?}" + ); + // The live turn after the resume still streams. + assert!(events.contains(&AgentEvent::TextDelta { + text: "after resume".into() + })); + assert!(matches!( + events.first(), + Some(AgentEvent::SessionStarted { session_id, .. }) if session_id == "s-resumed" + )); +} + +/// A session Hermes cannot find answers `null`; the harness starts fresh +/// instead of failing the run. +#[tokio::test] +async fn unknown_resume_falls_back_to_a_fresh_session() { + let mut req = request("scenario:happy"); + req.resume = Some("resume-fail".into()); + req.cwd = "/tmp".into(); + let events = run_once(&harness(), req).await; + + assert!(matches!( + events.first(), + Some(AgentEvent::SessionStarted { session_id, .. }) if session_id == "s-fresh" + )); + assert!(matches!( + events.last(), + Some(AgentEvent::Done { + status: DoneStatus::Completed, + .. + }) + )); +} + +/// Comet's sandbox level picks Hermes's edit-approval mode; the fixture asserts +/// the wire value and fails the turn if it is wrong. +#[tokio::test] +async fn read_only_without_auto_approve_selects_the_default_mode() { + let mut req = request("scenario:readonly"); + req.sandbox = SandboxLevel::ReadOnly; + req.auto_approve = false; + let events = run_once(&harness(), req).await; + assert!( + matches!( + events.last(), + Some(AgentEvent::Done { + status: DoneStatus::Completed, + .. + }) + ), + "{events:?}" + ); +} + +/// `session/request_permission` round-trips through `request_input`; a "Yes" +/// selects the allow option, a "No" the reject one. +#[tokio::test] +async fn permission_requests_bridge_to_request_input() { + for (answer, expected) in [("Yes", "picked:allow_once"), ("No", "picked:deny")] { + let (controls, steer, _token) = controls(answer); + drop(steer); + let mut req = request("scenario:permission"); + req.auto_approve = false; + let events = run_to_end(&harness(), req, controls).await; + assert!( + events.contains(&AgentEvent::TextDelta { + text: expected.into() + }), + "answering {answer} should send {expected}: {events:?}" + ); + } +} + +/// `auto_approve` allows without ever asking the user. +#[tokio::test] +async fn auto_approve_allows_without_consulting_the_user() { + let (steer_tx, steer_rx) = mpsc::channel(8); + drop(steer_tx); + let asked = std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let seen = asked.clone(); + let controls = RunControls { + request_input: Box::new(move |_questions| { + seen.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + let (tx, rx) = oneshot::channel(); + let _ = tx.send(Vec::new()); + rx + }), + steering: steer_rx, + interrupt: CancellationToken::new(), + }; + let events = run_to_end(&harness(), request("scenario:permission"), controls).await; + assert_eq!( + asked.load(std::sync::atomic::Ordering::SeqCst), + 0, + "auto_approve must not ask" + ); + assert!(events.contains(&AgentEvent::TextDelta { + text: "picked:allow_once".into() + })); +} + +/// A steer sent mid-turn is absorbed by the running turn: Comet emits `Steered`, +/// swallows Hermes's "Redirected…" acknowledgement, and still ends with exactly +/// ONE Done — the ack response is not a turn end. +#[tokio::test] +async fn mid_turn_steer_emits_steered_and_swallows_the_ack() { + let (controls, steer_tx, _token) = controls("Yes"); + let stream = harness() + .run(request("scenario:steer"), controls) + .await + .expect("run starts"); + tokio::pin!(stream); + + let mut events: Vec = Vec::new(); + let collect = async { + while let Some(ev) = stream.next().await { + let ev = ev.expect("stream event"); + // Steer only once the first chunk proves the turn is live. + if ev + == (AgentEvent::TextDelta { + text: "first".into(), + }) + { + steer_tx + .send(SteerMessage { + prompt: "steered text".into(), + message_id: None, + }) + .await + .expect("steer accepted"); + // Close the mailbox so the session reaps after the turn. + drop(steer_tx.clone()); + } + let done = matches!(ev, AgentEvent::Done { .. }); + events.push(ev); + if done { + break; + } + } + }; + tokio::time::timeout(Duration::from_secs(10), collect) + .await + .expect("run finished in time"); + + assert!( + events + .iter() + .any(|e| matches!(e, AgentEvent::Steered { .. })), + "{events:?}" + ); + assert!( + !events.contains(&AgentEvent::TextDelta { + text: "Redirected the active turn with your correction.".into() + }), + "the steer ack must not reach the transcript: {events:?}" + ); + assert!(events.contains(&AgentEvent::TextDelta { + text: "second".into() + })); + assert_eq!( + events + .iter() + .filter(|e| matches!(e, AgentEvent::Done { .. })) + .count(), + 1, + "the ack response must not be mistaken for a turn end: {events:?}" + ); +} + +/// Cancelling the token sends `session/cancel`; the pending prompt resolves +/// `cancelled` and the stream ends Interrupted. +#[tokio::test] +async fn interrupt_cancels_the_session_and_ends_interrupted() { + let (controls, _steer, token) = controls("Yes"); + let stream = harness() + .run(request("scenario:interrupt"), controls) + .await + .expect("run starts"); + tokio::pin!(stream); + + let mut events: Vec = Vec::new(); + let collect = async { + while let Some(ev) = stream.next().await { + let ev = ev.expect("stream event"); + if ev + == (AgentEvent::TextDelta { + text: "working".into(), + }) + { + token.cancel(); + } + let done = matches!(ev, AgentEvent::Done { .. }); + events.push(ev); + if done { + break; + } + } + }; + tokio::time::timeout(Duration::from_secs(10), collect) + .await + .expect("run finished in time"); + + assert!( + matches!( + events.last(), + Some(AgentEvent::Done { + status: DoneStatus::Interrupted, + .. + }) + ), + "{events:?}" + ); +} + +/// Interrupting a persistent session that is idle BETWEEN turns still closes +/// the stream with a Done — the "never end without a Done after an interrupt" +/// contract the Codex harness also holds. +#[tokio::test] +async fn interrupt_while_idle_between_turns_still_ends_with_done() { + // Keep the steering mailbox open so the session survives its first turn. + let (controls, _steer_tx, token) = controls("Yes"); + let mut req = request("scenario:happy"); + req.cwd = "/tmp".into(); + let stream = harness().run(req, controls).await.expect("run starts"); + tokio::pin!(stream); + + let mut dones = 0usize; + let mut statuses = Vec::new(); + let collect = async { + while let Some(ev) = stream.next().await { + if let AgentEvent::Done { status, .. } = ev.expect("stream event") { + dones += 1; + statuses.push(status); + if dones == 1 { + // The turn is over but the session is still alive. + token.cancel(); + } else { + break; + } + } + } + }; + tokio::time::timeout(Duration::from_secs(10), collect) + .await + .expect("run finished in time"); + + assert_eq!(statuses.first(), Some(&DoneStatus::Completed)); + assert_eq!( + statuses.get(1), + Some(&DoneStatus::Interrupted), + "an idle interrupt must still emit a terminal Done: {statuses:?}" + ); +} + +/// A JSON-RPC error on `session/prompt` ends the run as Errored, carrying the +/// server's message rather than a silent success. +#[tokio::test] +async fn prompt_error_ends_the_run_errored() { + let events = run_once(&harness(), request("scenario:promptfail")).await; + let Some(AgentEvent::Done { + status, + error: Some(error), + .. + }) = events.last() + else { + panic!("expected an errored Done: {events:?}"); + }; + assert_eq!(*status, DoneStatus::Errored); + assert!(error.contains("provider exploded"), "{error}"); +} + +/// The live catalog comes from `session/new`'s `models.availableModels`, and is +/// served from cache on the second call. +#[tokio::test] +async fn models_are_discovered_live_and_cached() { + let harness = harness(); + let models = harness.models().await.expect("models discovered"); + assert_eq!(models.len(), 2); + assert_eq!(models[0].id, "xai-oauth:grok-4.5"); + assert_eq!(models[0].label, "xAI · grok-4.5"); + assert_eq!(models[0].description.as_deref(), Some("Provider: xAI")); + // Hermes exposes no per-turn effort knob. + assert!(models[0].reasoning_levels.is_empty()); + assert!(models[1].description.is_none()); + + let again = harness.models().await.expect("cached models"); + assert_eq!(again, models); +} + +/// End-to-end against the REAL `hermes acp`. Ignored by default: it needs an +/// installed Hermes with a configured provider and hits the network. Run with +/// `cargo test -p comet-harness --test hermes -- --ignored --nocapture`. +#[tokio::test] +#[ignore = "requires an installed, provider-configured hermes CLI + network"] +async fn live_hermes_cli_streams_a_real_turn() { + let harness = HermesHarness::new(); + + let models = harness.models().await.expect("live model discovery"); + assert!(!models.is_empty(), "a configured hermes reports models"); + eprintln!("discovered {} models, first: {:?}", models.len(), models[0]); + + let dir = tempfile::tempdir().expect("tempdir"); + let (controls, steer, _token) = controls("Yes"); + drop(steer); + let mut req = request("Reply with exactly: PONG"); + req.model = None; // whatever the device is configured for + req.cwd = dir.path().display().to_string(); + + let stream = harness.run(req, controls).await.expect("run starts"); + let events = tokio::time::timeout( + Duration::from_secs(240), + stream.map(|r| r.expect("stream event")).collect::>(), + ) + .await + .expect("live turn finished in time"); + + assert!( + matches!(events.first(), Some(AgentEvent::SessionStarted { .. })), + "{events:?}" + ); + let text: String = events + .iter() + .filter_map(|e| match e { + AgentEvent::TextDelta { text } => Some(text.as_str()), + _ => None, + }) + .collect(); + eprintln!("assistant text: {text:?}"); + assert!(text.contains("PONG"), "expected PONG, got {text:?}"); + assert!( + matches!( + events.last(), + Some(AgentEvent::Done { + status: DoneStatus::Completed, + .. + }) + ), + "{events:?}" + ); +} + +/// A missing binary is a typed NotInstalled, never a panic or a hang. +#[tokio::test] +async fn missing_binary_reports_not_installed() { + let harness = HermesHarness::new().with_executable("/nonexistent/hermes"); + assert!(matches!( + harness.models().await, + Err(comet_harness::HarnessError::NotInstalled(_)) + )); + let (controls, _steer, _token) = controls("Yes"); + assert!(matches!( + harness.run(request("x"), controls).await.err(), + Some(comet_harness::HarnessError::NotInstalled(_)) + )); +} diff --git a/crates/proto/src/agent.rs b/crates/proto/src/agent.rs index 4d623c36..a88a1e3d 100644 --- a/crates/proto/src/agent.rs +++ b/crates/proto/src/agent.rs @@ -8,6 +8,8 @@ pub enum HarnessId { ClaudeCode, Codex, Cursor, + /// Hermes Agent (NousResearch/hermes-agent), driven over ACP. + Hermes, /// Test harness; never shown in production pickers. Mock, } diff --git a/crates/ui/assets/icons/hermes-mark.svg b/crates/ui/assets/icons/hermes-mark.svg new file mode 100644 index 00000000..a1c7a04c --- /dev/null +++ b/crates/ui/assets/icons/hermes-mark.svg @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/crates/ui/src/icons.rs b/crates/ui/src/icons.rs index 43716901..56ceedc0 100644 --- a/crates/ui/src/icons.rs +++ b/crates/ui/src/icons.rs @@ -116,6 +116,11 @@ icon_assets![ (CLAUDE_MARK, "claude-mark"), (OPENAI_MARK, "openai-mark"), (CURSOR_MARK, "cursor-mark"), + // Hermes has no vector mark to port — its app icon is a full raster + // illustration that doesn't reduce to a 16px monochrome glyph. This is a + // hand-drawn winged helmet (Hermes, the messenger) in the Solar Linear + // style, like the terminal/git-branch/return ports. + (HERMES_MARK, "hermes-mark"), ]; /// The Claude mark's brand orange (`#D97757`) — comet keeps it even on the diff --git a/crates/ui/src/pickers.rs b/crates/ui/src/pickers.rs index 59387e63..178efb81 100644 --- a/crates/ui/src/pickers.rs +++ b/crates/ui/src/pickers.rs @@ -2284,6 +2284,7 @@ pub(crate) fn harness_brand_icon(harness: HarnessId) -> (&'static str, Option (crate::icons::OPENAI_MARK, None), HarnessId::Cursor => (crate::icons::CURSOR_MARK, None), + HarnessId::Hermes => (crate::icons::HERMES_MARK, None), } } diff --git a/crates/ui/src/settings/accounts.rs b/crates/ui/src/settings/accounts.rs index 535ecdad..d74ed279 100644 --- a/crates/ui/src/settings/accounts.rs +++ b/crates/ui/src/settings/accounts.rs @@ -1179,6 +1179,7 @@ impl Render for AccountsPage { let provider_icon = |harness: HarnessId| match harness { HarnessId::Codex => (crate::icons::OPENAI_MARK, None), HarnessId::Cursor => (crate::icons::CURSOR_MARK, None), + HarnessId::Hermes => (crate::icons::HERMES_MARK, None), _ => ( crate::icons::CLAUDE_MARK, Some(crate::icons::claude_brand()), From 147534f3526f1ac5a3a897d2c267b6a317bad3b2 Mon Sep 17 00:00:00 2001 From: Juan Miret Date: Sat, 1 Aug 2026 18:28:38 -0300 Subject: [PATCH 2/3] hermes agent piker --- crates/engine/src/registry.rs | 4 +- crates/harness/src/claude/mod.rs | 2 +- crates/harness/src/codex/mod.rs | 2 +- crates/harness/src/hermes/catalog.rs | 137 +++++++++++++++++-- crates/harness/src/hermes/mod.rs | 42 +++++- crates/harness/tests/fixtures/fake-hermes.sh | 16 +++ crates/harness/tests/hermes.rs | 27 +++- 7 files changed, 211 insertions(+), 19 deletions(-) diff --git a/crates/engine/src/registry.rs b/crates/engine/src/registry.rs index 83ca8f1f..a5b927e9 100644 --- a/crates/engine/src/registry.rs +++ b/crates/engine/src/registry.rs @@ -213,8 +213,8 @@ pub fn default_registry() -> HarnessRegistry { // Hermes (NousResearch/hermes-agent) over ACP, same lazy pattern. The // static descriptor mirrors HermesHarness exactly: "Hermes", StepBoundary // steering (a mid-turn prompt is redirected into the running turn), and an - // EMPTY reasoning ladder — effort is a property of the provider/model - // picked in `hermes model`, not a per-turn ACP knob. + // empty HARNESS-WIDE reasoning ladder. Hermes's live Codex/Claude model + // rows receive those underlying harnesses' model-specific traits instead. registry.register_lazy( HarnessDescriptor { id: HarnessId::Hermes, diff --git a/crates/harness/src/claude/mod.rs b/crates/harness/src/claude/mod.rs index 9eb818b3..6e36751b 100644 --- a/crates/harness/src/claude/mod.rs +++ b/crates/harness/src/claude/mod.rs @@ -12,7 +12,7 @@ //! - Interrupt: cancelling [`RunControls::interrupt`] sends the protocol-level //! interrupt control request, then escalates to SIGTERM and SIGKILL. -mod catalog; +pub(crate) mod catalog; mod normalize; mod wire; diff --git a/crates/harness/src/codex/mod.rs b/crates/harness/src/codex/mod.rs index 3e6a85dc..72320477 100644 --- a/crates/harness/src/codex/mod.rs +++ b/crates/harness/src/codex/mod.rs @@ -24,7 +24,7 @@ //! escalating to SIGTERM → SIGKILL if the child is unresponsive; the stream //! always ends with `Done { status: Interrupted }`. -mod catalog; +pub(crate) mod catalog; mod normalize; use std::collections::{HashSet, VecDeque}; diff --git a/crates/harness/src/hermes/catalog.rs b/crates/harness/src/hermes/catalog.rs index 62f165fb..027dd188 100644 --- a/crates/harness/src/hermes/catalog.rs +++ b/crates/harness/src/hermes/catalog.rs @@ -4,17 +4,91 @@ //! providers the user has authenticated on this device, so the catalog is //! discovered from `session/new`'s `models.availableModels` rather than //! hardcoded. Model ids are provider-qualified (`xai-oauth:grok-4.5`) and are -//! passed back verbatim to `session/set_model`. +//! passed back verbatim to `session/set_model`. When the underlying model is +//! also offered by Comet's Codex or Claude harness, its picker traits are +//! overlaid onto the live row so changing harness does not hide model controls. -use comet_proto::{Model, ReasoningLevel, SandboxLevel}; +use comet_proto::{Model, ModelOption, ReasoningLevel, SandboxLevel}; use serde_json::Value; -/// Hermes exposes no reasoning-effort control over ACP — effort is a property -/// of the selected provider/model, chosen in `hermes model`, not a per-turn -/// knob. Advertising an empty ladder keeps the composer from offering a -/// setting the harness would silently drop. +/// There is no harness-wide ladder: Hermes is provider-agnostic, so each live +/// model row receives the underlying Codex/Claude model's own ladder instead. pub(crate) const REASONING_LEVELS: &[ReasoningLevel] = &[]; +/// The provider-qualified ACP id's model half. OpenRouter-style ids can retain +/// a namespace (`openrouter:anthropic/claude-opus-5`), so callers also try the +/// final path component when matching Comet's curated catalogs. +fn model_id_candidates(id: &str) -> impl Iterator { + let model = id.split_once(':').map_or(id, |(_, model)| model); + [model, model.rsplit('/').next().unwrap_or(model)].into_iter() +} + +fn shared_harness_traits(id: &str) -> Option<(Vec, Vec)> { + let candidates: Vec<&str> = model_id_candidates(id).collect(); + crate::codex::catalog::static_models() + .into_iter() + .chain(crate::claude::catalog::static_models()) + .find(|model| candidates.iter().any(|candidate| *candidate == model.id)) + .map(|model| { + // Hermes's provider-facing effort vocabulary is the seven ordinary + // levels used by its desktop app. Claude Code's prompt/settings + // special modes are harness-specific and cannot cross ACP. + let reasoning_levels = model + .reasoning_levels + .into_iter() + .filter(|level| { + !matches!( + level, + ReasoningLevel::Ultracode | ReasoningLevel::Ultrathink + ) + }) + .collect(); + // The Hermes desktop model menu exposes reasoning + fast. Do not + // leak Claude Code-only context-window settings into this harness. + let options = model + .options + .into_iter() + .filter(|option| matches!(option.id.as_str(), "serviceTier" | "fastMode")) + .collect(); + (reasoning_levels, options) + }) +} + +/// Hermes's generic reasoning setting accepts the shared effort vocabulary. +/// Harness-specific special modes degrade to their underlying xhigh effort. +pub(crate) fn reasoning_effort(level: ReasoningLevel) -> &'static str { + match level { + ReasoningLevel::Minimal => "minimal", + ReasoningLevel::Low => "low", + ReasoningLevel::Medium => "medium", + ReasoningLevel::High => "high", + ReasoningLevel::XHigh | ReasoningLevel::Ultracode | ReasoningLevel::Ultrathink => "xhigh", + ReasoningLevel::Max => "max", + ReasoningLevel::Ultra => "ultra", + } +} + +/// Normalize Comet's Codex/Claude speed controls to Hermes's per-session fast +/// setting. Hermes names the provider request value `priority`. +pub(crate) fn service_tier( + model_id: Option<&str>, + options: &serde_json::Map, +) -> Option<&'static str> { + let supports_fast = model_id + .and_then(shared_harness_traits) + .is_some_and(|(_, options)| !options.is_empty()); + if !supports_fast { + return None; + } + let codex_fast = options.get("serviceTier").and_then(Value::as_str) == Some("fast"); + let claude_fast = options.get("fastMode").and_then(Value::as_str) == Some("on"); + Some(if codex_fast || claude_fast { + "priority" + } else { + "normal" + }) +} + /// Hermes's ACP session modes are its edit-approval policy (`_MODE_*` in /// `acp_adapter/server.py`). Comet's sandbox level plus `auto_approve` pick one: /// @@ -61,12 +135,14 @@ pub(crate) fn models_from_session(result: &Value) -> Vec { .and_then(Value::as_str) .filter(|s| !s.is_empty()) .map(str::to_owned); + let (reasoning_levels, options) = + shared_harness_traits(id).unwrap_or_else(|| (Vec::new(), Vec::new())); Some(Model { id: id.to_owned(), label: label.to_owned(), description, - reasoning_levels: Vec::new(), - options: Vec::new(), + reasoning_levels, + options, }) }) .collect() @@ -120,19 +196,32 @@ mod tests { {"modelId": "xai-oauth:grok-4.5", "name": "xAI · grok-4.5", "description": "Provider: xAI"}, {"modelId": "openai-codex:gpt-5.5", "name": "OpenAI Codex · gpt-5.5"}, + {"modelId": "anthropic:claude-opus-5", "name": "Anthropic · claude-opus-5"}, {"name": "no id — dropped"}, ] } }); let models = models_from_session(&result); - assert_eq!(models.len(), 2); + assert_eq!(models.len(), 3); assert_eq!(models[0].id, "xai-oauth:grok-4.5"); assert_eq!(models[0].label, "xAI · grok-4.5"); assert_eq!(models[0].description.as_deref(), Some("Provider: xAI")); // A missing description stays None rather than echoing the label. assert_eq!(models[1].description, None); - // Hermes has no per-turn effort control. + // Non-Claude/Codex providers retain the raw ACP catalog traits. assert!(models[0].reasoning_levels.is_empty()); + // Provider-qualified models carry the same traits Comet exposes for + // the underlying Codex / Claude harness model. + assert!(models[1].reasoning_levels.contains(&ReasoningLevel::XHigh)); + assert!(models[1].options.iter().any(|o| o.id == "serviceTier")); + assert!(models[2].reasoning_levels.contains(&ReasoningLevel::Max)); + assert!( + !models[2] + .reasoning_levels + .contains(&ReasoningLevel::Ultrathink) + ); + assert!(models[2].options.iter().any(|o| o.id == "fastMode")); + assert!(!models[2].options.iter().any(|o| o.id == "contextWindow")); assert_eq!( current_model(&result).as_deref(), Some("xai-oauth:grok-4.5") @@ -147,4 +236,32 @@ mod tests { assert!(!stop_reason_interrupted("refusal")); assert!(!stop_reason_interrupted("max_tokens")); } + + #[test] + fn run_traits_normalize_to_hermes_config_values() { + assert_eq!(reasoning_effort(ReasoningLevel::Minimal), "minimal"); + assert_eq!(reasoning_effort(ReasoningLevel::Ultrathink), "xhigh"); + assert_eq!( + service_tier(Some("xai-oauth:grok-4.5"), &serde_json::Map::new()), + None + ); + assert_eq!( + service_tier(Some("openai-codex:gpt-5.5"), &serde_json::Map::new()), + Some("normal") + ); + assert_eq!( + service_tier( + Some("openai-codex:gpt-5.5"), + &serde_json::from_value(json!({"serviceTier": "fast"})).unwrap() + ), + Some("priority") + ); + assert_eq!( + service_tier( + Some("anthropic:claude-opus-5"), + &serde_json::from_value(json!({"fastMode": "on"})).unwrap() + ), + Some("priority") + ); + } } diff --git a/crates/harness/src/hermes/mod.rs b/crates/harness/src/hermes/mod.rs index 6cd81e9f..2b98a774 100644 --- a/crates/harness/src/hermes/mod.rs +++ b/crates/harness/src/hermes/mod.rs @@ -53,7 +53,8 @@ use comet_proto::{ use crate::jsonrpc::{Incoming, RpcClient}; use crate::{Harness, HarnessError, RunControls}; use catalog::{ - REASONING_LEVELS, current_model, models_from_session, session_mode, stop_reason_interrupted, + REASONING_LEVELS, current_model, models_from_session, reasoning_effort, service_tier, + session_mode, stop_reason_interrupted, }; /// How long a discovered model catalog stays fresh. Discovery costs a full @@ -525,7 +526,8 @@ async fn run_session(session: Session) { )); } - // Model + edit-approval mode. A rejected set is logged, not fatal: + // Model + per-session traits + edit-approval mode. A rejected set is + // logged, not fatal: // the session still runs on Hermes's configured defaults. if let Some(model) = request.model.as_ref().filter(|m| !m.is_empty()) && current_model(&result).as_ref() != Some(model) @@ -541,6 +543,42 @@ async fn run_session(session: Session) { "session/set_model({model}) rejected; using the session default: {e}" ); } + if let Some(reasoning) = request.reasoning { + let effort = reasoning_effort(reasoning); + if let Err(e) = client + .request( + "session/set_config_option", + json!({ + "sessionId": session_id, + "configId": "reasoning_effort", + "value": effort, + }), + ) + .await + { + tracing::warn!( + target: "comet_harness::hermes", + "session reasoning effort {effort} rejected; using the session default: {e}" + ); + } + } + if let Some(tier) = service_tier(request.model.as_deref(), &request.model_options) + && let Err(e) = client + .request( + "session/set_config_option", + json!({ + "sessionId": session_id, + "configId": "service_tier", + "value": tier, + }), + ) + .await + { + tracing::warn!( + target: "comet_harness::hermes", + "session service tier {tier} rejected; using the session default: {e}" + ); + } let mode = session_mode(request.sandbox, request.auto_approve); if let Err(e) = client .request( diff --git a/crates/harness/tests/fixtures/fake-hermes.sh b/crates/harness/tests/fixtures/fake-hermes.sh index 45823d52..d9e28438 100755 --- a/crates/harness/tests/fixtures/fake-hermes.sh +++ b/crates/harness/tests/fixtures/fake-hermes.sh @@ -60,6 +60,8 @@ fi # ---- set_model / set_mode preamble, then the turn -------------------------- SAW_SET_MODEL=no SAW_MODE='' +SAW_REASONING='' +SAW_SERVICE_TIER='' promptline='' while read -r line; do case "$line" in @@ -72,6 +74,14 @@ while read -r line; do SAW_MODE=$(printf '%s' "$line" | sed 's/.*"modeId":"\([a-z_]*\)".*/\1/') emit "{\"id\":$(rid "$line"),\"result\":{}}" ;; + *'"method":"session/set_config_option"'*'"configId":"reasoning_effort"'*) + SAW_REASONING=$(printf '%s' "$line" | sed 's/.*"value":"\([a-z]*\)".*/\1/') + emit "{\"id\":$(rid "$line"),\"result\":{\"configOptions\":[]}}" + ;; + *'"method":"session/set_config_option"'*'"configId":"service_tier"'*) + SAW_SERVICE_TIER=$(printf '%s' "$line" | sed 's/.*"value":"\([a-z]*\)".*/\1/') + emit "{\"id\":$(rid "$line"),\"result\":{\"configOptions\":[]}}" + ;; *'"method":"session/prompt"'*) promptline="$line" break @@ -111,6 +121,12 @@ case "$promptline" in emit "{\"id\":$pid,\"result\":{\"stopReason\":\"end_turn\",$USAGE}}" ;; +*scenario:traits*) + [ "$SAW_REASONING" = high ] || exit 1 + [ "$SAW_SERVICE_TIER" = priority ] || exit 1 + emit "{\"id\":$pid,\"result\":{\"stopReason\":\"end_turn\"}}" + ;; + *scenario:resumed*) msg '"after resume"' emit "{\"id\":$pid,\"result\":{\"stopReason\":\"end_turn\",$USAGE}}" diff --git a/crates/harness/tests/hermes.rs b/crates/harness/tests/hermes.rs index f164639a..a2490a57 100644 --- a/crates/harness/tests/hermes.rs +++ b/crates/harness/tests/hermes.rs @@ -9,8 +9,8 @@ use tokio::sync::{mpsc, oneshot}; use comet_harness::{CancellationToken, Harness, HermesHarness, RunControls, SteerMessage}; use comet_proto::{ - AgentEvent, DoneStatus, HarnessId, RunRequest, SandboxLevel, TodoItem, ToolCall, - UserInputAnswer, + AgentEvent, DoneStatus, HarnessId, ReasoningLevel, RunRequest, SandboxLevel, TodoItem, + ToolCall, UserInputAnswer, }; fn fixture_path() -> PathBuf { @@ -213,6 +213,25 @@ async fn happy_path_maps_chunks_tool_calls_plan_usage_and_done() { )); } +#[tokio::test] +async fn reasoning_and_service_tier_are_set_before_the_turn() { + let mut req = request("scenario:traits"); + req.reasoning = Some(ReasoningLevel::High); + req.model_options + .insert("serviceTier".into(), "fast".into()); + let events = run_once(&harness(), req).await; + assert!( + matches!( + events.last(), + Some(AgentEvent::Done { + status: DoneStatus::Completed, + .. + }) + ), + "{events:?}" + ); +} + /// `session/load` replays the whole prior transcript before responding. Comet's /// doc already holds those parts, so not one replayed event may escape. #[tokio::test] @@ -512,8 +531,10 @@ async fn models_are_discovered_live_and_cached() { assert_eq!(models[0].id, "xai-oauth:grok-4.5"); assert_eq!(models[0].label, "xAI · grok-4.5"); assert_eq!(models[0].description.as_deref(), Some("Provider: xAI")); - // Hermes exposes no per-turn effort knob. + // Non-Claude/Codex providers retain the raw ACP catalog traits. assert!(models[0].reasoning_levels.is_empty()); + assert!(models[1].reasoning_levels.contains(&ReasoningLevel::XHigh)); + assert!(models[1].options.iter().any(|o| o.id == "serviceTier")); assert!(models[1].description.is_none()); let again = harness.models().await.expect("cached models"); From e3fd0909e01c5ed0eb6564121ac46e075167deab Mon Sep 17 00:00:00 2001 From: Juan Miret Date: Mon, 3 Aug 2026 14:20:06 -0300 Subject: [PATCH 3/3] Fix Hermes PATH setup after rebase --- crates/harness/src/hermes/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/harness/src/hermes/mod.rs b/crates/harness/src/hermes/mod.rs index 2b98a774..55c8f8ca 100644 --- a/crates/harness/src/hermes/mod.rs +++ b/crates/harness/src/hermes/mod.rs @@ -161,7 +161,7 @@ impl HermesHarness { let exe = self.resolve_executable()?; let mut cmd = Command::new(&exe); cmd.arg("acp"); - crate::prepend_exe_dir_to_path(&mut cmd, &exe); + crate::compose_child_path(&mut cmd, &exe); if let Some(cwd) = cwd.filter(|c| !c.is_empty()) { cmd.current_dir(cwd); }