From e1cb5336e0d408d58e52a01a8da9f73dba1ceafd Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Wed, 2 Sep 2026 16:32:41 +0000 Subject: [PATCH] fix(llm-gateway): say a gated model is unavailable, not an auth failure A model behind a rollout flag was denied with the free-tier gate's wording and its "(rate_limit)" compat suffix, so Desktop could not tell the denial apart from a generic failure and offered a payment method for something no payment unlocks. - The flag denial now carries reason="model_not_available" and drops the "(rate_limit)" suffix, which only ever belonged to the free-tier shim. - The Desktop classifier reads the structured code and reason instead of matching free-tier wording, and routes a rollout gate to its own prompt. - The model listing drops models whose access flag the caller does not hold, so a picker no longer offers a model every request will reject. Generated-By: PostHog Desktop Task-Id: 68fea8bf-f61a-4194-a8ef-b022767ecb29 --- .../src/billing/usageLimitContent.test.ts | 15 ++++++++ .../core/src/billing/usageLimitContent.ts | 10 ++++++ .../src/pi-runtime/piSessionController.ts | 4 ++- .../packages/shared/src/analytics-events.ts | 2 +- .../packages/shared/src/errors.test.ts | 25 +++++++++++++ .../desktop/packages/shared/src/errors.ts | 34 ++++++++++++++++-- .../llm-gateway/src/llm_gateway/api/models.py | 36 ++++++++++++++++--- .../src/llm_gateway/dependencies.py | 8 ++++- .../llm-gateway/tests/test_dependencies.py | 6 +++- services/llm-gateway/tests/test_models_api.py | 32 ++++++++++++++--- 10 files changed, 156 insertions(+), 16 deletions(-) diff --git a/products/desktop/packages/core/src/billing/usageLimitContent.test.ts b/products/desktop/packages/core/src/billing/usageLimitContent.test.ts index 0ff269a6eda7..d33333da17ed 100644 --- a/products/desktop/packages/core/src/billing/usageLimitContent.test.ts +++ b/products/desktop/packages/core/src/billing/usageLimitContent.test.ts @@ -25,6 +25,21 @@ describe("usageLimitContent", () => { expect(content.actionLabel).toBeNull(); }); + it.each([true, false] as const)( + "never sends a billing prompt for an unavailable model (canManageBilling=%s)", + (canManageBilling) => { + const content = usageLimitContent({ + cause: "model_unavailable", + resetLabel: null, + subscribed: false, + canManageBilling, + }); + expect(content.title).toBe("This model isn't available"); + expect(content.description).toContain("Pick another model"); + expect(content.actionLabel).toBeNull(); + }, + ); + it.each([ // Confirmed-free org: allocation used up, the fix is adding a card. [false, "Free usage used up", "Add payment method"], diff --git a/products/desktop/packages/core/src/billing/usageLimitContent.ts b/products/desktop/packages/core/src/billing/usageLimitContent.ts index 5cd032dfa467..343ab466c20b 100644 --- a/products/desktop/packages/core/src/billing/usageLimitContent.ts +++ b/products/desktop/packages/core/src/billing/usageLimitContent.ts @@ -34,6 +34,16 @@ export function usageLimitContent(args: { }; } + if (cause === "model_unavailable") { + return { + title: "This model isn't available", + description: + "It's in preview and isn't turned on for your account yet. Pick another model to keep going.", + actionLabel: null, + dismissLabel: "Got it", + }; + } + if (cause === "org_limit") { if (!canManageBilling) { return { diff --git a/products/desktop/packages/core/src/pi-runtime/piSessionController.ts b/products/desktop/packages/core/src/pi-runtime/piSessionController.ts index f3b00f1d5319..ec2e0184cc18 100644 --- a/products/desktop/packages/core/src/pi-runtime/piSessionController.ts +++ b/products/desktop/packages/core/src/pi-runtime/piSessionController.ts @@ -1177,7 +1177,9 @@ export class PiSessionController { failure: PromptFailure, ): string { if (failure.kind === "usage_limit") { - return "Usage limit reached"; + return failure.limitCause === "model_unavailable" + ? "Model not available" + : "Usage limit reached"; } if (failure.kind === "transient") { return "Provider temporarily unavailable"; diff --git a/products/desktop/packages/shared/src/analytics-events.ts b/products/desktop/packages/shared/src/analytics-events.ts index 419603f4f749..d0bb4241602f 100644 --- a/products/desktop/packages/shared/src/analytics-events.ts +++ b/products/desktop/packages/shared/src/analytics-events.ts @@ -1262,7 +1262,7 @@ export type UpgradePromptClickedSurface = | "billing_announcement" | "model_picker"; -type UpgradePromptCause = "model_gate" | "org_limit"; +type UpgradePromptCause = "model_gate" | "model_unavailable" | "org_limit"; export interface UpgradePromptShownProperties { surface: UpgradePromptShownSurface; diff --git a/products/desktop/packages/shared/src/errors.test.ts b/products/desktop/packages/shared/src/errors.test.ts index 85cf0ff41bb8..665cfcc48cac 100644 --- a/products/desktop/packages/shared/src/errors.test.ts +++ b/products/desktop/packages/shared/src/errors.test.ts @@ -99,6 +99,21 @@ describe("classifyGatewayLimitError", () => { "API Error: 403 Model 'gpt-5.5' needs a paid PostHog plan. (rate_limit)", "model_gate", ], + [ + // The structured code alone, with wording the patterns don't cover. + `Internal error: API Error: 403 {"error":{"message":"Nope.","type":"permission_error","code":"model_gate"}}`, + "model_gate", + ], + [ + // A model behind a rollout flag: same code, plus the reason that keeps + // it away from the plan-upgrade prompt. + `Internal error: API Error: 403 {"error":{"message":"Model 'moonshotai/kimi-k3' is not available for your account. Choose another model.","type":"permission_error","code":"model_gate","reason":"model_not_available"}}`, + "model_unavailable", + ], + [ + "API Error: 403 Model 'moonshotai/kimi-k3' is not available for your account. Choose another model.", + "model_unavailable", + ], [ // Bare FastAPI detail from gateways predating the error envelope. `Internal error: API Error: 403 {"detail":"Model 'claude-opus-4-8' needs a paid PostHog plan."}`, @@ -182,6 +197,16 @@ describe("isFatalSessionError", () => { expect(isFatalSessionError(message)).toBe(true); }); + it("does not tear the session down over a model the account can't use", () => { + // The ACP layer wraps the gate 403 as "Internal error: …", which the fatal + // patterns would otherwise match. + expect( + isFatalSessionError( + `Internal error: API Error: 403 {"error":{"message":"Model 'moonshotai/kimi-k3' is not available for your account. Choose another model.","type":"permission_error","code":"model_gate","reason":"model_not_available"}}`, + ), + ).toBe(false); + }); + it("does not treat a rate-limit error as fatal even if a fatal phrase is present", () => { expect(isFatalSessionError("process exited", "rate limit exceeded")).toBe( false, diff --git a/products/desktop/packages/shared/src/errors.ts b/products/desktop/packages/shared/src/errors.ts index d7d5cd15d696..db4654e69479 100644 --- a/products/desktop/packages/shared/src/errors.ts +++ b/products/desktop/packages/shared/src/errors.ts @@ -74,10 +74,28 @@ const RATE_LIMIT_PATTERNS = [ "[429]", ] as const; -export type GatewayLimitCause = "model_gate" | "org_limit"; +export type GatewayLimitCause = + | "model_gate" + | "model_unavailable" + | "org_limit"; + +/** + * The gateway's structured denial fields, matched as they reach us: the ACP + * layer embeds the whole error body in the message string, so the JSON is + * text by the time it gets here. Wording patterns stay as a fallback for the + * SDK surfaces that reduce the body to its message alone. + */ +const MODEL_GATE_CODE_REGEX = /"code"\s*:\s*"model_gate"/; +const MODEL_UNAVAILABLE_REASON_REGEX = /"reason"\s*:\s*"model_not_available"/; const MODEL_GATE_PATTERNS = ["needs a paid posthog plan"] as const; +// A model behind a rollout flag the account doesn't hold. No payment unlocks +// it, so it must never reach the plan-upgrade prompt. +const MODEL_UNAVAILABLE_PATTERNS = [ + "is not available for your account", +] as const; + const ORG_LIMIT_PATTERNS = [ "cloud usage limit reached", "reached its posthog desktop usage limit", @@ -146,7 +164,17 @@ export function classifyGatewayLimitError( ): GatewayLimitCause | null { const matches = (patterns: readonly string[]) => includesAny(errorMessage, patterns) || includesAny(errorDetails, patterns); - if (matches(MODEL_GATE_PATTERNS)) return "model_gate"; + const matchesRegex = (regex: RegExp) => + regex.test(errorMessage) || (!!errorDetails && regex.test(errorDetails)); + if ( + matchesRegex(MODEL_UNAVAILABLE_REASON_REGEX) || + matches(MODEL_UNAVAILABLE_PATTERNS) + ) { + return "model_unavailable"; + } + if (matchesRegex(MODEL_GATE_CODE_REGEX) || matches(MODEL_GATE_PATTERNS)) { + return "model_gate"; + } if (matches(ORG_LIMIT_PATTERNS)) return "org_limit"; return null; } @@ -253,7 +281,7 @@ export function isFatalSessionError( if (isRateLimitError(errorMessage, errorDetails)) return false; if (isTurnEndedWithoutResponseError(errorMessage, errorDetails)) return false; if (isTransientUpstreamError(errorMessage, errorDetails)) return false; - if (classifyGatewayLimitError(errorMessage, errorDetails) === "model_gate") { + if (classifyGatewayLimitError(errorMessage, errorDetails) !== null) { return false; } return ( diff --git a/services/llm-gateway/src/llm_gateway/api/models.py b/services/llm-gateway/src/llm_gateway/api/models.py index 13aecfb7e814..2c081e48acad 100644 --- a/services/llm-gateway/src/llm_gateway/api/models.py +++ b/services/llm-gateway/src/llm_gateway/api/models.py @@ -1,3 +1,4 @@ +import asyncio from decimal import Decimal from typing import Literal @@ -7,10 +8,13 @@ from llm_gateway.auth.models import AuthenticatedUser from llm_gateway.auth.service import InvalidProjectScopeError, UnauthorizedProjectScopeError, get_auth_service +from llm_gateway.config import get_settings +from llm_gateway.flags import evaluate_flag from llm_gateway.products.config import ( FREE_TIER_RESTRICTION_REASON, CreditBucket, filter_to_free_tier_models, + get_required_model_flag, validate_product, ) from llm_gateway.rate_limiting.model_cost_service import ModelCostService @@ -73,6 +77,10 @@ class ModelsResponse(BaseModel): models: list[ModelObject] = [] # Alias for `data` — codex-acp expects this field +def _models_response(models: list[ModelObject]) -> ModelsResponse: + return ModelsResponse(data=models, models=models) + + def _format_rate(rate: float) -> str: return format(Decimal(str(rate)), "f") @@ -113,7 +121,7 @@ def _build_response(product: str) -> ModelsResponse: ) for m in models ] - return ModelsResponse(data=model_objects, models=model_objects) + return _models_response(model_objects) async def _authenticated_caller(request: Request) -> AuthenticatedUser | None: @@ -153,10 +161,29 @@ async def _caller_confirmed_free_tier(request: Request, user: AuthenticatedUser return not quota_status.code_usage_billing_active +async def _drop_flag_gated_models(models: list[ModelObject], user: AuthenticatedUser | None) -> list[ModelObject]: + """Models behind an access flag the caller does not hold, removed from the listing. + Enforcement rejects them on the request path, so listing one only offers a pick that + fails after a picker already committed to it. Dropped rather than marked `allowed: False`: + a mark reads as a plan restriction, and no plan change clears a rollout flag. + Unidentifiable callers keep the full list — there is no identity to evaluate.""" + if user is None or get_settings().debug: + return models + required_flag = {m.id: flag for m in models if (flag := get_required_model_flag(m.id)) is not None} + if not required_flag: + return models + flags = sorted(set(required_flag.values())) + # Same fail-closed default as enforcement: an unavailable evaluation blocks. + results = await asyncio.gather(*(evaluate_flag(flag, user.distinct_id) for flag in flags)) + enabled = dict(zip(flags, results, strict=True)) + return [m for m in models if m.id not in required_flag or enabled.get(required_flag[m.id]) is True] + + @models_router.get("/v1/models") async def list_models(request: Request) -> ModelsResponse: - await _authenticated_caller(request) - return _build_response("llm_gateway") + user = await _authenticated_caller(request) + response = _build_response("llm_gateway") + return _models_response(await _drop_flag_gated_models(response.data, user)) @models_router.get("/{product}/v1/models") @@ -165,6 +192,7 @@ async def list_models_for_product(product: str, request: Request) -> ModelsRespo response = _build_response(product) user = await _authenticated_caller(request) + response = _models_response(await _drop_flag_gated_models(response.data, user)) if resolved != "posthog_code": return response @@ -178,4 +206,4 @@ async def list_models_for_product(product: str, request: Request) -> ModelsRespo else m.model_copy(update={"allowed": False, "restriction_reason": FREE_TIER_RESTRICTION_REASON}) for m in response.data ] - return ModelsResponse(data=annotated, models=annotated) + return _models_response(annotated) diff --git a/services/llm-gateway/src/llm_gateway/dependencies.py b/services/llm-gateway/src/llm_gateway/dependencies.py index 043c0905a460..598a90e66e45 100644 --- a/services/llm-gateway/src/llm_gateway/dependencies.py +++ b/services/llm-gateway/src/llm_gateway/dependencies.py @@ -343,9 +343,15 @@ async def enforce_throttles( status_code=status.HTTP_403_FORBIDDEN, detail={ "error": { - "message": f"Model '{model}' is not available. Choose another model. (rate_limit)", + "message": f"Model '{model}' is not available for your account. Choose another model.", "type": "permission_error", + # `code` keeps clients that read only the code on their model-picker + # prompt; `reason` tells the ones that read it that no plan change + # unlocks this, so they prompt for another model instead of a payment + # method. The free-tier shim's `(rate_limit)` suffix is deliberately + # absent — this denial never clears on a retry. "code": "model_gate", + "reason": "model_not_available", } }, ) diff --git a/services/llm-gateway/tests/test_dependencies.py b/services/llm-gateway/tests/test_dependencies.py index 4f236f2a2a92..8b5caea4d43c 100644 --- a/services/llm-gateway/tests/test_dependencies.py +++ b/services/llm-gateway/tests/test_dependencies.py @@ -442,7 +442,11 @@ async def test_preview_model_blocked_when_flag_off_or_unavailable(self, flag_res error = exc_info.value.detail["error"] assert error["code"] == "model_gate" assert "moonshotai/kimi-k3" in error["message"] - assert error["message"].endswith("(rate_limit)") + # A rollout flag never clears on a retry, so the free-tier gate's + # "(rate_limit)" shim must not ride along; `reason` tells clients that + # no payment method unlocks this model. + assert "(rate_limit)" not in error["message"] + assert error["reason"] == "model_not_available" @pytest.mark.asyncio async def test_preview_model_allowed_when_flag_enabled(self) -> None: diff --git a/services/llm-gateway/tests/test_models_api.py b/services/llm-gateway/tests/test_models_api.py index 7691a937edbf..dc5ccdb0a3f0 100644 --- a/services/llm-gateway/tests/test_models_api.py +++ b/services/llm-gateway/tests/test_models_api.py @@ -385,14 +385,36 @@ def test_unbilled_org_gets_full_list_with_premium_models_marked(self, app, mock_ assert premium["allowed"] is False assert premium["restriction_reason"] == "paid_plan_required" # exact, not subset: the default free model must survive the allowlist - # and the annotation, or free-tier callers have no usable model - assert {m["id"] for m in body["data"] if m["allowed"]} == { - "@cf/zai-org/glm-5.2", - "deepseek-ai/deepseek-v4-flash-0731", - } + # and the annotation, or free-tier callers have no usable model. + # DeepSeek is free-tier but flag-gated, and no flag clears here. + assert {m["id"] for m in body["data"] if m["allowed"]} == {"@cf/zai-org/glm-5.2"} # codex reads the `models` mirror; the marks must be there too assert body["models"] == body["data"] + @pytest.mark.parametrize( + "flag_enabled,listed", + [(True, True), (False, False), (None, False)], + ids=["flag_on", "flag_off", "flag_unavailable"], + ) + def test_flag_gated_model_is_listed_only_when_its_flag_clears( + self, app, mock_db_pool, flag_enabled: bool | None, listed: bool + ): + # Enforcement rejects a gated model the caller's flag doesn't clear, and + # fails closed on an evaluation outage — the listing must agree, or the + # picker offers a model every request will 403. + _wire_authenticated_user(mock_db_pool, "gated-user") + + with ( + patch("llm_gateway.api.models.evaluate_flag", AsyncMock(return_value=flag_enabled)), + TestClient(app) as c, + ): + response = c.get("/posthog_code/v1/models", headers={"Authorization": "Bearer phx_gated_models"}) + + assert response.status_code == 200 + body = response.json() + assert ("deepseek-ai/deepseek-v4-flash-0731" in {m["id"] for m in body["data"]}) is listed + assert body["models"] == body["data"] + def test_billed_org_sees_full_list(self, app, mock_db_pool): from unittest.mock import AsyncMock