From cf4760f6f524b8c2fd1f8cfc94411facb12ebd12 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 12 Sep 2026 20:29:13 -0700 Subject: [PATCH 1/2] Show connected state plainly in the model picker --- src/config/index.ts | 22 ++++++-- src/config/oauth-catalog.test.ts | 91 ++++++++++++++++++++++++++++++++ src/tui/model-catalog.test.ts | 26 +++++++++ src/tui/model-catalog.ts | 24 +++++++-- src/tui/provider-setup.test.ts | 20 +++++++ src/tui/provider/choices.ts | 23 +++++--- 6 files changed, 193 insertions(+), 13 deletions(-) create mode 100644 src/config/oauth-catalog.test.ts 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..45f85ae84 100644 --- a/src/tui/model-catalog.test.ts +++ b/src/tui/model-catalog.test.ts @@ -218,4 +218,30 @@ 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/); + }); }); diff --git a/src/tui/model-catalog.ts b/src/tui/model-catalog.ts index 8ca701b10..d3dc0adaa 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,11 @@ export function describeModelCatalogOption( return { what: whatLine(model), - impact: pricingImpact(pricing, model), + impact: pricingImpact( + pricing, + option.id.slice(0, option.id.indexOf(":")), + 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. */ From 0af42e127b7d970accde77c42f5214faa64b7a19 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 12 Sep 2026 21:54:07 -0700 Subject: [PATCH 2/2] Parse colon-less picker ids without truncating the provider slice(0, indexOf(":")) drops the last character when the id has no colon, mis-attributing the row. split(":")[0] keeps the full id. --- src/tui/model-catalog.test.ts | 10 ++++++++++ src/tui/model-catalog.ts | 4 +++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/tui/model-catalog.test.ts b/src/tui/model-catalog.test.ts index 45f85ae84..28e3cfdde 100644 --- a/src/tui/model-catalog.test.ts +++ b/src/tui/model-catalog.test.ts @@ -244,4 +244,14 @@ describe("describeModelCatalogOption", () => { 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 d3dc0adaa..44fac0a33 100644 --- a/src/tui/model-catalog.ts +++ b/src/tui/model-catalog.ts @@ -324,7 +324,9 @@ export function describeModelCatalogOption( what: whatLine(model), impact: pricingImpact( pricing, - option.id.slice(0, option.id.indexOf(":")), + // 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",