diff --git a/dashboard/src/ui/dashboard/components/__tests__/limitDisplay.test.jsx b/dashboard/src/ui/dashboard/components/__tests__/limitDisplay.test.jsx index 2e8b6c13..5dcc3987 100644 --- a/dashboard/src/ui/dashboard/components/__tests__/limitDisplay.test.jsx +++ b/dashboard/src/ui/dashboard/components/__tests__/limitDisplay.test.jsx @@ -1,5 +1,8 @@ import { describe, expect, it } from "vitest"; +import { render, screen } from "@testing-library/react"; +import React from "react"; import { + LimitChips, limitIdForLabel, getCardLimitWindows, getCardTierClasses, @@ -145,3 +148,51 @@ describe("getCardLimitWindows with counts", () => { expect(premium.displayPct).toBeCloseTo(47.3); }); }); + + +describe("LimitChips — the three states a provider can be in", () => { + function chips(codex) { + return render(); + } + + it("renders nothing when the provider is not configured", () => { + // Correct: no credential means no quota to report and nothing broken. A + // status line here would put a permanent message under a tool the user does + // not even use. + const { container } = chips({ configured: false }); + expect(container.firstChild).toBeNull(); + }); + + it("renders a status line when the fetch failed", () => { + chips({ configured: true, error: "Codex API returned 500" }); + expect(screen.getByText(/Codex API returned 500/)).toBeTruthy(); + }); + + it("renders the NOTICE when there is no error and no windows", () => { + // The state that did not exist. issue 52 asked for "a neutral empty state + // instead of a red Fetch failed"; with no error and no windows the + // component fell through to `return null` and the chip disappeared — + // which issue 105 records as worse than never having had a chip. + chips({ + configured: true, + error: null, + notice: "No usage data for this sign-in. Run `codex` to sign in again.", + primary_window: null, + secondary_window: null, + }); + expect(screen.getByText(/sign in again/)).toBeTruthy(); + }); + + it("still renders nothing when there is no error, no windows AND no notice", () => { + // The old behaviour, kept deliberately: a provider with genuinely nothing to + // say should not occupy space. The fix is that a FAILURE now always carries + // either an error or a notice, so it never lands here. + const { container } = chips({ + configured: true, + error: null, + primary_window: null, + secondary_window: null, + }); + expect(container.firstChild).toBeNull(); + }); +}); diff --git a/dashboard/src/ui/dashboard/components/limitDisplay.jsx b/dashboard/src/ui/dashboard/components/limitDisplay.jsx index 7cc1264c..40f597c9 100644 --- a/dashboard/src/ui/dashboard/components/limitDisplay.jsx +++ b/dashboard/src/ui/dashboard/components/limitDisplay.jsx @@ -188,6 +188,9 @@ function LimitChip({ label, displayPct, counts, reset, mode }) { * - usageLimits not loaded yet → omit (no layout jump on arrival). * - provider not a limits provider / not configured → hide. * - configured but errored → subtle status line. + * - configured, no error, no windows, but a notice → the same subtle line. This + * is the "neutral empty state" issue 52 asked for; before it existed the chip just + * disappeared, which issue 105 records as worse than never having had one. */ export function LimitChips({ label, usageLimits, mode = LIMIT_DISPLAY_MODES.USED }) { if (!usageLimits || typeof usageLimits !== "object") return null; @@ -209,6 +212,17 @@ export function LimitChips({ label, usageLimits, mode = LIMIT_DISPLAY_MODES.USED } const windows = getCardLimitWindows(id, data, effectiveMode); + // The third state. Without it, "configured, no error, no windows" fell through + // to `return null` and the chip vanished — which is how issue 52's intent of "a + // neutral empty state instead of a red error" became no state at all, and the + // outcome issue 105 calls worse than never having had a chip. + if (windows.length === 0 && data.notice) { + return ( +
+ {data.notice} +
+ ); + } if (windows.length === 0) return null; return ( diff --git a/src/lib/usage-limits.js b/src/lib/usage-limits.js index f4ba98d4..0ff2db5b 100644 --- a/src/lib/usage-limits.js +++ b/src/lib/usage-limits.js @@ -218,10 +218,23 @@ async function fetchCodexUsageLimits( method: "GET", headers, }); - // 401/403/404 from wham means "no usage data available for this auth state" — render - // a neutral empty state instead of a red "Fetch failed" error. + // 401/403/404 from wham means "no usage data available for this auth state" — + // a free or multi-account user, or a token that went stale. #52 asked for a + // neutral state here rather than a red "Fetch failed". + // + // It got NO state. With no error and no windows, LimitChips falls through to + // `windows.length === 0 -> return null` and the chip disappears entirely, + // which is the outcome #105 calls worse than never having had a chip: the user + // has been trained to look there and now reads absence as "plenty left". + // + // `notice` is the third state the UI was missing. Visible and subtle, so #52's + // intent survives without #105's failure mode. if (res.status === 401 || res.status === 403 || res.status === 404) { - return { primary_window: null, secondary_window: null }; + return { + primary_window: null, + secondary_window: null, + notice: "No usage data for this sign-in. Run `codex` to sign in again.", + }; } if (!res.ok) { throw new Error(`Codex API returned ${res.status}`); @@ -2013,6 +2026,10 @@ async function getUsageLimits({ codex = { configured: true, error: null, + // A 4xx from wham produces no windows and no error. Carrying its `notice` + // through is what keeps the chip on screen instead of falling through to + // LimitChips' `windows.length === 0 -> return null`. + notice: codexResult.value.notice || null, plan_type: codexPlanType || null, primary_window: codexResult.value.primary_window, secondary_window: codexResult.value.secondary_window, diff --git a/test/usage-limits-failure-visible.test.js b/test/usage-limits-failure-visible.test.js new file mode 100644 index 00000000..31b02e7d --- /dev/null +++ b/test/usage-limits-failure-visible.test.js @@ -0,0 +1,242 @@ +"use strict"; + +// A quota chip exists so someone notices they are near a limit. A chip that +// silently DISAPPEARS when its fetcher breaks is worse than no chip, because the +// user has been trained to look there and now reads absence as "plenty left". +// +// The rendering rule is in dashboard/src/ui/dashboard/components/limitDisplay.jsx: +// +// if (!data || !data.configured) return null; // the chip VANISHES +// if (data.error) { ...status line... } // the chip is VISIBLE +// +// So `configured: false` after a genuine fetch failure is the defect. After a +// MISSING CREDENTIAL it is correct — there is nothing to show and nothing broke. +// +// #105 asked whether every fetcher gets this right, and recorded it as unknown: +// "what is untested is whether every fetcher's failure path actually sets +// `error` rather than returning an unconfigured-looking object." +// +// It does. This file is what makes that stay true. usage-limits.js talks to nine +// providers' undocumented private endpoints and any of them can change without +// notice; the failure shows up per-user at runtime and never in CI. This does +// not detect upstream drift — nothing short of live credentials does — but it +// does stop OUR refactors from turning a visible error into a vanished chip. + +const assert = require("node:assert/strict"); +const fs = require("node:fs"); +const os = require("node:os"); +const path = require("node:path"); +const { test } = require("node:test"); + +const MODULE_PATH = require.resolve("../src/lib/usage-limits"); + +// getUsageLimits memoises for CACHE_TTL_MS, so each scenario needs a fresh +// module instance or the second call returns the first one's answer. +function freshModule() { + delete require.cache[MODULE_PATH]; + return require("../src/lib/usage-limits"); +} + +// A home that gets every reachable provider PAST its credential gate, so the +// failure being tested is the network call rather than "not signed in". +function credentialedHome() { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "tt-limits-")); + const write = (rel, body) => { + const full = path.join(home, rel); + fs.mkdirSync(path.dirname(full), { recursive: true }); + fs.writeFileSync(full, typeof body === "string" ? body : JSON.stringify(body)); + }; + write(".codex/auth.json", { + tokens: { access_token: "test-token", refresh_token: "test-refresh", account_id: "acct" }, + last_refresh: new Date().toISOString(), + }); + write(".gemini/oauth_creds.json", { + access_token: "test-token", + refresh_token: "test-refresh", + expiry_date: Date.now() + 3_600_000, + }); + write(".config/github-copilot/apps.json", { + "github.com": { oauth_token: "test-token", user: "someone" }, + }); + // Kimi gates on config.toml existing, then reads credentials/kimi-code.json. + write(".kimi-code/config.toml", "[auth]\n"); + write(".kimi-code/credentials/kimi-code.json", { + access_token: "test-token", + refresh_token: "test-refresh", + expires_at: new Date(Date.now() + 3_600_000).toISOString(), + }); + return home; +} + +// spawnSync's contract: it RETURNS { error, status, stdout }, it does not throw. +// A commandRunner that throws is not a valid stand-in — an early version of this +// probe did that and produced a crash that looked like a product defect. +const missingBinary = () => ({ + error: Object.assign(new Error("spawn ENOENT"), { code: "ENOENT" }), + status: null, + stdout: "", + stderr: "", +}); + +const FAILURES = { + "HTTP 500": async () => ({ + ok: false, + status: 500, + statusText: "Internal Server Error", + headers: { get: () => null }, + text: async () => "upstream exploded", + json: async () => ({}), + }), + "network unreachable": async () => { + throw Object.assign(new Error("fetch failed"), { code: "ECONNREFUSED" }); + }, + "401 expired token": async () => ({ + ok: false, + status: 401, + statusText: "Unauthorized", + headers: { get: () => null }, + text: async () => "unauthorized", + json: async () => ({}), + }), + "a body that is not JSON": async () => ({ + ok: true, + status: 200, + headers: { get: () => "text/html" }, + text: async () => "captive portal", + json: async () => { + throw new Error("Unexpected token < in JSON at position 0"); + }, + }), +}; + +async function limitsUnder(fetchImpl, overrides = {}) { + const { getUsageLimits } = freshModule(); + const home = credentialedHome(); + return getUsageLimits({ + home, + env: { HOME: home, ZAI_API_KEY: "test-key" }, + platform: "linux", + securityRunner: () => { + throw new Error("no keychain on this platform"); + }, + fetchImpl, + commandRunner: missingBinary, + requestFn: async () => { + throw new Error("antigravity unreachable"); + }, + providerTimeoutMs: 3000, + ...overrides, + }); +} + +// Reachable here means: the fixture above gets past the credential gate, so a +// failure is genuinely a failed FETCH. Cursor is absent because its gate needs a +// real Cursor install plus a SQLite state DB holding a JWT; kiro and antigravity +// gate on a binary and a running process, and returning `configured: false` when +// neither exists is correct, not a vanished chip. +const REACHABLE = ["codex", "zai", "gemini", "copilot"]; + +for (const [label, fetchImpl] of Object.entries(FAILURES)) { + test(`${label}: every reachable provider stays VISIBLE with a labelled error`, async () => { + const limits = await limitsUnder(fetchImpl); + for (const provider of REACHABLE) { + const entry = limits?.[provider]; + assert.ok(entry, `${provider}: no entry at all`); + assert.equal( + entry.configured, + true, + `${provider} went to configured:false after a fetch failure — the chip vanishes and` + + ` the user reads that as "plenty left". Entry: ${JSON.stringify(entry)}`, + ); + // Visible has two forms, and the chip renders both in the same subtle + // line: `error` for "this broke", `notice` for "nothing to report for + // this sign-in". What must never happen is neither — that is the state + // that renders nothing at all. + const visible = + (typeof entry.error === "string" && entry.error.trim()) || + (typeof entry.notice === "string" && entry.notice.trim()); + assert.ok( + visible, + `${provider}: configured, but neither error nor notice — the chip renders no numbers` + + ` and no reason, so it disappears. Entry: ${JSON.stringify(entry)}`, + ); + } + }); +} + +test("a 4xx is a NOTICE, not an error — neutral wording, still on screen", async () => { + // The distinction #52 asked for. A stale or free-tier sign-in is not a fault, + // so it must not read as one; but the earlier attempt at "not an error" was + // "no state at all", and the chip vanished. + const limits = await limitsUnder(FAILURES["401 expired token"]); + const codex = limits?.codex; + assert.equal(codex.configured, true); + assert.equal(codex.error, null, "a 4xx must not be reported as a failure"); + assert.match(codex.notice, /sign in/i, `expected an actionable notice, got ${codex.notice}`); +}); + +test("a provider that times out is visible, not absent", async () => { + // withProviderTimeout rejects with "