diff --git a/src/config/index.ts b/src/config/index.ts index 5abb8ebd4..517efd3ee 100644 --- a/src/config/index.ts +++ b/src/config/index.ts @@ -1132,18 +1132,32 @@ export async function loadConfig( // provider connect (mid-session, no restart) can rebuild the picker's // catalog after writing new credentials, instead of only taking effect on // the next process start. -function mergeOAuthCatalog( +export function mergeOAuthCatalog( settings: Settings | null, resolved: ResolvedProvider, codexProfiles: readonly CodexProfile[], xaiProfiles: readonly XaiProfile[], ): ProviderCatalogEntry[] { + const codexEntries = codexProfilesToCatalogEntries(codexProfiles); + const xaiEntries = xaiProfilesToCatalogEntries(xaiProfiles); + // A legacy bare `codex`/`xai` settings row (the original single-instance + // connect key) reads as a second, separately-added provider next to the + // credential-backed `/` entries. Drop it once that family + // has a live profile; when nothing is connected the bare row is the only + // ChatGPT/Grok access and stays. + const dropBare = new Set([ + ...(codexEntries.length > 0 ? ["codex"] : []), + ...(xaiEntries.length > 0 ? ["xai"] : []), + ]); return [ ...buildProviderCatalog(settings, resolved).filter( - (e) => !isCodexProviderName(e.name) && !isXaiProviderName(e.name), + (e) => + !isCodexProviderName(e.name) && + !isXaiProviderName(e.name) && + !dropBare.has(e.name), ), - ...codexProfilesToCatalogEntries(codexProfiles), - ...xaiProfilesToCatalogEntries(xaiProfiles), + ...codexEntries, + ...xaiEntries, ].map((entry) => isOpenCodeGoProvider(entry) ? { ...entry, models: [...selectableGoModelIds()] } diff --git a/src/config/oauth-catalog.test.ts b/src/config/oauth-catalog.test.ts new file mode 100644 index 000000000..7efd74bc5 --- /dev/null +++ b/src/config/oauth-catalog.test.ts @@ -0,0 +1,91 @@ +import { describe, expect, test } from "bun:test"; +import type { CodexProfile } from "../auth/codex/store.js"; +import type { XaiProfile } from "../auth/xai/store.js"; +import { mergeOAuthCatalog } from "./index.js"; +import type { + ProviderSettings, + ResolvedProvider, + Settings, +} from "./settings.js"; + +// Regression tests for CL-5606: after a successful ChatGPT browser login the +// merged catalog must not list a separately-added legacy bare `codex` row +// alongside the credential-backed `codex/` entry. + +const resolved: ResolvedProvider = { + providerName: "openai", + baseURL: "https://api.openai.com/v1", + apiKey: "sk-test", + model: "gpt-5", +}; + +function settingsWith(providers: Record): Settings { + return { providers } as Settings; +} + +const entry = (): ProviderSettings => ({ + baseURL: "https://chatgpt.com/backend-api/codex/responses", + models: ["gpt-5.1-codex-max"], +}); + +const codexDefault: CodexProfile = { + name: "default", + tokens: { access: "codex-access", refresh: "r", expiresAt: 1 }, + createdAt: 0, +}; +const xaiWork: XaiProfile = { + name: "work", + tokens: { access: "xai-access", refresh: "r", expiresAt: 1 }, + createdAt: 0, +}; + +describe("mergeOAuthCatalog legacy bare-row dedupe (CL-5606)", () => { + test("a legacy bare codex row is dropped once codex/default is connected", () => { + const merged = mergeOAuthCatalog( + settingsWith({ codex: entry(), "codex/default": entry() }), + resolved, + [codexDefault], + [], + ); + expect(merged.map((p) => p.name)).toEqual(["codex/default"]); + }); + + test("a legacy bare xai row is dropped once xai/work is connected", () => { + const merged = mergeOAuthCatalog( + settingsWith({ xai: entry() }), + resolved, + [], + [xaiWork], + ); + expect(merged.map((p) => p.name)).toEqual(["xai/work"]); + }); + + test("a bare codex row survives when nothing credential-backed exists", () => { + // Not connected: no auth-store profile and no qualified entry. The legacy + // single-instance row is the only ChatGPT access — keep it. + const merged = mergeOAuthCatalog( + settingsWith({ codex: entry() }), + resolved, + [], + [], + ); + expect(merged.map((p) => p.name)).toEqual(["codex"]); + }); + + test("unrelated and API-key rows are untouched by the dedupe", () => { + const merged = mergeOAuthCatalog( + settingsWith({ + openai: { + baseURL: "https://api.openai.com/v1", + apiKey: "sk-test", + models: ["gpt-5"], + }, + codex: entry(), + }), + resolved, + [codexDefault], + [], + ); + expect(merged.map((p) => p.name)).toEqual(["openai", "codex/default"]); + }); +}); diff --git a/src/tui/model-catalog.test.ts b/src/tui/model-catalog.test.ts index 92faf5218..28e3cfdde 100644 --- a/src/tui/model-catalog.test.ts +++ b/src/tui/model-catalog.test.ts @@ -218,4 +218,40 @@ describe("describeModelCatalogOption", () => { ); expect(description?.impact).toMatch(/pricing unknown/i); }); + + test("connected ChatGPT rows state plan billing plainly instead of unknown pricing (CL-5606)", () => { + // Subscription-billed models have no per-token price; "Pricing unknown" + // misreads as metered billing with a missing rate. + const description = describeModelCatalogOption( + { + id: "codex/default:gpt-5.1-codex-max", + label: "gpt-5.1-codex-max * [Codex default]", + }, + { pricing: null }, + ); + expect(description?.impact).not.toMatch(/pricing unknown/i); + expect(description?.impact).toMatch(/ChatGPT subscription/); + }); + + test("connected Grok rows state plan billing plainly instead of unknown pricing (CL-5606)", () => { + const description = describeModelCatalogOption( + { + id: "xai/work:grok-4", + label: "grok-4 * [xAI work]", + }, + { pricing: null }, + ); + expect(description?.impact).not.toMatch(/pricing unknown/i); + expect(description?.impact).toMatch(/subscription/); + }); + + test("colon-less ids keep the full provider instead of dropping the last character", () => { + // slice(0, indexOf(":")) truncates a colon-less id (indexOf is -1), so + // "codex/" became "code" and missed subscription billing. + const description = describeModelCatalogOption( + { id: "codex/", label: "default * [Codex default]" }, + { pricing: null }, + ); + expect(description?.impact).toMatch(/ChatGPT subscription/); + }); }); diff --git a/src/tui/model-catalog.ts b/src/tui/model-catalog.ts index 8ca701b10..44fac0a33 100644 --- a/src/tui/model-catalog.ts +++ b/src/tui/model-catalog.ts @@ -251,9 +251,23 @@ function formatPrice(perToken: number): string { /** Rough per-Mtok multiplier over the "standard" $3 input tier, for decision-relevant framing. */ const STANDARD_INPUT_PER_MTOK = 3; -function pricingImpact(pricing: PricingCache | null, model: string): string { +function pricingImpact( + pricing: PricingCache | null, + providerId: string, + model: string, +): string { const price = lookupModelPricing(pricing, model); - if (price === null) return "Pricing unknown for this model."; + if (price === null) { + // Subscription-billed rows (ChatGPT/Grok OAuth) have no per-token price; + // "unknown" misreads as metered billing with a missing rate. + if (providerId.startsWith("codex/")) { + return "Billed through your ChatGPT subscription, not per-token."; + } + if (providerId.startsWith("xai/")) { + return "Billed through your Grok subscription, not per-token."; + } + return "Pricing unknown for this model."; + } const inputPerMtok = price.inputPricePerToken * 1_000_000; const ratio = inputPerMtok / STANDARD_INPUT_PER_MTOK; const ratioText = @@ -308,7 +322,13 @@ export function describeModelCatalogOption( return { what: whatLine(model), - impact: pricingImpact(pricing, model), + impact: pricingImpact( + pricing, + // Exact provider parse: slice(0, indexOf(":")) drops the last + // character of a colon-less id (indexOf returns -1). + option.id.split(":")[0] ?? option.id, + model, + ), tone: "plain", }; } diff --git a/src/tui/provider-setup.test.ts b/src/tui/provider-setup.test.ts index defc79860..c5eb88757 100644 --- a/src/tui/provider-setup.test.ts +++ b/src/tui/provider-setup.test.ts @@ -252,6 +252,26 @@ describe("provider setup pure helpers", () => { expect(custom?.accountCount).toBe(0); }); + test("Alt+A ChatGPT row hides the login CTA once connected (CL-5606)", () => { + // After a successful browser login the ChatGPT row must not present the + // "Login via Browser" connect option as if unconnected. The row stays + // listed (CL-5899: a second account remains reachable) — only the + // connected-state rendering changes. + const choices = providerChoices(); + const connected = addProviderSelectorChoices(choices, [ + { name: "codex/default" }, + ]); + const codex = connected.find((r) => r.id === "codex"); + expect(codex?.accountCount).toBe(1); + expect(codex?.label).not.toContain("Login via Browser"); + expect(codex?.label).toContain("ChatGPT"); + + const disconnected = addProviderSelectorChoices(choices, []); + const login = disconnected.find((r) => r.id === "codex"); + expect(login?.accountCount).toBe(0); + expect(login?.label).toContain("Login via Browser"); + }); + test("a connected Codex account counts under its profile-qualified name (CL-5606)", () => { // The ChatGPT-via-browser choice is keyed "codex", but a signed-in // account lands in the catalog as "codex/" — one row per diff --git a/src/tui/provider/choices.ts b/src/tui/provider/choices.ts index beae03699..6b89e1b5b 100644 --- a/src/tui/provider/choices.ts +++ b/src/tui/provider/choices.ts @@ -263,12 +263,23 @@ export function addProviderSelectorChoices( readonly hint: string; readonly accountCount: number; }[] { - return choices.map((choice) => ({ - id: choice.id, - label: choice.label, - hint: choice.hint, - accountCount: connectedAccountCount(choice, providers), - })); + return choices.map((choice) => { + const accountCount = connectedAccountCount(choice, providers); + // Once an OAuth kind has a connected account the browser-login CTA in its + // label ("ChatGPT — Login via Browser") reads as if still unconnected, so + // render the connected state plainly instead. The row stays listed so a + // second account remains reachable. + const connected = + choice.oauth !== null && !choice.custom && accountCount > 0; + return { + id: choice.id, + label: connected + ? `${choice.label.split(" — ")[0]} · ${accountCount} connected` + : choice.label, + hint: choice.hint, + accountCount, + }; + }); } /** Pick-list rows for the provider step. */