From bc72d0397075543040082cd6d7e9bead518a140b Mon Sep 17 00:00:00 2001 From: Abhishek Sharma Date: Fri, 18 Sep 2026 07:30:27 -0700 Subject: [PATCH] test(usage): pin fetcherProviders against supportedProviders (#13134) The two lists are sibling pure-data modules that have to agree, and nothing compared them. A provider in fetcherProviders but not supportedProviders computes a quota nobody ever asks for; one in supportedProviders with no dispatcher case is accepted and then falls through to `default: "Usage API not implemented"`. That seam has produced the same bug three times -- #9603 (bailian, whose own comment reads "fetcher existed, list entry missing"), #11722, and #12256 (openrouter credits, which #13080 then asked for again six days after the fix landed because the card had never appeared). usage-families-split.test.ts pins the dispatcher against fetcherProviders, so the open-sse side cannot drift; this pins the other edge. Divergence stays allowed but has to be declared with a reason: opencode, opencode-zen, xai have a fetcher, not offered to the dashboard xiaomi-mimo-token-plan offered, no fetcher -- but inert, because it authenticates by API key and is absent from PROVIDER_LIMITS_APIKEY_PROVIDERS, so isSupportedUsageConnection refuses it before the dispatcher is reached I could not establish intent for the first three from the tree, so they are pinned rather than "corrected" -- `xai-oauth`/`xao` are offered while the API-key `xai` is not, which reads like it may be deliberate. A third test fails if a declared divergence stops diverging, so the allowlists cannot go stale and quietly excuse a future recurrence of the same id. Mutations, each killed by the right test: new fetcher, no dashboard entry -> the forward guard + the stale check dashboard entry, no fetcher -> the reverse guard + the stale check a declared divergence gets fixed -> the stale check alone duplicate id in either list -> the duplicate test alone 105 related tests pass (this, usage-families-split, provider-plugin-manifest, provider-limits*), eslint clean. Test-only; no production file touched. --- tests/unit/usage-provider-list-drift.test.ts | 115 +++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 tests/unit/usage-provider-list-drift.test.ts diff --git a/tests/unit/usage-provider-list-drift.test.ts b/tests/unit/usage-provider-list-drift.test.ts new file mode 100644 index 0000000000..b5eefc7cf9 --- /dev/null +++ b/tests/unit/usage-provider-list-drift.test.ts @@ -0,0 +1,115 @@ +/** + * usage-provider-list-drift.test.ts + * + * `fetcherProviders.ts` and `supportedProviders.ts` are sibling pure-data lists + * that have to agree: the first says a provider has a wired + * `getUsageForProvider`, the second says the dashboard and server gates will + * accept its usage. A provider in the first but not the second computes a quota + * nobody asks for; one in the second but not the first is accepted and then + * falls through the dispatcher to `default: "Usage API not implemented"`. + * + * Nothing compared them, and that seam has now produced the same bug three + * times — #9603 (bailian: "fetcher existed, list entry missing"), #11722, and + * #12256 (openrouter credits, requested again in #13080 six days after the fix + * landed because the card had never appeared). `usage-families-split.test.ts` + * pins the dispatcher against `fetcherProviders.ts`, so the open-sse side + * cannot drift; this pins the other edge. + * + * Divergences are allowed but must be declared with a reason. An undeclared one + * is the bug. + */ +import test from "node:test"; +import assert from "node:assert/strict"; + +const { USAGE_FETCHER_PROVIDERS } = + await import("../../open-sse/services/usage/fetcherProviders.ts"); +const { USAGE_SUPPORTED_PROVIDERS } = + await import("../../open-sse/services/usage/supportedProviders.ts"); + +/** + * Has a usage fetcher, deliberately not offered to the dashboard. + * + * These three predate this test and I could not establish intent from the tree, + * so they are pinned rather than "corrected" — the set may not grow silently, + * and converting any entry into a real dashboard listing is a call for someone + * who knows why it was left out. `xai-oauth`/`xao` *are* listed while the + * API-key `xai` is not, which reads like it could be deliberate. + */ +const FETCHER_WITHOUT_DASHBOARD_ENTRY = new Set(["opencode", "opencode-zen", "xai"]); + +/** + * Offered to the dashboard with no fetcher behind it. + * + * `xiaomi-mimo-token-plan` is inert rather than broken: it authenticates by API + * key and is absent from `PROVIDER_LIMITS_APIKEY_PROVIDERS`, so + * `isSupportedUsageConnection` refuses it before the dispatcher is ever + * reached. Listed here so the entry is understood as dead config instead of + * being "fixed" into a live path that would return "Usage API not implemented". + */ +const DASHBOARD_ENTRY_WITHOUT_FETCHER = new Set(["xiaomi-mimo-token-plan"]); + +const sorted = (values: Iterable) => [...values].sort(); + +test("every provider with a usage fetcher is offered to the dashboard", () => { + const supported = new Set(USAGE_SUPPORTED_PROVIDERS); + const undeclared = USAGE_FETCHER_PROVIDERS.filter( + (provider) => !supported.has(provider) && !FETCHER_WITHOUT_DASHBOARD_ENTRY.has(provider) + ); + + assert.deepEqual( + undeclared, + [], + "these providers have a getUsageForProvider implementation but are missing from " + + "USAGE_SUPPORTED_PROVIDERS, so their quota is computed and never requested. " + + "Add them there (and to PROVIDER_LIMITS_APIKEY_PROVIDERS if they authenticate " + + "by API key), or add them to FETCHER_WITHOUT_DASHBOARD_ENTRY with the reason." + ); +}); + +test("every provider offered to the dashboard has a usage fetcher", () => { + const fetchers = new Set(USAGE_FETCHER_PROVIDERS); + const undeclared = USAGE_SUPPORTED_PROVIDERS.filter( + (provider) => !fetchers.has(provider) && !DASHBOARD_ENTRY_WITHOUT_FETCHER.has(provider) + ); + + assert.deepEqual( + undeclared, + [], + "these providers are in USAGE_SUPPORTED_PROVIDERS with no dispatcher case, so " + + "getUsageForProvider returns 'Usage API not implemented'. Add a fetcher, drop " + + "the entry, or declare it in DASHBOARD_ENTRY_WITHOUT_FETCHER with the reason." + ); +}); + +test("the declared divergences are the real ones, not stale", () => { + // Without this, an entry whose drift has since been fixed would sit in the + // allowlists forever, quietly excusing a future recurrence of the same id. + const supported = new Set(USAGE_SUPPORTED_PROVIDERS); + const fetchers = new Set(USAGE_FETCHER_PROVIDERS); + + assert.deepEqual( + sorted(USAGE_FETCHER_PROVIDERS.filter((p) => !supported.has(p))), + sorted(FETCHER_WITHOUT_DASHBOARD_ENTRY), + "FETCHER_WITHOUT_DASHBOARD_ENTRY lists an id that no longer diverges — remove it" + ); + assert.deepEqual( + sorted(USAGE_SUPPORTED_PROVIDERS.filter((p) => !fetchers.has(p))), + sorted(DASHBOARD_ENTRY_WITHOUT_FETCHER), + "DASHBOARD_ENTRY_WITHOUT_FETCHER lists an id that no longer diverges — remove it" + ); +}); + +test("neither list has duplicates", () => { + // A duplicated id makes the counts lie and hides a paste error in a 50-entry + // literal, which is how a wrong id survives review. + assert.deepEqual( + sorted(new Set(USAGE_FETCHER_PROVIDERS)), + sorted(USAGE_FETCHER_PROVIDERS), + "USAGE_FETCHER_PROVIDERS contains a duplicate" + ); + assert.deepEqual( + sorted(new Set(USAGE_SUPPORTED_PROVIDERS)), + sorted(USAGE_SUPPORTED_PROVIDERS), + "USAGE_SUPPORTED_PROVIDERS contains a duplicate" + ); +});