diff --git a/plugins/video_gen/fal/__init__.py b/plugins/video_gen/fal/__init__.py index 9760891966..94841727e2 100644 --- a/plugins/video_gen/fal/__init__.py +++ b/plugins/video_gen/fal/__init__.py @@ -221,9 +221,19 @@ def _submit_fal_video_request(endpoint: str, arguments: Dict[str, Any]): try: return _get_managed_fal_video_client(managed_gateway).submit(endpoint, arguments=arguments, headers=headers) except Exception as exc: - from tools.fal_common import _extract_http_status + from tools.fal_common import _extract_http_status, _managed_fal_billing_error status = _extract_http_status(exc) if status is not None and 400 <= status < 500: + billing = _managed_fal_billing_error(exc) + if billing is not None: + raise ValueError( + f"Nous Subscription gateway rejected endpoint '{endpoint}' (HTTP {status}): " + f"{billing['message']} ({billing['error_code']}; " + f"{billing['code']}: {billing['detail']}). " + "This is a Nous Portal billing configuration issue, not a missing local API key. " + "The managed route cannot run this endpoint until Nous enables its billing meter; " + "a direct FAL_KEY is an optional bypass." + ) from exc raise ValueError(f"Nous Subscription gateway rejected endpoint '{endpoint}' (HTTP {status}). This model may not yet be enabled " f"on the Nous Portal's FAL proxy. Either:\n • Set FAL_KEY in your environment to use FAL.ai directly, or\n" f" • Pick a different model via `hermes tools` → Video Generation.") from exc diff --git a/tests/plugins/video_gen/test_fal_plugin.py b/tests/plugins/video_gen/test_fal_plugin.py index 821d2db24d..eec18e0318 100644 --- a/tests/plugins/video_gen/test_fal_plugin.py +++ b/tests/plugins/video_gen/test_fal_plugin.py @@ -29,6 +29,54 @@ def test_fal_provider_registers(): assert DEFAULT_MODEL in {"pixverse-v6", "ltx-2.3"} +def test_managed_billing_error_preserves_structured_diagnostic(monkeypatch): + """Video and image callers share the same Nous billing diagnosis.""" + from plugins.video_gen import fal as fal_plugin + + class Response: + status_code = 409 + + @staticmethod + def json(): + return { + "error": { + "code": "BILLING_ERROR", + "message": "Charge authorization failed", + "details": { + "upstreamPayload": { + "code": "unsupported_pricing_meter", + "error": "Unsupported resolver usage meter", + } + }, + } + } + + class BillingError(RuntimeError): + def __init__(self): + super().__init__("gateway billing failure") + self.response = Response() + + error = BillingError() + client = Mock() + client.submit.side_effect = error + monkeypatch.setattr(fal_plugin, "_load_fal_client", lambda: object()) + monkeypatch.setattr( + fal_plugin, "_resolve_managed_fal_video_gateway", lambda: object() + ) + monkeypatch.setattr( + fal_plugin, "_get_managed_fal_video_client", lambda gateway: client + ) + + with pytest.raises(ValueError) as exc_info: + fal_plugin._submit_fal_video_request("fal-ai/video", {"prompt": "x"}) + + message = str(exc_info.value) + assert "BILLING_ERROR" in message + assert "unsupported_pricing_meter" in message + assert "Nous Portal billing" in message + assert "may not yet be enabled" not in message + + def test_kling_4k_uses_start_image_url(): """Kling v3 4K's image-to-video endpoint expects start_image_url, not image_url. The family must declare image_param_key='start_image_url'.""" @@ -161,9 +209,10 @@ class TestFamilyRouting: fake.submit = _submit # type: ignore monkeypatch.setitem(sys.modules, "fal_client", fake) - # Reset the lazy global so it picks up our stub + # Pin the stub directly so the clean test environment does not attempt + # to auto-install the optional fal-client distribution. from plugins.video_gen import fal as fal_plugin - fal_plugin._fal_client = None + fal_plugin._fal_client = fake # Also reset the managed client cache fal_plugin._managed_fal_video_client = None fal_plugin._managed_fal_video_client_config = None @@ -532,7 +581,7 @@ class TestUpscalePass: monkeypatch.setitem(sys.modules, "fal_client", fake) from plugins.video_gen import fal as fal_plugin - fal_plugin._fal_client = None + fal_plugin._fal_client = fake fal_plugin._managed_fal_video_client = None fal_plugin._managed_fal_video_client_config = None diff --git a/tests/tools/test_fal_common.py b/tests/tools/test_fal_common.py index 5884982cf7..2d04232c48 100644 --- a/tests/tools/test_fal_common.py +++ b/tests/tools/test_fal_common.py @@ -317,6 +317,39 @@ class TestManagedFalSyncClientSubmit: client._request_handle_class.assert_called_once() assert result is client._request_handle_class.return_value + def test_submit_with_idempotency_key_is_not_retried(self): + """The Nous gateway cannot replay an accepted submit reliably. + + A retry can replace the original billing/entitlement response with a + misleading ``idempotency key ... without a reusable request handle`` + conflict, so keyed submits must make exactly one POST attempt. + """ + http_client = MagicMock() + response = MagicMock() + response.json.return_value = { + "request_id": "req-1", + "response_url": "https://q.example.com/resp", + "status_url": "https://q.example.com/status", + "cancel_url": "https://q.example.com/cancel", + } + http_client.request.return_value = response + client, _, _ = self._make_client(http_client=http_client) + client._maybe_retry_request = MagicMock() + client._raise_for_status = MagicMock() + client._request_handle_class = MagicMock() + + client.submit( + "my-app", {"prompt": "hello"}, + headers={"X-Idempotency-Key": "submission-1"}, + ) + + http_client.request.assert_called_once_with( + "POST", "https://queue.example.com/my-app", + json={"prompt": "hello"}, timeout=120.0, + headers={"X-Idempotency-Key": "submission-1"}, + ) + client._maybe_retry_request.assert_not_called() + def test_submit_with_path(self): client, _, _ = self._make_client() response = MagicMock() diff --git a/tests/tools/test_image_generation.py b/tests/tools/test_image_generation.py index 0fdf952101..da250fcd8b 100644 --- a/tests/tools/test_image_generation.py +++ b/tests/tools/test_image_generation.py @@ -393,15 +393,22 @@ class TestRegistryIntegration: # --------------------------------------------------------------------------- class _MockResponse: - def __init__(self, status_code: int): + def __init__(self, status_code: int, payload=None): self.status_code = status_code + self._payload = payload + self.text = "" if payload is None else __import__("json").dumps(payload) + + def json(self): + if self._payload is None: + raise ValueError("not json") + return self._payload class _MockHttpxError(Exception): """Simulates httpx.HTTPStatusError which exposes .response.status_code.""" - def __init__(self, status_code: int, message: str = "Bad Request"): + def __init__(self, status_code: int, message: str = "Bad Request", payload=None): super().__init__(message) - self.response = _MockResponse(status_code) + self.response = _MockResponse(status_code, payload) class TestExtractHttpStatus: @@ -423,6 +430,11 @@ class TestExtractHttpStatus: class TestManagedGatewayErrorTranslation: """4xx from the Nous managed gateway should be translated to a user-actionable message.""" + @pytest.fixture(autouse=True) + def _skip_optional_fal_dependency(self, image_tool, monkeypatch): + """These unit tests replace the managed client and need no FAL SDK.""" + monkeypatch.setattr(image_tool, "_load_fal_client", lambda: None) + def test_4xx_translates_to_value_error_with_remediation(self, image_tool, monkeypatch): """403 from managed gateway → ValueError mentioning FAL_KEY + hermes tools.""" from unittest.mock import MagicMock @@ -451,6 +463,49 @@ class TestManagedGatewayErrorTranslation: # Original exception chained for debugging assert exc_info.value.__cause__ is bad_request + def test_billing_meter_error_is_preserved_instead_of_called_model_unavailable( + self, image_tool, monkeypatch + ): + """Portal billing configuration is the root cause, not a missing model.""" + from unittest.mock import MagicMock + + managed_gateway = MagicMock() + managed_gateway.gateway_origin = "https://fal-queue-gateway.example.com" + managed_gateway.nous_user_token = "test-token" + monkeypatch.setattr( + image_tool, "_resolve_managed_fal_gateway", lambda: managed_gateway + ) + payload = { + "error": { + "code": "BILLING_ERROR", + "message": "Charge authorization failed", + "details": { + "upstreamPayload": { + "code": "unsupported_pricing_meter", + "error": "Unsupported resolver usage meter", + } + }, + } + } + billing_error = _MockHttpxError(409, payload=payload) + mock_managed_client = MagicMock() + mock_managed_client.submit.side_effect = billing_error + monkeypatch.setattr( + image_tool, "_get_managed_fal_client", lambda gw: mock_managed_client + ) + + with pytest.raises(ValueError) as exc_info: + image_tool._submit_fal_request( + "openai/gpt-image-2.5/flare/text-to-image", {"prompt": "x"} + ) + + msg = str(exc_info.value) + assert "Charge authorization failed" in msg + assert "BILLING_ERROR" in msg + assert "unsupported_pricing_meter" in msg + assert "Nous Portal billing" in msg + assert "may not yet be enabled" not in msg + def test_non_http_exception_from_managed_bubbles_up(self, image_tool, monkeypatch): """Connection errors, timeouts, etc. from managed mode aren't 4xx — diff --git a/tests/tools/test_managed_media_gateways.py b/tests/tools/test_managed_media_gateways.py index 8117e44fcd..8d4171fc71 100644 --- a/tests/tools/test_managed_media_gateways.py +++ b/tests/tools/test_managed_media_gateways.py @@ -74,6 +74,9 @@ def _install_fake_tools_package(): get_session_info=lambda: {}, ) ) + lazy_deps_module = types.ModuleType("tools.lazy_deps") + setattr(lazy_deps_module, "ensure", lambda *args, **kwargs: None) + sys.modules["tools.lazy_deps"] = lazy_deps_module sys.modules["tools.managed_tool_gateway"] = _load_tool_module( "tools.managed_tool_gateway", "managed_tool_gateway.py", @@ -93,7 +96,7 @@ def _install_fake_fal_client(captured): "cancel_url": "http://127.0.0.1:3009/requests/req-123/cancel", } - def _maybe_retry_request(client, method, url, json=None, timeout=None, headers=None): + def _capture_request(client, method, url, json=None, timeout=None, headers=None): captured["submit_via"] = "managed_client" captured["http_client"] = client captured["method"] = method @@ -103,6 +106,13 @@ def _install_fake_fal_client(captured): captured["headers"] = headers return FakeResponse() + def _maybe_retry_request(client, method, url, json=None, timeout=None, headers=None): + return _capture_request(client, method, url, json, timeout, headers) + + class FakeHttpClient: + def request(self, method, url, json=None, timeout=None, headers=None): + return _capture_request(self, method, url, json, timeout, headers) + class SyncRequestHandle: def __init__(self, request_id, response_url, status_url, cancel_url, client): captured["request_id"] = request_id @@ -117,7 +127,7 @@ def _install_fake_fal_client(captured): captured["client_key"] = key captured["client_timeout"] = default_timeout self.default_timeout = default_timeout - self._client = object() + self._client = FakeHttpClient() fal_client_module = types.SimpleNamespace( submit=submit, diff --git a/tests/tools/test_video_generation_tool_surface_matrix.py b/tests/tools/test_video_generation_tool_surface_matrix.py index 96f7e74f12..d32d9b08ce 100644 --- a/tests/tools/test_video_generation_tool_surface_matrix.py +++ b/tests/tools/test_video_generation_tool_surface_matrix.py @@ -59,6 +59,8 @@ def matrix_env(tmp_path, monkeypatch): fake_fal.submit = _submit # type: ignore monkeypatch.setitem(__import__("sys").modules, "fal_client", fake_fal) + import tools.fal_common as fal_common + monkeypatch.setattr(fal_common, "import_fal_client", lambda: fake_fal) # httpx stub for xAI import httpx @@ -100,11 +102,7 @@ def matrix_env(tmp_path, monkeypatch): async def _no_sleep(*a, **k): return None monkeypatch.setattr(asyncio, "sleep", _no_sleep) - # Reset FAL plugin's lazy fal_client cache so it picks up the stub - from plugins.video_gen import fal as fal_plugin - fal_plugin._fal_client = None - - # Force discovery + # Force discovery. from hermes_cli.plugins import _ensure_plugins_discovered _ensure_plugins_discovered(force=True) diff --git a/tools/fal_common.py b/tools/fal_common.py index c43881ef11..7670329dc2 100644 --- a/tools/fal_common.py +++ b/tools/fal_common.py @@ -46,6 +46,30 @@ def _extract_http_status(exc: BaseException) -> Optional[int]: return status if isinstance(status, int) else None +def _managed_fal_billing_error(exc: BaseException) -> Optional[Dict[str, str]]: + """Return normalized Nous billing details from a managed-gateway error.""" + response = getattr(exc, "response", None) + if response is None: + return None + try: + payload = response.json() + except Exception: # noqa: BLE001 — diagnostics must not mask the provider error + return None + error = payload.get("error") if isinstance(payload, dict) else None + if not isinstance(error, dict) or error.get("code") != "BILLING_ERROR": + return None + raw_details = error.get("details") + details = raw_details if isinstance(raw_details, dict) else {} + raw_upstream = details.get("upstreamPayload") + upstream = raw_upstream if isinstance(raw_upstream, dict) else {} + return { + "message": str(error.get("message") or "Charge authorization failed"), + "error_code": str(error.get("code") or "BILLING_ERROR"), + "code": str(upstream.get("code") or details.get("chargeIntentErrorCode") or "billing_error"), + "detail": str(upstream.get("error") or "Nous Portal rejected the charge authorization"), + } + + def _require(value: Any, what: str) -> Any: if value is None: raise RuntimeError(f"{what} is required for managed FAL gateway mode") @@ -93,9 +117,28 @@ class _ManagedFalSyncClient: if self._add_timeout_header is None: raise RuntimeError("fal_client.client.add_timeout_header is required for timeout requests") self._add_timeout_header(start_timeout, request_headers) - response = self._maybe_retry_request( - self._http_client, "POST", url, json=arguments, - timeout=getattr(self._sync_client, "default_timeout", 120.0), headers=request_headers) + request_kwargs = { + "json": arguments, + "timeout": getattr(self._sync_client, "default_timeout", 120.0), + "headers": request_headers, + } + # The Nous gateway currently records a keyed submission before billing + # authorization finishes, but cannot replay the resulting error. The + # SDK's automatic 409 retry therefore replaces the real billing error + # with an idempotency conflict. Make one attempt when the caller supplied + # a key; an ambiguous transport failure is safer than a possible duplicate + # generation or a masked entitlement failure. + has_idempotency_key = any( + str(key).lower() == "x-idempotency-key" for key in request_headers + ) + if has_idempotency_key: + response = self._http_client.request("POST", url, **request_kwargs) + else: + retry_request = self._maybe_retry_request + if retry_request is None: # guarded in __init__; keeps static analysis honest + raise RuntimeError("fal_client.client request helpers are required") + response = retry_request( + self._http_client, "POST", url, **request_kwargs) self._raise_for_status(response) data = response.json() return self._request_handle_class( diff --git a/tools/image_generation_tool.py b/tools/image_generation_tool.py index 853f2e44f9..328d4dcd14 100644 --- a/tools/image_generation_tool.py +++ b/tools/image_generation_tool.py @@ -29,7 +29,10 @@ def _load_fal_client() -> Any: from tools.debug_helpers import DebugSession -from tools.fal_common import _ManagedFalSyncClient, _extract_http_status, _normalize_fal_queue_url_format +from tools.fal_common import ( + _ManagedFalSyncClient, _extract_http_status, _managed_fal_billing_error, + _normalize_fal_queue_url_format, +) from tools.image_generation_catalog import ( DEFAULT_ASPECT_RATIO, DEFAULT_MODEL, FAL_MODELS, UPSCALER_CREATIVITY, UPSCALER_DEFAULT_PROMPT, UPSCALER_FACTOR, UPSCALER_GUIDANCE_SCALE, UPSCALER_MODEL, UPSCALER_NEGATIVE_PROMPT, @@ -131,6 +134,16 @@ def _submit_fal_request(model: str, arguments: Dict[str, Any]): # (allowlist miss, billing gate): give remediation instead of a raw httpx error. status = _extract_http_status(exc) if status is not None and 400 <= status < 500: + billing = _managed_fal_billing_error(exc) + if billing is not None: + raise ValueError( + f"Nous Subscription gateway rejected model '{model}' (HTTP {status}): " + f"{billing['message']} ({billing['error_code']}; " + f"{billing['code']}: {billing['detail']}). " + "This is a Nous Portal billing configuration issue, not a missing local API key. " + "The managed route cannot run this model until Nous enables its billing meter; " + "a direct FAL_KEY is an optional bypass." + ) from exc gateway_message = "" if status in {401, 402, 403}: gateway_message = "\n\n" + nous_tool_gateway_unavailable_message(