From 19113b34d1b52dfdebe8b3fd46349b4f74c0731f Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 19 Aug 2026 15:49:32 -0700 Subject: [PATCH] test(tools): repin selector/picker tests to the provider-string contract Update the sibling tests that pinned the old use_gateway-writing contract: image/video selector and reconfigure rows now assert the single provider string ('nous' managed / 'fal' BYOK) plus legacy-key popping, the stt/video picker writes drop the use_gateway expectation, the web_server managed-browser select asserts the persisted 'nous' cloud_provider, and explicit-local STT pins no-cloud-fallback against a stored raw-config selection. --- .../test_imagegen_managed_gateway.py | 84 +++++++++++-------- tests/hermes_cli/test_stt_picker.py | 5 +- tests/hermes_cli/test_video_gen_picker.py | 3 +- tests/hermes_cli/test_web_server.py | 5 +- tests/tools/test_transcription.py | 3 +- 5 files changed, 59 insertions(+), 41 deletions(-) diff --git a/tests/hermes_cli/test_imagegen_managed_gateway.py b/tests/hermes_cli/test_imagegen_managed_gateway.py index 7886439842..7daf364112 100644 --- a/tests/hermes_cli/test_imagegen_managed_gateway.py +++ b/tests/hermes_cli/test_imagegen_managed_gateway.py @@ -1,15 +1,18 @@ -"""Regression tests for image_gen use_gateway persistence (managed FAL clobber). +"""Regression tests for image_gen provider persistence (managed FAL clobber). -Bug: ``_select_plugin_image_gen_provider`` hardcoded -``image_gen.use_gateway = False``. When a user picked FAL through the -Nous-subscription managed flow, ``_write_provider_config`` first set -``use_gateway = True`` — then the image selector ran and clobbered it back -to False, silently routing every generation through the user's personal -FAL_KEY instead of the Nous Tool Gateway (real incident: personal key -drained to zero while the subscription sat unused). +Historical bug: ``_select_plugin_image_gen_provider`` hardcoded the direct +(non-managed) routing. When a user picked FAL through the Nous-subscription +managed flow, the managed write landed first — then the image selector ran +and clobbered it back to direct, silently routing every generation through +the user's personal FAL_KEY instead of the Nous Tool Gateway (real incident: +personal key drained to zero while the subscription sat unused). -The video twin (``_select_plugin_video_gen_provider``) already accepted a -``use_gateway`` kwarg; these tests pin the image path to the same contract. +Current contract (strict provider-string selection): each picker row writes +exactly ONE provider string per category — ``image_gen.provider: nous`` for +the managed "Nous Subscription" row, ``image_gen.provider: fal`` for the +BYOK FAL row — and any legacy ``use_gateway`` key is popped so the +read-time shim (use_gateway: true ⇒ nous) cannot override the fresh pick. +The video twin (``_select_plugin_video_gen_provider``) shares the contract. """ from hermes_cli.tools_config import ( @@ -28,43 +31,50 @@ def _quiet(monkeypatch): monkeypatch.setattr(tc, "_configure_videogen_model_for_plugin", lambda *a, **k: None) -def test_image_gen_selector_preserves_managed_gateway_flag(monkeypatch): - """Managed pick: use_gateway=True must survive the selector.""" +def test_image_gen_selector_preserves_managed_selection(monkeypatch): + """Managed pick: the 'nous' provider string must survive the selector.""" _quiet(monkeypatch) config = {} - # The managed flow first writes the managed flag... + # The managed flow first persists the managed selection... _write_provider_config( {"image_gen_plugin_name": "fal"}, config, managed_feature="image_gen" ) - assert config["image_gen"]["use_gateway"] is True + assert config["image_gen"]["provider"] == "nous" + assert "use_gateway" not in config["image_gen"] - # ...then the selector runs; passing the managed flag must NOT clobber it. + # ...then the selector runs; the managed kwarg must NOT clobber it + # back onto the vendor name (direct-key routing). _select_plugin_image_gen_provider("fal", config, use_gateway=True) - assert config["image_gen"]["provider"] == "fal" - assert config["image_gen"]["use_gateway"] is True + assert config["image_gen"]["provider"] == "nous" + assert "use_gateway" not in config["image_gen"] -def test_image_gen_selector_direct_key_pick_clears_gateway(monkeypatch): - """Non-managed pick keeps the historical default: direct key, no gateway.""" +def test_image_gen_selector_direct_key_pick_writes_vendor(monkeypatch): + """Non-managed pick writes the vendor name and pops the legacy flag.""" _quiet(monkeypatch) config = {"image_gen": {"use_gateway": True}} _select_plugin_image_gen_provider("fal", config) assert config["image_gen"]["provider"] == "fal" - assert config["image_gen"]["use_gateway"] is False + assert "use_gateway" not in config["image_gen"] -def test_image_and_video_selectors_share_the_gateway_contract(monkeypatch): +def test_image_and_video_selectors_share_the_selection_contract(monkeypatch): """The two selectors are twins: same kwarg, same persistence behavior.""" _quiet(monkeypatch) - for use_gateway in (True, False): - config = {} + for use_gateway, expected in ((True, "nous"), (False, "fal")): + config = { + "image_gen": {"use_gateway": not use_gateway}, + "video_gen": {"use_gateway": not use_gateway}, + } _select_plugin_image_gen_provider("fal", config, use_gateway=use_gateway) _select_plugin_video_gen_provider("fal", config, use_gateway=use_gateway) - assert config["image_gen"]["use_gateway"] is use_gateway - assert config["video_gen"]["use_gateway"] is use_gateway + assert config["image_gen"]["provider"] == expected + assert config["video_gen"]["provider"] == expected + assert "use_gateway" not in config["image_gen"] + assert "use_gateway" not in config["video_gen"] def _quiet_reconfigure(monkeypatch): @@ -82,11 +92,12 @@ def _quiet_reconfigure(monkeypatch): monkeypatch.setattr(ns, "ensure_nous_portal_access", lambda **k: True) -def test_reconfigure_managed_fal_row_keeps_gateway_flag(monkeypatch): +def test_reconfigure_managed_fal_row_keeps_managed_selection(monkeypatch): """The sibling bug of fe63353cb: the legacy-backend model-pick step in - _reconfigure_provider hardcoded use_gateway=False AFTER the managed - branch wrote True — a Nous Subscription user re-entering the picker to - change models was silently flipped onto their personal FAL_KEY.""" + _reconfigure_provider hardcoded the direct selection AFTER the managed + branch wrote the managed one — a Nous Subscription user re-entering the + picker to change models was silently flipped onto their personal + FAL_KEY.""" _quiet_reconfigure(monkeypatch) import hermes_cli.tools_config as tc @@ -98,17 +109,17 @@ def test_reconfigure_managed_fal_row_keeps_gateway_flag(monkeypatch): "override_env_vars": ["FAL_KEY"], "imagegen_backend": "fal", } - config = {"image_gen": {"model": "fal-ai/gpt-image-2"}} + config = {"image_gen": {"model": "fal-ai/gpt-image-2", "use_gateway": True}} tc._reconfigure_provider(managed_row, config) - assert config["image_gen"]["provider"] == "fal" - assert config["image_gen"]["use_gateway"] is True + assert config["image_gen"]["provider"] == "nous" + assert "use_gateway" not in config["image_gen"] -def test_reconfigure_direct_fal_row_clears_gateway_flag(monkeypatch): - """Direct-key FAL reconfig must still clear the flag (historical - behavior for genuinely non-managed picks).""" +def test_reconfigure_direct_fal_row_writes_vendor_selection(monkeypatch): + """Direct-key FAL reconfig writes the vendor name and pops any stale + legacy use_gateway key so the read-time shim can't resurrect 'nous'.""" _quiet_reconfigure(monkeypatch) import hermes_cli.tools_config as tc @@ -121,4 +132,5 @@ def test_reconfigure_direct_fal_row_clears_gateway_flag(monkeypatch): tc._reconfigure_provider(direct_row, config) - assert config["image_gen"]["use_gateway"] is False + assert config["image_gen"]["provider"] == "fal" + assert "use_gateway" not in config["image_gen"] diff --git a/tests/hermes_cli/test_stt_picker.py b/tests/hermes_cli/test_stt_picker.py index 198362c450..0fec1c813d 100644 --- a/tests/hermes_cli/test_stt_picker.py +++ b/tests/hermes_cli/test_stt_picker.py @@ -56,11 +56,12 @@ class TestSttCategory: class TestConfigWrites: def test_write_provider_config_sets_stt_provider(self): - config = {} + config = {"stt": {"use_gateway": True}} prov = _stt_provider_named("Groq") _write_provider_config(prov, config, managed_feature=None) assert config["stt"]["provider"] == "groq" - assert config["stt"]["use_gateway"] is False + # Legacy key is popped so the read-time shim can't override the pick. + assert "use_gateway" not in config["stt"] def test_apply_provider_selection_stt(self): diff --git a/tests/hermes_cli/test_video_gen_picker.py b/tests/hermes_cli/test_video_gen_picker.py index 8c2ebeb17e..c740ef3942 100644 --- a/tests/hermes_cli/test_video_gen_picker.py +++ b/tests/hermes_cli/test_video_gen_picker.py @@ -113,7 +113,8 @@ class TestReconfigureWritesProvider: assert config["video_gen"]["provider"] == "xai_fake" assert config["video_gen"]["model"] == "xai_fake-video-v1" - assert config["video_gen"]["use_gateway"] is False + # Non-managed pick: no legacy use_gateway key is written. + assert "use_gateway" not in config["video_gen"] class TestPluginVideoProvidersRow: diff --git a/tests/hermes_cli/test_web_server.py b/tests/hermes_cli/test_web_server.py index e8487de5fb..5c1fb1e985 100644 --- a/tests/hermes_cli/test_web_server.py +++ b/tests/hermes_cli/test_web_server.py @@ -2671,9 +2671,12 @@ class TestNewEndpoints: assert data["needs_nous_auth"] is True assert data["feature"] == "browser" # The selection is still persisted — activation is what's gated. + # Managed rows store the single 'nous' provider string (the runtime + # maps it to the Browser Use cloud through the Nous Tool Gateway). from hermes_cli.config import load_config cfg = load_config() - assert cfg["browser"]["cloud_provider"] == "browser-use" + assert cfg["browser"]["cloud_provider"] == "nous" + assert "use_gateway" not in cfg["browser"] # -- Web capability split (search vs extract backends) ------------------ diff --git a/tests/tools/test_transcription.py b/tests/tools/test_transcription.py index 2d8b8b04e9..a6a81b0149 100644 --- a/tests/tools/test_transcription.py +++ b/tests/tools/test_transcription.py @@ -43,7 +43,8 @@ class TestGetProvider: monkeypatch.delenv("GROQ_API_KEY", raising=False) with patch("tools.transcription_tools._HAS_FASTER_WHISPER", False), \ patch("tools.transcription_tools._HAS_OPENAI", True), \ - patch("tools.transcription_tools._has_local_command", return_value=False): + patch("tools.transcription_tools._has_local_command", return_value=False), \ + patch("tools.tool_backend_helpers.read_selection", return_value="local"): from tools.transcription_tools import _get_provider assert _get_provider({"provider": "local"}) == "none"