From 34abf954bda5e5428c20eb715ad96c5772159692 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Thu, 3 Sep 2026 09:40:49 -0700 Subject: [PATCH] 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 63279301bcb, 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. --- acp_adapter/auth.py | 5 ++++ acp_adapter/edit_approval.py | 10 +++++++ hermes_cli/auth.py | 18 +++++++++++ hermes_cli/models.py | 25 ++++++++++++++++ hermes_state_messages.py | 9 ++++++ hermes_state_sessions.py | 15 ++++++++++ tests/acp/test_auth.py | 17 +++++++++++ tests/acp/test_edit_approval.py | 3 +- .../acp_adapter/test_detect_provider_entra.py | 1 + tests/gateway/test_watchdog_review_76354.py | 2 +- tests/hermes_cli/test_billing_scope_stepup.py | 30 +++++++++++++++++++ tests/hermes_cli/test_model_validation.py | 28 +++++++++++++++++ tests/test_hermes_state.py | 6 ++-- tests/test_message_reactions.py | 10 +++++-- tests/test_toolsets.py | 11 +++++++ toolsets.py | 8 +++++ 16 files changed, 191 insertions(+), 7 deletions(-) diff --git a/acp_adapter/auth.py b/acp_adapter/auth.py index a1ad9982d4..3b0ba0fa79 100644 --- a/acp_adapter/auth.py +++ b/acp_adapter/auth.py @@ -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. diff --git a/acp_adapter/edit_approval.py b/acp_adapter/edit_approval.py index 5c38efc9e0..e7659281c9 100644 --- a/acp_adapter/edit_approval.py +++ b/acp_adapter/edit_approval.py @@ -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(): diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index 3507903bf4..d7075b2762 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -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") diff --git a/hermes_cli/models.py b/hermes_cli/models.py index f8e738f965..c44afab89b 100644 --- a/hermes_cli/models.py +++ b/hermes_cli/models.py @@ -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) diff --git a/hermes_state_messages.py b/hermes_state_messages.py index ede2f15959..4deb712b9b 100644 --- a/hermes_state_messages.py +++ b/hermes_state_messages.py @@ -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: diff --git a/hermes_state_sessions.py b/hermes_state_sessions.py index c5fb21a2ea..ade7128b4f 100644 --- a/hermes_state_sessions.py +++ b/hermes_state_sessions.py @@ -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: diff --git a/tests/acp/test_auth.py b/tests/acp/test_auth.py index 4d04dfdde7..ea92337b5f 100644 --- a/tests/acp/test_auth.py +++ b/tests/acp/test_auth.py @@ -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( diff --git a/tests/acp/test_edit_approval.py b/tests/acp/test_edit_approval.py index 8cdbf82fd2..a7c53d6d89 100644 --- a/tests/acp/test_edit_approval.py +++ b/tests/acp/test_edit_approval.py @@ -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(): diff --git a/tests/acp_adapter/test_detect_provider_entra.py b/tests/acp_adapter/test_detect_provider_entra.py index 5ffd0b3886..6d9409ab9d 100644 --- a/tests/acp_adapter/test_detect_provider_entra.py +++ b/tests/acp_adapter/test_detect_provider_entra.py @@ -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 diff --git a/tests/gateway/test_watchdog_review_76354.py b/tests/gateway/test_watchdog_review_76354.py index a726b45eeb..94eaaa3c85 100644 --- a/tests/gateway/test_watchdog_review_76354.py +++ b/tests/gateway/test_watchdog_review_76354.py @@ -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"] == "" diff --git a/tests/hermes_cli/test_billing_scope_stepup.py b/tests/hermes_cli/test_billing_scope_stepup.py index 157a4cb476..3841921393 100644 --- a/tests/hermes_cli/test_billing_scope_stepup.py +++ b/tests/hermes_cli/test_billing_scope_stepup.py @@ -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 # --------------------------------------------------------------------------- diff --git a/tests/hermes_cli/test_model_validation.py b/tests/hermes_cli/test_model_validation.py index 6ff144816a..304492f87c 100644 --- a/tests/hermes_cli/test_model_validation.py +++ b/tests/hermes_cli/test_model_validation.py @@ -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: diff --git a/tests/test_hermes_state.py b/tests/test_hermes_state.py index 3861d2cd90..dba894bd96 100644 --- a/tests/test_hermes_state.py +++ b/tests/test_hermes_state.py @@ -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): diff --git a/tests/test_message_reactions.py b/tests/test_message_reactions.py index 431be907af..0cae4dbc8b 100644 --- a/tests/test_message_reactions.py +++ b/tests/test_message_reactions.py @@ -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): diff --git a/tests/test_toolsets.py b/tests/test_toolsets.py index ce35cf80a5..018f178bd1 100644 --- a/tests/test_toolsets.py +++ b/tests/test_toolsets.py @@ -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 diff --git a/toolsets.py b/toolsets.py index a66c95950e..6df4907967 100644 --- a/toolsets.py +++ b/toolsets.py @@ -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}