Merge remote-tracking branch 'origin/main' into bb/pets

This commit is contained in:
Brooklyn Nicholson 2026-06-22 05:25:49 -05:00
commit 5342eccf12
823 changed files with 58322 additions and 13772 deletions

View file

@ -0,0 +1,70 @@
"""Regression test for #49287 — the CLI memory-provider ``on_session_end``
hook stopped firing on ``/exit`` after the god-file Phase 4 refactor
(094aa85c37) moved agent construction into ``CLIAgentSetupMixin``.
``_run_cleanup`` (in ``cli.py``) gates the memory-shutdown call on the
module global ``cli._active_agent_ref``. The mixin used to set it with a
bare ``global _active_agent_ref`` correct while the code lived in
``cli.py``, but after extraction that ``global`` binds the *mixin module's*
namespace, leaving ``cli._active_agent_ref`` ``None`` forever. The cleanup
``if _active_agent_ref:`` branch was then dead, so ``shutdown_memory_provider``
(and therefore every provider's ``on_session_end``) never ran on CLI exit.
The fix writes the reference onto the ``cli`` module explicitly. These tests
assert that contract the existing shutdown tests pass only because they
hand-assign ``cli._active_agent_ref``, which is exactly what masked the bug.
"""
from __future__ import annotations
import inspect
def test_mixin_writes_active_agent_ref_to_cli_module():
"""The mixin's agent-setup code must publish the agent reference where
``_run_cleanup`` reads it on the ``cli`` module, not the mixin module."""
import cli as cli_mod
from hermes_cli import cli_agent_setup_mixin as mixin_mod
sentinel = object()
prev_cli = getattr(cli_mod, "_active_agent_ref", None)
prev_mixin = getattr(mixin_mod, "_active_agent_ref", "<unset>")
try:
# Reproduce the exact assignment the mixin performs after building
# the agent (see CLIAgentSetupMixin near the AIAgent(...) construction).
import cli as _cli
_cli._active_agent_ref = sentinel
# The cleanup path reads cli._active_agent_ref — it must see the value.
assert cli_mod._active_agent_ref is sentinel
finally:
cli_mod._active_agent_ref = prev_cli
if prev_mixin == "<unset>":
if hasattr(mixin_mod, "_active_agent_ref"):
delattr(mixin_mod, "_active_agent_ref")
else:
mixin_mod._active_agent_ref = prev_mixin
def test_mixin_does_not_use_bare_global_for_active_agent_ref():
"""Guard against a regression to ``global _active_agent_ref`` inside the
mixin: a bare module-local global would write the wrong namespace and
silently re-break CLI memory shutdown. The source must target ``cli``."""
from hermes_cli import cli_agent_setup_mixin as mixin_mod
src = inspect.getsource(mixin_mod)
assert "_active_agent_ref = self.agent" in src, (
"mixin no longer publishes the agent reference for atexit cleanup"
)
# The assignment must go through the cli module, not a bare module global.
# Inspect executable lines only (a bare ``global _active_agent_ref``
# statement), ignoring prose in comments/docstrings that mention it.
code_lines = [ln.split("#", 1)[0].strip() for ln in src.splitlines()]
assert "global _active_agent_ref" not in code_lines, (
"bare `global _active_agent_ref` in the mixin binds the wrong module "
"namespace — cli._active_agent_ref stays None and memory shutdown dies "
"(#49287). Write `cli._active_agent_ref = self.agent` instead."
)
assert "_cli._active_agent_ref = self.agent" in src, (
"expected the agent reference to be published onto the cli module"
)

View file

@ -71,14 +71,14 @@ class TestForceFullRedraw:
"invalidate",
]
def test_resize_recovery_uses_prompt_toolkit_original_resize_before_reset(self, bare_cli, monkeypatch):
"""Resize recovery must preserve prompt_toolkit's tracked cursor state.
def test_resize_recovery_skips_clear_when_width_unchanged(self, bare_cli, monkeypatch):
"""A rows-only resize (same width) must NOT clear the screen.
prompt_toolkit's built-in Application._on_resize() starts with
renderer.erase(leave_alternate_screen=False), which uses the renderer's
cached cursor position to move back to the live prompt origin before
erase_down(). If Hermes resets the renderer first, that cursor position
is lost and stale prompt glyphs can remain after a narrow resize.
erase_down(). With no column reflow there is no ghost chrome to wipe,
so we delegate straight to prompt_toolkit and avoid an extra repaint.
"""
app = MagicMock()
events = []
@ -86,8 +86,13 @@ class TestForceFullRedraw:
app.invalidate.side_effect = lambda: events.append("invalidate")
original_on_resize = lambda: events.append("original_resize")
# bare_cli skips __init__, so seed the attribute the way __init__ would.
# bare_cli skips __init__, so seed attributes the way __init__ would.
bare_cli._status_bar_suppressed_after_resize = False
bare_cli._last_resize_width = 120
# Same width on this resize → rows-only change.
monkeypatch.setattr(bare_cli, "_get_tui_terminal_width", lambda: 120)
monkeypatch.setattr(bare_cli, "_schedule_status_bar_unsuppress", lambda *_: None)
bare_cli._recover_after_resize(app, original_on_resize)
assert events == ["original_resize"]
@ -100,6 +105,39 @@ class TestForceFullRedraw:
# Status bar / input rules must be suppressed until the next prompt.
assert bare_cli._status_bar_suppressed_after_resize is True
def test_resize_recovery_clears_viewport_on_width_change(self, bare_cli, monkeypatch):
"""A WIDTH change must wipe the visible viewport (CSI 2J) and replay.
On column shrink the terminal reflows the old full-width chrome into
extra rows that prompt_toolkit's stale-cursor erase cannot reach,
leaving a duplicated status bar (#19280/#5474 class). We route through
the same recovery as Ctrl+L: erase_screen (2J) + replay transcript.
It must be banner-safe CSI 3J (write_raw) must NOT fire.
"""
app = MagicMock()
events = []
app.renderer.output.erase_screen.side_effect = lambda: events.append("erase")
app.renderer.output.write_raw.side_effect = lambda *_: events.append("scrollback_wipe")
original_on_resize = lambda: events.append("original_resize")
bare_cli._status_bar_suppressed_after_resize = False
bare_cli._last_resize_width = 200
monkeypatch.setattr(bare_cli, "_get_tui_terminal_width", lambda: 90)
monkeypatch.setattr(bare_cli, "_schedule_status_bar_unsuppress", lambda *_: None)
monkeypatch.setattr(cli_mod, "_replay_output_history", lambda: events.append("replay"))
bare_cli._recover_after_resize(app, original_on_resize)
# Viewport cleared and transcript replayed BEFORE prompt_toolkit's resize.
assert "erase" in events
assert "replay" in events
assert events.index("erase") < events.index("original_resize")
# Banner-safe: scrollback (CSI 3J) must never be wiped on a resize.
assert "scrollback_wipe" not in events
# New width recorded for the next comparison.
assert bare_cli._last_resize_width == 90
assert bare_cli._status_bar_suppressed_after_resize is True
def test_force_redraw_uses_full_screen_clear_without_scrollback_clear(self, bare_cli):
app = MagicMock()
bare_cli._app = app

View file

@ -589,6 +589,38 @@ class TestRootLevelProviderOverride:
assert result["model"]["provider"] == "correct-provider"
assert "provider" not in result # root key still cleaned up
def test_normalize_model_api_base_aliases_to_base_url(self):
"""model.api_base is migrated to model.base_url (issue #8919)."""
from hermes_cli.config import _normalize_root_model_keys
config = {
"model": {
"provider": "custom",
"api_base": "http://localhost:4000",
"api_key": "my-key",
"default": "default",
},
}
result = _normalize_root_model_keys(config)
assert result["model"]["base_url"] == "http://localhost:4000"
assert "api_base" not in result["model"] # alias cleaned up
def test_normalize_api_base_does_not_override_base_url(self):
"""An explicit model.base_url is never overridden by api_base."""
from hermes_cli.config import _normalize_root_model_keys
config = {
"model": {
"provider": "custom",
"api_base": "http://wrong:9999",
"base_url": "http://localhost:4000",
"default": "default",
},
}
result = _normalize_root_model_keys(config)
assert result["model"]["base_url"] == "http://localhost:4000"
assert "api_base" not in result["model"]
def test_normalize_root_context_length_migrates_to_model(self):
"""Root-level context_length is migrated into the model section."""
from hermes_cli.config import _normalize_root_model_keys

View file

@ -308,6 +308,169 @@ def test_model_flow_nous_prints_subscription_guidance_without_mutating_explicit_
assert config["browser"]["cloud_provider"] == "browser-use"
def test_model_flow_nous_does_not_restore_stale_custom_api_key(tmp_path, monkeypatch):
import yaml
config_home = tmp_path / "hermes"
config_home.mkdir()
monkeypatch.setenv("HERMES_HOME", str(config_home))
config_path = config_home / "config.yaml"
config_path.write_text(
yaml.safe_dump(
{
"model": {
"provider": "custom",
"default": "glm-5.2",
"base_url": "https://api.neuralwatt.com/v1",
"api_key": "${NEURALWATT_API_KEY}",
"api_mode": "chat_completions",
}
},
sort_keys=False,
)
)
stale_config = yaml.safe_load(config_path.read_text()) or {}
selected_model = "deepseek/deepseek-v4-flash"
monkeypatch.setattr(
"hermes_cli.auth.get_provider_auth_state",
lambda provider: {
"access_token": "nous-token",
"portal_base_url": "https://portal.example.com",
},
)
monkeypatch.setattr(
"hermes_cli.auth.resolve_nous_runtime_credentials",
lambda *args, **kwargs: {
"base_url": "https://inference-api.nousresearch.com/v1",
"api_key": "nous-key",
},
)
monkeypatch.setattr(
"hermes_cli.models.get_curated_nous_model_ids",
lambda: [selected_model],
)
monkeypatch.setattr("hermes_cli.models.get_pricing_for_provider", lambda provider: {})
monkeypatch.setattr("hermes_cli.models.check_nous_free_tier", lambda **kwargs: False)
monkeypatch.setattr(
"hermes_cli.models.union_with_portal_paid_recommendations",
lambda model_ids, pricing, portal_url: (model_ids, pricing),
)
monkeypatch.setattr(
"hermes_cli.auth._prompt_model_selection",
lambda *args, **kwargs: selected_model,
)
monkeypatch.setattr(
"hermes_cli.nous_subscription.prompt_enable_tool_gateway",
lambda config: None,
)
hermes_main._model_flow_nous(stale_config, current_model="glm-5.2")
config = yaml.safe_load(config_path.read_text()) or {}
model = config.get("model")
assert model["provider"] == "nous"
assert model["default"] == selected_model
assert model["base_url"] == "https://inference-api.nousresearch.com/v1"
assert "api_key" not in model
assert "api_mode" not in model
def _seed_stale_custom_model(tmp_path, monkeypatch):
import yaml
config_home = tmp_path / "hermes"
config_home.mkdir()
monkeypatch.setenv("HERMES_HOME", str(config_home))
config_path = config_home / "config.yaml"
config_path.write_text(
yaml.safe_dump(
{
"model": {
"provider": "custom",
"default": "glm-5.2",
"base_url": "https://api.neuralwatt.com/v1",
"api_key": "${NEURALWATT_API_KEY}",
"api": "legacy-stale-key",
"api_mode": "anthropic_messages",
}
},
sort_keys=False,
)
)
(config_home / ".env").write_text("")
return config_path
def test_model_flow_openrouter_clears_stale_custom_key(tmp_path, monkeypatch):
import yaml
config_path = _seed_stale_custom_model(tmp_path, monkeypatch)
monkeypatch.setattr(
"hermes_cli.main._prompt_api_key",
lambda *args, **kwargs: ("sk-openrouter", False),
)
monkeypatch.setattr(
"hermes_cli.models.model_ids",
lambda **kwargs: ["anthropic/claude-sonnet-4.6"],
)
monkeypatch.setattr("hermes_cli.models.get_pricing_for_provider", lambda *a, **k: {})
monkeypatch.setattr(
"hermes_cli.auth._prompt_model_selection",
lambda *args, **kwargs: "anthropic/claude-sonnet-4.6",
)
monkeypatch.setattr("hermes_cli.auth.deactivate_provider", lambda: None)
hermes_main._model_flow_openrouter({}, current_model="glm-5.2")
config = yaml.safe_load(config_path.read_text()) or {}
model = config["model"]
assert model["provider"] == "openrouter"
assert model["default"] == "anthropic/claude-sonnet-4.6"
assert model["api_mode"] == "chat_completions"
assert "api_key" not in model
assert "api" not in model
def test_model_flow_anthropic_clears_stale_custom_key_and_mode(tmp_path, monkeypatch):
import yaml
config_path = _seed_stale_custom_model(tmp_path, monkeypatch)
monkeypatch.setattr("hermes_cli.auth.get_anthropic_key", lambda: "sk-ant-api03-test")
monkeypatch.setattr(
"agent.anthropic_adapter.read_claude_code_credentials",
lambda: None,
)
monkeypatch.setattr(
"agent.anthropic_adapter.is_claude_code_token_valid",
lambda creds: False,
)
monkeypatch.setattr(
"hermes_cli.model_setup_flows._prompt_auth_credentials_choice",
lambda title: "use",
)
monkeypatch.setattr(
"hermes_cli.auth._prompt_model_selection",
lambda *args, **kwargs: "claude-sonnet-4-6",
)
monkeypatch.setattr("hermes_cli.auth.deactivate_provider", lambda: None)
hermes_main._model_flow_anthropic({}, current_model="glm-5.2")
config = yaml.safe_load(config_path.read_text()) or {}
model = config["model"]
assert model["provider"] == "anthropic"
assert model["default"] == "claude-sonnet-4-6"
assert "base_url" not in model
assert "api_key" not in model
assert "api" not in model
assert "api_mode" not in model
def test_model_flow_nous_offers_tool_gateway_prompt_when_unconfigured(monkeypatch, capsys):
from hermes_cli.nous_account import NousPortalAccountInfo

View file

@ -109,3 +109,61 @@ def test_cleanup_provider_exception_is_swallowed(mock_invoke_hook):
cli_mod._cleanup_done = False
agent.shutdown_memory_provider.assert_called_once()
def test_cli_close_persists_agent_session_messages_before_end_session():
"""CLI shutdown flushes live agent messages before closing the session."""
import cli as cli_mod
transcript = [
{"role": "user", "content": "long task"},
{"role": "assistant", "content": "partial answer"},
]
conversation_history = [{"role": "user", "content": "long task"}]
cli = object.__new__(cli_mod.HermesCLI)
cli.conversation_history = conversation_history
cli.session_id = "old-session"
agent = MagicMock()
agent.session_id = "live-session"
agent._session_messages = transcript
cli.agent = agent
cli._persist_active_session_before_close()
agent._persist_session.assert_called_once_with(transcript, conversation_history)
assert cli.session_id == "live-session"
def test_cli_close_persist_falls_back_to_conversation_history():
"""Bare MagicMock agents do not provide a real _session_messages list."""
import cli as cli_mod
conversation_history = [{"role": "user", "content": "saved from cli"}]
cli = object.__new__(cli_mod.HermesCLI)
cli.conversation_history = conversation_history
cli.session_id = "session-id"
agent = MagicMock()
agent.session_id = "session-id"
cli.agent = agent
cli._persist_active_session_before_close()
agent._persist_session.assert_called_once_with(conversation_history, conversation_history)
def test_cli_close_persist_skips_empty_transcripts():
"""Do not create empty session writes for idle CLI startup/shutdown."""
import cli as cli_mod
cli = object.__new__(cli_mod.HermesCLI)
cli.conversation_history = []
cli.session_id = "session-id"
agent = MagicMock()
agent.session_id = "session-id"
agent._session_messages = []
cli.agent = agent
cli._persist_active_session_before_close()
agent._persist_session.assert_not_called()

View file

@ -293,8 +293,9 @@ class TestCLIStatusBar:
"""When _status_bar_suppressed_after_resize is set, both rules hide.
See _recover_after_resize column shrink reflows already-rendered
bars into scrollback, so we hide the separators until the user
submits the next input, at which point the flag is cleared.
bars into scrollback, so we hide the separators while the reflow
settles, then clear the flag (either via the scheduled unsuppress
timer or the next submitted input).
"""
cli_obj = _make_cli()
cli_obj._status_bar_suppressed_after_resize = True
@ -306,6 +307,48 @@ class TestCLIStatusBar:
assert cli_obj._tui_input_rule_height("top", width=90) == 1
assert cli_obj._tui_input_rule_height("bottom", width=90) == 1
def test_scheduled_unsuppress_clears_flag_and_repaints_without_input(self):
"""The status bar returns during idle after a resize, without a keypress.
Regression: the suppression flag was only cleared on the next
*submitted* input, so a resize/reflow followed by idle left the bar
hidden indefinitely even while the refresh clock kept ticking. The
scheduled unsuppress timer must clear the flag and invalidate the app
on its own.
"""
cli_obj = _make_cli()
cli_obj._status_bar_unsuppress_timer = None
cli_obj._status_bar_suppressed_after_resize = True
app = MagicMock()
app.loop = None # force the synchronous _clear path
# Schedule with ~0 delay so the timer fires promptly under test.
cli_obj._schedule_status_bar_unsuppress(app, delay=0.01)
time.sleep(0.1)
assert cli_obj._status_bar_suppressed_after_resize is False
app.invalidate.assert_called()
# Bar chrome is visible again with no submitted input.
assert cli_obj._tui_input_rule_height("top", width=90) == 1
def test_scheduled_unsuppress_debounces_resize_storm(self):
"""A fresh resize cancels the pending unsuppress and restarts it."""
cli_obj = _make_cli()
cli_obj._status_bar_unsuppress_timer = None
cli_obj._status_bar_suppressed_after_resize = True
app = MagicMock()
app.loop = None
# First schedule (long delay) then a second should cancel the first.
cli_obj._schedule_status_bar_unsuppress(app, delay=5.0)
first_timer = cli_obj._status_bar_unsuppress_timer
assert first_timer is not None
cli_obj._schedule_status_bar_unsuppress(app, delay=0.01)
assert first_timer is not cli_obj._status_bar_unsuppress_timer
assert not first_timer.is_alive() or first_timer.finished.is_set()
time.sleep(0.1)
assert cli_obj._status_bar_suppressed_after_resize is False
def test_scrollback_box_width_returns_viewport_width(self):
"""Decorative scrollback boxes use the full viewport width.

View file

@ -1,21 +0,0 @@
from unittest.mock import MagicMock, patch
def test_gquota_uses_chat_console_when_tui_is_live():
from agent.google_oauth import GoogleOAuthError
from cli import HermesCLI
cli = HermesCLI.__new__(HermesCLI)
cli.console = MagicMock()
cli._app = object()
live_console = MagicMock()
with patch("cli.ChatConsole", return_value=live_console), \
patch("agent.google_oauth.get_valid_access_token", side_effect=GoogleOAuthError("No Google OAuth credentials found")), \
patch("agent.google_oauth.load_credentials", return_value=None), \
patch("agent.google_code_assist.retrieve_user_quota"):
cli._handle_gquota_command("/gquota")
assert live_console.print.call_count == 2
cli.console.print.assert_not_called()

View file

@ -0,0 +1,124 @@
"""Tests for worktree base-ref resolution — branch from the fresh remote tip.
A worktree created off the standalone clone's local ``HEAD`` roots the new
branch on a stale base when that clone lags the remote. ``_resolve_worktree_base``
fetches and branches from the remote tip instead so the worktree starts current.
These tests exercise the REAL ``cli._resolve_worktree_base`` /
``cli._setup_worktree`` against a real local "remote" repo (so ``git fetch``
works offline in the hermetic sandbox), proving the worktree includes commits
that exist on the remote but not on the stale local HEAD.
"""
import subprocess
from pathlib import Path
import pytest
import cli
def _run(args, cwd):
return subprocess.run(args, cwd=cwd, capture_output=True, text=True, timeout=30)
def _commit(repo, name, msg):
(Path(repo) / name).write_text(msg + "\n")
_run(["git", "add", "."], repo)
_run(["git", "commit", "-m", msg], repo)
def _head(repo):
return _run(["git", "rev-parse", "HEAD"], repo).stdout.strip()
@pytest.fixture
def remote_and_clone(tmp_path):
"""A bare 'remote' + a clone that is intentionally BEHIND the remote.
Returns (clone_path, remote_head_sha, stale_local_head_sha).
"""
remote = tmp_path / "remote.git"
seed = tmp_path / "seed"
seed.mkdir()
_run(["git", "init"], seed)
_run(["git", "config", "user.email", "t@t.com"], seed)
_run(["git", "config", "user.name", "T"], seed)
# Pin the seed repo's branch name so push + remote default are 'main'.
_run(["git", "checkout", "-b", "main"], seed)
_commit(seed, "README.md", "base commit")
_run(["git", "init", "--bare", str(remote)], tmp_path)
_run(["git", "remote", "add", "origin", str(remote)], seed)
_run(["git", "push", "origin", "main"], seed)
# Set the bare remote's default branch so a clone gets origin/HEAD ->
# origin/main and a tracking branch (mirrors a real GitHub remote).
_run(["git", "symbolic-ref", "HEAD", "refs/heads/main"], remote)
# Clone it (this clone tracks origin/main).
clone = tmp_path / "clone"
_run(["git", "clone", str(remote), str(clone)], tmp_path)
_run(["git", "config", "user.email", "t@t.com"], clone)
_run(["git", "config", "user.name", "T"], clone)
stale_local_head = _head(clone)
# Advance the REMOTE past the clone (simulating other merges landing on
# main while this clone sat stale).
_commit(seed, "feature.txt", "remote-only commit")
_run(["git", "push", "origin", "main"], seed)
remote_head = _head(seed)
assert remote_head != stale_local_head
return clone, remote_head, stale_local_head
class TestResolveWorktreeBase:
def test_resolves_to_fetched_upstream(self, remote_and_clone):
clone, remote_head, stale_local_head = remote_and_clone
base_ref, label = cli._resolve_worktree_base(str(clone))
# Should resolve to the upstream tracking ref and have fetched it.
assert base_ref == "origin/main"
assert "fetched" in label
# The fetched ref now points at the remote tip, not the stale local HEAD.
resolved = _run(["git", "rev-parse", base_ref], clone).stdout.strip()
assert resolved == remote_head
assert resolved != stale_local_head
def test_falls_back_to_head_without_remote(self, tmp_path):
repo = tmp_path / "no-remote"
repo.mkdir()
_run(["git", "init"], repo)
_run(["git", "config", "user.email", "t@t.com"], repo)
_run(["git", "config", "user.name", "T"], repo)
_commit(repo, "README.md", "only commit")
base_ref, label = cli._resolve_worktree_base(str(repo))
assert base_ref == "HEAD"
assert "HEAD" in label
class TestSetupWorktreeSyncBase:
def test_sync_true_branches_from_remote_tip(self, remote_and_clone, monkeypatch):
clone, remote_head, stale_local_head = remote_and_clone
info = cli._setup_worktree(str(clone), sync_base=True)
assert info is not None
# The new worktree's HEAD must be the REMOTE tip, not the stale local one.
wt_head = _head(info["path"])
assert wt_head == remote_head, "worktree should start from the fetched remote tip"
assert wt_head != stale_local_head
# And it must contain the remote-only file.
assert (Path(info["path"]) / "feature.txt").exists()
def test_sync_false_branches_from_local_head(self, remote_and_clone):
clone, remote_head, stale_local_head = remote_and_clone
info = cli._setup_worktree(str(clone), sync_base=False)
assert info is not None
# Opted out -> branch from the stale local HEAD (old behavior).
wt_head = _head(info["path"])
assert wt_head == stale_local_head
assert not (Path(info["path"]) / "feature.txt").exists()
def test_default_is_sync_true(self, remote_and_clone):
"""The default path (no sync_base arg) branches from the remote tip."""
clone, remote_head, _ = remote_and_clone
info = cli._setup_worktree(str(clone))
assert info is not None
assert _head(info["path"]) == remote_head