mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-25 05:52:34 +00:00
fix(tools): video_gen picker reflects active xAI selection and runs xai_grok post_setup
Two bugs in the `hermes tools` reconfigure flow caused picking xAI Grok Imagine for video_gen (or image_gen) to feel like a no-op: 1. `_is_provider_active()` had a branch for `image_gen_plugin_name` but none for `video_gen_plugin_name`, so a row marked as the active xAI video provider was never recognized as active. The picker fell through to the env-var fallback in `_detect_active_provider_index()`, which matched the FAL row (because `FAL_KEY` is set), so the picker visually defaulted to FAL even though the user had selected xAI. 2. `_plugin_video_gen_providers()` and `_plugin_image_gen_providers()` built picker rows from the plugin's `get_setup_schema()` but only copied `name`, `badge`, `tag`, `env_vars`. The xAI plugins declare `post_setup: "xai_grok"` so the picker should run the OAuth / API-key prompt hook after selection — that key was silently dropped, so the hook never fired from the picker rows. Adds the missing `video_gen_plugin_name` branch (placed before the `managed_nous_feature` block, mirroring the existing image_gen branch) and propagates `post_setup` from the plugin schema into both picker-row builders. Adds focused tests in `test_video_gen_picker.py` and `test_image_gen_picker.py`.
This commit is contained in:
parent
b62c997973
commit
e4d7a5dffa
3 changed files with 141 additions and 18 deletions
|
|
@ -1505,15 +1505,16 @@ def _plugin_image_gen_providers() -> list[dict]:
|
||||||
continue
|
continue
|
||||||
if not isinstance(schema, dict):
|
if not isinstance(schema, dict):
|
||||||
continue
|
continue
|
||||||
rows.append(
|
row = {
|
||||||
{
|
"name": schema.get("name", provider.display_name),
|
||||||
"name": schema.get("name", provider.display_name),
|
"badge": schema.get("badge", ""),
|
||||||
"badge": schema.get("badge", ""),
|
"tag": schema.get("tag", ""),
|
||||||
"tag": schema.get("tag", ""),
|
"env_vars": schema.get("env_vars", []),
|
||||||
"env_vars": schema.get("env_vars", []),
|
"image_gen_plugin_name": provider.name,
|
||||||
"image_gen_plugin_name": provider.name,
|
}
|
||||||
}
|
if schema.get("post_setup"):
|
||||||
)
|
row["post_setup"] = schema["post_setup"]
|
||||||
|
rows.append(row)
|
||||||
return rows
|
return rows
|
||||||
|
|
||||||
|
|
||||||
|
|
@ -1542,15 +1543,16 @@ def _plugin_video_gen_providers() -> list[dict]:
|
||||||
continue
|
continue
|
||||||
if not isinstance(schema, dict):
|
if not isinstance(schema, dict):
|
||||||
continue
|
continue
|
||||||
rows.append(
|
row = {
|
||||||
{
|
"name": schema.get("name", provider.display_name),
|
||||||
"name": schema.get("name", provider.display_name),
|
"badge": schema.get("badge", ""),
|
||||||
"badge": schema.get("badge", ""),
|
"tag": schema.get("tag", ""),
|
||||||
"tag": schema.get("tag", ""),
|
"env_vars": schema.get("env_vars", []),
|
||||||
"env_vars": schema.get("env_vars", []),
|
"video_gen_plugin_name": provider.name,
|
||||||
"video_gen_plugin_name": provider.name,
|
}
|
||||||
}
|
if schema.get("post_setup"):
|
||||||
)
|
row["post_setup"] = schema["post_setup"]
|
||||||
|
rows.append(row)
|
||||||
return rows
|
return rows
|
||||||
|
|
||||||
|
|
||||||
|
|
@ -1814,6 +1816,11 @@ def _is_provider_active(provider: dict, config: dict) -> bool:
|
||||||
image_cfg = config.get("image_gen", {})
|
image_cfg = config.get("image_gen", {})
|
||||||
return isinstance(image_cfg, dict) and image_cfg.get("provider") == plugin_name
|
return isinstance(image_cfg, dict) and image_cfg.get("provider") == plugin_name
|
||||||
|
|
||||||
|
video_plugin_name = provider.get("video_gen_plugin_name")
|
||||||
|
if video_plugin_name:
|
||||||
|
video_cfg = config.get("video_gen", {})
|
||||||
|
return isinstance(video_cfg, dict) and video_cfg.get("provider") == video_plugin_name
|
||||||
|
|
||||||
managed_feature = provider.get("managed_nous_feature")
|
managed_feature = provider.get("managed_nous_feature")
|
||||||
if managed_feature:
|
if managed_feature:
|
||||||
features = get_nous_subscription_features(config)
|
features = get_nous_subscription_features(config)
|
||||||
|
|
|
||||||
|
|
@ -103,6 +103,33 @@ class TestPluginPickerInjection:
|
||||||
visible = tools_config._visible_providers(browser, {})
|
visible = tools_config._visible_providers(browser, {})
|
||||||
assert all(p.get("image_gen_plugin_name") is None for p in visible)
|
assert all(p.get("image_gen_plugin_name") is None for p in visible)
|
||||||
|
|
||||||
|
def test_post_setup_propagated_when_declared(self, monkeypatch):
|
||||||
|
from hermes_cli import tools_config
|
||||||
|
|
||||||
|
image_gen_registry.register_provider(_FakeProvider(
|
||||||
|
"xai_img",
|
||||||
|
schema={
|
||||||
|
"name": "xAI Grok Imagine",
|
||||||
|
"badge": "paid",
|
||||||
|
"tag": "grok image",
|
||||||
|
"env_vars": [],
|
||||||
|
"post_setup": "xai_grok",
|
||||||
|
},
|
||||||
|
))
|
||||||
|
|
||||||
|
rows = tools_config._plugin_image_gen_providers()
|
||||||
|
match = next(r for r in rows if r.get("image_gen_plugin_name") == "xai_img")
|
||||||
|
assert match["post_setup"] == "xai_grok"
|
||||||
|
|
||||||
|
def test_post_setup_omitted_when_not_declared(self, monkeypatch):
|
||||||
|
from hermes_cli import tools_config
|
||||||
|
|
||||||
|
image_gen_registry.register_provider(_FakeProvider("plain_img"))
|
||||||
|
|
||||||
|
rows = tools_config._plugin_image_gen_providers()
|
||||||
|
match = next(r for r in rows if r.get("image_gen_plugin_name") == "plain_img")
|
||||||
|
assert "post_setup" not in match
|
||||||
|
|
||||||
|
|
||||||
class TestPluginCatalog:
|
class TestPluginCatalog:
|
||||||
def test_plugin_catalog_returns_models(self):
|
def test_plugin_catalog_returns_models(self):
|
||||||
|
|
|
||||||
|
|
@ -146,3 +146,92 @@ class TestReconfigureWritesProvider:
|
||||||
assert config["video_gen"]["provider"] == "noenv_video"
|
assert config["video_gen"]["provider"] == "noenv_video"
|
||||||
assert config["video_gen"]["model"] == "noenv_video-video-v1"
|
assert config["video_gen"]["model"] == "noenv_video-video-v1"
|
||||||
assert config["video_gen"]["use_gateway"] is False
|
assert config["video_gen"]["use_gateway"] is False
|
||||||
|
|
||||||
|
|
||||||
|
class TestPluginVideoProvidersRow:
|
||||||
|
"""Tests for _plugin_video_gen_providers row contents."""
|
||||||
|
|
||||||
|
def test_post_setup_propagated_when_declared(self, monkeypatch):
|
||||||
|
from hermes_cli import tools_config
|
||||||
|
|
||||||
|
video_gen_registry.register_provider(_FakeVideoProvider(
|
||||||
|
"xai_video",
|
||||||
|
schema={
|
||||||
|
"name": "xAI Grok Imagine",
|
||||||
|
"badge": "paid",
|
||||||
|
"tag": "grok video",
|
||||||
|
"env_vars": [],
|
||||||
|
"post_setup": "xai_grok",
|
||||||
|
},
|
||||||
|
))
|
||||||
|
|
||||||
|
rows = tools_config._plugin_video_gen_providers()
|
||||||
|
match = next(r for r in rows if r.get("video_gen_plugin_name") == "xai_video")
|
||||||
|
assert match["post_setup"] == "xai_grok"
|
||||||
|
|
||||||
|
def test_post_setup_omitted_when_not_declared(self, monkeypatch):
|
||||||
|
from hermes_cli import tools_config
|
||||||
|
|
||||||
|
video_gen_registry.register_provider(_FakeVideoProvider("plain_video"))
|
||||||
|
|
||||||
|
rows = tools_config._plugin_video_gen_providers()
|
||||||
|
match = next(r for r in rows if r.get("video_gen_plugin_name") == "plain_video")
|
||||||
|
assert "post_setup" not in match
|
||||||
|
|
||||||
|
|
||||||
|
class TestVideoPluginProviderActive:
|
||||||
|
"""Tests for _is_provider_active recognizing video_gen_plugin_name."""
|
||||||
|
|
||||||
|
def test_active_when_video_gen_provider_matches(self):
|
||||||
|
from hermes_cli import tools_config
|
||||||
|
|
||||||
|
config = {"video_gen": {"provider": "xai"}}
|
||||||
|
row = {"name": "xAI Grok Imagine", "video_gen_plugin_name": "xai"}
|
||||||
|
|
||||||
|
assert tools_config._is_provider_active(row, config) is True
|
||||||
|
|
||||||
|
def test_inactive_when_video_gen_provider_differs(self):
|
||||||
|
from hermes_cli import tools_config
|
||||||
|
|
||||||
|
config = {"video_gen": {"provider": "fal"}}
|
||||||
|
row = {"name": "xAI Grok Imagine", "video_gen_plugin_name": "xai"}
|
||||||
|
|
||||||
|
assert tools_config._is_provider_active(row, config) is False
|
||||||
|
|
||||||
|
def test_inactive_when_video_gen_section_missing(self):
|
||||||
|
from hermes_cli import tools_config
|
||||||
|
|
||||||
|
row = {"name": "xAI Grok Imagine", "video_gen_plugin_name": "xai"}
|
||||||
|
assert tools_config._is_provider_active(row, {}) is False
|
||||||
|
|
||||||
|
def test_detect_active_index_picks_video_plugin_match(self, monkeypatch):
|
||||||
|
"""When xAI is the configured video_gen provider, the picker should
|
||||||
|
default to the xAI row even if FAL_KEY happens to be set in env.
|
||||||
|
|
||||||
|
Regression: previously _detect_active_provider_index() saw
|
||||||
|
_is_provider_active(xai) return False (no video_gen branch),
|
||||||
|
skipped xAI (empty env_vars), and matched the FAL row via the
|
||||||
|
env-var fallback — so the picker visually defaulted to FAL even
|
||||||
|
though the user picked xAI. The xAI row uses empty env_vars
|
||||||
|
because authentication is handled via xAI Grok OAuth (post_setup
|
||||||
|
hook).
|
||||||
|
"""
|
||||||
|
from hermes_cli import tools_config
|
||||||
|
|
||||||
|
monkeypatch.setattr(
|
||||||
|
tools_config,
|
||||||
|
"get_env_value",
|
||||||
|
lambda key: "fal-key" if key == "FAL_KEY" else "",
|
||||||
|
)
|
||||||
|
|
||||||
|
config = {"video_gen": {"provider": "xai"}}
|
||||||
|
providers = [
|
||||||
|
{"name": "xAI Grok Imagine", "env_vars": [], "video_gen_plugin_name": "xai"},
|
||||||
|
{
|
||||||
|
"name": "FAL.ai",
|
||||||
|
"env_vars": [{"key": "FAL_KEY", "prompt": "FAL"}],
|
||||||
|
"video_gen_plugin_name": "fal",
|
||||||
|
},
|
||||||
|
]
|
||||||
|
|
||||||
|
assert tools_config._detect_active_provider_index(providers, config) == 0
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue