fix(harnesses): surface Hermes in the web harness picker (#1940)
* fix(harnesses): surface Hermes in the web harness picker Hermes is a valid, installable harness (present in valid_harnesses and harness_modules with declared capabilities) but had no harness_labels entry, so harness_catalog() -- which iterates the labels -- dropped it from GET /v1/harnesses. The web picker therefore never listed Hermes even though "omnigent setup" (which hardcodes the row) shows it ready. Add the label, matching the subprocess-harness convention of codex/cursor/pi. The frontend already maps the hermes harness to HermesIcon, so no frontend change is needed. Closes #1939 Co-authored-by: Isaac Signed-off-by: manffred-calvosanchez_data <manffred.calvosanchez@databricks.com> * fix(harnesses): thread the spawn env for the hermes picker row Adding hermes to `harness_labels` makes it selectable in the web picker, but hermes had no spawn-env builder and no `model_env_keys` entry, so `_build_spawn_env_from_spec` returned None for it and the subprocess started with no per-session config at all. The wrap then applied its own defaults, and three picker choices became silent no-ops: - a selected sandbox fell back to the wrap's `caller_process` + `sandbox=none`, so a session the UI showed as sandboxed ran unconfined; - the session workspace fell back to the runner-wide `OMNIGENT_RUNNER_WORKSPACE` instead of the folder the user picked; - `/model` was rejected up front, since `harness_supports_model_override` derives from `model_env_keys`. Add `_build_hermes_spawn_env`, modelled on the kimi builder: hermes owns its file-based auth (`hermes setup` / `hermes model`, credentials under its `HERMES_HOME`), so there is no gateway/provider surface to configure and the builder threads only model, cwd, skills filter, and the serialized `os_env`. Unlike kimi it does emit `HARNESS_HERMES_SKILLS_FILTER`, which the executor turns into its `-s` / `--ignore-rules` argv. `HARNESS_HERMES_BUNDLE_DIR` stays unset: it is reserved in the wrap with no `hermes chat` flag to carry it, so emitting it would set a var the executor cannot pass on. Register the builder on the `hermes` arm of the runner dispatch chain, matching the eleven sibling builtins, and add the model env key so `/model` reaches the subprocess. Tests: hermes joins the shared parametrized cwd and `OMNIGENT_*_PATH` suites, gains four builder tests beside its kimi peer, a dispatch-chain guard (having a builder does not prove the chain reaches it), and a guard that the picker row's model plumbing exists. Each fails on the unfixed tree for its own reason. Co-authored-by: Isaac Signed-off-by: Dhruv Gupta <dhruv0811@gmail.com> --------- Signed-off-by: manffred-calvosanchez_data <manffred.calvosanchez@databricks.com> Signed-off-by: Dhruv Gupta <dhruv0811@gmail.com> Co-authored-by: Dhruv Gupta <dhruv0811@gmail.com>
This commit is contained in:
@@ -784,6 +784,7 @@ _BUILTIN_CONTRIBUTION = HarnessContribution(
|
||||
"copilot": "HARNESS_COPILOT_MODEL",
|
||||
"cursor": "HARNESS_CURSOR_MODEL",
|
||||
"goose": "HARNESS_GOOSE_MODEL",
|
||||
"hermes": "HARNESS_HERMES_MODEL",
|
||||
"kimi": "HARNESS_KIMI_MODEL",
|
||||
"openai-agents": "HARNESS_OPENAI_AGENTS_MODEL",
|
||||
"pi": "HARNESS_PI_MODEL",
|
||||
@@ -810,6 +811,7 @@ _BUILTIN_CONTRIBUTION = HarnessContribution(
|
||||
"codex": "Codex",
|
||||
"copilot": "Copilot",
|
||||
"cursor": "Cursor",
|
||||
"hermes": "Hermes",
|
||||
# openai-agents is intentionally omitted from the picker catalog: it
|
||||
# stays a valid harness for YAML specs (and the credential-free
|
||||
# integration mock LLM), but is no longer offered as a UI pick.
|
||||
|
||||
@@ -9096,6 +9096,7 @@ def _build_spawn_env_from_spec(
|
||||
_build_copilot_spawn_env,
|
||||
_build_cursor_spawn_env,
|
||||
_build_goose_spawn_env,
|
||||
_build_hermes_spawn_env,
|
||||
_build_kimi_spawn_env,
|
||||
_build_openai_agents_sdk_spawn_env,
|
||||
_build_pi_spawn_env,
|
||||
@@ -9116,6 +9117,8 @@ def _build_spawn_env_from_spec(
|
||||
env = _build_antigravity_spawn_env(effective_spec)
|
||||
elif harness == "kimi":
|
||||
env = _build_kimi_spawn_env(effective_spec, cwd=cwd)
|
||||
elif harness == "hermes":
|
||||
env = _build_hermes_spawn_env(effective_spec, cwd=cwd, workdir=workdir)
|
||||
elif harness == "qwen":
|
||||
env = _build_qwen_spawn_env(effective_spec, cwd=cwd, workdir=workdir)
|
||||
elif harness == "goose":
|
||||
|
||||
@@ -1982,6 +1982,53 @@ def _build_kimi_spawn_env(
|
||||
return env
|
||||
|
||||
|
||||
def _build_hermes_spawn_env(
|
||||
spec: AgentSpec,
|
||||
*,
|
||||
cwd: Path | None = None,
|
||||
workdir: Path | None = None,
|
||||
) -> dict[str, str]:
|
||||
"""Build the env-var dict the hermes harness wrap reads.
|
||||
|
||||
Maps ``spec.executor`` fields → the ``HARNESS_HERMES_*`` env vars defined
|
||||
in :mod:`omnigent.inner.hermes_harness`. Hermes owns its own file-based
|
||||
auth (``hermes setup`` / ``hermes model``, credentials under its
|
||||
``HERMES_HOME``), so — like :func:`_build_kimi_spawn_env` — this threads
|
||||
only the model, working directory, skills filter, and ``os_env`` sandbox
|
||||
spec; there is no gateway/provider env surface to configure.
|
||||
|
||||
A hermes session with no spawn env is not inert: the wrap falls back to
|
||||
``sandbox=none`` and the runner-wide launch directory, so the sandbox and
|
||||
workspace a session selected have to be threaded here to take effect.
|
||||
|
||||
:param spec: The agent spec.
|
||||
:param cwd: Runtime working directory for the hermes subprocess — the
|
||||
session workspace, NOT the agent bundle dir. Threaded as
|
||||
``HARNESS_HERMES_CWD``; when unset the wrap falls back to
|
||||
``OMNIGENT_RUNNER_WORKSPACE``.
|
||||
:param workdir: The bundle's on-disk path. Accepted for signature parity
|
||||
with the sibling builders but not threaded: ``HARNESS_HERMES_BUNDLE_DIR``
|
||||
is reserved (there is no ``hermes chat`` flag for it yet), so the wrap
|
||||
would read a value it cannot pass on.
|
||||
:returns: A dict of env-var overrides.
|
||||
"""
|
||||
env: dict[str, str] = {}
|
||||
model = _resolve_spec_model(spec)
|
||||
if model is not None:
|
||||
env["HARNESS_HERMES_MODEL"] = model
|
||||
if cwd is not None:
|
||||
env["HARNESS_HERMES_CWD"] = str(cwd)
|
||||
# Always set so the wrap doesn't fall back to "all" and override an
|
||||
# explicit ``skills: none`` from the spec. Hermes turns this into its
|
||||
# ``-s`` / ``--ignore-rules`` argv (see hermes_executor._build_args).
|
||||
env["HARNESS_HERMES_SKILLS_FILTER"] = json.dumps(spec.skills_filter)
|
||||
os_env_payload = _serialize_os_env(spec.os_env)
|
||||
if os_env_payload is not None:
|
||||
env["HARNESS_HERMES_OS_ENV"] = os_env_payload
|
||||
_apply_harness_path_override(env, "hermes")
|
||||
return env
|
||||
|
||||
|
||||
def _build_antigravity_spawn_env(spec: AgentSpec) -> dict[str, str]:
|
||||
"""
|
||||
Map ``spec.executor`` fields → the ``HARNESS_ANTIGRAVITY_*`` env vars the
|
||||
|
||||
@@ -1411,6 +1411,43 @@ def test_build_spawn_env_applies_model_override(
|
||||
assert overridden["HARNESS_CLAUDE_SDK_MODEL"] == "claude-sonnet-4-6"
|
||||
|
||||
|
||||
def test_build_spawn_env_routes_hermes(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""The dispatch chain routes ``hermes`` to its builder.
|
||||
|
||||
Regression: hermes had no arm here, so this returned ``None`` and the
|
||||
subprocess got no spawn env at all — the session's sandbox fell back to the
|
||||
wrap's ``sandbox=none`` default and its workspace to the runner's launch
|
||||
directory, both silently. Having the builder is not enough; the chain has
|
||||
to reach it.
|
||||
|
||||
:param tmp_path: Pytest temp dir for an isolated provider config.
|
||||
:param monkeypatch: Pytest monkeypatch fixture.
|
||||
"""
|
||||
from omnigent.inner.datamodel import OSEnvSandboxSpec, OSEnvSpec
|
||||
|
||||
monkeypatch.setenv("OMNIGENT_CONFIG_HOME", str(tmp_path))
|
||||
monkeypatch.setenv("OMNIGENT_DISABLE_KEYRING", "1")
|
||||
monkeypatch.delenv("OMNIGENT_HERMES_PATH", raising=False)
|
||||
workspace = tmp_path / "workspace"
|
||||
workspace.mkdir()
|
||||
spec = AgentSpec(
|
||||
spec_version=1,
|
||||
name="x",
|
||||
executor=ExecutorSpec(type="omnigent", config={"harness": "hermes"}),
|
||||
os_env=OSEnvSpec(type="caller_process", sandbox=OSEnvSandboxSpec(type="linux_bwrap")),
|
||||
)
|
||||
|
||||
env = _build_spawn_env_from_spec(spec, "hermes", cwd=workspace)
|
||||
|
||||
assert env is not None, "hermes is not routed to a spawn-env builder"
|
||||
assert env["HARNESS_HERMES_CWD"] == str(workspace)
|
||||
assert json.loads(env["HARNESS_HERMES_OS_ENV"])["sandbox"]["type"] == "linux_bwrap"
|
||||
# The /model override reaches hermes through the same model env key.
|
||||
overridden = _build_spawn_env_from_spec(spec, "hermes", model_override="hermes-4-70b")
|
||||
assert overridden is not None
|
||||
assert overridden["HARNESS_HERMES_MODEL"] == "hermes-4-70b"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_resolve_harness_config_applies_harness_override(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
|
||||
@@ -30,6 +30,7 @@ from omnigent.runtime.workflow import (
|
||||
_build_claude_sdk_spawn_env,
|
||||
_build_codex_spawn_env,
|
||||
_build_goose_spawn_env,
|
||||
_build_hermes_spawn_env,
|
||||
_build_kimi_spawn_env,
|
||||
_build_openai_agents_sdk_spawn_env,
|
||||
_build_pi_spawn_env,
|
||||
@@ -1448,6 +1449,86 @@ def test_kimi_os_env_serialized(config_home: Path) -> None:
|
||||
assert decoded["sandbox"]["type"] == "darwin_seatbelt"
|
||||
|
||||
|
||||
# ── Hermes Agent CLI spawn-env ────────────────────────────────────────────
|
||||
|
||||
|
||||
def test_hermes_spawn_env_threads_spec_model_and_skills(config_home: Path) -> None:
|
||||
"""The hermes builder emits the model plus the skills filter, and no
|
||||
gateway vars: Hermes owns its file-based auth (``hermes setup`` /
|
||||
``hermes model``) and has no per-spawn provider override to configure.
|
||||
|
||||
Regression: hermes had no builder at all, so a spec model never reached the
|
||||
subprocess and it silently ran on whatever default Hermes had configured."""
|
||||
_write_config(config_home, {"providers": {}})
|
||||
spec = _make_spec(harness="hermes", model="hermes-4-405b")
|
||||
|
||||
env = _build_hermes_spawn_env(spec, cwd=None, workdir=None)
|
||||
|
||||
assert env == {
|
||||
"HARNESS_HERMES_MODEL": "hermes-4-405b",
|
||||
"HARNESS_HERMES_SKILLS_FILTER": '"all"',
|
||||
}
|
||||
|
||||
|
||||
def test_hermes_ignores_global_default_provider(config_home: Path) -> None:
|
||||
"""An openai default provider injects no creds into the hermes env.
|
||||
|
||||
Same reasoning as kimi: Hermes reads credentials from its own
|
||||
``auth.json`` / ``.env`` under ``HERMES_HOME``, so injecting an ambient key
|
||||
the executor cannot pass through would mis-bill the user against a
|
||||
provider their Hermes install never uses."""
|
||||
_write_config(config_home, _openai_default_config())
|
||||
spec = _make_spec(harness="hermes")
|
||||
|
||||
env = _build_hermes_spawn_env(spec, cwd=None, workdir=None)
|
||||
|
||||
assert "HARNESS_HERMES_GATEWAY_BASE_URL" not in env
|
||||
assert "HARNESS_HERMES_GATEWAY_API_KEY" not in env
|
||||
assert "HARNESS_HERMES_DATABRICKS_PROFILE" not in env
|
||||
|
||||
|
||||
def test_hermes_os_env_serialized(config_home: Path) -> None:
|
||||
"""``spec.os_env`` is serialized into ``HARNESS_HERMES_OS_ENV`` so the wrap
|
||||
rebuilds the sandbox spec instead of falling back to ``sandbox=none``.
|
||||
|
||||
This is what makes a sandbox picked in the web session dialog actually
|
||||
confine hermes — without it the harness ran unconfined while the UI
|
||||
reported a sandbox."""
|
||||
import json as _json
|
||||
|
||||
from omnigent.inner.datamodel import OSEnvSandboxSpec, OSEnvSpec
|
||||
|
||||
_write_config(config_home, {"providers": {}})
|
||||
os_env = OSEnvSpec(
|
||||
type="caller_process",
|
||||
cwd=None,
|
||||
sandbox=OSEnvSandboxSpec(type="linux_bwrap"),
|
||||
fork=False,
|
||||
)
|
||||
spec = _make_spec(harness="hermes", os_env=os_env)
|
||||
|
||||
env = _build_hermes_spawn_env(spec, cwd=None, workdir=None)
|
||||
|
||||
assert "HARNESS_HERMES_OS_ENV" in env
|
||||
decoded = _json.loads(env["HARNESS_HERMES_OS_ENV"])
|
||||
assert decoded["sandbox"]["type"] == "linux_bwrap"
|
||||
|
||||
|
||||
def test_hermes_omits_reserved_bundle_dir(config_home: Path, tmp_path: Path) -> None:
|
||||
"""``workdir`` is accepted for signature parity but not threaded.
|
||||
|
||||
``HARNESS_HERMES_BUNDLE_DIR`` is reserved in the wrap — there is no
|
||||
``hermes chat`` flag for it — so emitting it would set a var the executor
|
||||
cannot pass on. Locks the deliberate omission so a future reader doesn't
|
||||
"fix" it without wiring the argv side."""
|
||||
_write_config(config_home, {"providers": {}})
|
||||
spec = _make_spec(harness="hermes")
|
||||
|
||||
env = _build_hermes_spawn_env(spec, cwd=None, workdir=tmp_path)
|
||||
|
||||
assert "HARNESS_HERMES_BUNDLE_DIR" not in env
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# harness.<canonical>.command → OMNIGENT_<NAME>_PATH (spawn-env builders)
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -1477,6 +1558,7 @@ def _call_builder(builder: object, spec: AgentSpec) -> dict[str, str]: # type:
|
||||
("kimi", _build_kimi_spawn_env, "OMNIGENT_KIMI_PATH"),
|
||||
("goose", _build_goose_spawn_env, "OMNIGENT_GOOSE_PATH"),
|
||||
("qwen", _build_qwen_spawn_env, "OMNIGENT_QWEN_PATH"),
|
||||
("hermes", _build_hermes_spawn_env, "OMNIGENT_HERMES_PATH"),
|
||||
],
|
||||
)
|
||||
def test_spawn_env_threads_config_command_to_path(
|
||||
@@ -1504,6 +1586,7 @@ def test_spawn_env_threads_config_command_to_path(
|
||||
("kimi", _build_kimi_spawn_env, "OMNIGENT_KIMI_PATH"),
|
||||
("goose", _build_goose_spawn_env, "OMNIGENT_GOOSE_PATH"),
|
||||
("qwen", _build_qwen_spawn_env, "OMNIGENT_QWEN_PATH"),
|
||||
("hermes", _build_hermes_spawn_env, "OMNIGENT_HERMES_PATH"),
|
||||
],
|
||||
)
|
||||
def test_spawn_env_ambient_env_wins_over_config_command(
|
||||
|
||||
@@ -26,6 +26,7 @@ from omnigent.runtime.workflow import (
|
||||
_build_copilot_spawn_env,
|
||||
_build_cursor_spawn_env,
|
||||
_build_goose_spawn_env,
|
||||
_build_hermes_spawn_env,
|
||||
_build_qwen_spawn_env,
|
||||
)
|
||||
from omnigent.spec.types import AgentSpec, ExecutorSpec
|
||||
@@ -39,6 +40,7 @@ _BUILDERS = [
|
||||
("goose", _build_goose_spawn_env, "HARNESS_GOOSE_CWD"),
|
||||
("copilot", _build_copilot_spawn_env, "HARNESS_COPILOT_CWD"),
|
||||
("acp", _build_acp_spawn_env, "HARNESS_ACP_CWD"),
|
||||
("hermes", _build_hermes_spawn_env, "HARNESS_HERMES_CWD"),
|
||||
]
|
||||
|
||||
|
||||
|
||||
@@ -176,6 +176,32 @@ def test_catalog_rows_include_capabilities() -> None:
|
||||
assert value is None or isinstance(value, (str, bool))
|
||||
|
||||
|
||||
def test_catalog_includes_hermes() -> None:
|
||||
"""Hermes must appear in the web picker catalog (regression: it was a
|
||||
valid harness with capabilities but had no ``harness_labels`` entry, so
|
||||
``harness_catalog`` — which iterates labels — dropped it)."""
|
||||
rows = harness_catalog()
|
||||
hermes = next((row for row in rows if row["id"] == "hermes"), None)
|
||||
assert hermes is not None, "hermes missing from harness_catalog()"
|
||||
assert hermes["label"] == "Hermes"
|
||||
# The catalog only lists valid harnesses and hermes declares capabilities,
|
||||
# so the row must carry the feature matrix like its subprocess peers.
|
||||
assert "capabilities" in hermes
|
||||
|
||||
|
||||
def test_hermes_picker_row_has_spawn_env_plumbing() -> None:
|
||||
"""A picker row is only honest if the session's choices reach the harness.
|
||||
|
||||
Hermes' model env key is what both threads ``/model`` into the spawn env and
|
||||
(via ``_SDK_MODEL_OVERRIDE_HARNESSES``) makes the server accept the override
|
||||
instead of rejecting it up front."""
|
||||
from omnigent.harness_plugins import model_env_keys
|
||||
from omnigent.model_override import harness_supports_model_override
|
||||
|
||||
assert model_env_keys()["hermes"] == "HARNESS_HERMES_MODEL"
|
||||
assert harness_supports_model_override("hermes")
|
||||
|
||||
|
||||
def test_catalog_rows_carry_setup_steps() -> None:
|
||||
"""Every row exposes an ordered, JSON-serializable setup checklist."""
|
||||
rows = {row["id"]: row for row in harness_catalog()}
|
||||
|
||||
Reference in New Issue
Block a user