From f5c2882acb3586ed2e3f841848e0d68533c824b4 Mon Sep 17 00:00:00 2001 From: hongming Date: Wed, 27 May 2026 19:10:21 -0700 Subject: [PATCH 1/2] =?UTF-8?q?feat(canvas):=20P3=20internal#718=20?= =?UTF-8?q?=E2=80=94=20consume=20registry-served=20/templates=20list,=20re?= =?UTF-8?q?tire=20hardcoded=20provider=20vocab=20(#4/#5)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P3 item 2. The canvas Provider/Model selector + Config-tab billing-mode now consume the registry-served GET /templates fields (registry_backed / registry_providers / registry_models from PR-A) instead of re-deriving provider knowledge client-side. Retires the hardcoded vocabularies as the PRIMARY path: - ProviderModelSelector (#4): new buildProviderCatalogFromRegistry(providers, models) builds the dropdown catalog from the registry payload — provider label = registry display_name, bucket = DERIVED provider, billing + auth_env from the registry — instead of inferVendor / VENDOR_LABELS / BARE_VENDOR_PATTERNS. The selector takes an optional pre-built `catalog` prop and uses it verbatim when supplied. inferVendor/buildProviderCatalog remain ONLY as the fallback for non-registry runtimes / older backends. - ConfigTab (#5): when the selected runtime is registry-backed, the provider catalog + selector models come from registry_providers/registry_models, and billingModeForSelectedProvider(provider, catalog) reads the DERIVED provider's billing_mode off the registry catalog. The hardcoded billingModeForProvider ('' | 'platform' → platform_managed else byok) stays as the fallback only. So the billing-mode the UI shows/sends reflects the DERIVED provider (folds in the closed #1931's canvas intent). Federation/back-compat preserved: a non-registry runtime (external/mock/kimi/ future third-party) or an older backend that doesn't serve the registry fields yields registry_backed=false → the canvas keeps the template-served models + its heuristic, unchanged. NO hard-reject (the canvas just can't render an option the registry didn't serve for registry-backed runtimes). Out of scope (per brief): the manifest runtime allowlist (SUPPORTED_RUNTIME_VALUES / FALLBACK_RUNTIME_OPTIONS) is NOT a provider vocabulary and is untouched; PUT /workspaces/:id/provider is NOT retired (that CTO #3 follow-through is a later phase). Stacked on PR-A (workspace-server registry-served /templates); re-target to main after PR-A merges. TDD: ProviderModelSelector.registry.test.tsx (catalog bucketed by derived provider, labelled from display_name, carries billing_mode + auth_env, no empty buckets), ConfigTab.registryBilling.test.tsx (billing reads registry catalog; falls back to the legacy rule with no catalog / unknown provider). Full canvas suite green (3380 passed / 1 skipped), tsc clean for touched files, eslint 0. internal#718 P3 — not merged; CTO merge-go after Five-Axis (UI-affecting). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/components/ProviderModelSelector.tsx | 102 +++++++++++++- .../ProviderModelSelector.registry.test.tsx | 110 +++++++++++++++ canvas/src/components/tabs/ConfigTab.tsx | 128 ++++++++++++++++-- .../ConfigTab.registryBilling.test.tsx | 55 ++++++++ 4 files changed, 381 insertions(+), 14 deletions(-) create mode 100644 canvas/src/components/__tests__/ProviderModelSelector.registry.test.tsx create mode 100644 canvas/src/components/tabs/__tests__/ConfigTab.registryBilling.test.tsx diff --git a/canvas/src/components/ProviderModelSelector.tsx b/canvas/src/components/ProviderModelSelector.tsx index e04fbc6d9..1bb677be8 100644 --- a/canvas/src/components/ProviderModelSelector.tsx +++ b/canvas/src/components/ProviderModelSelector.tsx @@ -49,6 +49,33 @@ export interface ProviderEntry { wildcard: boolean; /** Optional tooltip text (rendered as native title=). */ tooltip?: string; + /** Billing mode the DERIVED provider implies, when this entry came from the + * registry-backed payload (internal#718 P3): "platform_managed" | "byok". + * Undefined for entries built by the legacy inferVendor heuristic. */ + billingMode?: "platform_managed" | "byok"; +} + +/** RegistryProvider mirrors one entry of GET /templates `registry_providers` + * (workspace-server registryProviderView): the registry's native provider for + * a runtime, with its display label, auth-env NAMES, and billing mode. This is + * the SSOT the dropdown labels come from — the canvas drops VENDOR_LABELS for + * registry-backed runtimes (internal#718 P3, retire-list #4). */ +export interface RegistryProvider { + name: string; + display_name?: string; + auth_env?: string[]; + billing_mode?: "platform_managed" | "byok"; + deprecated?: boolean; +} + +/** RegistryModel mirrors one entry of GET /templates `registry_models`: a + * native model id annotated with its DERIVED provider (registry name) and the + * billing_mode that provider implies. */ +export interface RegistryModel { + id: string; + name?: string; + provider?: string; + billing_mode?: "platform_managed" | "byok"; } export interface SelectorValue { @@ -68,6 +95,13 @@ interface Props { models: SelectorModel[]; value: SelectorValue; onChange: (next: SelectorValue) => void; + /** Optional pre-built provider catalog. When provided, the selector uses it + * verbatim instead of re-inferring one from `models` via + * buildProviderCatalog — the registry-backed path (internal#718 P3), where + * the parent builds the catalog from the registry-served providers/models + * so dropdown labels + billing come from the provider-registry SSOT rather + * than the inferVendor heuristic. Omitted = legacy heuristic over `models`. */ + catalog?: ProviderEntry[]; /** Display variant. "grid" = label+control side-by-side (used in ConfigTab * Runtime section). "stack" = vertical (used in MissingKeysModal). */ variant?: "grid" | "stack"; @@ -251,6 +285,66 @@ export function buildProviderCatalog(models: SelectorModel[]): ProviderEntry[] { return Array.from(buckets.values()); } +/** Build the provider catalog from a REGISTRY-BACKED GET /templates payload + * (registry_providers + registry_models) — internal#718 P3, retire-list #4. + * + * Unlike buildProviderCatalog (which RE-INFERS vendor from model-id prefixes + * + env via inferVendor/VENDOR_LABELS/BARE_VENDOR_PATTERNS), this trusts the + * registry: each model carries its DERIVED `provider` (a registry provider + * name) and the dropdown label/billing/auth come from the matching + * `registry_providers` entry. The canvas can render no provider/model the + * registry did not serve ("only registered selectable"), and the billing-mode + * shown reflects the derived provider rather than a hardcoded rule. + * + * A provider with no served model is omitted (no empty buckets). Models whose + * `provider` doesn't match a registry_providers entry still get a bucket + * keyed by the raw provider name (defensive — should not happen for a + * well-formed registry payload), so a model is never silently dropped. */ +export function buildProviderCatalogFromRegistry( + registryProviders: RegistryProvider[], + registryModels: RegistryModel[], +): ProviderEntry[] { + const byName = new Map(); + for (const p of registryProviders) byName.set(p.name, p); + + // Bucket models by their derived provider name, preserving registry order. + const buckets = new Map(); + for (const m of registryModels) { + const vendor = (m.provider ?? "").trim(); + if (!vendor) continue; // un-annotated registry model — skip from the + // provider cascade (selectable elsewhere via free-text); it has no + // derived provider to bucket under. + const meta = byName.get(vendor); + const wildcard = m.id.includes("*"); + let entry = buckets.get(vendor); + if (!entry) { + entry = { + id: `registry|${vendor}`, + vendor, + label: meta?.display_name || vendor, + envVars: meta?.auth_env ?? [], + models: [], + wildcard, + billingMode: meta?.billing_mode ?? m.billing_mode, + tooltip: VENDOR_TOOLTIPS[vendor], + }; + buckets.set(vendor, entry); + } + entry.models.push({ id: m.id, name: m.name, provider: vendor }); + entry.wildcard = entry.wildcard || wildcard; + } + + // Decorate label with model-count when ≥2 concrete models share the bucket, + // matching buildProviderCatalog's UX. + for (const e of buckets.values()) { + if (!e.wildcard && e.models.length > 1) { + e.label = `${e.label} (${e.models.length} models)`; + } + } + + return Array.from(buckets.values()); +} + /** Find the provider entry that contains a given model id. Used by * callers to back-derive the provider when only the model is known * (e.g. ConfigTab loading from saved state). */ @@ -283,6 +377,7 @@ export function ProviderModelSelector({ models, value, onChange, + catalog: catalogProp, variant = "stack", allowCustomModelEscape = false, disabled = false, @@ -293,7 +388,12 @@ export function ProviderModelSelector({ const providerSelectId = `${baseId}-provider`; const modelSelectId = `${baseId}-model`; - const catalog = useMemo(() => buildProviderCatalog(models), [models]); + // Registry-backed path (internal#718 P3): use the parent-supplied catalog + // verbatim; otherwise re-infer one from `models` via the legacy heuristic. + const catalog = useMemo( + () => catalogProp ?? buildProviderCatalog(models), + [catalogProp, models], + ); const selected = useMemo( () => catalog.find((p) => p.id === value.providerId) ?? null, [catalog, value.providerId], diff --git a/canvas/src/components/__tests__/ProviderModelSelector.registry.test.tsx b/canvas/src/components/__tests__/ProviderModelSelector.registry.test.tsx new file mode 100644 index 000000000..ac2798127 --- /dev/null +++ b/canvas/src/components/__tests__/ProviderModelSelector.registry.test.tsx @@ -0,0 +1,110 @@ +// @vitest-environment jsdom +// +// internal#718 P3 (retire-list #4) — when GET /templates serves a +// registry-backed selectable list (registry_providers + registry_models with +// display_name / billing_mode / derived provider), the canvas builds the +// provider catalog FROM that registry data instead of re-inferring vendor +// from model-id prefixes (VENDOR_LABELS / BARE_VENDOR_PATTERNS / inferVendor). +// The heuristic path stays only as the fallback for non-registry runtimes / +// older backends. + +import { describe, it, expect } from "vitest"; +import { + buildProviderCatalogFromRegistry, + type RegistryProvider, + type RegistryModel, +} from "../ProviderModelSelector"; + +// Mirrors the registry-served claude-code payload from GET /templates +// (registry_providers / registry_models). display_name + billing_mode come +// from the registry, NOT from the canvas VENDOR_LABELS map. +const CLAUDE_CODE_REGISTRY_PROVIDERS: RegistryProvider[] = [ + { + name: "anthropic-oauth", + display_name: "Claude Code subscription", + auth_env: ["CLAUDE_CODE_OAUTH_TOKEN"], + billing_mode: "byok", + }, + { + name: "anthropic-api", + display_name: "Anthropic API", + auth_env: ["ANTHROPIC_API_KEY"], + billing_mode: "byok", + }, + { + name: "platform", + display_name: "Platform", + auth_env: ["ANTHROPIC_API_KEY", "MOLECULE_LLM_USAGE_TOKEN"], + billing_mode: "platform_managed", + }, +]; + +const CLAUDE_CODE_REGISTRY_MODELS: RegistryModel[] = [ + { id: "sonnet", provider: "anthropic-oauth", billing_mode: "byok" }, + { id: "opus", provider: "anthropic-oauth", billing_mode: "byok" }, + { id: "claude-opus-4-7", provider: "anthropic-api", billing_mode: "byok" }, + { id: "anthropic/claude-opus-4-7", provider: "platform", billing_mode: "platform_managed" }, +]; + +describe("buildProviderCatalogFromRegistry", () => { + it("buckets models by their DERIVED registry provider, not by inferred vendor", () => { + const catalog = buildProviderCatalogFromRegistry( + CLAUDE_CODE_REGISTRY_PROVIDERS, + CLAUDE_CODE_REGISTRY_MODELS, + ); + + const byVendor = new Map(catalog.map((p) => [p.vendor, p])); + // anthropic-oauth bucket holds the two OAuth-derived models. + const oauth = byVendor.get("anthropic-oauth"); + expect(oauth).toBeDefined(); + expect(oauth!.models.map((m) => m.id).sort()).toEqual(["opus", "sonnet"]); + // platform bucket holds the platform-namespaced model. + const platform = byVendor.get("platform"); + expect(platform).toBeDefined(); + expect(platform!.models.map((m) => m.id)).toEqual(["anthropic/claude-opus-4-7"]); + }); + + it("labels providers from the registry display_name, not VENDOR_LABELS", () => { + const catalog = buildProviderCatalogFromRegistry( + CLAUDE_CODE_REGISTRY_PROVIDERS, + CLAUDE_CODE_REGISTRY_MODELS, + ); + const oauth = catalog.find((p) => p.vendor === "anthropic-oauth"); + // Registry display_name "Claude Code subscription" (decorated with the + // model count by the catalog builder is acceptable; assert it carries the + // registry label, not an inferred one). + expect(oauth!.label).toContain("Claude Code subscription"); + }); + + it("carries the registry billing_mode per provider", () => { + const catalog = buildProviderCatalogFromRegistry( + CLAUDE_CODE_REGISTRY_PROVIDERS, + CLAUDE_CODE_REGISTRY_MODELS, + ); + expect(catalog.find((p) => p.vendor === "anthropic-oauth")!.billingMode).toBe("byok"); + expect(catalog.find((p) => p.vendor === "platform")!.billingMode).toBe("platform_managed"); + }); + + it("surfaces the registry auth_env on the provider entry", () => { + const catalog = buildProviderCatalogFromRegistry( + CLAUDE_CODE_REGISTRY_PROVIDERS, + CLAUDE_CODE_REGISTRY_MODELS, + ); + expect(catalog.find((p) => p.vendor === "anthropic-oauth")!.envVars).toEqual([ + "CLAUDE_CODE_OAUTH_TOKEN", + ]); + }); + + it("only includes providers that actually have at least one served model", () => { + // anthropic-api is a registry provider but has no model in this slice → + // it should not appear as an empty bucket. + const models: RegistryModel[] = [ + { id: "sonnet", provider: "anthropic-oauth", billing_mode: "byok" }, + ]; + const catalog = buildProviderCatalogFromRegistry( + CLAUDE_CODE_REGISTRY_PROVIDERS, + models, + ); + expect(catalog.map((p) => p.vendor)).toEqual(["anthropic-oauth"]); + }); +}); diff --git a/canvas/src/components/tabs/ConfigTab.tsx b/canvas/src/components/tabs/ConfigTab.tsx index 27027a203..149914706 100644 --- a/canvas/src/components/tabs/ConfigTab.tsx +++ b/canvas/src/components/tabs/ConfigTab.tsx @@ -11,8 +11,12 @@ import { ExternalConnectionSection } from "./ExternalConnectionSection"; import { ProviderModelSelector, buildProviderCatalog, + buildProviderCatalogFromRegistry, findProviderForModel, type SelectorValue, + type ProviderEntry, + type RegistryProvider, + type RegistryModel, } from "../ProviderModelSelector"; import { isExternalLikeRuntime } from "@/lib/externalRuntimes"; @@ -258,6 +262,17 @@ interface RuntimeOption { // canvas falls back to deriving unique vendor prefixes from // models[].id (still adapter-driven, just inferred). providers: string[]; + // registryBacked / registryProviders / registryModels come from the + // registry-served GET /templates fields (internal#718 P3). When + // registryBacked is true, the selectable provider+model list is built from + // the registry (registryProviders/registryModels) — display labels + + // billing mode + derived provider come from the provider-registry SSOT, not + // the canvas VENDOR_LABELS / billingModeForProvider vocabularies. When + // false (non-registry runtime / older backend), the canvas falls back to + // the template-served models[] + its inferVendor heuristic. + registryBacked: boolean; + registryProviders: RegistryProvider[]; + registryModels: RegistryModel[]; } // deriveProvidersFromModels — when a template doesn't ship an explicit @@ -322,6 +337,32 @@ export function billingModeForProvider(provider: string): LLMBillingMode { return "byok"; } +// billingModeForSelectedProvider — internal#718 P3 (retire-list #5): the +// billing mode the Config tab shows/sends for the selected PROVIDER, sourced +// from the registry-served catalog when available rather than the hardcoded +// billingModeForProvider rule. +// +// When the runtime is registry-backed, GET /templates serves each provider's +// DERIVED billing_mode (platform_managed for the closed platform provider, +// byok otherwise) on the ProviderEntry. We read it off the catalog so the UI +// reflects the registry SSOT — the same predicate billing/credential emission +// keys off the derived provider. +// +// Falls back to billingModeForProvider when: no catalog (non-registry runtime +// / older backend), or the provider string isn't carried by the catalog +// (e.g. a stale saved value). The fallback keeps the legacy behavior intact +// for everything the registry doesn't yet speak to. +export function billingModeForSelectedProvider( + provider: string, + catalog?: ProviderEntry[], +): LLMBillingMode { + if (catalog && catalog.length > 0) { + const entry = catalog.find((p) => p.vendor === provider.trim()); + if (entry?.billingMode) return entry.billingMode; + } + return billingModeForProvider(provider); +} + // Fallback used when /templates can't be fetched (offline, older backend). // Keep in sync with manifest.json workspace_templates as a defensive default. // Model + env suggestions only flow when the backend is reachable. @@ -339,10 +380,10 @@ const RUNTIMES_WITH_OWN_CONFIG = new Set(["external", "kimi", "kimi-cli" const SUPPORTED_RUNTIME_VALUES = new Set(["claude-code", "codex", "openclaw", "hermes"]); const FALLBACK_RUNTIME_OPTIONS: RuntimeOption[] = [ - { value: "claude-code", label: "Claude Code", models: [], providers: [] }, - { value: "codex", label: "Codex", models: [], providers: [] }, - { value: "openclaw", label: "OpenClaw", models: [], providers: [] }, - { value: "hermes", label: "Hermes", models: [], providers: [] }, + { value: "claude-code", label: "Claude Code", models: [], providers: [], registryBacked: false, registryProviders: [], registryModels: [] }, + { value: "codex", label: "Codex", models: [], providers: [], registryBacked: false, registryProviders: [], registryModels: [] }, + { value: "openclaw", label: "OpenClaw", models: [], providers: [], registryBacked: false, registryProviders: [], registryModels: [] }, + { value: "hermes", label: "Hermes", models: [], providers: [], registryBacked: false, registryProviders: [], registryModels: [] }, ]; export function ConfigTab({ workspaceId }: Props) { @@ -533,7 +574,18 @@ export function ConfigTab({ workspaceId }: Props) { useEffect(() => { let cancelled = false; - api.get>("/templates") + api.get>("/templates") .then((rows) => { if (cancelled || !Array.isArray(rows)) return; const byRuntime = new Map(); @@ -545,8 +597,23 @@ export function ConfigTab({ workspaceId }: Props) { const existing = byRuntime.get(v); const models = Array.isArray(r.models) ? r.models : []; const providers = Array.isArray(r.providers) ? r.providers : []; - if (!existing || models.length > existing.models.length) { - byRuntime.set(v, { value: v, label: r.name || v, models, providers }); + const registryProviders = Array.isArray(r.registry_providers) ? r.registry_providers : []; + const registryModels = Array.isArray(r.registry_models) ? r.registry_models : []; + const registryBacked = r.registry_backed === true && registryModels.length > 0; + // Prefer the richer payload: a registry-backed entry, then more + // template models. Keeps the "last/richer template wins" intent. + const score = (o: RuntimeOption) => (o.registryBacked ? 1000 : 0) + o.models.length; + const candidate: RuntimeOption = { + value: v, + label: r.name || v, + models, + providers, + registryBacked, + registryProviders, + registryModels, + }; + if (!existing || score(candidate) > score(existing)) { + byRuntime.set(v, candidate); } } if (byRuntime.size > 0) setRuntimeOptions(Array.from(byRuntime.values())); @@ -557,7 +624,13 @@ export function ConfigTab({ workspaceId }: Props) { // Models + env hints for the currently-selected runtime. const selectedRuntime = runtimeOptions.find((o) => o.value === (config.runtime || "")) ?? null; - const availableModels: ModelSpec[] = selectedRuntime?.models ?? []; + // Memoised so its identity is stable across renders — it feeds several + // useMemo dependency arrays below (registry/legacy catalog, selector models) + // and a fresh `[]` literal each render would defeat their memoisation. + const availableModels: ModelSpec[] = useMemo( + () => selectedRuntime?.models ?? [], + [selectedRuntime?.models], + ); // Provider suggestions for the legacy free-text input fallback (used // when /templates returned no models for this runtime, e.g. hermes // workspaces). Prefer the runtime's declarative providers list, @@ -571,9 +644,37 @@ export function ConfigTab({ workspaceId }: Props) { // Vendor-aware catalog shared with the selector. Memoised so the // catalog identity is stable across renders (selector relies on it). + // + // internal#718 P3: when the runtime is registry-backed, build the catalog + // FROM the registry-served providers/models (display labels + billing + + // derived provider from the provider-registry SSOT) instead of re-inferring + // vendor from model-id prefixes. Falls back to the inferVendor heuristic + // for non-registry runtimes / older backends. + const registryBacked = selectedRuntime?.registryBacked ?? false; const providerCatalog = useMemo( - () => buildProviderCatalog(availableModels), - [availableModels], + () => + registryBacked + ? buildProviderCatalogFromRegistry( + selectedRuntime?.registryProviders ?? [], + selectedRuntime?.registryModels ?? [], + ) + : buildProviderCatalog(availableModels), + [registryBacked, selectedRuntime?.registryProviders, selectedRuntime?.registryModels, availableModels], + ); + // Models fed to the selector dropdown: the registry-served native set for a + // registry-backed runtime (so the dropdown can render no unregistered + // option), else the template-served models. + const selectorModels: ModelSpec[] = useMemo( + () => + registryBacked + ? (selectedRuntime?.registryModels ?? []).map((m) => ({ + id: m.id, + name: m.name, + // carry the derived provider so the selector buckets correctly + ...(m.provider ? { provider: m.provider } : {}), + })) + : availableModels, + [registryBacked, selectedRuntime?.registryModels, availableModels], ); // Derive the selector's current value from the form state. Provider @@ -893,9 +994,10 @@ export function ConfigTab({ workspaceId }: Props) { — empty = "auto-derive from model slug" was the pre-PR-5 behavior; selecting any provider here writes LLM_PROVIDER and triggers an auto-restart. */} - {availableModels.length > 0 ? ( + {selectorModels.length > 0 ? ( { setSelectorValue(next); @@ -908,7 +1010,7 @@ export function ConfigTab({ workspaceId }: Props) { setConfig((prev) => { const v = next.model; const prevModelId = prev.runtime_config?.model || prev.model || ""; - const prevSpec = availableModels.find((m) => m.id === prevModelId) ?? null; + const prevSpec = selectorModels.find((m) => m.id === prevModelId) ?? null; const prevRequired = prev.runtime_config?.required_env ?? []; const wasTemplateDriven = prevRequired.length === 0 || diff --git a/canvas/src/components/tabs/__tests__/ConfigTab.registryBilling.test.tsx b/canvas/src/components/tabs/__tests__/ConfigTab.registryBilling.test.tsx new file mode 100644 index 000000000..815d7c8e5 --- /dev/null +++ b/canvas/src/components/tabs/__tests__/ConfigTab.registryBilling.test.tsx @@ -0,0 +1,55 @@ +// @vitest-environment jsdom +// +// internal#718 P3 (retire-list #5) — the billing-mode the Config tab shows / +// sends must reflect the DERIVED provider per the registry, not the hardcoded +// billingModeForProvider("" | "platform" → platform_managed else byok) rule. +// When the runtime is registry-backed, billingModeForSelectedProvider reads the +// registry-served billing_mode off the provider catalog entry. The hardcoded +// rule remains only as the fallback for non-registry runtimes / older backends. + +import { describe, it, expect } from "vitest"; +import { billingModeForSelectedProvider } from "../ConfigTab"; +import { + buildProviderCatalogFromRegistry, + type RegistryProvider, + type RegistryModel, +} from "../../ProviderModelSelector"; + +const REGISTRY_PROVIDERS: RegistryProvider[] = [ + { name: "anthropic-oauth", display_name: "Claude Code subscription", auth_env: ["CLAUDE_CODE_OAUTH_TOKEN"], billing_mode: "byok" }, + { name: "platform", display_name: "Platform", auth_env: ["ANTHROPIC_API_KEY"], billing_mode: "platform_managed" }, +]; +const REGISTRY_MODELS: RegistryModel[] = [ + { id: "sonnet", provider: "anthropic-oauth", billing_mode: "byok" }, + { id: "anthropic/claude-opus-4-7", provider: "platform", billing_mode: "platform_managed" }, +]; + +describe("billingModeForSelectedProvider (registry-driven)", () => { + const catalog = buildProviderCatalogFromRegistry(REGISTRY_PROVIDERS, REGISTRY_MODELS); + + it("reads platform_managed from the registry for the platform provider", () => { + expect(billingModeForSelectedProvider("platform", catalog)).toBe("platform_managed"); + }); + + it("reads byok from the registry for a BYOK provider", () => { + // anthropic-oauth derives to byok via the REGISTRY (not because the + // hardcoded rule treats every non-'platform' string as byok — that rule + // would also say byok here, so use a case the hardcoded rule gets WRONG + // to prove the registry is authoritative): + expect(billingModeForSelectedProvider("anthropic-oauth", catalog)).toBe("byok"); + }); + + it("falls back to the hardcoded rule when no registry catalog is supplied", () => { + // Non-registry runtime / older backend → catalog empty/undefined → the + // legacy mapping still applies ('' | 'platform' → platform_managed). + expect(billingModeForSelectedProvider("", undefined)).toBe("platform_managed"); + expect(billingModeForSelectedProvider("platform", undefined)).toBe("platform_managed"); + expect(billingModeForSelectedProvider("minimax", undefined)).toBe("byok"); + }); + + it("falls back to the hardcoded rule when the provider is not in the registry catalog", () => { + // A provider string the registry catalog doesn't carry (stale saved + // value) → fall back to the legacy rule rather than guessing. + expect(billingModeForSelectedProvider("some-byo-vendor", catalog)).toBe("byok"); + }); +}); -- 2.52.0 From 8546502ab8819e2a9fdffd631d0621c25b009dab Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Thu, 28 May 2026 02:43:10 +0000 Subject: [PATCH 2/2] test(canvas): make registryBilling test discriminate registry-vs-hardcoded billing precedence (#1978 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit agent-reviewer #7790 (blocking) found that ConfigTab.registryBilling.test.tsx did not actually pin retire-list #5's core claim — both existing assertions ("platform"→platform_managed, "anthropic-oauth"→byok) return the SAME value under both the registry-authoritative impl and a regression to the old hardcoded billingModeForProvider rule, so the test was tautological and a regression would still pass. The misleading comment on the anthropic-oauth case claimed it was "a case the hardcoded rule gets WRONG" but the hardcoded rule actually agrees there too. This commit adds a genuine disagreement case: a registry provider "managed-federated" whose registry-served billing_mode is "platform_managed" even though its name is not "" / "platform" (so the legacy billingModeForProvider rule would return "byok"). The new test asserts the two rules disagree on this input (sanity) and then asserts billingModeForSelectedProvider returns the REGISTRY value ("platform_managed"), which is only reachable by honoring the catalog. Load-bearing proof: with the registry-first impl, the new test PASSES; when billingModeForSelectedProvider is temporarily forced to fall through to the hardcoded rule, the new test (and only the new test) FAILS with expected 'platform_managed' / received 'byok' — proving it pins the registry-wins contract. Also fixes the misleading "hardcoded rule gets WRONG" comment on the anthropic-oauth case (explicitly annotates it as non-discriminating and points to the new disagreement case as the registry-WINS proof). Implementation (billingModeForSelectedProvider) untouched — confirmed byte-identical to PR #1978 HEAD (f2d7f1da). Verification: - targeted: 5 passed (was 4 — adds the discriminating case) - regressed-impl: only the new test fails, others pass (= they are non-discriminating as the review found) - full canvas vitest: 223 files / 3381 passed | 1 skipped (3382) — +1 vs the 3380/1 baseline - tsc: 0 new errors (touched file clean; pre-existing 223 baseline unchanged with my diff stashed) - eslint on touched file: 0 Refs: #1978, review #7790, internal#718 P3 retire-list #5. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../ConfigTab.registryBilling.test.tsx | 33 ++++++++++++++++--- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/canvas/src/components/tabs/__tests__/ConfigTab.registryBilling.test.tsx b/canvas/src/components/tabs/__tests__/ConfigTab.registryBilling.test.tsx index 815d7c8e5..a0329db27 100644 --- a/canvas/src/components/tabs/__tests__/ConfigTab.registryBilling.test.tsx +++ b/canvas/src/components/tabs/__tests__/ConfigTab.registryBilling.test.tsx @@ -8,7 +8,7 @@ // rule remains only as the fallback for non-registry runtimes / older backends. import { describe, it, expect } from "vitest"; -import { billingModeForSelectedProvider } from "../ConfigTab"; +import { billingModeForSelectedProvider, billingModeForProvider } from "../ConfigTab"; import { buildProviderCatalogFromRegistry, type RegistryProvider, @@ -18,10 +18,23 @@ import { const REGISTRY_PROVIDERS: RegistryProvider[] = [ { name: "anthropic-oauth", display_name: "Claude Code subscription", auth_env: ["CLAUDE_CODE_OAUTH_TOKEN"], billing_mode: "byok" }, { name: "platform", display_name: "Platform", auth_env: ["ANTHROPIC_API_KEY"], billing_mode: "platform_managed" }, + // DISCRIMINATING fixture (review #7790): a provider whose registry-served + // billing_mode DISAGREES with the hardcoded name-based rule. Its name is not + // "platform"/"" so billingModeForProvider() would call it "byok", yet the + // registry serves "platform_managed" (the federation-ready shape the SSOT is + // built for — a managed provider that isn't literally named "platform"). + // billingModeForSelectedProvider MUST return the REGISTRY value here; the + // only way to get "platform_managed" out is to honor the catalog, so this + // case fails if the impl ever regresses to the hardcoded rule. + { name: "managed-federated", display_name: "Managed (federated)", auth_env: [], billing_mode: "platform_managed" }, ]; const REGISTRY_MODELS: RegistryModel[] = [ { id: "sonnet", provider: "anthropic-oauth", billing_mode: "byok" }, { id: "anthropic/claude-opus-4-7", provider: "platform", billing_mode: "platform_managed" }, + // model bucketed under the disagreeing provider so the catalog builds an + // entry for it (buildProviderCatalogFromRegistry only emits a provider entry + // for providers that own at least one model). + { id: "managed/some-model", provider: "managed-federated", billing_mode: "platform_managed" }, ]; describe("billingModeForSelectedProvider (registry-driven)", () => { @@ -32,13 +45,23 @@ describe("billingModeForSelectedProvider (registry-driven)", () => { }); it("reads byok from the registry for a BYOK provider", () => { - // anthropic-oauth derives to byok via the REGISTRY (not because the - // hardcoded rule treats every non-'platform' string as byok — that rule - // would also say byok here, so use a case the hardcoded rule gets WRONG - // to prove the registry is authoritative): + // anthropic-oauth derives to byok via the REGISTRY. (Note: the hardcoded + // rule would ALSO say byok for this non-'platform' name, so on its own this + // assertion does NOT prove the registry is authoritative — it agrees either + // way. The registry-WINS proof is the disagreement case below.) expect(billingModeForSelectedProvider("anthropic-oauth", catalog)).toBe("byok"); }); + it("lets the registry billing_mode WIN when it disagrees with the hardcoded rule", () => { + // 'managed-federated' is not '' / 'platform', so the legacy name-based rule + // classifies it byok — but the registry serves platform_managed. The + // registry is the SSOT, so billingModeForSelectedProvider must return + // platform_managed. This is the discriminating case: it FAILS if the impl + // regresses to billingModeForProvider (which would return byok here). + expect(billingModeForProvider("managed-federated")).toBe("byok"); // sanity: the rules genuinely disagree + expect(billingModeForSelectedProvider("managed-federated", catalog)).toBe("platform_managed"); + }); + it("falls back to the hardcoded rule when no registry catalog is supplied", () => { // Non-registry runtime / older backend → catalog empty/undefined → the // legacy mapping still applies ('' | 'platform' → platform_managed). -- 2.52.0