fix(setup): write config for image_gen and video_gen in apply_nous_managed_defaults (#35109)
apply_nous_managed_defaults() was adding image_gen and video_gen to the 'changed' return set without writing any config values. The caller (tools_command first_install flow) uses 'changed' to skip manual configuration, so these tools ended up in platform_toolsets but with no video_gen.provider, video_gen.use_gateway, or image_gen.use_gateway in config.yaml. At runtime the FAL plugin's is_available() returned False because there was no FAL_KEY and no use_gateway config — the tool never loaded despite being 'enabled' in the toolset list. For image_gen this was a latent bug masked by the gateway offer prompt (prompt_enable_tool_gateway) running earlier in the setup flow and writing image_gen.use_gateway=True via apply_gateway_defaults(). But if the user skipped the gateway offer, image_gen would silently break the same way. For video_gen (added in PR #33259) the bug was always hit because the gateway offer ran before the user checked video_gen in the toolset checklist. Fix: write provider/use_gateway config values before adding to 'changed', matching the pattern used by web, tts, and browser.
This commit is contained in:
@ -587,9 +587,20 @@ def apply_nous_managed_defaults(
|
||||
changed.add("browser")
|
||||
|
||||
if "image_gen" in selected_toolsets and not fal_key_is_configured():
|
||||
image_cfg = config.get("image_gen")
|
||||
if not isinstance(image_cfg, dict):
|
||||
image_cfg = {}
|
||||
config["image_gen"] = image_cfg
|
||||
image_cfg["use_gateway"] = True
|
||||
changed.add("image_gen")
|
||||
|
||||
if "video_gen" in selected_toolsets and not fal_key_is_configured():
|
||||
video_cfg = config.get("video_gen")
|
||||
if not isinstance(video_cfg, dict):
|
||||
video_cfg = {}
|
||||
config["video_gen"] = video_cfg
|
||||
video_cfg["provider"] = "fal"
|
||||
video_cfg["use_gateway"] = True
|
||||
changed.add("video_gen")
|
||||
|
||||
return changed
|
||||
|
||||
@ -231,3 +231,93 @@ def test_get_gateway_eligible_tools_ignores_quoted_false_opt_in(monkeypatch):
|
||||
assert "web" in has_direct
|
||||
assert "web" not in already_managed
|
||||
assert set(unconfigured) == {"image_gen", "video_gen", "tts", "browser"}
|
||||
|
||||
|
||||
def test_apply_nous_managed_defaults_writes_video_gen_config(monkeypatch):
|
||||
"""apply_nous_managed_defaults must write video_gen.provider and
|
||||
video_gen.use_gateway when a Nous subscriber selects video_gen
|
||||
without a direct FAL_KEY."""
|
||||
monkeypatch.setattr(ns, "managed_nous_tools_enabled", lambda **kw: True)
|
||||
monkeypatch.delenv("FAL_KEY", raising=False)
|
||||
monkeypatch.setattr(ns, "fal_key_is_configured", lambda: False)
|
||||
monkeypatch.setattr(
|
||||
ns, "get_nous_portal_account_info",
|
||||
lambda **kw: _account(logged_in=True, paid=True),
|
||||
)
|
||||
|
||||
config = {"model": {"provider": "nous"}}
|
||||
changed = ns.apply_nous_managed_defaults(
|
||||
config, enabled_toolsets=["video_gen"],
|
||||
)
|
||||
|
||||
assert "video_gen" in changed
|
||||
assert config["video_gen"]["provider"] == "fal"
|
||||
assert config["video_gen"]["use_gateway"] is True
|
||||
|
||||
|
||||
def test_apply_nous_managed_defaults_writes_image_gen_config(monkeypatch):
|
||||
"""apply_nous_managed_defaults must write image_gen.use_gateway
|
||||
when a Nous subscriber selects image_gen without a direct FAL_KEY."""
|
||||
monkeypatch.setattr(ns, "managed_nous_tools_enabled", lambda **kw: True)
|
||||
monkeypatch.delenv("FAL_KEY", raising=False)
|
||||
monkeypatch.setattr(ns, "fal_key_is_configured", lambda: False)
|
||||
monkeypatch.setattr(
|
||||
ns, "get_nous_portal_account_info",
|
||||
lambda **kw: _account(logged_in=True, paid=True),
|
||||
)
|
||||
|
||||
config = {"model": {"provider": "nous"}}
|
||||
changed = ns.apply_nous_managed_defaults(
|
||||
config, enabled_toolsets=["image_gen"],
|
||||
)
|
||||
|
||||
assert "image_gen" in changed
|
||||
assert config["image_gen"]["use_gateway"] is True
|
||||
|
||||
|
||||
def test_apply_nous_managed_defaults_skips_fal_tools_when_key_present(monkeypatch):
|
||||
"""When FAL_KEY is set, apply_nous_managed_defaults should not touch
|
||||
image_gen or video_gen config — the user's direct key takes precedence."""
|
||||
monkeypatch.setattr(ns, "managed_nous_tools_enabled", lambda **kw: True)
|
||||
monkeypatch.setenv("FAL_KEY", "fal-direct-key")
|
||||
monkeypatch.setattr(ns, "fal_key_is_configured", lambda: True)
|
||||
monkeypatch.setattr(
|
||||
ns, "get_nous_portal_account_info",
|
||||
lambda **kw: _account(logged_in=True, paid=True),
|
||||
)
|
||||
|
||||
config = {"model": {"provider": "nous"}}
|
||||
changed = ns.apply_nous_managed_defaults(
|
||||
config, enabled_toolsets=["image_gen", "video_gen"],
|
||||
)
|
||||
|
||||
assert "image_gen" not in changed
|
||||
assert "video_gen" not in changed
|
||||
assert "image_gen" not in config
|
||||
assert "video_gen" not in config
|
||||
|
||||
|
||||
def test_apply_nous_managed_defaults_preserves_existing_video_gen_section(monkeypatch):
|
||||
"""When video_gen config already exists as a dict, the function should
|
||||
update it in-place rather than replacing it."""
|
||||
monkeypatch.setattr(ns, "managed_nous_tools_enabled", lambda **kw: True)
|
||||
monkeypatch.delenv("FAL_KEY", raising=False)
|
||||
monkeypatch.setattr(ns, "fal_key_is_configured", lambda: False)
|
||||
monkeypatch.setattr(
|
||||
ns, "get_nous_portal_account_info",
|
||||
lambda **kw: _account(logged_in=True, paid=True),
|
||||
)
|
||||
|
||||
config = {
|
||||
"model": {"provider": "nous"},
|
||||
"video_gen": {"model": "pixverse-v6"},
|
||||
}
|
||||
changed = ns.apply_nous_managed_defaults(
|
||||
config, enabled_toolsets=["video_gen"],
|
||||
)
|
||||
|
||||
assert "video_gen" in changed
|
||||
assert config["video_gen"]["provider"] == "fal"
|
||||
assert config["video_gen"]["use_gateway"] is True
|
||||
# Pre-existing keys should be preserved
|
||||
assert config["video_gen"]["model"] == "pixverse-v6"
|
||||
|
||||
@ -757,8 +757,68 @@ def test_first_install_nous_auto_configures_managed_defaults(monkeypatch):
|
||||
assert config["web"]["backend"] == "firecrawl"
|
||||
assert config["tts"]["provider"] == "openai"
|
||||
assert config["browser"]["cloud_provider"] == "browser-use"
|
||||
assert config["image_gen"]["use_gateway"] is True
|
||||
assert configured == []
|
||||
|
||||
|
||||
def test_first_install_nous_auto_configures_video_gen(monkeypatch):
|
||||
"""When a Nous subscriber checks video_gen in the toolset checklist,
|
||||
apply_nous_managed_defaults must write video_gen.provider and
|
||||
video_gen.use_gateway so the FAL plugin can route through the gateway
|
||||
at runtime. Regression test for the bug where video_gen was marked as
|
||||
auto-configured but no config was actually written."""
|
||||
monkeypatch.setattr("hermes_cli.nous_subscription.managed_nous_tools_enabled", lambda: True)
|
||||
config = {
|
||||
"model": {"provider": "nous"},
|
||||
"platform_toolsets": {"cli": []},
|
||||
}
|
||||
for env_var in (
|
||||
"VOICE_TOOLS_OPENAI_KEY",
|
||||
"OPENAI_API_KEY",
|
||||
"ELEVENLABS_API_KEY",
|
||||
"FIRECRAWL_API_KEY",
|
||||
"FIRECRAWL_API_URL",
|
||||
"TAVILY_API_KEY",
|
||||
"PARALLEL_API_KEY",
|
||||
"BROWSERBASE_API_KEY",
|
||||
"BROWSERBASE_PROJECT_ID",
|
||||
"BROWSER_USE_API_KEY",
|
||||
"FAL_KEY",
|
||||
):
|
||||
monkeypatch.delenv(env_var, raising=False)
|
||||
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.tools_config._prompt_toolset_checklist",
|
||||
lambda *args, **kwargs: {"video_gen"},
|
||||
)
|
||||
monkeypatch.setattr("hermes_cli.tools_config.save_config", lambda config: None)
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.tools_config._get_enabled_platforms",
|
||||
lambda: ["cli"],
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.nous_subscription.get_nous_portal_account_info",
|
||||
lambda *args, **kwargs: NousPortalAccountInfo(
|
||||
logged_in=True,
|
||||
source="jwt",
|
||||
fresh=False,
|
||||
paid_service_access=True,
|
||||
),
|
||||
)
|
||||
|
||||
configured = []
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.tools_config._configure_toolset",
|
||||
lambda ts_key, config: configured.append(ts_key),
|
||||
)
|
||||
|
||||
tools_command(first_install=True, config=config)
|
||||
|
||||
assert config["video_gen"]["provider"] == "fal"
|
||||
assert config["video_gen"]["use_gateway"] is True
|
||||
# video_gen should NOT appear in the manual configure list — it's auto-configured
|
||||
assert "video_gen" not in configured
|
||||
|
||||
# ── Platform / toolset consistency ────────────────────────────────────────────
|
||||
|
||||
|
||||
|
||||
@ -249,9 +249,12 @@ class TestFlushAll:
|
||||
mgr = _make_manager(write_frequency="async")
|
||||
sess = _make_session()
|
||||
sess.add_message("user", "pending")
|
||||
mgr._async_queue.put(sess)
|
||||
|
||||
with patch.object(mgr, "_flush_session") as mock_flush:
|
||||
# Put the item AFTER the mock is installed so the background
|
||||
# writer thread (if it dequeues before flush_all) still hits
|
||||
# the mock rather than the real _flush_session.
|
||||
mgr._async_queue.put(sess)
|
||||
mgr.flush_all()
|
||||
# Called at least once for the queued item
|
||||
assert mock_flush.call_count >= 1
|
||||
|
||||
Reference in New Issue
Block a user