From 91ecc88b718ae289c094f9ca36533dbb6acd458b Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sun, 25 Oct 2026 22:28:34 -0700 Subject: [PATCH] feat(plugins): public model-setup helpers and secret_source_label for provider plugins The github-copilot labs plugin's own `hermes model` flows still imported three core privates (r4-labs/CORE_GAPS.md): _persist_model, _note_setup_failure and env_loader's format_secret_source_suffix. They encode core config/metric/secret-source rules a plugin must not copy, so core publishes documented names: hermes_cli.model_setup.persist_model_selection and note_setup_failure (new public module, in the plugin-validate allow-list) and agent.secret_scope.secret_source_label (already a public module). Documented in model-provider-plugin.md. The private names stay (core flows use them; no catalog pin imports them). --- agent/secret_scope.py | 8 ++++ hermes_cli/model_setup.py | 25 ++++++++++ hermes_cli/plugin_validate_private_imports.py | 2 +- tests/hermes_cli/test_model_setup_public.py | 47 +++++++++++++++++++ .../developer-guide/model-provider-plugin.md | 10 ++++ 5 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 hermes_cli/model_setup.py create mode 100644 tests/hermes_cli/test_model_setup_public.py diff --git a/agent/secret_scope.py b/agent/secret_scope.py index f3b3213193..2b88d2267a 100644 --- a/agent/secret_scope.py +++ b/agent/secret_scope.py @@ -261,6 +261,14 @@ def get_secret(name: str, default: Optional[str] = None) -> Optional[str]: return _environ_or(name, default) +def secret_source_label(name: str) -> str: + """``" (from Bitwarden)"``-style suffix naming the external secret source (a secret-source plugin) + that supplied env var *name*; ``""`` for ``.env`` / shell values. Public for plugins' status lines.""" + from hermes_cli.env_loader import format_secret_source_suffix + + return format_secret_source_suffix(name) + + def get_secret_str(name: str, default: str = "") -> str: """``get_secret`` for callers that want a ``str``: ``default`` only when the secret is genuinely unset. Still raises ``UnscopedSecretError`` — swallowing it hides a spawn-site bug.""" diff --git a/hermes_cli/model_setup.py b/hermes_cli/model_setup.py new file mode 100644 index 0000000000..eed769664c --- /dev/null +++ b/hermes_cli/model_setup.py @@ -0,0 +1,25 @@ +"""Public helpers for a provider plugin's own ``hermes model`` step (``ProviderProfile.model_setup_flow``). + +Documented in website/docs/developer-guide/model-provider-plugin.md: these are the contract a plugin +flow calls instead of re-implementing core's config rules. +""" + +from __future__ import annotations + + +def persist_model_selection(model: str, provider: str, *, base_url: str | None = None, + api_mode: str | None = None) -> dict: + """Save *model* on *provider* as the default: writes the ``model:`` section (``provider``, then + ``base_url`` and ``api_mode`` when given), scrubs inline endpoint credentials from it, saves + config.yaml and deactivates the previously active OAuth provider. Returns the saved section.""" + from hermes_cli.model_setup_flows_common import _persist_model + + return _persist_model(model, provider, base_url=base_url, api_mode=api_mode) + + +def note_setup_failure(failure_class: str) -> None: + """Record why the flow ends without a pick (``"auth"``, ``"no_models"``, ...) for the opt-in + provider-setup metric; a no-op outside a tracked ``hermes model`` / setup flow.""" + from hermes_cli.model_setup_flows_common import _note_setup_failure + + _note_setup_failure(failure_class) diff --git a/hermes_cli/plugin_validate_private_imports.py b/hermes_cli/plugin_validate_private_imports.py index b42d15c6e9..dd4b584034 100644 --- a/hermes_cli/plugin_validate_private_imports.py +++ b/hermes_cli/plugin_validate_private_imports.py @@ -28,7 +28,7 @@ _PUBLIC = frozenset({ # developer-guide / plugins docs "agent.account_usage", "agent.acp_client", "agent.browser_provider", "agent.context_engine", "agent.credential_pool", "agent.delegation_backends", "agent.delegation_provider", "hermes_cli.auth_model_picker:prompt_model_selection", - "hermes_cli.auth:resolve_external_process_provider_credentials", + "hermes_cli.auth:resolve_external_process_provider_credentials", "hermes_cli.model_setup", "agent.image_gen_provider", "agent.memory_manager", "agent.memory_provider", "agent.secret_scope", "agent.secret_sources.base", "agent.subagent_lifecycle", "agent.terminal_env_provider", "agent.transports", "agent.transports.chat_completions", "agent.video_gen_provider", diff --git a/tests/hermes_cli/test_model_setup_public.py b/tests/hermes_cli/test_model_setup_public.py new file mode 100644 index 0000000000..a0a006aaed --- /dev/null +++ b/tests/hermes_cli/test_model_setup_public.py @@ -0,0 +1,47 @@ +"""Public helpers a provider plugin's ``model_setup_flow`` calls (documented in model-provider-plugin.md).""" + +from hermes_cli.plugin_validate_private_imports import private_import_findings + + +def test_persist_model_selection_writes_the_model_section(tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + (tmp_path / "config.yaml").write_text( + "model:\n default: old\n provider: openrouter\n api_key: inline-secret\n") + from hermes_cli.model_setup import persist_model_selection + + persist_model_selection("m-1", "fake-provider", base_url="https://api.example.test/v1", + api_mode="chat_completions") + + from hermes_cli.config import load_config + + model = load_config()["model"] + assert (model["default"], model["provider"]) == ("m-1", "fake-provider") + assert model["base_url"] == "https://api.example.test/v1" and model["api_mode"] == "chat_completions" + assert "api_key" not in model # endpoint credentials of the previous route never carry over + + +def test_note_setup_failure_reaches_the_setup_metric(monkeypatch): + seen = [] + monkeypatch.setattr("hermes_cli.observability.shared_metrics_setup.note_provider_setup_failure", seen.append) + from hermes_cli.model_setup import note_setup_failure + + note_setup_failure("auth") + assert seen == ["auth"] + + +def test_secret_source_label_names_an_external_source(monkeypatch): + monkeypatch.setattr("hermes_cli.env_loader.get_secret_source", + lambda name: "bitwarden" if name == "FAKE_TOKEN" else None) + from agent.secret_scope import secret_source_label + + assert secret_source_label("FAKE_TOKEN") == " (from Bitwarden)" + assert secret_source_label("OTHER_TOKEN") == "" + + +def test_plugin_validate_treats_the_helpers_as_public(tmp_path): + (tmp_path / "plugin.yaml").write_text("name: fake-provider\nkind: model-provider\n") + (tmp_path / "__init__.py").write_text( + "from hermes_cli.model_setup import persist_model_selection, note_setup_failure\n" + "from agent.secret_scope import secret_source_label\n" + "from hermes_cli.model_setup_flows_common import _persist_model\n") + assert private_import_findings(tmp_path) == ["__init__.py:3 hermes_cli.model_setup_flows_common._persist_model"] diff --git a/website/docs/developer-guide/model-provider-plugin.md b/website/docs/developer-guide/model-provider-plugin.md index 0a34fe38e7..97979eb6f7 100644 --- a/website/docs/developer-guide/model-provider-plugin.md +++ b/website/docs/developer-guide/model-provider-plugin.md @@ -335,6 +335,16 @@ meaning says otherwise. | `base_url_env_var` | Env var overriding `base_url` on the registry row when not in `env_vars` | `""` | | `registry_after` | Registry id this row follows in priority-ordered scans | `""` (appended) | +### Public helpers for a `model_setup_flow` + +A provider's own `hermes model` step calls these instead of re-implementing core's config rules: + +| Helper | What it does | +|---|---| +| `hermes_cli.model_setup.persist_model_selection(model, provider, *, base_url=None, api_mode=None)` | Writes the `model:` section (`provider`, then `base_url` / `api_mode` when given), scrubs inline endpoint credentials, saves `config.yaml` and deactivates the previously active OAuth provider. Returns the saved section. | +| `hermes_cli.model_setup.note_setup_failure(failure_class)` | Records why the flow ended without a pick (`"auth"`, `"no_models"`, ...) for the opt-in provider-setup metric; a no-op outside a tracked flow. | +| `agent.secret_scope.secret_source_label(name)` | `" (from Bitwarden)"`-style suffix naming the secret-source plugin that supplied env var `name`; `""` for `.env` / shell values. | + ## Account usage Model-provider plugins can provide account or plan usage to `/usage` by