From 59529afee05541538c46774414a4866ffbd709c4 Mon Sep 17 00:00:00 2001 From: Siddharth Balyan <52913345+alt-glitch@users.noreply.github.com> Date: Sun, 26 Jul 2026 13:19:43 -0700 Subject: [PATCH] feat(billing): carry the payment-method union through to clients (#71542) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(billing): add payment_method union to the billing-state wire type * feat(billing): carry the payment-method union through the gateway NAS now sends a typed `paymentMethod` union on /api/billing/state alongside the legacy `card` field. The gateway parses payloads field-by-field, so the new field was dropped on the floor before reaching TUI/Desktop. Parse it into PaymentMethodInfo and re-emit it as snake_case `payment_method`, matching the translation the rest of this payload already does. The payment method id is deliberately not carried through — clients have no use for it. No client rendering changes: `card` stays populated for cards, so every existing consumer behaves exactly as before and the new field is inert until a surface opts into reading it. * fix(billing): send only the fields each payment-method kind declares The serializer emitted every key for every kind, so a Link method went out carrying brand, last4 and wallet set to null. That contradicts the shared type, where each kind declares its own fields: a client testing `'brand' in pm` would read every Link method as a card, and one trusting the declared non-null `brand` could crash on it. Send each kind's own fields, and forward an unrecognized kind by name alone so a client that predates it can still say something honest. The shared type gains the matching fallback arm its own comment already promised. Tests now follow a payload from the server response through to the client wire for each kind, rather than checking parsing and serializing separately — which is why the old expectation locked in the wrong shape without noticing. * fix(billing): keep the payment-method kind narrowable Typing the fallback arm's kind as `string & {}` borrowed a trick that only works on unions of plain strings. On a union of objects it makes the discriminant non-literal, so TypeScript stops narrowing on every arm — even `if (pm.kind === 'card')` no longer gives you `brand`. The first client to use this would have hit a compile error and reached for a cast. An unrecognised kind now arrives as `unknown`, carrying the real name alongside it, so every arm has a literal discriminant. A type-level test pins this: it fails to compile if the discriminant stops narrowing. The parser settles which kind it is, the way the card parser already does, so the record cannot hold fields that do not belong to its kind and the serializer no longer re-checks. The type comment also stops claiming `card` is a safe signal — it is null for Link, so `!card` does not mean "nothing on file". --- agent/billing_view.py | 53 +++++++++++ .../src/billing-payment-method.test-d.ts | 24 +++++ apps/shared/src/billing-types.ts | 44 +++++++++ apps/shared/src/index.ts | 1 + tests/agent/test_billing_view.py | 36 ++++++++ tests/tui_gateway/test_billing_rpc.py | 91 ++++++++++++++++++- tui_gateway/server.py | 27 ++++++ 7 files changed, 274 insertions(+), 2 deletions(-) create mode 100644 apps/shared/src/billing-payment-method.test-d.ts diff --git a/agent/billing_view.py b/agent/billing_view.py index a535aee1f142..29e8068c2275 100644 --- a/agent/billing_view.py +++ b/agent/billing_view.py @@ -107,6 +107,22 @@ class CardInfo: return f"{self.masked} — {label}" if label else self.masked +@dataclass(frozen=True) +class PaymentMethodInfo: + """The payment method on file. `kind` is "card", "link", or "unknown" + — anything else is normalised to "unknown" at parse time, so consumers + only ever see fields that belong to the kind they are looking at.""" + + kind: str + brand: Optional[str] = None + last4: Optional[str] = None + wallet: Optional[str] = None + email: Optional[str] = None + resolved_via: Optional[str] = None + #: What the server called it, when we did not recognise the kind. + raw_kind: Optional[str] = None + + @dataclass(frozen=True) class MonthlyCap: limit_usd: Optional[Decimal] = None @@ -150,6 +166,7 @@ class BillingState: min_usd: Optional[Decimal] = None max_usd: Optional[Decimal] = None card: Optional[CardInfo] = None + payment_method: Optional[PaymentMethodInfo] = None monthly_cap: Optional[MonthlyCap] = None auto_reload: Optional[AutoReload] = None portal_url: Optional[str] = None @@ -201,6 +218,41 @@ def _parse_card(raw: Any) -> Optional[CardInfo]: return CardInfo(brand=brand, last4=last4, resolved_via=resolved_via) +def _parse_payment_method(raw: Any) -> Optional[PaymentMethodInfo]: + if not isinstance(raw, dict): + return None + kind = raw.get("kind") + if not isinstance(kind, str): + return None + + def _optional_string(key: str) -> Optional[str]: + value = raw.get(key) + return value if isinstance(value, str) else None + + resolved_via = _optional_string("resolvedVia") + brand = _optional_string("brand") + last4 = _optional_string("last4") + # Settle the kind here, the way _parse_card settles a card, so nothing + # downstream has to re-check which fields this kind is allowed to have. + if kind == "card" and brand and last4: + return PaymentMethodInfo( + kind="card", + brand=brand, + last4=last4, + wallet=_optional_string("wallet"), + resolved_via=resolved_via, + ) + if kind == "link": + return PaymentMethodInfo( + kind="link", + email=_optional_string("email"), + resolved_via=resolved_via, + ) + return PaymentMethodInfo( + kind="unknown", raw_kind=kind, resolved_via=resolved_via + ) + + def _parse_monthly_cap(raw: Any) -> Optional[MonthlyCap]: if not isinstance(raw, dict): return None @@ -274,6 +326,7 @@ def billing_state_from_payload( min_usd=parse_money(bounds.get("minUsd")), max_usd=parse_money(bounds.get("maxUsd")), card=_parse_card(payload.get("card")), + payment_method=_parse_payment_method(payload.get("paymentMethod")), monthly_cap=_parse_monthly_cap(payload.get("monthlyCap")), auto_reload=_parse_auto_reload(payload.get("autoReload")), portal_url=portal_url, diff --git a/apps/shared/src/billing-payment-method.test-d.ts b/apps/shared/src/billing-payment-method.test-d.ts new file mode 100644 index 000000000000..7d2de8dba998 --- /dev/null +++ b/apps/shared/src/billing-payment-method.test-d.ts @@ -0,0 +1,24 @@ +/** + * Compile-time guard for BillingPaymentMethod. + * + * There is nothing to run here — the point is that `tsc` accepts this file. + * An earlier revision typed the fallback arm's `kind` as `string & {}`, which + * makes the discriminant non-literal and silently defeats narrowing for every + * arm: the `pm.brand` read below stops compiling. Keeping this file honest + * keeps `kind` narrowable. + */ + +import type { BillingPaymentMethod } from './billing-types' + +export function describePaymentMethod(pm: BillingPaymentMethod): string { + switch (pm.kind) { + case 'card': + return pm.wallet ? `${pm.wallet} ${pm.brand} ${pm.last4}` : `${pm.brand} ${pm.last4}` + + case 'link': + return pm.email ?? 'Link' + + case 'unknown': + return pm.raw_kind + } +} diff --git a/apps/shared/src/billing-types.ts b/apps/shared/src/billing-types.ts index 9a7801c22334..21b356e2f7d2 100644 --- a/apps/shared/src/billing-types.ts +++ b/apps/shared/src/billing-types.ts @@ -122,6 +122,47 @@ export interface BillingCardInfo { resolved_via?: null | string } +/** + * The org's payment method on file. + * + * This is the authoritative field. `card` is a lossy older view of the same + * thing: it is populated only when the method is a card, and is null for + * every other kind — so `!card` does NOT mean "no payment method on file". + * A surface that gates on `card` alone will tell a Link customer they have + * nothing on file. + * + * Older gateways omit this field entirely, so absence means "this gateway + * didn't say", not "nothing on file". + * + * A kind this client predates arrives as `unknown` rather than as its real + * name, which keeps `kind` narrowable — every arm is a literal, so + * `if (pm.kind === 'card')` gives you the card fields. (The `string & {}` + * trick used by BillingRefusalCode does not work here: on an object union it + * makes the discriminant non-literal and defeats narrowing for every arm.) + */ +export type BillingPaymentMethod = + | { + kind: 'card' + brand: string + last4: string + /** Wallet that wrapped the card (e.g. "apple_pay", "google_pay"), if any. */ + wallet: string | null + /** Card-resolution rung ("subPin" | "customerDefault" | "autoRefill") or null. */ + resolved_via: null | string + } + | { + kind: 'link' + /** Link displays as the account email; can be absent on the Stripe side. */ + email: null | string + resolved_via: null | string + } + | { + kind: 'unknown' + /** What the server actually called it, for logs and neutral copy. */ + raw_kind: string + resolved_via: null | string + } + export interface BillingMonthlyCap { is_default_ceiling: boolean limit_display: string @@ -159,6 +200,9 @@ export interface BillingStateResponse { can_change_plan?: boolean can_charge: boolean card: BillingCardInfo | null + // Typed payment-method union (newer gateways only); `card` remains the + // compatibility field and stays populated for kind "card". + payment_method?: BillingPaymentMethod | null charge_presets: string[] charge_presets_display: string[] cli_billing_enabled: boolean diff --git a/apps/shared/src/index.ts b/apps/shared/src/index.ts index 09e10e40052e..21c40a716dbc 100644 --- a/apps/shared/src/index.ts +++ b/apps/shared/src/index.ts @@ -13,6 +13,7 @@ export type { BillingErrorPayload, BillingMonthlyCap, BillingMutationResponse, + BillingPaymentMethod, BillingRefusalCode, BillingStateResponse, ChargeFailureReason, diff --git a/tests/agent/test_billing_view.py b/tests/agent/test_billing_view.py index 89824fa32610..14e0cdc883e9 100644 --- a/tests/agent/test_billing_view.py +++ b/tests/agent/test_billing_view.py @@ -25,6 +25,7 @@ from agent.billing_view import ( BillingState, CardInfo, MonthlyCap, + PaymentMethodInfo, billing_state_from_payload, build_billing_state, format_money, @@ -215,6 +216,41 @@ def test_state_owner_tier_parse(): ) +def test_state_parses_link_payment_method(): + payload = _owner_payload() + payload["paymentMethod"] = { + "kind": "link", + "email": "billing@example.com", + "paymentMethodId": "pm_secret", + "purpose": "top-up", + "resolvedVia": "customerDefault", + } + + state = billing_state_from_payload(payload) + + assert state.payment_method == PaymentMethodInfo( + kind="link", + email="billing@example.com", + resolved_via="customerDefault", + ) + + +def test_state_without_payment_method_keeps_it_absent(): + state = billing_state_from_payload(_owner_payload()) + + assert state.payment_method is None + + +@pytest.mark.parametrize("raw_payment_method", ["link", {"email": "billing@example.com"}]) +def test_state_ignores_malformed_payment_method(raw_payment_method): + payload = _owner_payload() + payload["paymentMethod"] = raw_payment_method + + state = billing_state_from_payload(payload) + + assert state.payment_method is None + + @pytest.mark.parametrize( "raw_card,expected", [ diff --git a/tests/tui_gateway/test_billing_rpc.py b/tests/tui_gateway/test_billing_rpc.py index 3d29993bfda4..3259ac6fb257 100644 --- a/tests/tui_gateway/test_billing_rpc.py +++ b/tests/tui_gateway/test_billing_rpc.py @@ -16,7 +16,7 @@ import pytest import tui_gateway.server as srv import hermes_cli.nous_billing as nb import agent.billing_view as bv -from agent.billing_view import BillingState, CardInfo, MonthlyCap +from agent.billing_view import BillingState, CardInfo, MonthlyCap, PaymentMethodInfo def _call(method: str, params: dict) -> dict: @@ -41,6 +41,11 @@ def test_billing_state_serializes_decimals_as_strings(monkeypatch): min_usd=Decimal("10"), max_usd=Decimal("10000"), card=CardInfo(brand="visa", last4="4242"), + payment_method=PaymentMethodInfo( + kind="link", + email="billing@example.com", + resolved_via="customerDefault", + ), monthly_cap=MonthlyCap( limit_usd=Decimal("1000"), spent_this_month_usd=Decimal("180"), is_default_ceiling=True ), @@ -53,7 +58,18 @@ def test_billing_state_serializes_decimals_as_strings(monkeypatch): assert res["balance_usd"] == "142.5" assert res["balance_display"] == "$142.50" assert res["charge_presets"] == ["100", "250"] - assert res["card"]["masked"] == "visa ····4242" + assert res["card"] == { + "brand": "visa", + "last4": "4242", + "masked": "visa ····4242", + "display": "visa ····4242", + "resolved_via": None, + } + assert res["payment_method"] == { + "kind": "link", + "email": "billing@example.com", + "resolved_via": "customerDefault", + } assert res["monthly_cap"]["is_default_ceiling"] is True assert res["is_admin"] is True and res["can_charge"] is True @@ -67,6 +83,77 @@ def test_billing_state_fail_open(monkeypatch): assert res["ok"] is True and res["logged_in"] is False +@pytest.mark.parametrize( + "raw_payment_method,expected", + [ + ( + { + "kind": "card", + "brand": "visa", + "last4": "4242", + "wallet": "apple_pay", + "paymentMethodId": "pm_card", + "resolvedVia": "subPin", + }, + { + "kind": "card", + "brand": "visa", + "last4": "4242", + "wallet": "apple_pay", + "resolved_via": "subPin", + }, + ), + ( + { + "kind": "link", + "email": "billing@example.com", + "resolvedVia": "customerDefault", + }, + { + "kind": "link", + "email": "billing@example.com", + "resolved_via": "customerDefault", + }, + ), + # A kind added after this gateway shipped: normalised to "unknown", + # keeping the real name for logs and neutral copy. + ( + {"kind": "future_method", "resolvedVia": "subPin"}, + {"kind": "unknown", "raw_kind": "future_method", "resolved_via": "subPin"}, + ), + ], +) +def test_billing_state_carries_payment_method_from_server_to_client( + monkeypatch, raw_payment_method, expected +): + payload = { + "org": {"id": "org_1", "name": "Acme", "role": "OWNER"}, + "balanceUsd": "10", + "paymentMethod": raw_payment_method, + } + monkeypatch.setattr( + bv, + "build_billing_state", + lambda *a, **kw: bv.billing_state_from_payload(payload), + ) + + res = _call("billing.state", {}) + + assert res["payment_method"] == expected + + +def test_billing_state_serializes_absent_payment_method(monkeypatch): + monkeypatch.setattr( + bv, + "build_billing_state", + lambda *a, **kw: BillingState(logged_in=False), + ) + + res = _call("billing.state", {}) + + assert res["payment_method"] is None + + # --------------------------------------------------------------------------- # billing.charge — typed error envelopes # --------------------------------------------------------------------------- diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 10ea2538a631..ec43de410a5a 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -9337,6 +9337,32 @@ def _serialize_billing_state(state) -> dict: "display": state.card.display, "resolved_via": state.card.resolved_via, } + payment_method = None + if state.payment_method is not None: + pm = state.payment_method + # Each kind sends only its own fields. Emitting every key with nulls + # would contradict the shared type — a client checking `'brand' in pm` + # would read every Link method as a card. + if pm.kind == "card": + payment_method = { + "kind": "card", + "brand": pm.brand, + "last4": pm.last4, + "wallet": pm.wallet, + "resolved_via": pm.resolved_via, + } + elif pm.kind == "link": + payment_method = { + "kind": "link", + "email": pm.email, + "resolved_via": pm.resolved_via, + } + else: + payment_method = { + "kind": "unknown", + "raw_kind": pm.raw_kind, + "resolved_via": pm.resolved_via, + } monthly_cap = None if state.monthly_cap is not None: mc = state.monthly_cap @@ -9386,6 +9412,7 @@ def _serialize_billing_state(state) -> dict: "min_usd": _s(state.min_usd), "max_usd": _s(state.max_usd), "card": card, + "payment_method": payment_method, "monthly_cap": monthly_cap, "auto_reload": auto_reload, "portal_url": state.portal_url,