feat(billing): carry the payment-method union through to clients (#71542)

* 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".
This commit is contained in:
Siddharth Balyan 2026-07-26 13:19:43 -07:00 committed by GitHub
parent 8943c9958b
commit 59529afee0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 274 additions and 2 deletions

View file

@ -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,

View file

@ -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
}
}

View file

@ -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

View file

@ -13,6 +13,7 @@ export type {
BillingErrorPayload,
BillingMonthlyCap,
BillingMutationResponse,
BillingPaymentMethod,
BillingRefusalCode,
BillingStateResponse,
ChargeFailureReason,

View file

@ -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",
[

View file

@ -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
# ---------------------------------------------------------------------------

View file

@ -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,