Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions src/config/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<kind>/<profile>` 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()] }
Expand Down
91 changes: 91 additions & 0 deletions src/config/oauth-catalog.test.ts
Original file line number Diff line number Diff line change
@@ -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/<profile>` entry.

const resolved: ResolvedProvider = {
providerName: "openai",
baseURL: "https://api.openai.com/v1",
apiKey: "sk-test",
model: "gpt-5",
};

function settingsWith(providers: Record<string, ProviderSettings>): 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"]);
});
});
36 changes: 36 additions & 0 deletions src/tui/model-catalog.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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/);
});
});
26 changes: 23 additions & 3 deletions src/tui/model-catalog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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",
};
}
20 changes: 20 additions & 0 deletions src/tui/provider-setup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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/<profile>" — one row per
Expand Down
23 changes: 17 additions & 6 deletions src/tui/provider/choices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
Loading