review-fix(public-api): restore get_session_activity, latest_user_message_row_id, resolve_multiple_toolsets, has_provider, nous_token_has_billing_scope, curated_models_for_provider, clear_edit_approval_requester + tests
All public on BASE 63279301bc, dropped by the simplify refactor (their tests were deleted or
rewritten to the replacement API). Restore each with BASE signature/body as a thin wrapper over the
surviving implementation, and restore the tests at the original call sites: test_message_reactions
again asserts the role=user contract (a newer assistant message is never the default target);
test_hermes_state / test_watchdog_review_76354 go back to get_session_activity(); toolsets, acp auth,
edit_approval, billing-scope and curated-models tests restored/extended.
This commit is contained in:
@@ -27,6 +27,11 @@ def detect_provider() -> Optional[str]:
|
||||
return None
|
||||
|
||||
|
||||
def has_provider() -> bool:
|
||||
"""Return True if Hermes can resolve any runtime provider credentials."""
|
||||
return detect_provider() is not None
|
||||
|
||||
|
||||
def build_auth_methods() -> list[Any]:
|
||||
"""Return registry-compatible ACP auth methods for Hermes.
|
||||
|
||||
|
||||
@@ -56,6 +56,16 @@ def reset_edit_approval_requester(token: Token) -> None:
|
||||
_EDIT_APPROVAL_REQUESTER.reset(token)
|
||||
|
||||
|
||||
def clear_edit_approval_requester() -> None:
|
||||
"""Clear the current requester; primarily used by tests."""
|
||||
|
||||
_EDIT_APPROVAL_REQUESTER.set(None)
|
||||
|
||||
|
||||
def get_edit_approval_requester() -> EditApprovalRequester | None:
|
||||
return _EDIT_APPROVAL_REQUESTER.get()
|
||||
|
||||
|
||||
def _read_text_if_exists(path: str) -> str | None:
|
||||
p = Path(path).expanduser()
|
||||
if p.is_file():
|
||||
|
||||
@@ -998,6 +998,24 @@ def get_provider_auth_state(provider_id: str) -> Optional[Dict[str, Any]]:
|
||||
return _load_provider_state(_load_auth_store(), provider_id)
|
||||
|
||||
|
||||
def nous_token_has_billing_scope() -> bool:
|
||||
"""Return True if the currently-held Nous token carries ``billing:manage``.
|
||||
|
||||
Reads the persisted ``scope`` string saved at login (``_save_provider_state``
|
||||
stores ``token_data.get("scope") or scope``). A space-delimited match. Used by
|
||||
the lazy step-up: if False, the first billing call will 403 ``insufficient_scope``
|
||||
anyway, but checking up front lets a surface skip a doomed round-trip.
|
||||
"""
|
||||
try:
|
||||
state = get_provider_auth_state("nous") or {}
|
||||
except Exception:
|
||||
return False
|
||||
scope = state.get("scope")
|
||||
if not isinstance(scope, str):
|
||||
return False
|
||||
return NOUS_BILLING_MANAGE_SCOPE in scope.split()
|
||||
|
||||
|
||||
def get_active_provider() -> Optional[str]:
|
||||
"""Return the currently active provider ID from auth store."""
|
||||
return _load_auth_store().get("active_provider")
|
||||
|
||||
@@ -758,6 +758,31 @@ def _anthropic_models_url(base_url: Optional[str] = None) -> str:
|
||||
return endpoint + ("/models" if endpoint.endswith("/v1") else "/v1/models")
|
||||
|
||||
|
||||
def curated_models_for_provider(
|
||||
provider: Optional[str],
|
||||
*,
|
||||
force_refresh: bool = False,
|
||||
) -> list[tuple[str, str]]:
|
||||
"""Return ``(model_id, description)`` tuples for a provider's model list.
|
||||
|
||||
Tries to fetch the live model list from the provider's API first,
|
||||
falling back to the static ``_PROVIDER_MODELS`` catalog if the API
|
||||
is unreachable.
|
||||
"""
|
||||
normalized = normalize_provider(provider)
|
||||
if normalized == "openrouter":
|
||||
return fetch_openrouter_models(force_refresh=force_refresh)
|
||||
|
||||
# Try live API first (Codex, Nous, etc. all support /models)
|
||||
live = provider_model_ids(normalized)
|
||||
if live:
|
||||
return [(m, "") for m in live]
|
||||
|
||||
# Fallback to static catalog
|
||||
models = _PROVIDER_MODELS.get(normalized, [])
|
||||
return [(m, "") for m in models]
|
||||
|
||||
|
||||
def _provider_keys(provider: str) -> set[str]:
|
||||
key = (provider or "").strip().lower()
|
||||
normalized = normalize_provider(provider)
|
||||
|
||||
@@ -396,6 +396,15 @@ class SessionMessagesMixin:
|
||||
(session_id, role, int(offset)))
|
||||
return row[0] if row else None
|
||||
|
||||
def latest_user_message_row_id(self, session_id: str) -> Optional[int]:
|
||||
"""Row id of the most recent active user message, or ``None``.
|
||||
|
||||
The agent's default reaction target: "the message that triggered me",
|
||||
so the model never has to thread row ids through a tool call (mirrors
|
||||
the photon adapter's ``_record_last_inbound``).
|
||||
"""
|
||||
return self.latest_message_row_id(session_id, role="user")
|
||||
|
||||
def get_message_role(self, session_id: str, row_id: int) -> Optional[str]:
|
||||
"""Role of the active message at *row_id* in *session_id*, or ``None``."""
|
||||
if not session_id:
|
||||
|
||||
@@ -538,6 +538,21 @@ class SessionSessionsMixin:
|
||||
("", ActivityProvenance.UNKNOWN.value, session_id), patience_s=self._ACTIVITY_WRITE_PATIENCE_S,
|
||||
)
|
||||
|
||||
def get_session_activity(self, session_id: str) -> Optional[Dict[str, Any]]:
|
||||
"""Return the durable activity snapshot for *session_id*, or None."""
|
||||
if not session_id:
|
||||
return None
|
||||
row = self.get_session(session_id)
|
||||
if not row:
|
||||
return None
|
||||
from agent.session_activity import build_activity_snapshot
|
||||
|
||||
return build_activity_snapshot(
|
||||
last_activity_at=row.get("last_activity_at"),
|
||||
last_activity_description=row.get("last_activity_description"),
|
||||
last_activity_provenance=row.get("last_activity_provenance"),
|
||||
)
|
||||
|
||||
def update_session_meta(
|
||||
self, session_id: str, model_config_json: str, model: Optional[str] = None,
|
||||
) -> None:
|
||||
|
||||
@@ -3,10 +3,27 @@
|
||||
from acp_adapter.auth import (
|
||||
TERMINAL_SETUP_AUTH_METHOD_ID,
|
||||
build_auth_methods,
|
||||
has_provider,
|
||||
detect_provider,
|
||||
)
|
||||
|
||||
|
||||
class TestHasProvider:
|
||||
def test_has_provider_with_resolved_runtime(self, monkeypatch):
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.runtime_provider.resolve_runtime_provider",
|
||||
lambda: {"provider": "openrouter", "api_key": "sk-or-test"},
|
||||
)
|
||||
assert has_provider() is True
|
||||
|
||||
def test_has_provider_false_without_credentials(self, monkeypatch):
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.runtime_provider.resolve_runtime_provider",
|
||||
lambda: {"provider": "openrouter", "api_key": ""},
|
||||
)
|
||||
assert has_provider() is False
|
||||
|
||||
|
||||
class TestDetectProvider:
|
||||
def test_detect_openrouter(self, monkeypatch):
|
||||
monkeypatch.setattr(
|
||||
|
||||
@@ -9,6 +9,7 @@ from pathlib import Path
|
||||
from acp_adapter.edit_approval import (
|
||||
EditProposal,
|
||||
build_acp_edit_tool_call,
|
||||
clear_edit_approval_requester,
|
||||
set_edit_approval_requester,
|
||||
should_auto_approve_edit,
|
||||
)
|
||||
@@ -16,7 +17,7 @@ from model_tools import handle_function_call
|
||||
|
||||
|
||||
def teardown_function() -> None:
|
||||
set_edit_approval_requester(None)
|
||||
clear_edit_approval_requester()
|
||||
|
||||
|
||||
def test_acp_permission_tool_call_uses_edit_kind_and_diff_content():
|
||||
|
||||
@@ -34,6 +34,7 @@ class TestDetectProviderEntra:
|
||||
side_effect=_fake_runtime,
|
||||
):
|
||||
assert _acp_auth.detect_provider() == "azure-foundry"
|
||||
assert _acp_auth.has_provider() is True
|
||||
|
||||
def test_string_api_key_still_works(self):
|
||||
from acp_adapter import auth as _acp_auth
|
||||
|
||||
@@ -77,7 +77,7 @@ def test_s1_clear_labels_noop_skips_transaction(tmp_path, monkeypatch):
|
||||
calls.clear()
|
||||
db.clear_session_activity_labels(sid)
|
||||
assert len(calls) == 1
|
||||
activity = db.get_session(sid)
|
||||
activity = db.get_session_activity(sid)
|
||||
assert activity["last_activity_description"] == ""
|
||||
|
||||
|
||||
|
||||
@@ -7,10 +7,40 @@ import pytest
|
||||
import hermes_cli.auth as auth
|
||||
from hermes_cli.auth import (
|
||||
NOUS_BILLING_MANAGE_SCOPE,
|
||||
nous_token_has_billing_scope,
|
||||
step_up_nous_billing_scope,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# nous_token_has_billing_scope
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestNousTokenHasBillingScope:
|
||||
def test_true_when_scope_string_contains_billing_manage(self, monkeypatch):
|
||||
monkeypatch.setattr(
|
||||
auth, "get_provider_auth_state",
|
||||
lambda pid: {"scope": f"openid {NOUS_BILLING_MANAGE_SCOPE} inference"},
|
||||
)
|
||||
assert nous_token_has_billing_scope() is True
|
||||
|
||||
def test_false_when_scope_missing_or_not_a_string(self, monkeypatch):
|
||||
monkeypatch.setattr(auth, "get_provider_auth_state", lambda pid: {"scope": "openid inference"})
|
||||
assert nous_token_has_billing_scope() is False
|
||||
monkeypatch.setattr(auth, "get_provider_auth_state", lambda pid: {"scope": None})
|
||||
assert nous_token_has_billing_scope() is False
|
||||
monkeypatch.setattr(auth, "get_provider_auth_state", lambda pid: None)
|
||||
assert nous_token_has_billing_scope() is False
|
||||
|
||||
def test_false_when_auth_state_lookup_raises(self, monkeypatch):
|
||||
def _boom(pid):
|
||||
raise RuntimeError("auth store unreadable")
|
||||
|
||||
monkeypatch.setattr(auth, "get_provider_auth_state", _boom)
|
||||
assert nous_token_has_billing_scope() is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# step_up_nous_billing_scope
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -7,6 +7,7 @@ from hermes_cli.models import (
|
||||
azure_foundry_model_api_mode,
|
||||
copilot_model_api_mode,
|
||||
fetch_github_model_catalog,
|
||||
curated_models_for_provider,
|
||||
fetch_api_models,
|
||||
fetch_lmstudio_models,
|
||||
github_model_reasoning_efforts,
|
||||
@@ -56,6 +57,33 @@ class TestParseModelInput:
|
||||
assert model == "anthropic/claude-sonnet-4.5"
|
||||
|
||||
|
||||
# -- curated_models_for_provider ---------------------------------------------
|
||||
|
||||
class TestCuratedModelsForProvider:
|
||||
def test_openrouter_returns_curated_list(self):
|
||||
with patch(
|
||||
"hermes_cli.models.fetch_openrouter_models",
|
||||
return_value=[
|
||||
("anthropic/claude-opus-4.6", "recommended"),
|
||||
("qwen/qwen3.6-plus", ""),
|
||||
],
|
||||
):
|
||||
models = curated_models_for_provider("openrouter")
|
||||
assert len(models) > 0
|
||||
assert any("claude" in m[0] for m in models)
|
||||
|
||||
def test_unknown_provider_returns_empty(self):
|
||||
assert curated_models_for_provider("totally-unknown") == []
|
||||
|
||||
def test_live_catalog_projected_to_tuples_else_static_fallback(self):
|
||||
with patch("hermes_cli.models.provider_model_ids", return_value=["m-live"]):
|
||||
assert curated_models_for_provider("nous") == [("m-live", "")]
|
||||
with patch("hermes_cli.models.provider_model_ids", return_value=[]), patch.dict(
|
||||
"hermes_cli.models._PROVIDER_MODELS", {"nous": ["m-static"]}
|
||||
):
|
||||
assert curated_models_for_provider("nous") == [("m-static", "")]
|
||||
|
||||
|
||||
# -- normalize_provider ------------------------------------------------------
|
||||
|
||||
class TestNormalizeProvider:
|
||||
|
||||
@@ -2375,7 +2375,7 @@ class TestListSessionsRich:
|
||||
assert row["last_activity_description"] == "starting API call #1"
|
||||
assert row["last_activity_provenance"] == "unknown"
|
||||
|
||||
activity = db.get_session("s1")
|
||||
activity = db.get_session_activity("s1")
|
||||
assert activity["last_activity_at"] == heartbeat
|
||||
assert activity["last_activity_description"] == "starting API call #1"
|
||||
assert "phase" not in activity
|
||||
@@ -2405,7 +2405,7 @@ class TestListSessionsRich:
|
||||
assert row["last_activity_at"] == heartbeat
|
||||
assert row["last_activity_description"] == ""
|
||||
assert row["last_activity_provenance"] == "unknown"
|
||||
activity = db.get_session("s1")
|
||||
activity = db.get_session_activity("s1")
|
||||
assert activity["last_activity_at"] == heartbeat
|
||||
assert activity["last_activity_description"] == ""
|
||||
assert activity["last_activity_provenance"] == "unknown"
|
||||
@@ -2448,7 +2448,7 @@ class TestListSessionsRich:
|
||||
rows = db.list_gateway_sessions(active_only=True)
|
||||
assert len(rows) == 1
|
||||
assert rows[0]["last_active"] == heartbeat
|
||||
activity = db.get_session("gw-1")
|
||||
activity = db.get_session_activity("gw-1")
|
||||
assert activity["last_activity_description"] == "compressing context"
|
||||
|
||||
def test_order_by_last_active_surfaces_recently_touched_older_session_first(self, db):
|
||||
|
||||
@@ -152,12 +152,18 @@ def test_reacting_never_mutates_message_content(session, db):
|
||||
def test_latest_user_message_is_the_agents_default_target(session, db):
|
||||
"""The agent reacts to "the message that triggered me" without an id."""
|
||||
key, rows = session
|
||||
assert db.latest_message_row_id(key) == rows[0]
|
||||
assert db.latest_user_message_row_id(key) == rows[0]
|
||||
|
||||
db.append_message(key, "user", "thanks!")
|
||||
newest = db.get_messages_as_conversation(key, include_row_ids=True)[-1]["_row_id"]
|
||||
|
||||
assert db.latest_message_row_id(key) == newest
|
||||
assert db.latest_user_message_row_id(key) == newest
|
||||
|
||||
# Role-targeting contract: a newer ASSISTANT message must not become the
|
||||
# agent's default target — it always means the latest USER message.
|
||||
db.append_message(key, "assistant", "you're welcome")
|
||||
assert db.latest_user_message_row_id(key) == newest
|
||||
assert db.latest_message_row_id(key, role="assistant") != newest
|
||||
|
||||
|
||||
def test_row_id_is_opt_in_and_never_reaches_the_provider(session, db):
|
||||
|
||||
@@ -6,6 +6,7 @@ from toolsets import (
|
||||
TOOLSETS,
|
||||
get_toolset,
|
||||
resolve_toolset,
|
||||
resolve_multiple_toolsets,
|
||||
get_all_toolsets,
|
||||
validate_toolset,
|
||||
create_custom_toolset,
|
||||
@@ -105,6 +106,16 @@ class TestResolveToolset:
|
||||
|
||||
|
||||
|
||||
class TestResolveMultipleToolsets:
|
||||
def test_combines_and_deduplicates(self):
|
||||
tools = resolve_multiple_toolsets(["web", "terminal"])
|
||||
assert "web_search" in tools
|
||||
assert "web_extract" in tools
|
||||
assert "terminal" in tools
|
||||
# No duplicates
|
||||
assert len(tools) == len(set(tools))
|
||||
|
||||
|
||||
class TestValidateToolset:
|
||||
def test_valid(self):
|
||||
assert validate_toolset("web") is True
|
||||
|
||||
@@ -370,6 +370,14 @@ def resolve_toolset(name: str, visited: Set[str] = None, *, include_registry: bo
|
||||
return result
|
||||
|
||||
|
||||
def resolve_multiple_toolsets(toolset_names: List[str]) -> List[str]:
|
||||
"""Resolve several toolsets and return the combined, deduplicated, sorted tool names."""
|
||||
all_tools = set()
|
||||
for name in toolset_names:
|
||||
all_tools.update(resolve_toolset(name))
|
||||
return sorted(all_tools)
|
||||
|
||||
|
||||
def _get_plugin_toolset_names() -> Set[str]:
|
||||
"""Registry toolset names absent from the static TOOLSETS dict."""
|
||||
return {n for n in _registry_call("get_registered_toolset_names", ()) if n not in TOOLSETS}
|
||||
|
||||
Reference in New Issue
Block a user