Skip to content

Commit c2d3d30

Browse files
Show connected state plainly in the model picker (#973)
* Show connected state plainly in the model picker * 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.
1 parent 9e77d85 commit c2d3d30

6 files changed

Lines changed: 205 additions & 13 deletions

File tree

src/config/index.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1139,18 +1139,32 @@ export async function loadConfig(
11391139
// provider connect (mid-session, no restart) can rebuild the picker's
11401140
// catalog after writing new credentials, instead of only taking effect on
11411141
// the next process start.
1142-
function mergeOAuthCatalog(
1142+
export function mergeOAuthCatalog(
11431143
settings: Settings | null,
11441144
resolved: ResolvedProvider,
11451145
codexProfiles: readonly CodexProfile[],
11461146
xaiProfiles: readonly XaiProfile[],
11471147
): ProviderCatalogEntry[] {
1148+
const codexEntries = codexProfilesToCatalogEntries(codexProfiles);
1149+
const xaiEntries = xaiProfilesToCatalogEntries(xaiProfiles);
1150+
// A legacy bare `codex`/`xai` settings row (the original single-instance
1151+
// connect key) reads as a second, separately-added provider next to the
1152+
// credential-backed `<kind>/<profile>` entries. Drop it once that family
1153+
// has a live profile; when nothing is connected the bare row is the only
1154+
// ChatGPT/Grok access and stays.
1155+
const dropBare = new Set([
1156+
...(codexEntries.length > 0 ? ["codex"] : []),
1157+
...(xaiEntries.length > 0 ? ["xai"] : []),
1158+
]);
11481159
return [
11491160
...buildProviderCatalog(settings, resolved).filter(
1150-
(e) => !isCodexProviderName(e.name) && !isXaiProviderName(e.name),
1161+
(e) =>
1162+
!isCodexProviderName(e.name) &&
1163+
!isXaiProviderName(e.name) &&
1164+
!dropBare.has(e.name),
11511165
),
1152-
...codexProfilesToCatalogEntries(codexProfiles),
1153-
...xaiProfilesToCatalogEntries(xaiProfiles),
1166+
...codexEntries,
1167+
...xaiEntries,
11541168
].map((entry) =>
11551169
isOpenCodeGoProvider(entry)
11561170
? { ...entry, models: [...selectableGoModelIds()] }

src/config/oauth-catalog.test.ts

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
import { describe, expect, test } from "bun:test";
2+
import type { CodexProfile } from "../auth/codex/store.js";
3+
import type { XaiProfile } from "../auth/xai/store.js";
4+
import { mergeOAuthCatalog } from "./index.js";
5+
import type {
6+
ProviderSettings,
7+
ResolvedProvider,
8+
Settings,
9+
} from "./settings.js";
10+
11+
// Regression tests for CL-5606: after a successful ChatGPT browser login the
12+
// merged catalog must not list a separately-added legacy bare `codex` row
13+
// alongside the credential-backed `codex/<profile>` entry.
14+
15+
const resolved: ResolvedProvider = {
16+
providerName: "openai",
17+
baseURL: "https://api.openai.com/v1",
18+
apiKey: "sk-test",
19+
model: "gpt-5",
20+
};
21+
22+
function settingsWith(providers: Record<string, ProviderSettings>): Settings {
23+
return { providers } as Settings;
24+
}
25+
26+
const entry = (): ProviderSettings => ({
27+
baseURL: "https://chatgpt.com/backend-api/codex/responses",
28+
models: ["gpt-5.1-codex-max"],
29+
});
30+
31+
const codexDefault: CodexProfile = {
32+
name: "default",
33+
tokens: { access: "codex-access", refresh: "r", expiresAt: 1 },
34+
createdAt: 0,
35+
};
36+
const xaiWork: XaiProfile = {
37+
name: "work",
38+
tokens: { access: "xai-access", refresh: "r", expiresAt: 1 },
39+
createdAt: 0,
40+
};
41+
42+
describe("mergeOAuthCatalog legacy bare-row dedupe (CL-5606)", () => {
43+
test("a legacy bare codex row is dropped once codex/default is connected", () => {
44+
const merged = mergeOAuthCatalog(
45+
settingsWith({ codex: entry(), "codex/default": entry() }),
46+
resolved,
47+
[codexDefault],
48+
[],
49+
);
50+
expect(merged.map((p) => p.name)).toEqual(["codex/default"]);
51+
});
52+
53+
test("a legacy bare xai row is dropped once xai/work is connected", () => {
54+
const merged = mergeOAuthCatalog(
55+
settingsWith({ xai: entry() }),
56+
resolved,
57+
[],
58+
[xaiWork],
59+
);
60+
expect(merged.map((p) => p.name)).toEqual(["xai/work"]);
61+
});
62+
63+
test("a bare codex row survives when nothing credential-backed exists", () => {
64+
// Not connected: no auth-store profile and no qualified entry. The legacy
65+
// single-instance row is the only ChatGPT access — keep it.
66+
const merged = mergeOAuthCatalog(
67+
settingsWith({ codex: entry() }),
68+
resolved,
69+
[],
70+
[],
71+
);
72+
expect(merged.map((p) => p.name)).toEqual(["codex"]);
73+
});
74+
75+
test("unrelated and API-key rows are untouched by the dedupe", () => {
76+
const merged = mergeOAuthCatalog(
77+
settingsWith({
78+
openai: {
79+
baseURL: "https://api.openai.com/v1",
80+
apiKey: "sk-test",
81+
models: ["gpt-5"],
82+
},
83+
codex: entry(),
84+
}),
85+
resolved,
86+
[codexDefault],
87+
[],
88+
);
89+
expect(merged.map((p) => p.name)).toEqual(["openai", "codex/default"]);
90+
});
91+
});

src/tui/model-catalog.test.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,4 +218,40 @@ describe("describeModelCatalogOption", () => {
218218
);
219219
expect(description?.impact).toMatch(/pricing unknown/i);
220220
});
221+
222+
test("connected ChatGPT rows state plan billing plainly instead of unknown pricing (CL-5606)", () => {
223+
// Subscription-billed models have no per-token price; "Pricing unknown"
224+
// misreads as metered billing with a missing rate.
225+
const description = describeModelCatalogOption(
226+
{
227+
id: "codex/default:gpt-5.1-codex-max",
228+
label: "gpt-5.1-codex-max * [Codex default]",
229+
},
230+
{ pricing: null },
231+
);
232+
expect(description?.impact).not.toMatch(/pricing unknown/i);
233+
expect(description?.impact).toMatch(/ChatGPT subscription/);
234+
});
235+
236+
test("connected Grok rows state plan billing plainly instead of unknown pricing (CL-5606)", () => {
237+
const description = describeModelCatalogOption(
238+
{
239+
id: "xai/work:grok-4",
240+
label: "grok-4 * [xAI work]",
241+
},
242+
{ pricing: null },
243+
);
244+
expect(description?.impact).not.toMatch(/pricing unknown/i);
245+
expect(description?.impact).toMatch(/subscription/);
246+
});
247+
248+
test("colon-less ids keep the full provider instead of dropping the last character", () => {
249+
// slice(0, indexOf(":")) truncates a colon-less id (indexOf is -1), so
250+
// "codex/" became "code" and missed subscription billing.
251+
const description = describeModelCatalogOption(
252+
{ id: "codex/", label: "default * [Codex default]" },
253+
{ pricing: null },
254+
);
255+
expect(description?.impact).toMatch(/ChatGPT subscription/);
256+
});
221257
});

src/tui/model-catalog.ts

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -251,9 +251,23 @@ function formatPrice(perToken: number): string {
251251
/** Rough per-Mtok multiplier over the "standard" $3 input tier, for decision-relevant framing. */
252252
const STANDARD_INPUT_PER_MTOK = 3;
253253

254-
function pricingImpact(pricing: PricingCache | null, model: string): string {
254+
function pricingImpact(
255+
pricing: PricingCache | null,
256+
providerId: string,
257+
model: string,
258+
): string {
255259
const price = lookupModelPricing(pricing, model);
256-
if (price === null) return "Pricing unknown for this model.";
260+
if (price === null) {
261+
// Subscription-billed rows (ChatGPT/Grok OAuth) have no per-token price;
262+
// "unknown" misreads as metered billing with a missing rate.
263+
if (providerId.startsWith("codex/")) {
264+
return "Billed through your ChatGPT subscription, not per-token.";
265+
}
266+
if (providerId.startsWith("xai/")) {
267+
return "Billed through your Grok subscription, not per-token.";
268+
}
269+
return "Pricing unknown for this model.";
270+
}
257271
const inputPerMtok = price.inputPricePerToken * 1_000_000;
258272
const ratio = inputPerMtok / STANDARD_INPUT_PER_MTOK;
259273
const ratioText =
@@ -308,7 +322,13 @@ export function describeModelCatalogOption(
308322

309323
return {
310324
what: whatLine(model),
311-
impact: pricingImpact(pricing, model),
325+
impact: pricingImpact(
326+
pricing,
327+
// Exact provider parse: slice(0, indexOf(":")) drops the last
328+
// character of a colon-less id (indexOf returns -1).
329+
option.id.split(":")[0] ?? option.id,
330+
model,
331+
),
312332
tone: "plain",
313333
};
314334
}

src/tui/provider-setup.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,26 @@ describe("provider setup pure helpers", () => {
252252
expect(custom?.accountCount).toBe(0);
253253
});
254254

255+
test("Alt+A ChatGPT row hides the login CTA once connected (CL-5606)", () => {
256+
// After a successful browser login the ChatGPT row must not present the
257+
// "Login via Browser" connect option as if unconnected. The row stays
258+
// listed (CL-5899: a second account remains reachable) — only the
259+
// connected-state rendering changes.
260+
const choices = providerChoices();
261+
const connected = addProviderSelectorChoices(choices, [
262+
{ name: "codex/default" },
263+
]);
264+
const codex = connected.find((r) => r.id === "codex");
265+
expect(codex?.accountCount).toBe(1);
266+
expect(codex?.label).not.toContain("Login via Browser");
267+
expect(codex?.label).toContain("ChatGPT");
268+
269+
const disconnected = addProviderSelectorChoices(choices, []);
270+
const login = disconnected.find((r) => r.id === "codex");
271+
expect(login?.accountCount).toBe(0);
272+
expect(login?.label).toContain("Login via Browser");
273+
});
274+
255275
test("a connected Codex account counts under its profile-qualified name (CL-5606)", () => {
256276
// The ChatGPT-via-browser choice is keyed "codex", but a signed-in
257277
// account lands in the catalog as "codex/<profile>" — one row per

src/tui/provider/choices.ts

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -265,12 +265,23 @@ export function addProviderSelectorChoices(
265265
readonly hint: string;
266266
readonly accountCount: number;
267267
}[] {
268-
return choices.map((choice) => ({
269-
id: choice.id,
270-
label: choice.label,
271-
hint: choice.hint,
272-
accountCount: connectedAccountCount(choice, providers),
273-
}));
268+
return choices.map((choice) => {
269+
const accountCount = connectedAccountCount(choice, providers);
270+
// Once an OAuth kind has a connected account the browser-login CTA in its
271+
// label ("ChatGPT — Login via Browser") reads as if still unconnected, so
272+
// render the connected state plainly instead. The row stays listed so a
273+
// second account remains reachable.
274+
const connected =
275+
choice.oauth !== null && !choice.custom && accountCount > 0;
276+
return {
277+
id: choice.id,
278+
label: connected
279+
? `${choice.label.split(" — ")[0]} · ${accountCount} connected`
280+
: choice.label,
281+
hint: choice.hint,
282+
accountCount,
283+
};
284+
});
274285
}
275286

276287
/** Pick-list rows for the provider step. */

0 commit comments

Comments
 (0)