mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-27 11:22:03 +00:00
fix(agent): gate verify-on-stop nudge off for messaging surfaces
The verify-on-stop guard (PRs #52296, #52297) defaulted ON for every session, so on gateway messaging surfaces (Telegram, Discord, etc.) the model complied with the nudge by writing a hermes-verify temp script and emitting an ad-hoc verification summary, which the gateway delivered to the end user as chat noise. Resolve a surface-aware default instead. The DEFAULT_CONFIG value becomes the sentinel "auto", which verify_on_stop_enabled() resolves to ON for interactive coding surfaces (CLI, TUI, desktop) and programmatic callers, and OFF for conversational messaging surfaces. The surface is read from HERMES_SESSION_PLATFORM (what the gateway actually binds), with HERMES_SESSION_SOURCE and HERMES_PLATFORM as fallbacks, matching the sibling resolution in skill_commands.py and prompt_builder.py. An explicit HERMES_VERIFY_ON_STOP env var or a boolean agent.verify_on_stop config still overrides in either direction. The passive evidence ledger and the call site are untouched.
This commit is contained in:
parent
e62afaca62
commit
f168631be0
4 changed files with 199 additions and 13 deletions
|
|
@ -15,9 +15,77 @@ from typing import Any, Iterable
|
|||
|
||||
_MAX_CHANGED_PATHS_IN_NUDGE = 8
|
||||
|
||||
# Session identities (platform or source) that are NOT human conversational
|
||||
# messaging surfaces: interactive coding surfaces (CLI, TUI, desktop, codex,
|
||||
# local, gateway) and programmatic callers (API server, webhooks, tools).
|
||||
# Verify-on-stop stays ON by default for these. Any other resolved gateway
|
||||
# platform is a conversational messaging surface (Telegram, Discord, WhatsApp,
|
||||
# Signal, Slack, etc.) where the verification narrative would reach a human as
|
||||
# chat noise, so it defaults OFF. Mirrors LOCAL_SESSION_SOURCE_IDS in
|
||||
# apps/desktop/src/lib/session-source.ts; keep roughly in sync when adding a
|
||||
# local or programmatic surface. Default-deny by design: an unrecognized
|
||||
# identity is treated as messaging (OFF) so a new chat platform never leaks the
|
||||
# verification receipt before this set is updated.
|
||||
_NON_MESSAGING_SESSION_SURFACES = frozenset(
|
||||
{
|
||||
"",
|
||||
"cli",
|
||||
"codex",
|
||||
"desktop",
|
||||
"gateway",
|
||||
"local",
|
||||
"tui",
|
||||
"tool",
|
||||
"api_server",
|
||||
"webhook",
|
||||
"msgraph_webhook",
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
def _session_is_messaging_surface() -> bool:
|
||||
"""Return whether this turn is delivered over a human messaging channel.
|
||||
|
||||
The gateway binds the platform value (e.g. ``telegram``) to
|
||||
``HERMES_SESSION_PLATFORM``; the CLI and TUI set ``HERMES_SESSION_SOURCE``
|
||||
(e.g. ``cli``, ``tui``) instead. Both are consulted via the session-context
|
||||
helper (with an ``os.environ`` fallback), alongside the ``HERMES_PLATFORM``
|
||||
override, matching the sibling platform resolution in
|
||||
``agent/skill_commands.py`` and ``agent/prompt_builder.py``. A turn is a
|
||||
messaging surface when a resolved identity is present and is not a known
|
||||
non-messaging surface.
|
||||
"""
|
||||
try:
|
||||
from gateway.session_context import get_session_env
|
||||
|
||||
platform = (
|
||||
os.getenv("HERMES_PLATFORM")
|
||||
or get_session_env("HERMES_SESSION_PLATFORM", "")
|
||||
)
|
||||
source = get_session_env("HERMES_SESSION_SOURCE", "")
|
||||
except Exception:
|
||||
platform = os.getenv("HERMES_PLATFORM", "") or os.environ.get(
|
||||
"HERMES_SESSION_PLATFORM", ""
|
||||
)
|
||||
source = os.environ.get("HERMES_SESSION_SOURCE", "")
|
||||
for identity in (platform, source):
|
||||
identity = str(identity or "").strip().lower()
|
||||
if identity and identity not in _NON_MESSAGING_SESSION_SURFACES:
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def verify_on_stop_enabled(config: dict[str, Any] | None = None) -> bool:
|
||||
"""Return whether edit -> verify-before-finish behavior is enabled."""
|
||||
"""Return whether edit -> verify-before-finish behavior is enabled.
|
||||
|
||||
Precedence: an explicit ``HERMES_VERIFY_ON_STOP`` env var wins, then an
|
||||
explicit boolean ``agent.verify_on_stop`` config value, then a surface-aware
|
||||
default. The config default is the sentinel ``"auto"`` (see
|
||||
``DEFAULT_CONFIG``), which resolves to ON for interactive coding surfaces
|
||||
(CLI, TUI, desktop) and programmatic callers, and OFF for conversational
|
||||
messaging surfaces (Telegram, Discord, etc.) where the verification
|
||||
narrative would otherwise reach a human as chat noise.
|
||||
"""
|
||||
env = os.environ.get("HERMES_VERIFY_ON_STOP")
|
||||
if env is not None:
|
||||
return env.strip().lower() not in {"0", "false", "no", "off"}
|
||||
|
|
@ -29,9 +97,17 @@ def verify_on_stop_enabled(config: dict[str, Any] | None = None) -> bool:
|
|||
except Exception:
|
||||
config = {}
|
||||
agent_cfg = (config or {}).get("agent") if isinstance(config, dict) else None
|
||||
if isinstance(agent_cfg, dict) and "verify_on_stop" in agent_cfg:
|
||||
return bool(agent_cfg.get("verify_on_stop"))
|
||||
return True
|
||||
cfg_val = agent_cfg.get("verify_on_stop") if isinstance(agent_cfg, dict) else None
|
||||
if isinstance(cfg_val, bool):
|
||||
return cfg_val
|
||||
if isinstance(cfg_val, str):
|
||||
token = cfg_val.strip().lower()
|
||||
if token in {"1", "true", "yes", "on"}:
|
||||
return True
|
||||
if token in {"0", "false", "no", "off"}:
|
||||
return False
|
||||
# "auto", missing, or any other value -> surface-aware default.
|
||||
return not _session_is_messaging_surface()
|
||||
|
||||
|
||||
def _candidate_cwds(paths: Iterable[str]) -> list[Path]:
|
||||
|
|
|
|||
|
|
@ -637,7 +637,15 @@ agent:
|
|||
# primaries (default 3). The OpenAI SDK does its own low-level retries
|
||||
# underneath this wrapper — this is the Hermes-level loop.
|
||||
# api_max_retries: 3
|
||||
|
||||
|
||||
# After the agent edits code without fresh passing verification, nudge it to
|
||||
# verify before finishing. The default "auto" enables it on interactive
|
||||
# coding surfaces (CLI, TUI, desktop) and programmatic callers, and disables
|
||||
# it on conversational messaging surfaces (Telegram, Discord, etc.) where the
|
||||
# verification summary would reach a human as chat noise. Set true or false to
|
||||
# force it on or off; the HERMES_VERIFY_ON_STOP env var (1/0) takes precedence.
|
||||
# verify_on_stop: auto
|
||||
|
||||
# Enable verbose logging
|
||||
verbose: false
|
||||
|
||||
|
|
|
|||
|
|
@ -982,8 +982,12 @@ DEFAULT_CONFIG = {
|
|||
# Verification closure: after the agent edits files in a code workspace,
|
||||
# do not accept a final answer until fresh verification evidence exists
|
||||
# or the agent explains why it cannot run checks. The loop is bounded
|
||||
# and uses the passive verification ledger. Set false to disable.
|
||||
"verify_on_stop": True,
|
||||
# and uses the passive verification ledger. The default "auto" enables
|
||||
# it on interactive coding surfaces (CLI, TUI, desktop) and programmatic
|
||||
# callers, and disables it on conversational messaging surfaces
|
||||
# (Telegram, Discord, etc.) where the verification summary would reach a
|
||||
# human as chat noise. Set true or false to force it on or off.
|
||||
"verify_on_stop": "auto",
|
||||
# Staged inactivity warning: send a warning to the user at this
|
||||
# threshold before escalating to a full timeout. The warning fires
|
||||
# once per run and does not interrupt the agent. 0 = disable warning.
|
||||
|
|
|
|||
|
|
@ -2,6 +2,8 @@ import json
|
|||
import tempfile
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from agent.verification_evidence import (
|
||||
mark_workspace_edited,
|
||||
record_terminal_result,
|
||||
|
|
@ -25,21 +27,117 @@ def _make_project(root: Path) -> None:
|
|||
_node_project(root)
|
||||
|
||||
|
||||
def test_verify_on_stop_default_is_on(monkeypatch):
|
||||
monkeypatch.delenv("HERMES_VERIFY_ON_STOP", raising=False)
|
||||
@pytest.fixture
|
||||
def clear_verify_env(monkeypatch):
|
||||
"""Clear every env signal verify_on_stop_enabled consults.
|
||||
|
||||
Tests then set only the variable they exercise, mirroring how the CLI/TUI
|
||||
set HERMES_SESSION_SOURCE and the gateway sets HERMES_SESSION_PLATFORM.
|
||||
"""
|
||||
for var in (
|
||||
"HERMES_VERIFY_ON_STOP",
|
||||
"HERMES_PLATFORM",
|
||||
"HERMES_SESSION_PLATFORM",
|
||||
"HERMES_SESSION_SOURCE",
|
||||
):
|
||||
monkeypatch.delenv(var, raising=False)
|
||||
return monkeypatch
|
||||
|
||||
|
||||
def test_verify_on_stop_default_is_on(clear_verify_env):
|
||||
# No env, no messaging identity, no explicit config -> default ON.
|
||||
assert verify_on_stop_enabled({"agent": {}}) is True
|
||||
|
||||
|
||||
def test_verify_on_stop_env_can_disable(monkeypatch):
|
||||
monkeypatch.setenv("HERMES_VERIFY_ON_STOP", "0")
|
||||
def test_verify_on_stop_auto_sentinel_resolves_to_surface_default(clear_verify_env):
|
||||
# The DEFAULT_CONFIG sentinel must fall through to the surface-aware default,
|
||||
# not be coerced to a truthy string.
|
||||
assert verify_on_stop_enabled({"agent": {"verify_on_stop": "auto"}}) is True
|
||||
clear_verify_env.setenv("HERMES_SESSION_PLATFORM", "telegram")
|
||||
assert verify_on_stop_enabled({"agent": {"verify_on_stop": "auto"}}) is False
|
||||
|
||||
|
||||
def test_verify_on_stop_env_can_disable(clear_verify_env):
|
||||
clear_verify_env.setenv("HERMES_VERIFY_ON_STOP", "0")
|
||||
assert verify_on_stop_enabled({"agent": {"verify_on_stop": True}}) is False
|
||||
|
||||
|
||||
def test_verify_on_stop_config_can_disable(monkeypatch):
|
||||
monkeypatch.delenv("HERMES_VERIFY_ON_STOP", raising=False)
|
||||
def test_verify_on_stop_config_can_disable(clear_verify_env):
|
||||
assert verify_on_stop_enabled({"agent": {"verify_on_stop": False}}) is False
|
||||
|
||||
|
||||
def test_verify_on_stop_off_on_gateway_messaging_platform(clear_verify_env):
|
||||
# The gateway binds the platform value to HERMES_SESSION_PLATFORM and leaves
|
||||
# HERMES_SESSION_SOURCE empty, so a real Telegram turn must default OFF.
|
||||
clear_verify_env.setenv("HERMES_SESSION_PLATFORM", "telegram")
|
||||
assert verify_on_stop_enabled({"agent": {}}) is False
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"platform",
|
||||
["discord", "whatsapp_cloud", "signal", "slack", "matrix", "email", "sms"],
|
||||
)
|
||||
def test_verify_on_stop_off_for_each_messaging_platform(clear_verify_env, platform):
|
||||
clear_verify_env.setenv("HERMES_SESSION_PLATFORM", platform)
|
||||
assert verify_on_stop_enabled({"agent": {}}) is False
|
||||
|
||||
|
||||
def test_verify_on_stop_messaging_platform_is_case_insensitive(clear_verify_env):
|
||||
clear_verify_env.setenv("HERMES_SESSION_PLATFORM", " Telegram ")
|
||||
assert verify_on_stop_enabled({"agent": {}}) is False
|
||||
|
||||
|
||||
def test_verify_on_stop_uses_hermes_platform_override(clear_verify_env):
|
||||
# HERMES_PLATFORM mirrors the sibling platform resolution and also flags a
|
||||
# messaging surface.
|
||||
clear_verify_env.setenv("HERMES_PLATFORM", "discord")
|
||||
assert verify_on_stop_enabled({"agent": {}}) is False
|
||||
|
||||
|
||||
@pytest.mark.parametrize("source", ["cli", "tui", "desktop", "codex", "local"])
|
||||
def test_verify_on_stop_on_for_interactive_surfaces(clear_verify_env, source):
|
||||
# CLI/TUI/desktop set HERMES_SESSION_SOURCE; these are coding surfaces -> ON.
|
||||
clear_verify_env.setenv("HERMES_SESSION_SOURCE", source)
|
||||
assert verify_on_stop_enabled({"agent": {}}) is True
|
||||
|
||||
|
||||
@pytest.mark.parametrize("platform", ["api_server", "webhook", "msgraph_webhook"])
|
||||
def test_verify_on_stop_on_for_programmatic_surfaces(clear_verify_env, platform):
|
||||
clear_verify_env.setenv("HERMES_SESSION_PLATFORM", platform)
|
||||
assert verify_on_stop_enabled({"agent": {}}) is True
|
||||
|
||||
|
||||
def test_env_forces_verify_on_stop_on_for_messaging(clear_verify_env):
|
||||
clear_verify_env.setenv("HERMES_SESSION_PLATFORM", "telegram")
|
||||
clear_verify_env.setenv("HERMES_VERIFY_ON_STOP", "1")
|
||||
assert verify_on_stop_enabled({"agent": {}}) is True
|
||||
|
||||
|
||||
def test_config_forces_verify_on_stop_on_for_messaging(clear_verify_env):
|
||||
clear_verify_env.setenv("HERMES_SESSION_PLATFORM", "telegram")
|
||||
assert verify_on_stop_enabled({"agent": {"verify_on_stop": True}}) is True
|
||||
|
||||
|
||||
def test_verify_on_stop_default_path_through_load_config(tmp_path, clear_verify_env):
|
||||
# E2E: the sole production caller passes no config, so verify_on_stop_enabled
|
||||
# resolves through load_config() + DEFAULT_CONFIG. The "auto" sentinel must
|
||||
# reach the surface-aware default rather than being shadowed by a static
|
||||
# True. This is the path the unit-level tests above cannot exercise.
|
||||
clear_verify_env.setenv("HERMES_HOME", str(tmp_path / ".hermes"))
|
||||
|
||||
from hermes_cli.config import load_config
|
||||
|
||||
merged = load_config()
|
||||
assert merged["agent"]["verify_on_stop"] == "auto"
|
||||
|
||||
# Interactive (no messaging identity) resolves ON through the real loader.
|
||||
assert verify_on_stop_enabled() is True
|
||||
|
||||
# A messaging platform resolves OFF, proving the sentinel flows through.
|
||||
clear_verify_env.setenv("HERMES_SESSION_PLATFORM", "telegram")
|
||||
assert verify_on_stop_enabled() is False
|
||||
|
||||
|
||||
def test_no_nudge_after_fresh_pass(tmp_path, monkeypatch):
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes"))
|
||||
_node_project(tmp_path)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue