diff --git a/devops_bench/agents/cli/hermes/__init__.py b/devops_bench/agents/cli/hermes/__init__.py new file mode 100644 index 00000000..e87bec98 --- /dev/null +++ b/devops_bench/agents/cli/hermes/__init__.py @@ -0,0 +1,26 @@ +# Copyright 2026 The Kubernetes Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Hermes CLI agent package. + +Importing this package self-registers the agent under the ``"hermes"`` key. +""" + +from devops_bench.agents.cli.hermes.agent import HermesAgent +from devops_bench.agents.cli.hermes.parsing import ( + extract_tokens_from_db, + extract_trajectory_from_db, +) + +__all__ = ["HermesAgent", "extract_tokens_from_db", "extract_trajectory_from_db"] diff --git a/devops_bench/agents/cli/hermes/agent.py b/devops_bench/agents/cli/hermes/agent.py new file mode 100644 index 00000000..2dfdfad3 --- /dev/null +++ b/devops_bench/agents/cli/hermes/agent.py @@ -0,0 +1,337 @@ +# Copyright 2026 The Kubernetes Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Hermes CLI agent harness driving the ``hermes`` binary (local-only). + +Capability wiring is delivered through hermes's native channels, laid down in +the run-scoped home directory ``$HERMES_HOME``: + +* **State isolation** — ``HERMES_HOME`` points at ``/.hermes``, so + ``config.yaml`` and the ``state.db`` session store are per-run and never + touch the user's ``~/.hermes``. The eval harness snapshots the workspace + before the run, so ``.hermes`` is copied into the run's ``generated_files/`` + as a single directory rather than scattering hermes state across the diff. +* **MCP servers** — command-bearing bindings become ``mcp_servers`` entries in + ``$HERMES_HOME/config.yaml``. +* **Skills** — ``config.capabilities.skills.paths`` are materialized under + ``$HERMES_HOME/skills//SKILL.md``. Hermes's repo-local skill discovery + is switched off and its bundled catalog is cut down to hermes's own essential + ``hermes-agent`` skill, which always survives; beyond that one, the granted + skills are all the agent is offered. +* **Rules** — ``config.capabilities.rules.text`` is prepended to the prompt + (``hermes chat`` has no dedicated system-prompt flag). +* **Model auth** — ``config.api_key`` is threaded into the provider env vars + the resolved :class:`~devops_bench.core.model_providers.ProviderSpec` names. + +Trajectory and token usage are read back from the run's SQLite ``state.db``; +the parsers live in :mod:`~devops_bench.agents.cli.hermes.parsing`. + +``__init__`` assigns ``self.rules``, ``self.mcp_servers`` and ``self.skills`` +from the granted bindings, so the agent structurally satisfies +``SupportsRules`` / ``SupportsMcp`` / ``SupportsSkills``. +""" + +from __future__ import annotations + +import io +import os +import shutil +from pathlib import Path +from typing import TYPE_CHECKING + +from ruamel.yaml import YAML + +from devops_bench.agents.base import AGENTS, AgentHarness +from devops_bench.agents.cli.hermes.parsing import ( + extract_tokens_from_db, + extract_trajectory_from_db, +) +from devops_bench.agents.config import AgentConfig +from devops_bench.agents.result import AgentResult, empty_tokens +from devops_bench.agents.shared.cli_capabilities import ( + agent_workdir, + build_mcp_servers, + materialize_skills, + mcp_isolation_env, + prepend_rules, +) +from devops_bench.core import ConfigError, SubprocessError, get_logger +from devops_bench.core.model_providers import known_providers, resolve_provider +from devops_bench.core.subprocess import run + +if TYPE_CHECKING: # pragma: no cover - typing-only import + from devops_bench.agents.capabilities import McpBinding + +__all__ = ["HermesAgent"] + +_log = get_logger("agents.cli.hermes.agent") + +_HERMES_HOME_DIRNAME = ".hermes" +_CONFIG_FILE = "config.yaml" +_STATE_DB = "state.db" +_SKILLS_DIRNAME = "skills" + +# Marker file hermes checks on startup: with it present the bundled skill sync +# is cut down to hermes's own essential skill instead of installing all 82. +# Without it every run is granted a skill catalog (``devops``, ``mlops``, ...) +# the capability matrix never granted, and the run's ``.hermes`` carries the +# ~6 MB of skill packs into the artifacts. +_NO_BUNDLED_SKILLS_MARKER = ".no-bundled-skills" + +# Installation artifacts hermes materializes inside its home: a vendored binary, +# the models.dev catalog download, and its request cache. ~22 MB per task with +# nothing to say about what the agent did, so they are dropped before the +# harness snapshots the workspace. Run evidence (state.db, config, logs) stays. +_HOME_INSTALL_ARTIFACTS: tuple[str, ...] = ( + "bin", + "cache", + "models_dev_cache.json", + "models_dev_cache.etag", +) + +# ruamel's safe (non-round-trip) loader: block style keeps the generated config +# readable if a run is inspected after the fact. +_yaml = YAML(typ="safe") +_yaml.default_flow_style = False + + +# Canonical providers hermes can serve — the ones carrying a ``hermes_provider`` +# on their :class:`ProviderSpec`. Only used to spell out the error below. +_HERMES_SUPPORTED: tuple[str, ...] = tuple( + sorted( + { + spec.canonical + for alias in known_providers() + if (spec := resolve_provider(alias)).hermes_provider + } + ) +) + + +def _hermes_provider(provider: str) -> str: + """Map a bench provider alias onto the name ``hermes chat --provider`` takes. + + Args: + provider: Raw ``AGENT_PROVIDER`` value. + + Returns: + The hermes provider name. + + Raises: + ConfigError: If hermes has no equivalent for the resolved provider. + """ + spec = resolve_provider(provider) + if spec.hermes_provider is None: + # anthropic-vertex today: hermes's "vertex" serves Gemini only, so there + # is no way to ask it for Claude on Vertex. Fail loudly rather than let + # the run answer with a Gemini model instead. + raise ConfigError( + f"provider {spec.canonical!r} has no hermes equivalent; " + f"supported: {', '.join(_HERMES_SUPPORTED)}" + ) + return spec.hermes_provider + + +def _prune_install_artifacts(home: Path) -> None: + """Drop hermes's own installation files from the run home. + + Called once the state DB has been read, so the workspace snapshot the eval + harness takes carries the run's evidence rather than a copy of the hermes + installation. Best-effort: a failed removal only costs disk. + """ + for name in _HOME_INSTALL_ARTIFACTS: + target = home / name + try: + if target.is_dir(): + shutil.rmtree(target) + elif target.exists(): + target.unlink() + except OSError as exc: + _log.warning("Failed to prune %s from the hermes run home: %s", name, exc) + + +def _build_env(config: AgentConfig) -> dict[str, str]: + """Build the subprocess env overlay carrying provider credentials. + + Raises: + ConfigError: If ``config.provider`` names no known provider. Validation + is unconditional so a typo fails here rather than on a keyless run + that would otherwise reach hermes and answer from the wrong model. + """ + overlay: dict[str, str] = {} + if config.provider: + spec = resolve_provider(config.provider) + if config.api_key: + for var in spec.api_key_envs: + overlay[var] = config.api_key + overlay.update(config.extra_env) + return overlay + + +@AGENTS.register("hermes") +class HermesAgent(AgentHarness): + """Hermes CLI agent harness driving the local ``hermes`` binary. + + The run-scoped home is never seeded from the user's ``~/.hermes``, so a + benchmark run is reproducible and never picks up ambient developer state + (nor the credentials in the user's ``.env``, which the run home would carry + into the run's artifacts). + + Args: + config: Agent configuration; ``target`` overrides the binary path. + """ + + def __init__(self, config: AgentConfig | None = None) -> None: + AgentHarness.__init__(self, config) + caps = self.config.capabilities + self.rules = caps.rules + self.mcp_servers = caps.mcp_servers + self.skills = caps.skills + + def _resolve_hermes_bin(self) -> str: + """Resolve the ``hermes`` binary path, preferring an explicit target.""" + if self.config.target: + return os.path.expanduser(self.config.target) + candidate = os.path.expanduser("~/.local/bin/hermes") + return candidate if os.path.exists(candidate) else "hermes" + + def _prepare_config(self, run_dir: Path, mcp_servers: tuple[McpBinding, ...]) -> None: + """Write the run-scoped ``config.yaml`` granting the run's MCP servers. + + The run home is created fresh per run and is never seeded from the + user's ``~/.hermes`` — a benchmark run has to be reproducible and must + not pick up ambient developer state — so the document is built outright + rather than merged into whatever was already there. + + Also lays down the opt-out marker for hermes's bundled skill catalog, so + the granted skills are the only ones the agent is offered beyond hermes's + own essential ``hermes-agent`` skill, which the marker cannot remove. + """ + (run_dir / _NO_BUNDLED_SKILLS_MARKER).touch() + + # Hermes otherwise sources ``/.hermes/skills`` and + # ``/.agents/skills`` when the session starts inside a + # checkout, which would hand the agent skills the run never granted. + config_data: dict = {"skills": {"project_discovery": False}} + + servers = build_mcp_servers(mcp_servers) + if servers: + # MCP servers are spawned by hermes, so they inherit its env rather + # than the harness's; pass the run's isolation vars through + # explicitly or the servers fall back to the ambient ones. + for key, value in mcp_isolation_env(self.config.extra_env).items(): + for entry in servers.values(): + entry.setdefault("env", {})[key] = value + config_data["mcp_servers"] = servers + + buffer = io.StringIO() + _yaml.dump(config_data, buffer) + (run_dir / _CONFIG_FILE).write_text(buffer.getvalue(), encoding="utf-8") + + def _build_command(self, prompt: str) -> list[str]: + """Build the ``hermes chat`` argv for this run. + + Raises: + ConfigError: If the resolved provider has no hermes equivalent. + """ + # ``--query=`` as one token: the prompt is attacker-influenced + # task text, and a separate argv element starting with ``-`` would be + # parsed as a flag. + cmd = [self._resolve_hermes_bin(), "chat", f"--query={prompt}"] + if self.config.model: + cmd.extend(["-m", self.config.model]) + if self.config.provider: + cmd.extend(["--provider", _hermes_provider(self.config.provider)]) + return cmd + + def _execute(self, prompt: str, workspace_path: Path | None = None) -> AgentResult: + caps = self.config.capabilities + + with agent_workdir(workspace_path, prefix="hermes-run-") as workdir: + hermes_home = workdir / _HERMES_HOME_DIRNAME + hermes_home.mkdir(parents=True, exist_ok=True) + self._prepare_config(hermes_home, caps.mcp_servers) + if caps.skills.paths: + materialize_skills(hermes_home / _SKILLS_DIRNAME, caps.skills.paths) + + env_overlay = _build_env(self.config) + env_overlay["HERMES_HOME"] = str(hermes_home) + db_path = hermes_home / _STATE_DB + + try: + completed = run( + self._build_command(prepend_rules(caps.rules.text, prompt)), + check=False, + cwd=str(workdir), + timeout=self.config.timeout_sec, + extra_env=env_overlay, + ) + except SubprocessError as exc: + # With check=False the only SubprocessError left is the timeout. + # The killed run still flushed whatever it completed, so report + # that partial trajectory rather than discarding the run. + trajectory, errors = extract_trajectory_from_db(db_path) + tokens = extract_tokens_from_db(db_path) + _prune_install_artifacts(hermes_home) + return AgentResult( + output=( + f"Timeout expired.\n\n=== STDOUT ===\n{exc.stdout or ''}" + f"\n\n=== STDERR ===\n{exc.stderr or ''}" + ), + trajectory=trajectory, + tokens=tokens, + errors=[f"hermes agent timed out after {self.config.timeout_sec:g}s", *errors], + metadata={"timeout": True}, + ) + except OSError as exc: + # Binary missing/not executable: same canonical all-None token + # shape as every other path, so the row reads "unavailable". + return AgentResult( + output=f"Error: hermes binary unavailable: {exc}", + trajectory=[], + tokens=empty_tokens(), + errors=[f"hermes binary unavailable: {exc}"], + ) + + errors: list[str] = [] + metadata: dict = {} + if completed.returncode != 0: + stderr = (completed.stderr or "").strip() + errors.append( + f"hermes agent exited {completed.returncode}: {stderr or ''}" + ) + metadata["returncode"] = completed.returncode + + trajectory, export_errors = extract_trajectory_from_db(db_path) + errors.extend(export_errors) + tokens = extract_tokens_from_db(db_path) + if completed.returncode == 0 and not tokens["total"]: + # hermes exits 0 on an unknown provider, a rejected API key, or + # an API call whose retries all failed. Without this the run + # looks like a genuinely bad answer instead of an infra failure, + # because no model usage was ever recorded. + stderr = (completed.stderr or "").strip() + errors.append( + "hermes agent recorded no model usage despite exiting 0; " + f"the model was never reached: {stderr or ''}" + ) + _prune_install_artifacts(hermes_home) + + return AgentResult( + output=completed.stdout or "", + trajectory=trajectory, + tokens=tokens, + errors=errors, + metadata=metadata, + ) diff --git a/devops_bench/agents/cli/hermes/parsing.py b/devops_bench/agents/cli/hermes/parsing.py new file mode 100644 index 00000000..b9109cd1 --- /dev/null +++ b/devops_bench/agents/cli/hermes/parsing.py @@ -0,0 +1,205 @@ +# Copyright 2026 The Kubernetes Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Trajectory and token parsers for the Hermes ``state.db`` SQLite database.""" + +from __future__ import annotations + +import json +import sqlite3 +from pathlib import Path +from urllib.parse import quote + +from devops_bench.agents.result import ToolCall, empty_tokens + +__all__: list[str] = ["extract_tokens_from_db", "extract_trajectory_from_db"] + +# Hermes ``sessions`` columns -> the canonical buckets in +# :data:`~devops_bench.agents.result.TOKEN_BUCKETS`. Hermes normalizes every +# provider's usage into its own ``CanonicalUsage`` before persisting, so +# ``input_tokens`` already excludes cache reads and writes. +_SESSION_TOKEN_COLUMNS: dict[str, str] = { + "input_tokens": "input", + "cache_read_tokens": "cached", + "cache_write_tokens": "cache_write", + "reasoning_tokens": "reasoning", + "output_tokens": "output", +} + + +def _connect_ro(db_path: Path) -> sqlite3.Connection: + """Open ``db_path`` read-only, so a live ``state.db`` is never locked.""" + # quote() keeps a ``?`` or ``#`` in the path from being read as the URI's + # query or fragment delimiter. + return sqlite3.connect(f"file:{quote(str(db_path))}?mode=ro", uri=True) + + +def extract_tokens_from_db(db_path: Path) -> dict[str, int | None]: + """Read canonical token usage from a Hermes ``state.db``. + + The ``sessions`` table carries per-session token counts (``input_tokens`` / + ``output_tokens`` / ``reasoning_tokens`` / ``cache_read_tokens`` / + ``cache_write_tokens``), summed across all sessions — the DB is run-scoped, + and a run may write more than one session row. Never raises: an older + Hermes schema (missing columns), a missing/corrupt DB, or any read failure + yields all-``None`` buckets rather than a fabricated ``0``. + + Hermes stores ``output_tokens`` as the provider's full completion count with + ``reasoning_tokens`` as a subset of it, whereas the canonical ``output`` + bucket excludes reasoning; reasoning is subtracted out here so ``total`` + counts it once and still matches Hermes's own prompt-plus-completion figure. + + Args: + db_path: Path to the run's ``state.db``. + + Returns: + The canonical token dict; ``total`` is the sum of the reported buckets, + or ``None`` when nothing was reported. + """ + tokens = empty_tokens() + if not db_path.exists(): + return tokens + select = ", ".join(f"SUM({column})" for column in _SESSION_TOKEN_COLUMNS) + try: + conn = _connect_ro(db_path) + try: + row = conn.execute(f"SELECT {select} FROM sessions").fetchone() + finally: + conn.close() + except sqlite3.Error: + return tokens + if row is None: + return tokens + for bucket, value in zip(_SESSION_TOKEN_COLUMNS.values(), row, strict=True): + # SUM yields int, float (REAL affinity), or NULL for an all-NULL column. + if isinstance(value, int | float) and not isinstance(value, bool): + tokens[bucket] = int(value) + if tokens["output"] is not None and tokens["reasoning"]: + tokens["output"] = max(0, tokens["output"] - tokens["reasoning"]) + reported = [ + value for bucket in _SESSION_TOKEN_COLUMNS.values() if (value := tokens[bucket]) is not None + ] + if reported: + tokens["total"] = sum(reported) + return tokens + + +def _parse_arguments(raw: object) -> dict: + """Normalize a tool call's ``arguments`` field to a dict. + + Providers disagree on the encoding: OpenAI-style adapters send a JSON + *string*, others send an already-decoded object. Anything that is neither is + preserved verbatim under ``raw_args`` rather than dropped. + """ + if raw is None: + return {} + if isinstance(raw, dict): + return raw + if isinstance(raw, str): + try: + decoded = json.loads(raw) + except json.JSONDecodeError: + return {"raw_args": raw} + return decoded if isinstance(decoded, dict) else {"raw_args": raw} + return {"raw_args": raw} + + +def extract_trajectory_from_db(db_path: Path) -> tuple[list[dict], list[str]]: + """Extract the canonical trajectory from a Hermes ``state.db``. + + Reads the ``messages`` rows of every session in the run-scoped DB: an + ``assistant`` row's ``tool_calls`` JSON opens a + :class:`~devops_bench.agents.result.ToolCall`, and the matching ``tool`` row + (keyed on ``tool_call_id``) folds the result in. All sessions are read, not + just the newest, so the trajectory covers the same rows + :func:`extract_tokens_from_db` sums its usage over. Extraction misses are + reported on the returned error list rather than yielding a silent-empty + trajectory. + + Args: + db_path: Path to the run's ``state.db``. + + Returns: + A ``(trajectory, errors)`` tuple. ``trajectory`` is a list of + ``ToolCall.to_dict()`` mappings in call order. + """ + errors: list[str] = [] + trajectory: list[dict] = [] + + if not db_path.exists(): + errors.append(f"State database not found at {db_path}") + return [], errors + + try: + conn = _connect_ro(db_path) + try: + cursor = conn.cursor() + if not cursor.execute("SELECT COUNT(*) FROM sessions").fetchone()[0]: + errors.append("No session found in state database") + return [], errors + + # Ordered by ``sessions.rowid``, not by ``id``: hermes session ids + # are UUIDs, so lexical ordering interleaves sessions arbitrarily. + # rowid is insertion order and exists on every rowid table, whatever + # the schema version. + cursor.execute( + "SELECT m.role, m.content, m.tool_calls, m.tool_call_id, m.tool_name" + " FROM messages m JOIN sessions s ON m.session_id = s.id" + " ORDER BY s.rowid, m.id" + ) + messages = cursor.fetchall() + finally: + conn.close() + except sqlite3.Error as exc: + errors.append(f"Database error: {exc}") + return [], errors + + calls_by_id: dict[str, dict] = {} + for role, content, tool_calls_json, tool_call_id, tool_name in messages: + if role == "assistant" and tool_calls_json: + try: + tool_calls = json.loads(tool_calls_json) + except json.JSONDecodeError as exc: + errors.append(f"Failed to parse tool calls JSON: {exc}") + continue + if not isinstance(tool_calls, list): + errors.append(f"Tool calls JSON is not a list: {type(tool_calls).__name__}") + continue + for entry in tool_calls: + if not isinstance(entry, dict): + errors.append(f"Skipped non-object tool call entry: {type(entry).__name__}") + continue + function = entry.get("function") + if not isinstance(function, dict): + function = {} + args = _parse_arguments(function.get("arguments")) + call = ToolCall(name=function.get("name") or "unknown", args=args, status="called") + trajectory.append(call.to_dict()) + if entry.get("id"): + calls_by_id[entry["id"]] = trajectory[-1] + elif role == "tool" and tool_call_id: + pending = calls_by_id.get(tool_call_id) + if pending is not None: + pending["result"] = content + pending["status"] = "completed" + else: + # An orphan result still belongs in the trajectory (the call row + # may predate the session cut), but the gap is worth surfacing. + call = ToolCall( + name=tool_name or "unknown", args={}, result=content, status="completed" + ) + trajectory.append(call.to_dict()) + errors.append(f"Found tool response for unknown tool_call_id: {tool_call_id}") + + return trajectory, errors diff --git a/devops_bench/agents/cli/openclaw/agent.py b/devops_bench/agents/cli/openclaw/agent.py index fa844b26..baca88b4 100644 --- a/devops_bench/agents/cli/openclaw/agent.py +++ b/devops_bench/agents/cli/openclaw/agent.py @@ -73,6 +73,8 @@ agent_workdir, build_mcp_servers, materialize_skills, + mcp_isolation_env, + prepend_rules, ) from devops_bench.core import SubprocessError, get_logger from devops_bench.core.errors import ConfigError @@ -255,17 +257,19 @@ def _build_openclaw_config(config: AgentConfig, mcp_servers: tuple[McpBinding, . binding nor a model needing a catalog entry (caller then skips the config write and leaves ``OPENCLAW_CONFIG_PATH`` unset). - Each MCP server entry inherits the run's ``KUBECONFIG`` (set by ``RunEnv``) as - an explicit ``env`` so the MCP server (e.g. gke-mcp) reads the run-scoped - cluster credentials directly instead of forcing the agent to re-fetch them. + Each MCP server entry gets the run-isolation vars as an explicit ``env`` + (resolved by + :func:`~devops_bench.agents.shared.cli_capabilities.mcp_isolation_env`) so + the MCP server (e.g. gke-mcp) reads the run-scoped cluster and gcloud config + directly instead of the operator's ambient ones. openclaw spawns the servers + itself, so they inherit its filtered env and not the harness's. """ payload: dict = {} servers = build_mcp_servers(mcp_servers) if servers: - kubeconfig = os.environ.get("KUBECONFIG") - if kubeconfig: + for key, value in mcp_isolation_env(config.extra_env).items(): for entry in servers.values(): - entry.setdefault("env", {})["KUBECONFIG"] = kubeconfig + entry.setdefault("env", {})[key] = value payload["mcp"] = {"servers": servers} payload.update(_build_model_override(config)) return payload @@ -327,25 +331,6 @@ def _oc_model_flag(config: AgentConfig) -> str: return f"--model {shlex.quote(model_id)} " -def _prepend_rules(rules_text: str, prompt: str) -> str: - """Return ``prompt`` with ``rules_text`` prepended as an operator brief. - - Empty / whitespace-only rules pass the prompt through unchanged so a default - :class:`AgentRules` is indistinguishable from "no preamble". A non-empty - brief is separated from the prompt by a blank line. - - Args: - rules_text: The bound rules text (``capabilities.rules.text``). - prompt: The task prompt for this run. - - Returns: - The combined string to hand to ``oc agent -m``. - """ - if not rules_text or not rules_text.strip(): - return prompt - return f"{rules_text.rstrip()}\n\n{prompt}" - - def _build_local_command(config: AgentConfig, prompt: str, agent_name: str, oc_bin: str) -> str: """Build the bash command that sets the model and runs ``oc agent --local``. @@ -433,7 +418,7 @@ def _execute(self, prompt: str, workspace_path: Path | None = None) -> AgentResu """ caps = self.config.capabilities oc_bin = self._resolve_oc_bin() - final_prompt = _prepend_rules(caps.rules.text, prompt) + final_prompt = prepend_rules(caps.rules.text, prompt) with agent_workdir(workspace_path, prefix="oc-run-") as workdir: state_dir = workdir / _OPENCLAW_STATE_DIRNAME diff --git a/devops_bench/agents/shared/cli_capabilities.py b/devops_bench/agents/shared/cli_capabilities.py index e271e42c..6ec7a98a 100644 --- a/devops_bench/agents/shared/cli_capabilities.py +++ b/devops_bench/agents/shared/cli_capabilities.py @@ -12,11 +12,12 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Capability materialization shared by the CLI agents (Gemini, openclaw). +"""Capability materialization shared by the CLI agents (Gemini, openclaw, hermes). -Both CLI agents render granted MCP bindings into a ``{name: {command, args}}`` -launch map and copy discovered ``SKILL.md`` files into the binary's workspace -skills tree. Importing this module pulls no provider SDK. +The CLI agents render granted MCP bindings into a ``{name: {command, args}}`` +launch map, copy discovered ``SKILL.md`` files into the binary's workspace skills +tree, prepend the granted rules to the prompt, and forward the run's isolation +env vars to the MCP children. Importing this module pulls no provider SDK. """ from __future__ import annotations @@ -24,17 +25,24 @@ import contextlib import os import tempfile -from collections.abc import Iterator +from collections.abc import Iterator, Mapping from pathlib import Path from typing import TYPE_CHECKING from devops_bench.agents.shared.skills import iter_skills from devops_bench.core import get_logger +from devops_bench.core.run_env import CHILD_SCOPED_ENVS if TYPE_CHECKING: from devops_bench.agents.capabilities import McpBinding -__all__ = ["agent_workdir", "build_mcp_servers", "materialize_skills"] +__all__ = [ + "agent_workdir", + "build_mcp_servers", + "materialize_skills", + "mcp_isolation_env", + "prepend_rules", +] _log = get_logger("agents.shared.cli_capabilities") @@ -106,6 +114,63 @@ def build_mcp_servers(mcp_servers: tuple[McpBinding, ...]) -> dict[str, dict]: return servers +def mcp_isolation_env(extra_env: Mapping[str, str]) -> dict[str, str]: + """Resolve the run-isolation vars an MCP child needs, in the CLI's precedence. + + A CLI spawns its MCP servers itself, so they inherit the CLI's environment + rather than the harness's — and both hermes and openclaw filter that child + env down to an allowlist plus whatever the server config names. Each var in + :data:`~devops_bench.core.run_env.CHILD_SCOPED_ENVS` therefore has to be + written into the server entry explicitly, or the server falls back to the + operator's ambient config instead of the run's. + + The same set applies to every CLI rather than a per-agent list: forwarding + one of these *narrows* what the child can reach, so a shorter list is not a + tighter sandbox but a looser one. + + ``run_env`` publishes the per-run values on ``os.environ`` while + ``extra_env`` is the operator's override, which wins — the same precedence + the CLI's own env overlay uses, so the servers and the CLI agree on which + cluster they are pointed at. Presence in ``extra_env`` decides, not + truthiness: an override set to ``""`` means "unset this", not "fall back to + the ambient value". + + Args: + extra_env: ``config.extra_env`` — the operator's override mapping. + + Returns: + The resolved ``{name: value}`` mapping, omitting anything that resolved + to empty or was set nowhere. + """ + resolved: dict[str, str] = {} + for key in CHILD_SCOPED_ENVS: + value = extra_env[key] if key in extra_env else os.environ.get(key) + if value: + resolved[key] = value + return resolved + + +def prepend_rules(rules_text: str, prompt: str) -> str: + """Return ``prompt`` with ``rules_text`` prepended as an operator brief. + + For the CLIs with no dedicated system-prompt flag, the granted rules ride on + the prompt itself. Empty / whitespace-only rules pass the prompt through + unchanged, so a default :class:`~devops_bench.agents.capabilities.AgentRules` + is indistinguishable from "no preamble"; a non-empty brief is separated from + the prompt by a blank line. + + Args: + rules_text: The bound rules text (``capabilities.rules.text``). + prompt: The task prompt for this run. + + Returns: + The combined string to hand to the CLI. + """ + if not rules_text or not rules_text.strip(): + return prompt + return f"{rules_text.rstrip()}\n\n{prompt}" + + def materialize_skills(skills_root: Path, paths: tuple[str, ...]) -> list[str]: """Copy discovered ``SKILL.md`` files into a CLI's workspace skills tree. diff --git a/devops_bench/core/model_providers.py b/devops_bench/core/model_providers.py index 22e986f4..bc9a0932 100644 --- a/devops_bench/core/model_providers.py +++ b/devops_bench/core/model_providers.py @@ -46,6 +46,12 @@ class ProviderSpec(BaseModel): ``MODELS.get`` (e.g. ``gemini`` / ``claude`` / ``ollama``). oc_provider: openclaw wire-provider id used in ``provider/model`` and the per-run ``_PROVIDER_TRANSPORT`` lookup. + hermes_provider: Name ``hermes chat --provider`` accepts, defaulting to + ``None`` — hermes cannot serve this provider at all. Not derivable from + ``adapter_family``: hermes's ``vertex`` is Google-Vertex-Gemini only + (so ``anthropic-vertex`` has no equivalent and must fail loudly + rather than answer from a Gemini model) and its OpenAI transport is + spelled ``openai-api``. api_key_envs: Env var name(s) a CLI harness sets from ``config.api_key``. Empty for ``anthropic-vertex`` / ``anthropic-bedrock`` / ``ollama`` (no key is ever threaded). ``google-vertex`` is keyless-ok but still @@ -61,6 +67,7 @@ class ProviderSpec(BaseModel): canonical: str adapter_family: str oc_provider: str + hermes_provider: str | None = None api_key_envs: tuple[str, ...] keyless_ok: bool backend: str | None = None @@ -78,6 +85,7 @@ class ProviderSpec(BaseModel): canonical="google", adapter_family="gemini", oc_provider="google", + hermes_provider="gemini", api_key_envs=("GEMINI_API_KEY", "GOOGLE_API_KEY"), keyless_ok=False, backend=None, @@ -86,6 +94,7 @@ class ProviderSpec(BaseModel): canonical="google-vertex", adapter_family="gemini", oc_provider="google-vertex", + hermes_provider="vertex", api_key_envs=("GOOGLE_CLOUD_API_KEY",), keyless_ok=True, backend="vertex", @@ -94,6 +103,7 @@ class ProviderSpec(BaseModel): canonical="anthropic", adapter_family="claude", oc_provider="anthropic", + hermes_provider="anthropic", api_key_envs=("ANTHROPIC_API_KEY",), keyless_ok=False, backend=None, # claude infers api/vertex/bedrock from the environment @@ -102,6 +112,7 @@ class ProviderSpec(BaseModel): canonical="anthropic-vertex", adapter_family="claude", oc_provider="anthropic-vertex", + hermes_provider=None, api_key_envs=(), keyless_ok=True, backend="vertex", @@ -110,6 +121,7 @@ class ProviderSpec(BaseModel): canonical="anthropic-bedrock", adapter_family="claude", oc_provider="anthropic-bedrock", + hermes_provider="bedrock", api_key_envs=(), keyless_ok=True, backend="bedrock", @@ -118,6 +130,7 @@ class ProviderSpec(BaseModel): canonical="openai", adapter_family="openai", # no adapter module today: get_model raises NotRegisteredError oc_provider="openai", + hermes_provider="openai-api", api_key_envs=("OPENAI_API_KEY",), keyless_ok=False, backend=None, @@ -126,6 +139,7 @@ class ProviderSpec(BaseModel): canonical="ollama", adapter_family="ollama", oc_provider="ollama", + hermes_provider="ollama", api_key_envs=(), # optional key handled by the adapter via AGENT_API_KEY keyless_ok=True, backend=None, diff --git a/devops_bench/core/run_env.py b/devops_bench/core/run_env.py index 7758d7f7..e99f40c1 100644 --- a/devops_bench/core/run_env.py +++ b/devops_bench/core/run_env.py @@ -42,10 +42,17 @@ from devops_bench.core.config import get_env from devops_bench.core.logging import get_logger -__all__ = ["RunEnv"] +__all__ = ["CHILD_SCOPED_ENVS", "RunEnv"] _log = get_logger("core.run_env") +#: The run-scoped config paths a child process must inherit to stay inside the +#: run. A subset of :data:`RunEnv._MUTATED_KEYS` — the run id and the parallel +#: flag are bookkeeping, not isolation. Lives here rather than in the agent +#: layer because this module is what points them at per-run paths: a var added +#: below has exactly one place to be added here too. +CHILD_SCOPED_ENVS: tuple[str, ...] = ("KUBECONFIG", "CLOUDSDK_CONFIG") + # GKE cluster names are capped at 40 chars and must match # ``[a-z]([-a-z0-9]*[a-z0-9])?``. _MAX_CLUSTER_NAME = 40 diff --git a/devops_bench/evalharness/default.py b/devops_bench/evalharness/default.py index 35b9ccdb..aaefb1b8 100644 --- a/devops_bench/evalharness/default.py +++ b/devops_bench/evalharness/default.py @@ -73,6 +73,7 @@ "devops_bench.agents.cli.gemini_cli", "devops_bench.agents.cli.openclaw", "devops_bench.agents.cli.antigravity", + "devops_bench.agents.cli.hermes", "devops_bench.agents.api.agent", ) diff --git a/docs/how-to/add-a-model-provider.md b/docs/how-to/add-a-model-provider.md index d3422ccd..03ce551b 100644 --- a/docs/how-to/add-a-model-provider.md +++ b/docs/how-to/add-a-model-provider.md @@ -83,6 +83,7 @@ _SPECS["your-key"] = ProviderSpec( canonical="your-key", adapter_family="", # the MODELS.get() key from step 1 oc_provider="your-key", # openclaw wire-provider id + hermes_provider="your-key", # `hermes chat --provider` name, None if unsupported api_key_envs=("YOURPROVIDER_API_KEY",), # () if keyless keyless_ok=False, backend=None, # or a backend hint your adapter reads diff --git a/tests/unit/agents/shared/test_cli_capabilities.py b/tests/unit/agents/shared/test_cli_capabilities.py index aa25f4a1..e6eb5f90 100644 --- a/tests/unit/agents/shared/test_cli_capabilities.py +++ b/tests/unit/agents/shared/test_cli_capabilities.py @@ -12,11 +12,13 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Tests for the CLI capability helpers shared by the Gemini/openclaw agents.""" +"""Tests for the CLI capability helpers shared by the Gemini/openclaw/hermes agents.""" from __future__ import annotations +import os from pathlib import Path +from unittest.mock import patch import pytest @@ -25,6 +27,8 @@ agent_workdir, build_mcp_servers, materialize_skills, + mcp_isolation_env, + prepend_rules, ) @@ -172,3 +176,32 @@ def test_agent_workdir_creates_and_cleans_up_temp_dir_when_no_path_supplied() -> assert workdir.name.startswith("agent-workdir-test-") assert not created.exists() + + +def test_prepend_rules_passes_the_prompt_through_when_rules_are_blank() -> None: + """A default ``AgentRules`` must be indistinguishable from "no preamble".""" + assert prepend_rules("", "do the thing") == "do the thing" + assert prepend_rules(" \n ", "do the thing") == "do the thing" + + +def test_prepend_rules_separates_the_brief_from_the_prompt_with_a_blank_line() -> None: + assert prepend_rules("be careful\n", "audit pods") == "be careful\n\naudit pods" + + +def test_mcp_isolation_env_reads_the_ambient_run_values() -> None: + with patch.dict(os.environ, {"KUBECONFIG": "/run/kubeconfig"}, clear=True): + assert mcp_isolation_env({}) == {"KUBECONFIG": "/run/kubeconfig"} + + +def test_mcp_isolation_env_prefers_the_configured_override() -> None: + """``extra_env`` wins, as it does in each CLI's own env overlay.""" + with patch.dict(os.environ, {"KUBECONFIG": "/run/kubeconfig"}, clear=True): + resolved = mcp_isolation_env({"KUBECONFIG": "/override"}) + + assert resolved == {"KUBECONFIG": "/override"} + + +def test_mcp_isolation_env_treats_a_blank_override_as_unset() -> None: + """Presence decides, not truthiness: ``""`` must not fall back to ambient.""" + with patch.dict(os.environ, {"KUBECONFIG": "/run/kubeconfig"}, clear=True): + assert mcp_isolation_env({"KUBECONFIG": ""}) == {} diff --git a/tests/unit/agents/test_agents_cli_hermes.py b/tests/unit/agents/test_agents_cli_hermes.py new file mode 100644 index 00000000..3f987ab6 --- /dev/null +++ b/tests/unit/agents/test_agents_cli_hermes.py @@ -0,0 +1,819 @@ +# Copyright 2026 The Kubernetes Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Unit tests for the Hermes CLI agent harness and its ``state.db`` parsers.""" + +from __future__ import annotations + +import json +import os +import sqlite3 +from collections.abc import Callable +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +import pytest +from ruamel.yaml import YAML + +from devops_bench.agents.base import AGENTS +from devops_bench.agents.capabilities import AllCapabilities, McpBinding, SkillBinding +from devops_bench.agents.cli.hermes import agent as agent_mod +from devops_bench.agents.cli.hermes.agent import HermesAgent, _build_env +from devops_bench.agents.cli.hermes.parsing import ( + extract_tokens_from_db, + extract_trajectory_from_db, +) +from devops_bench.agents.config import AgentConfig +from devops_bench.agents.result import TOKEN_BUCKETS, empty_tokens +from devops_bench.core.errors import ConfigError, SubprocessError +from devops_bench.results.normalize import normalize_tokens + +_yaml = YAML(typ="safe") + +_TOKEN_COLUMNS = ( + "input_tokens", + "output_tokens", + "reasoning_tokens", + "cache_read_tokens", + "cache_write_tokens", +) + + +@pytest.fixture +def db_path(tmp_path: Path) -> Path: + """Path a test ``state.db`` is created at (the file itself is not made).""" + return tmp_path / "state.db" + + +def _init_schema(path: Path) -> None: + """Create the ``sessions`` / ``messages`` tables hermes writes.""" + token_columns = "".join(f", {column} INTEGER" for column in _TOKEN_COLUMNS) + conn = sqlite3.connect(path) + conn.execute( + f"CREATE TABLE sessions (id TEXT PRIMARY KEY, started_at TIMESTAMP{token_columns})" + ) + conn.execute( + "CREATE TABLE messages (id INTEGER PRIMARY KEY AUTOINCREMENT, session_id TEXT," + " role TEXT, content TEXT, tool_calls TEXT, tool_call_id TEXT, tool_name TEXT)" + ) + conn.commit() + conn.close() + + +def _insert_session(path: Path, session_id: str, *counts: int | float | None) -> None: + conn = sqlite3.connect(path) + columns = ", ".join(_TOKEN_COLUMNS) + placeholders = ", ".join("?" * len(_TOKEN_COLUMNS)) + conn.execute( + f"INSERT INTO sessions (id, started_at, {columns}) VALUES (?, NULL, {placeholders})", + (session_id, *counts), + ) + conn.commit() + conn.close() + + +def _insert_message(path: Path, session_id: str, role: str, **fields: str | None) -> None: + conn = sqlite3.connect(path) + keys = ["session_id", "role", *fields] + placeholders = ", ".join("?" * len(keys)) + conn.execute( + f"INSERT INTO messages ({', '.join(keys)}) VALUES ({placeholders})", + (session_id, role, *fields.values()), + ) + conn.commit() + conn.close() + + +def _tool_calls_json(call_id: str | None, name: str | None, args: dict | str) -> str: + function: dict = {"arguments": args if isinstance(args, str) else json.dumps(args)} + if name is not None: + function["name"] = name + entry: dict = {"type": "function", "function": function} + if call_id is not None: + entry["id"] = call_id + return json.dumps([entry]) + + +# --- registration & configuration ------------------------------------------- + + +def test_registers_under_the_canonical_key() -> None: + """The eval harness resolves the agent by its lowercase registry key.""" + assert AGENTS.get("hermes") is HermesAgent + + +def test_build_env_routes_the_api_key_to_the_provider_vars() -> None: + env = _build_env( + AgentConfig(provider="google", api_key="test-key", extra_env={"FOO": "BAR"}), + ) + + assert env["GEMINI_API_KEY"] == "test-key" + assert env["GOOGLE_API_KEY"] == "test-key" + assert env["FOO"] == "BAR" + + +def test_build_env_omits_key_vars_when_no_key_is_configured() -> None: + """A keyless (ADC / Vertex) run must not export an empty credential.""" + assert _build_env(AgentConfig(provider="google-vertex")) == {} + + +def test_resolve_hermes_bin_prefers_the_configured_target() -> None: + agent = HermesAgent(AgentConfig(target="/custom/bin/hermes")) + assert agent._resolve_hermes_bin() == "/custom/bin/hermes" + + +@patch("os.path.exists") +def test_resolve_hermes_bin_falls_back_to_path_lookup(mock_exists: MagicMock) -> None: + agent = HermesAgent(AgentConfig(target=None)) + + mock_exists.return_value = True + assert agent._resolve_hermes_bin() == os.path.expanduser("~/.local/bin/hermes") + + mock_exists.return_value = False + assert agent._resolve_hermes_bin() == "hermes" + + +def test_build_command_maps_vertex_onto_the_vertex_backend() -> None: + cmd = HermesAgent( + AgentConfig(target="/bin/hermes", model="gemini-3-pro", provider="google-vertex") + )._build_command("hi") + + assert cmd == [ + "/bin/hermes", + "chat", + "--query=hi", + "-m", + "gemini-3-pro", + "--provider", + "vertex", + ] + + +@pytest.mark.parametrize( + ("provider", "expected"), + [ + ("google", "gemini"), + ("google-vertex", "vertex"), + ("anthropic", "anthropic"), + ("anthropic-bedrock", "bedrock"), + ("openai", "openai-api"), + ("ollama", "ollama"), + ], +) +def test_build_command_maps_each_provider_onto_a_hermes_provider_name( + provider: str, expected: str +) -> None: + """The emitted name must be one hermes's own registry accepts.""" + cmd = HermesAgent(AgentConfig(target="/bin/hermes", provider=provider))._build_command("hi") + + assert cmd == ["/bin/hermes", "chat", "--query=hi", "--provider", expected] + + +def test_build_command_rejects_a_provider_hermes_cannot_serve() -> None: + """Hermes's ``vertex`` is Gemini-only, so Claude-on-Vertex must not silently + route to a Gemini model.""" + agent = HermesAgent(AgentConfig(target="/bin/hermes", provider="anthropic-vertex")) + + with pytest.raises(ConfigError, match="no hermes equivalent"): + agent._build_command("hi") + + +def test_build_command_binds_a_dash_leading_prompt_to_the_query_flag() -> None: + """A prompt starting with ``-`` must not be parsed as a hermes flag.""" + cmd = HermesAgent(AgentConfig(target="/bin/hermes"))._build_command("--help me") + + assert cmd == ["/bin/hermes", "chat", "--query=--help me"] + + +def test_prepare_config_writes_the_granted_mcp_servers(tmp_path: Path) -> None: + agent = HermesAgent(AgentConfig()) + binding = McpBinding(name="k8s", command=("k8s-mcp", "--stdio")) + + agent._prepare_config(tmp_path, (binding,)) + + data = _yaml.load((tmp_path / "config.yaml").read_text(encoding="utf-8")) + assert data["mcp_servers"] == {"k8s": {"command": "k8s-mcp", "args": ["--stdio"]}} + + +def test_prepare_config_opts_out_of_the_skills_the_run_never_granted(tmp_path: Path) -> None: + """Hermes installs 82 bundled skills and sources repo-local ones; either + would hand the agent capabilities the matrix withheld.""" + HermesAgent(AgentConfig())._prepare_config(tmp_path, ()) + + assert (tmp_path / ".no-bundled-skills").exists() + data = _yaml.load((tmp_path / "config.yaml").read_text(encoding="utf-8")) + assert data["skills"]["project_discovery"] is False + + +def test_prune_install_artifacts_keeps_the_run_evidence(tmp_path: Path) -> None: + (tmp_path / "bin").mkdir() + (tmp_path / "bin" / "tirith").write_bytes(b"\x00") + (tmp_path / "cache").mkdir() + (tmp_path / "models_dev_cache.json").write_text("{}", encoding="utf-8") + (tmp_path / "state.db").write_bytes(b"\x00") + (tmp_path / "config.yaml").write_text("{}", encoding="utf-8") + + agent_mod._prune_install_artifacts(tmp_path) + + assert not (tmp_path / "bin").exists() + assert not (tmp_path / "cache").exists() + assert not (tmp_path / "models_dev_cache.json").exists() + assert (tmp_path / "state.db").exists() + assert (tmp_path / "config.yaml").exists() + + +def test_prepare_config_forwards_the_run_isolation_env_to_mcp_servers(tmp_path: Path) -> None: + """hermes filters an MCP child's env to an allowlist plus the declared keys, + so the run's cluster and cloud config have to be named explicitly.""" + agent = HermesAgent(AgentConfig()) + + with patch.dict( + os.environ, {"KUBECONFIG": "/run/kubeconfig", "CLOUDSDK_CONFIG": "/run/gcloud"} + ): + agent._prepare_config(tmp_path, (McpBinding(name="k8s", command=("k8s-mcp",)),)) + + data = _yaml.load((tmp_path / "config.yaml").read_text(encoding="utf-8")) + assert data["mcp_servers"]["k8s"]["env"] == { + "KUBECONFIG": "/run/kubeconfig", + "CLOUDSDK_CONFIG": "/run/gcloud", + } + + +def test_prepare_config_prefers_the_configured_isolation_override(tmp_path: Path) -> None: + """``extra_env`` wins over the ambient value, as it does in ``_build_env``.""" + agent = HermesAgent(AgentConfig(extra_env={"KUBECONFIG": "/override/kubeconfig"})) + + with patch.dict(os.environ, {"KUBECONFIG": "/run/kubeconfig"}): + agent._prepare_config(tmp_path, (McpBinding(name="k8s", command=("k8s-mcp",)),)) + + data = _yaml.load((tmp_path / "config.yaml").read_text(encoding="utf-8")) + assert data["mcp_servers"]["k8s"]["env"]["KUBECONFIG"] == "/override/kubeconfig" + + +def test_prepare_config_omits_the_env_block_without_isolation(tmp_path: Path) -> None: + """A non-isolated run leaves the server on the ambient environment.""" + agent = HermesAgent(AgentConfig()) + + with patch.dict(os.environ, {}, clear=True): + agent._prepare_config(tmp_path, (McpBinding(name="k8s", command=("k8s-mcp",)),)) + + data = _yaml.load((tmp_path / "config.yaml").read_text(encoding="utf-8")) + assert "env" not in data["mcp_servers"]["k8s"] + + +def test_prepare_config_does_not_read_user_state_by_default(tmp_path: Path) -> None: + """A benchmark run is reproducible: ``~/.hermes`` is not inherited.""" + home = tmp_path / "home" + (home / ".hermes").mkdir(parents=True) + (home / ".hermes" / "SOUL.md").write_text("ambient", encoding="utf-8") + run_dir = tmp_path / "run" + run_dir.mkdir() + + with patch.dict(os.environ, {"HOME": str(home)}): + HermesAgent(AgentConfig())._prepare_config(run_dir, ()) + + assert not (run_dir / "SOUL.md").exists() + + +_DEFAULT_COUNTS: tuple[int, ...] = (2748, 11267, 152, 334987, 12000) + + +def _seed_state_db(home: Path, *, counts: tuple[int, ...] = _DEFAULT_COUNTS) -> None: + _init_schema(home / "state.db") + _insert_session(home / "state.db", "s1", *counts) + + +def _fake_run( + *, + returncode: int = 0, + stdout: str = "ok", + stderr: str = "", + counts: tuple[int, ...] = _DEFAULT_COUNTS, + seed: Callable[[Path], None] | None = None, + seen: dict | None = None, +) -> Callable[..., SimpleNamespace]: + """Build a ``core.subprocess.run`` stand-in that seeds the run's ``state.db``. + + Tolerant of extra keywords on purpose: pinning ``run``'s exact signature in + every test means one new keyword there breaks all of them at once. + + Args: + returncode: Exit status the fake reports. + stdout: Captured stdout the fake reports. + stderr: Captured stderr the fake reports. + counts: Session token counts to seed, in ``_TOKEN_COLUMNS`` order. + seed: Optional extra seeding (e.g. messages), called with the run home. + seen: Optional dict the call's ``cmd`` / ``cwd`` / ``home`` are recorded in. + """ + + def fake_run(cmd: list[str], **kwargs) -> SimpleNamespace: + home = Path(kwargs["extra_env"]["HERMES_HOME"]) + _seed_state_db(home, counts=counts) + if seed is not None: + seed(home) + if seen is not None: + seen.update(cmd=cmd, cwd=kwargs["cwd"], home=str(home)) + return SimpleNamespace(returncode=returncode, stdout=stdout, stderr=stderr) + + return fake_run + + +def test_execute_reports_trajectory_and_tokens_from_the_state_db() -> None: + """End-to-end wiring: the run-scoped DB reaches ``AgentResult``.""" + + def seed(home: Path) -> None: + _insert_message( + home / "state.db", + "s1", + "assistant", + tool_calls=_tool_calls_json("call_1", "kubectl_apply", {"manifest": "nginx.yaml"}), + ) + _insert_message(home / "state.db", "s1", "tool", content="applied", tool_call_id="call_1") + + with patch.object(agent_mod, "run", side_effect=_fake_run(stdout="done", seed=seed)): + result = HermesAgent(AgentConfig(target="/bin/hermes")).run("hello") + + assert result.output == "done" + assert not result.has_errors() + assert result.trajectory == [ + { + "name": "kubectl_apply", + "args": {"manifest": "nginx.yaml"}, + "result": "applied", + "status": "completed", + } + ] + assert result.tokens == { + "input": 2748, + "cached": 334987, + "cache_write": 12000, + "reasoning": 152, + "output": 11267 - 152, + "total": 2748 + 334987 + 12000 + 11267, + } + + +def test_execute_runs_in_the_harness_workspace_when_given_one(tmp_path: Path) -> None: + """hermes runs *in* the workspace but keeps its own state out of it.""" + seen: dict = {} + + with patch.object(agent_mod, "run", side_effect=_fake_run(seen=seen)): + HermesAgent(AgentConfig(target="/bin/hermes")).run("hello", tmp_path) + + assert seen["cwd"] == str(tmp_path) + assert seen["home"] == str(tmp_path / ".hermes") + assert (tmp_path / ".hermes" / "config.yaml").exists() + # Only the state home is added to the workspace, so the harness's + # generated-file diff stays dominated by what the agent actually wrote. + assert [entry.name for entry in tmp_path.iterdir()] == [".hermes"] + + +def test_execute_materializes_granted_skills_under_the_state_home(tmp_path: Path) -> None: + src = tmp_path / "src" / "deploy" + src.mkdir(parents=True) + (src / "SKILL.md").write_text("---\nname: deploy\n---\nsteps", encoding="utf-8") + workspace = tmp_path / "ws" + workspace.mkdir() + + config = AgentConfig( + target="/bin/hermes", + capabilities=AllCapabilities(skills=SkillBinding(paths=(str(tmp_path / "src"),))), + ) + with patch.object(agent_mod, "run", side_effect=_fake_run()): + HermesAgent(config).run("hello", workspace) + + assert (workspace / ".hermes" / "skills" / "deploy" / "SKILL.md").exists() + + +def test_execute_records_a_nonzero_exit_without_losing_the_trajectory() -> None: + fake_run = _fake_run(returncode=3, stdout="partial", stderr="boom") + with patch.object(agent_mod, "run", side_effect=fake_run): + result = HermesAgent(AgentConfig(target="/bin/hermes")).run("hello") + + assert result.metadata["returncode"] == 3 + assert any("hermes agent exited 3: boom" in err for err in result.errors) + assert result.tokens["input"] == 2748 + + +def test_execute_flags_a_zero_exit_that_never_reached_the_model() -> None: + """hermes exits 0 on a rejected key or exhausted retries; an unflagged run + would be scored as a bad answer instead of an infrastructure failure.""" + + fake_run = _fake_run(stdout="", stderr="API key not valid", counts=(0, 0, 0, 0, 0)) + with patch.object(agent_mod, "run", side_effect=fake_run): + result = HermesAgent(AgentConfig(target="/bin/hermes")).run("hello") + + assert result.has_errors() + assert any("no model usage" in err and "API key not valid" in err for err in result.errors) + + +def test_execute_on_timeout_reports_what_the_killed_run_flushed() -> None: + def fake_run(cmd: list[str], **kwargs) -> SimpleNamespace: + home = Path(kwargs["extra_env"]["HERMES_HOME"]) + _seed_state_db(home) + _insert_message( + home / "state.db", + "s1", + "assistant", + tool_calls=_tool_calls_json("call_1", "kubectl_get", {}), + ) + raise SubprocessError(cmd, returncode=-1, stdout="out", stderr="err") + + with patch.object(agent_mod, "run", side_effect=fake_run): + result = HermesAgent(AgentConfig(target="/bin/hermes", timeout_sec=5)).run("hello") + + assert result.metadata["timeout"] is True + assert result.errors[0] == "hermes agent timed out after 5s" + assert [call["name"] for call in result.trajectory] == ["kubectl_get"] + assert result.tokens["input"] == 2748 + assert "out" in result.output and "err" in result.output + + +def test_execute_reports_canonical_none_tokens_when_the_binary_is_missing() -> None: + with patch.object(agent_mod, "run", side_effect=OSError("no such file")): + result = HermesAgent(AgentConfig(target="/bin/hermes")).run("hello") + + assert result.has_errors() + assert result.trajectory == [] + assert result.tokens == empty_tokens() + + +# --- extract_trajectory_from_db ---------------------------------------------- + + +def test_trajectory_missing_db_is_reported(db_path: Path) -> None: + trajectory, errors = extract_trajectory_from_db(db_path) + + assert trajectory == [] + assert "State database not found" in errors[0] + + +def test_trajectory_non_database_file_is_reported(db_path: Path) -> None: + db_path.write_text("not a database", encoding="utf-8") + + trajectory, errors = extract_trajectory_from_db(db_path) + + assert trajectory == [] + assert errors and errors[0].startswith("Database error:") + + +def test_trajectory_empty_db_reports_the_missing_table(db_path: Path) -> None: + db_path.touch() + + trajectory, errors = extract_trajectory_from_db(db_path) + + assert trajectory == [] + assert "Database error: no such table: sessions" in errors[0] + + +def test_trajectory_no_sessions_is_reported(db_path: Path) -> None: + _init_schema(db_path) + + trajectory, errors = extract_trajectory_from_db(db_path) + + assert trajectory == [] + assert "No session found in state database" in errors[0] + + +def test_trajectory_reads_every_session_in_insertion_order(db_path: Path) -> None: + """All sessions are read, matching what the token sum already covers. + + Session ids are UUIDs, so ordering is insertion order, not id order — here + the first-inserted session sorts *after* the second lexically. + """ + _init_schema(db_path) + _insert_session(db_path, "f3a9-uuid", *[0] * len(_TOKEN_COLUMNS)) + _insert_session(db_path, "0b21-uuid", *[0] * len(_TOKEN_COLUMNS)) + _insert_message( + db_path, "f3a9-uuid", "assistant", tool_calls=_tool_calls_json("a", "first_call", {}) + ) + _insert_message( + db_path, "0b21-uuid", "assistant", tool_calls=_tool_calls_json("b", "second_call", {}) + ) + + trajectory, errors = extract_trajectory_from_db(db_path) + + assert errors == [] + assert [call["name"] for call in trajectory] == ["first_call", "second_call"] + + +def test_trajectory_pairs_calls_with_their_results(db_path: Path) -> None: + _init_schema(db_path) + _insert_session(db_path, "s1", *[0] * len(_TOKEN_COLUMNS)) + _insert_message(db_path, "s1", "user", content="Deploy app") + _insert_message( + db_path, + "s1", + "assistant", + tool_calls=_tool_calls_json("call_1", "kubectl_apply", {"manifest": "nginx.yaml"}), + ) + _insert_message( + db_path, + "s1", + "tool", + content="Successfully applied", + tool_call_id="call_1", + tool_name="kubectl_apply", + ) + + trajectory, errors = extract_trajectory_from_db(db_path) + + assert errors == [] + assert trajectory == [ + { + "name": "kubectl_apply", + "args": {"manifest": "nginx.yaml"}, + "result": "Successfully applied", + "status": "completed", + } + ] + + +def test_trajectory_keeps_an_unanswered_call_as_called(db_path: Path) -> None: + _init_schema(db_path) + _insert_session(db_path, "s1", *[0] * len(_TOKEN_COLUMNS)) + _insert_message( + db_path, "s1", "assistant", tool_calls=_tool_calls_json("call_1", "kubectl_get", {}) + ) + + trajectory, errors = extract_trajectory_from_db(db_path) + + assert errors == [] + assert trajectory[0]["status"] == "called" + assert trajectory[0]["result"] is None + + +def test_trajectory_malformed_tool_calls_json_is_reported(db_path: Path) -> None: + _init_schema(db_path) + _insert_session(db_path, "s1", *[0] * len(_TOKEN_COLUMNS)) + _insert_message(db_path, "s1", "assistant", tool_calls="[invalid json") + + trajectory, errors = extract_trajectory_from_db(db_path) + + assert trajectory == [] + assert "Failed to parse tool calls JSON" in errors[0] + + +def test_trajectory_malformed_json_does_not_drop_later_calls(db_path: Path) -> None: + """One bad row must not truncate the rest of the session.""" + _init_schema(db_path) + _insert_session(db_path, "s1", *[0] * len(_TOKEN_COLUMNS)) + _insert_message(db_path, "s1", "assistant", tool_calls="[invalid json") + _insert_message( + db_path, "s1", "assistant", tool_calls=_tool_calls_json("call_2", "kubectl_get", {}) + ) + + trajectory, errors = extract_trajectory_from_db(db_path) + + assert len(errors) == 1 + assert [call["name"] for call in trajectory] == ["kubectl_get"] + + +def test_trajectory_non_json_arguments_are_preserved_raw(db_path: Path) -> None: + _init_schema(db_path) + _insert_session(db_path, "s1", *[0] * len(_TOKEN_COLUMNS)) + _insert_message( + db_path, + "s1", + "assistant", + tool_calls=_tool_calls_json("call_1", "bash", "kubectl get po"), + ) + + trajectory, errors = extract_trajectory_from_db(db_path) + + assert errors == [] + assert trajectory[0]["args"] == {"raw_args": "kubectl get po"} + + +def test_trajectory_decoded_object_arguments_are_used_as_is(db_path: Path) -> None: + """Non-OpenAI adapters store ``arguments`` already decoded, not as a string.""" + _init_schema(db_path) + _insert_session(db_path, "s1", *[0] * len(_TOKEN_COLUMNS)) + entry = {"id": "call_1", "function": {"name": "bash", "arguments": {"cmd": "kubectl get po"}}} + _insert_message(db_path, "s1", "assistant", tool_calls=json.dumps([entry])) + + trajectory, errors = extract_trajectory_from_db(db_path) + + assert errors == [] + assert trajectory[0]["args"] == {"cmd": "kubectl get po"} + + +@pytest.mark.parametrize( + ("payload", "expected_error"), + [ + ('{"function": {}}', "Tool calls JSON is not a list"), + ("[42]", "Skipped non-object tool call entry"), + ], +) +def test_trajectory_malformed_tool_call_shapes_are_reported( + db_path: Path, payload: str, expected_error: str +) -> None: + """Valid JSON of the wrong shape must not crash the whole extraction.""" + _init_schema(db_path) + _insert_session(db_path, "s1", *[0] * len(_TOKEN_COLUMNS)) + _insert_message(db_path, "s1", "assistant", tool_calls=payload) + _insert_message( + db_path, "s1", "assistant", tool_calls=_tool_calls_json("call_2", "kubectl_get", {}) + ) + + trajectory, errors = extract_trajectory_from_db(db_path) + + assert expected_error in errors[0] + assert [call["name"] for call in trajectory] == ["kubectl_get"] + + +def test_trajectory_non_object_function_falls_back_to_unknown(db_path: Path) -> None: + _init_schema(db_path) + _insert_session(db_path, "s1", *[0] * len(_TOKEN_COLUMNS)) + _insert_message( + db_path, "s1", "assistant", tool_calls=json.dumps([{"id": "c1", "function": "bash"}]) + ) + + trajectory, errors = extract_trajectory_from_db(db_path) + + assert errors == [] + assert trajectory == [{"name": "unknown", "args": {}, "result": None, "status": "called"}] + + +def test_trajectory_missing_tool_name_falls_back_to_unknown(db_path: Path) -> None: + _init_schema(db_path) + _insert_session(db_path, "s1", *[0] * len(_TOKEN_COLUMNS)) + _insert_message( + db_path, "s1", "assistant", tool_calls=_tool_calls_json("call_1", None, {"a": 1}) + ) + + trajectory, errors = extract_trajectory_from_db(db_path) + + assert errors == [] + assert trajectory[0]["name"] == "unknown" + + +def test_trajectory_orphan_tool_result_is_kept_and_reported(db_path: Path) -> None: + _init_schema(db_path) + _insert_session(db_path, "s1", *[0] * len(_TOKEN_COLUMNS)) + _insert_message( + db_path, + "s1", + "tool", + content="Some result", + tool_call_id="call_unknown", + tool_name="kubectl_delete", + ) + + trajectory, errors = extract_trajectory_from_db(db_path) + + assert "Found tool response for unknown tool_call_id: call_unknown" in errors[0] + assert trajectory == [ + {"name": "kubectl_delete", "args": {}, "result": "Some result", "status": "completed"} + ] + + +# --- extract_tokens_from_db -------------------------------------------------- + + +def test_tokens_fill_every_canonical_bucket(db_path: Path) -> None: + """The harness maps onto the shared schema — no bucket left unpopulated. + + Guards against ``TOKEN_BUCKETS`` gaining a bucket this parser silently skips. + """ + _init_schema(db_path) + _insert_session(db_path, "s1", 1, 2, 3, 4, 5) + + assert set(extract_tokens_from_db(db_path)) == set(TOKEN_BUCKETS) + assert all(value is not None for value in extract_tokens_from_db(db_path).values()) + + +def test_tokens_read_the_session_counts(db_path: Path) -> None: + """``output`` drops the reasoning subset; ``total`` still counts it once. + + Hermes stores ``output_tokens`` as the full provider completion count and + ``reasoning_tokens`` as a slice of it, so ``total`` here must equal Hermes's + own ``prompt_tokens + output_tokens``. + """ + _init_schema(db_path) + _insert_session(db_path, "s1", 2748, 11267, 152, 334987, 12000) + + assert extract_tokens_from_db(db_path) == { + "input": 2748, + "cached": 334987, + "cache_write": 12000, + "reasoning": 152, + "output": 11267 - 152, + "total": 2748 + 334987 + 12000 + 11267, + } + + +def test_tokens_reach_the_result_row_unchanged(db_path: Path) -> None: + """Cross-layer guard: the emitted keys are the ones ``normalize_tokens`` reads. + + A bucket named differently here would silently normalize to ``None`` on the + dashboard row rather than failing loudly. + """ + _init_schema(db_path) + _insert_session(db_path, "s1", 2748, 11267, 152, 334987, 12000) + + normalized = normalize_tokens(extract_tokens_from_db(db_path)) + + assert normalized.input == 2748 + assert normalized.output == 11267 - 152 + assert normalized.cached == 334987 + assert normalized.cache_write == 12000 + assert normalized.reasoning == 152 + assert normalized.total == 2748 + 334987 + 12000 + 11267 + + +def test_tokens_sum_across_sessions(db_path: Path) -> None: + # The DB is run-scoped; a run may write several session rows (compaction, + # sub-sessions), and session ids need not sort chronologically. + _init_schema(db_path) + _insert_session(db_path, "f3a9-uuid", 1, 1, 0, 0, 0) + _insert_session(db_path, "0b21-uuid", 500, 40, 5, 900, 30) + + tokens = extract_tokens_from_db(db_path) + + assert tokens["input"] == 501 + assert tokens["output"] == 41 - 5 + assert tokens["cached"] == 900 + assert tokens["total"] == 501 + 41 + 900 + 30 + + +def test_tokens_coerce_real_values(db_path: Path) -> None: + # SQLite columns are dynamically typed; a REAL count must not be dropped. + _init_schema(db_path) + _insert_session(db_path, "s1", 100, 7, 0, 0, 12000.0) + + tokens = extract_tokens_from_db(db_path) + + assert tokens["cache_write"] == 12000 + assert tokens["input"] == 100 + + +def test_tokens_null_columns_stay_none(db_path: Path) -> None: + # NULL counts (e.g. a crashed run) must surface as None, not a fake 0. + _init_schema(db_path) + _insert_session(db_path, "s1", 100, 7, None, None, None) + + tokens = extract_tokens_from_db(db_path) + + assert tokens["input"] == 100 + assert tokens["output"] == 7 + assert tokens["reasoning"] is None + assert tokens["cached"] is None + assert tokens["cache_write"] is None + assert tokens["total"] == 107 + + +def test_tokens_reasoning_beyond_the_completion_total_clamps_at_zero(db_path: Path) -> None: + """Defensive: a bad provider report must not yield a negative bucket.""" + _init_schema(db_path) + _insert_session(db_path, "s1", 100, 7, 40, 0, 0) + + tokens = extract_tokens_from_db(db_path) + + assert tokens["output"] == 0 + assert tokens["total"] == 140 + + +def test_tokens_survive_a_path_with_uri_delimiters(tmp_path: Path) -> None: + """A ``?`` in the run path must not truncate the read-only URI.""" + odd_dir = tmp_path / "run?id=1" + odd_dir.mkdir() + db_path = odd_dir / "state.db" + _init_schema(db_path) + _insert_session(db_path, "s1", 100, 7, 0, 0, 0) + + assert extract_tokens_from_db(db_path)["input"] == 100 + + +def test_tokens_no_session_rows_stay_none(db_path: Path) -> None: + """SUM over an empty table yields NULL, which must not become 0.""" + _init_schema(db_path) + + assert extract_tokens_from_db(db_path) == empty_tokens() + + +def test_tokens_old_schema_without_token_columns(db_path: Path) -> None: + conn = sqlite3.connect(db_path) + conn.execute("CREATE TABLE sessions (id TEXT PRIMARY KEY, started_at TIMESTAMP)") + conn.commit() + conn.close() + + assert extract_tokens_from_db(db_path) == empty_tokens() + + +def test_tokens_missing_or_invalid_db(db_path: Path) -> None: + assert extract_tokens_from_db(db_path) == empty_tokens() # no file + db_path.write_text("not a database", encoding="utf-8") + assert extract_tokens_from_db(db_path) == empty_tokens() diff --git a/tests/unit/agents/test_agents_cli_openclaw.py b/tests/unit/agents/test_agents_cli_openclaw.py index f488375b..62758865 100644 --- a/tests/unit/agents/test_agents_cli_openclaw.py +++ b/tests/unit/agents/test_agents_cli_openclaw.py @@ -518,19 +518,6 @@ def test_openclaw_agent_mirrors_rules_binding_onto_mixin_attribute() -> None: # --------------------------------------------------------------------------- -def test_prepend_rules_passes_prompt_through_when_rules_empty() -> None: - from devops_bench.agents.cli.openclaw.agent import _prepend_rules - - assert _prepend_rules("", "do the thing") == "do the thing" - assert _prepend_rules(" \n ", "do the thing") == "do the thing" - - -def test_prepend_rules_separates_brief_from_prompt_with_blank_line() -> None: - from devops_bench.agents.cli.openclaw.agent import _prepend_rules - - assert _prepend_rules("be careful", "audit pods") == "be careful\n\naudit pods" - - def test_execute_prepends_bound_rules_to_oc_prompt( monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: @@ -591,6 +578,24 @@ def test_build_openclaw_config_wraps_servers_under_mcp() -> None: assert cfg == {"mcp": {"servers": {"gke": {"command": "gke-mcp"}}}} +def test_build_openclaw_config_forwards_every_run_isolation_var( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Both isolation vars reach the server, not just ``KUBECONFIG``. + + openclaw spawns the server with a filtered env, so an unforwarded + ``CLOUDSDK_CONFIG`` leaves a gcloud-backed server reading the operator's + ambient credentials rather than the run's. + """ + monkeypatch.setenv("KUBECONFIG", "/run/kubeconfig") + monkeypatch.setenv("CLOUDSDK_CONFIG", "/run/gcloud") + cfg = _build_openclaw_config(AgentConfig(), (McpBinding(name="gke", command=("gke-mcp",)),)) + assert cfg["mcp"]["servers"]["gke"]["env"] == { + "KUBECONFIG": "/run/kubeconfig", + "CLOUDSDK_CONFIG": "/run/gcloud", + } + + def test_build_openclaw_config_empty_without_launchable_server_or_override() -> None: """No MCP binding and a catalog-known model → empty config (caller skips).""" assert _build_openclaw_config(AgentConfig(), ()) == {} diff --git a/tests/unit/models/test_models_base.py b/tests/unit/models/test_models_base.py index 110e05fe..0b2322ec 100644 --- a/tests/unit/models/test_models_base.py +++ b/tests/unit/models/test_models_base.py @@ -61,6 +61,7 @@ def install(backend: str | None = None) -> None: canonical="fake-provider", adapter_family="fake_family", oc_provider="fake-provider", + hermes_provider="fake-provider", api_key_envs=(), keyless_ok=True, backend=backend,