From ab5caad1c350fda833f2e388d00fee5ba50381c3 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Mon, 10 Aug 2026 08:57:59 -0700 Subject: [PATCH 1/8] Repair missing Content-Type on Codex streaming responses The Codex backend serves some models (observed live with the gpt-5.6 family) as HTTP 200 with a valid SSE body but no Content-Type header at all. The inference harness detects the response protocol from that header alone, so every turn on those models died with an unrecoverable protocol error despite a healthy stream. The fetch boundary now restores the header for Codex responses requests from the protocol the request's accept header declared; declared-but-unsupported types, ambiguous accept headers, and non-2xx responses keep the loud failure. --- src/provider/codex-responses-adapter.ts | 66 ++++++ src/provider/inference-dependencies.ts | 12 +- tests/unit/inference-response-kind.test.ts | 227 +++++++++++++++++++++ 3 files changed, 303 insertions(+), 2 deletions(-) create mode 100644 tests/unit/inference-response-kind.test.ts diff --git a/src/provider/codex-responses-adapter.ts b/src/provider/codex-responses-adapter.ts index 01aef3481..7f4646773 100644 --- a/src/provider/codex-responses-adapter.ts +++ b/src/provider/codex-responses-adapter.ts @@ -41,6 +41,72 @@ export const CODEX_SESSION_ID_OPTION = "codexSessionId"; const EMPTY_PARTIAL: PartialMessage = { text: "" }; +type FetchLike = ( + input: string | URL | Request, + init?: RequestInit, +) => Promise; + +function requestURL(input: string | URL | Request): string { + if (typeof input === "string") return input; + if (input instanceof URL) return input.toString(); + return input.url; +} + +// Content type the request's accept header committed to, or null when the +// commitment is ambiguous. Reads init headers first, falling back to a +// Request object's own headers so both fetch calling conventions are +// honored. Media types are prefix-matched per comma-separated entry so +// parameters do not defeat the match; a list naming BOTH supported +// protocols is ambiguous and yields null. +function acceptedContentType( + input: string | URL | Request, + init: RequestInit | undefined, +): string | null { + const headers = + init?.headers !== undefined + ? new Headers(init.headers) + : input instanceof Request + ? input.headers + : undefined; + const accept = headers?.get("accept"); + if (accept === undefined || accept === null) return null; + const supported = new Set(); + for (const entry of accept.toLowerCase().split(",")) { + const media = entry.trim(); + if (media.startsWith("text/event-stream")) supported.add("text/event-stream"); + else if (media.startsWith("application/json")) supported.add("application/json"); + } + if (supported.size !== 1) return null; + return [...supported][0] ?? null; +} + +// The Codex backend omits the Content-Type header entirely on some model +// streams (observed live with the gpt-5.6 family) while the body is a valid +// SSE stream. The vendored harness detects the response protocol from that +// header alone and fails the turn when it is absent, so the header is +// restored here — at the fetch boundary Corbits owns, scoped to Codex +// responses requests — from the protocol the request's accept header +// declared. Responses that declare any Content-Type, non-2xx responses, and +// requests whose accept header is ambiguous pass through untouched, keeping +// the harness's loud protocol-mismatch failure for genuine violations. +export function withCodexContentTypeRepair(fetchImpl: FetchLike): FetchLike { + return async (input, init) => { + const response = await fetchImpl(input, init); + if (!requestURL(input).endsWith(CODEX_RESPONSES_PATH)) return response; + if (!response.ok) return response; + if (response.headers.get("content-type") !== null) return response; + const declared = acceptedContentType(input, init); + if (declared === null) return response; + const headers = new Headers(response.headers); + headers.set("content-type", declared); + return new Response(response.body, { + status: response.status, + statusText: response.statusText, + headers, + }); + }; +} + // --------------------------------------------------------------------------- // Request building — internal turns → Responses `input` items // --------------------------------------------------------------------------- diff --git a/src/provider/inference-dependencies.ts b/src/provider/inference-dependencies.ts index dda245548..63c17396b 100644 --- a/src/provider/inference-dependencies.ts +++ b/src/provider/inference-dependencies.ts @@ -5,7 +5,10 @@ import * as codexResponses from "./codex-responses-adapter.js"; import * as grokResponses from "./grok-responses-adapter.js"; import * as bifrostAdapter from "./bifrost-adapter.js"; import * as openaiResponses from "./openai-responses-adapter.js"; -import { CODEX_RESPONSES_PROVIDER } from "./codex-responses-adapter.js"; +import { + CODEX_RESPONSES_PROVIDER, + withCodexContentTypeRepair, +} from "./codex-responses-adapter.js"; import { GROK_RESPONSES_PROVIDER } from "./grok-responses-adapter.js"; import { BIFROST_PROVIDER } from "./bifrost-adapter.js"; import { OPENAI_RESPONSES_PROVIDER } from "./openai-responses-adapter.js"; @@ -58,7 +61,12 @@ export function createInferenceDependencies(): Promise { if (cached === undefined) { cached = loadAdapterRegistry(manifest, { import: (specifier) => Promise.resolve(localModules[specifier]), - }).then(createDependencies); + }) + .then(createDependencies) + .then((deps) => ({ + ...deps, + fetch: withCodexContentTypeRepair(deps.fetch), + })); } return cached; } diff --git a/tests/unit/inference-response-kind.test.ts b/tests/unit/inference-response-kind.test.ts new file mode 100644 index 000000000..07798f0f1 --- /dev/null +++ b/tests/unit/inference-response-kind.test.ts @@ -0,0 +1,227 @@ +/** + * Regression coverage for Codex responses that omit the Content-Type header. + * The live backend does this for some models (observed with gpt-5.6-sol / + * gpt-5.6-luna): HTTP 200 with a valid SSE body and no Content-Type at all, + * which the vendored harness's protocol detection treats as an unrecoverable + * error. withCodexContentTypeRepair restores the header at the fetch boundary + * from the protocol the request's accept header declared, scoped to Codex + * responses requests only; everything else keeps the loud failure. + */ +import { describe, expect, test } from "bun:test"; +import { + createDefaultScheduler, + runInference, + type Dependencies, +} from "@intx/inference"; +import type { + ConversationTurn, + InferenceEvent, + InferenceSource, +} from "@intx/types/runtime"; +import { createInferenceDependencies } from "../../src/provider/inference-dependencies.js"; +import { + CODEX_RESPONSES_PROVIDER, + withCodexContentTypeRepair, +} from "../../src/provider/codex-responses-adapter.js"; +import { CODEX_RESPONSES_PATH } from "../../src/auth/codex/constants.js"; + +const CODEX_URL = `https://chatgpt.com/backend-api${CODEX_RESPONSES_PATH}`; + +const CODEX_SOURCE: InferenceSource = { + id: "codex/default", + provider: CODEX_RESPONSES_PROVIDER, + baseURL: "https://chatgpt.com/backend-api", + apiKey: "test-token", + model: "gpt-5.6-sol", +}; + +function userTurn(text: string): ConversationTurn { + return { role: "user", content: [{ type: "text", text }], timestamp: 0 }; +} + +const SSE_PAYLOADS = [ + { type: "response.created", response: { id: "resp_1", status: "in_progress" } }, + { type: "response.output_text.delta", item_id: "item_1", delta: "hello" }, + { + type: "response.completed", + response: { + id: "resp_1", + status: "completed", + usage: { input_tokens: 3, output_tokens: 1, total_tokens: 4 }, + }, + }, +]; + +/** SSE wire body from data payloads, streamed so Response infers no Content-Type. */ +function sseBody(payloads: readonly object[]): ReadableStream { + const wire = payloads.map((p) => `data: ${JSON.stringify(p)}\n\n`).join(""); + return new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode(wire)); + controller.close(); + }, + }); +} + +function headerlessResponse(body: BodyInit): Response { + const response = new Response(body, { status: 200 }); + response.headers.delete("content-type"); + return response; +} + +async function collect(iter: AsyncIterable): Promise { + const out: InferenceEvent[] = []; + for await (const ev of iter) out.push(ev); + return out; +} + +async function runCodexTurn(fetchImpl: Dependencies["fetch"]): Promise { + const base = await createInferenceDependencies(); + const deps: Dependencies = { + ...base, + fetch: withCodexContentTypeRepair(fetchImpl), + scheduler: createDefaultScheduler(), + }; + let seq = 0; + return collect( + runInference({ + turns: [userTurn("hi")], + source: CODEX_SOURCE, + nextSeq: () => ++seq, + deps, + }), + ); +} + +describe("withCodexContentTypeRepair through the harness", () => { + test("headerless 2xx SSE stream completes instead of failing the turn", async () => { + const events = await runCodexTurn(() => { + const response = headerlessResponse(sseBody(SSE_PAYLOADS)); + expect(response.headers.get("content-type")).toBeNull(); + return Promise.resolve(response); + }); + const types = events.map((e) => e.type); + + expect(types).not.toContain("inference.error"); + expect(types).toContain("inference.text.delta"); + expect(types).toContain("inference.done"); + }); + + test("declared unsupported Content-Type still fails loudly", async () => { + const events = await runCodexTurn(() => + Promise.resolve( + new Response("", { + status: 200, + headers: { "content-type": "text/html" }, + }), + ), + ); + const error = events.find((e) => e.type === "inference.error"); + + expect(error).toBeDefined(); + expect(JSON.stringify(error)).toContain("Unsupported response Content-Type"); + }); +}); + +describe("withCodexContentTypeRepair boundaries", () => { + const sseInit: RequestInit = { + method: "POST", + headers: { accept: "text/event-stream", "content-type": "application/json" }, + }; + + test("restores text/event-stream from an SSE accept header", async () => { + const fetchImpl = withCodexContentTypeRepair(() => + Promise.resolve(headerlessResponse(sseBody(SSE_PAYLOADS))), + ); + const response = await fetchImpl(CODEX_URL, sseInit); + expect(response.headers.get("content-type")).toBe("text/event-stream"); + }); + + test("restores application/json from a JSON accept header", async () => { + const fetchImpl = withCodexContentTypeRepair(() => + Promise.resolve(headerlessResponse(sseBody([]))), + ); + const response = await fetchImpl(CODEX_URL, { + method: "POST", + headers: { accept: "application/json" }, + }); + expect(response.headers.get("content-type")).toBe("application/json"); + }); + + test("leaves an ambiguous accept header unrepaired", async () => { + const fetchImpl = withCodexContentTypeRepair(() => + Promise.resolve(headerlessResponse(sseBody([]))), + ); + for (const accept of ["*/*", "application/json, text/event-stream"]) { + const response = await fetchImpl(CODEX_URL, { + method: "POST", + headers: { accept }, + }); + expect(response.headers.get("content-type")).toBeNull(); + } + }); + + test("reads the accept header from a Request-object input", async () => { + const fetchImpl = withCodexContentTypeRepair(() => + Promise.resolve(headerlessResponse(sseBody(SSE_PAYLOADS))), + ); + const response = await fetchImpl( + new Request(CODEX_URL, { + method: "POST", + headers: { accept: "text/event-stream" }, + }), + ); + expect(response.headers.get("content-type")).toBe("text/event-stream"); + }); + + test("leaves non-Codex URLs untouched", async () => { + const fetchImpl = withCodexContentTypeRepair(() => + Promise.resolve(headerlessResponse(sseBody([]))), + ); + const response = await fetchImpl("https://api.example.com/v1/messages", sseInit); + expect(response.headers.get("content-type")).toBeNull(); + }); + + test("createInferenceDependencies wires the repair into its fetch", async () => { + // A fresh module instance (cache-busted specifier) binds our stubbed + // globalThis.fetch at creation time, so this exercises the production + // wiring itself — the cached instance other tests share is untouched + // and no test-order dependence is introduced. + const originalFetch = globalThis.fetch; + globalThis.fetch = Object.assign( + () => Promise.resolve(headerlessResponse(sseBody(SSE_PAYLOADS))), + { preconnect: () => undefined }, + ) as typeof globalThis.fetch; + try { + const specifier = + "../../src/provider/inference-dependencies.js" + "?wiring-regression"; + const mod = (await import(specifier)) as { + createInferenceDependencies: () => Promise; + }; + const deps = await mod.createInferenceDependencies(); + const response = await deps.fetch(CODEX_URL, sseInit); + expect(response.headers.get("content-type")).toBe("text/event-stream"); + } finally { + globalThis.fetch = originalFetch; + } + }); + + test("leaves declared Content-Type and non-2xx responses untouched", async () => { + const declared = withCodexContentTypeRepair(() => + Promise.resolve( + new Response("{}", { status: 200, headers: { "content-type": "application/json" } }), + ), + ); + const declaredResponse = await declared(CODEX_URL, sseInit); + expect(declaredResponse.headers.get("content-type")).toBe("application/json"); + + const failing = withCodexContentTypeRepair(() => { + const response = new Response("nope", { status: 429 }); + response.headers.delete("content-type"); + return Promise.resolve(response); + }); + const failingResponse = await failing(CODEX_URL, sseInit); + expect(failingResponse.headers.get("content-type")).toBeNull(); + expect(failingResponse.status).toBe(429); + }); +}); From 6f82c29585205958821296a314e7df4fa5efe17d Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Mon, 10 Aug 2026 09:16:24 -0700 Subject: [PATCH 2/8] Support unlimited named OAuth accounts per provider with re-auth The connect flow always signed in under the same hardcoded profile name, so a second ChatGPT or xAI account silently overwrote the first one's tokens and there was no way to re-authorize an existing account. OAuth choices now insert a name step between provider selection and login: it prefills a suggested slug that auto-suffixes on collision, validates the entered slug (lowercase, restricted charset, length-capped) since it flows into a settings key, and asks for an explicit confirm before re-authorizing a name that already exists. The slug lives in its own form field rather than overloading the provider name field, which only gets the compound catalog name once login succeeds. --- src/tui/provider-setup-submit.test.ts | 3 + src/tui/provider-setup.test.ts | 262 +++++++++++++++++++++-- src/tui/provider-setup.ts | 292 ++++++++++++++++++++++++-- 3 files changed, 529 insertions(+), 28 deletions(-) diff --git a/src/tui/provider-setup-submit.test.ts b/src/tui/provider-setup-submit.test.ts index cbdf1d022..f4cc5de47 100644 --- a/src/tui/provider-setup-submit.test.ts +++ b/src/tui/provider-setup-submit.test.ts @@ -30,6 +30,7 @@ describe("buildProviderSubmitHandler", () => { baseURL: "https://api.openai.com/v1", apiKey: "", model: "gpt-5", + oauthProfile: "", }; const preset = { id: "openai", models: ["gpt-5"], anthropic: false, opencodeGo: false }; @@ -49,6 +50,7 @@ describe("buildProviderSubmitHandler", () => { baseURL: "http://localhost:11434/v1", apiKey: "", model: "llama3", + oauthProfile: "", }; // skipValidation avoids the live connection probe in this unit test. @@ -67,6 +69,7 @@ describe("buildProviderSubmitHandler", () => { baseURL: "https://api.openai.com/v1", apiKey: "sk-test-fake", model: "gpt-5", + oauthProfile: "", }; const preset = { id: "openai", models: ["gpt-5"], anthropic: false, opencodeGo: false }; diff --git a/src/tui/provider-setup.test.ts b/src/tui/provider-setup.test.ts index 5ee18401c..3851705c0 100644 --- a/src/tui/provider-setup.test.ts +++ b/src/tui/provider-setup.test.ts @@ -19,10 +19,13 @@ import { stepHeadline, stepReady, stepsFor, + suggestOAuthProfileSlug, summaryRows, TYPE_MODEL_ID, unconnectedProviderChoices, + validateOAuthProfileSlug, type OAuthLoginStarter, + type OAuthProfileLister, type ProviderFormValues, type ProviderSetupSubmit, type SubmitOpts, @@ -33,6 +36,7 @@ const EMPTY: ProviderFormValues = { baseURL: "", apiKey: "", model: "", + oauthProfile: "", } describe("provider setup pure helpers", () => { @@ -75,13 +79,15 @@ describe("provider setup pure helpers", () => { expect(secretFromMaskedEdit(secret, "")).toBe("") }) - test("a picked provider takes three steps, custom takes five", () => { + test("a picked provider takes three steps, custom takes five, oauth takes four", () => { const openai = providerChoiceById("openai") expect(openai?.baseURL).toBe("https://api.openai.com/v1") expect(stepsFor(openai ?? null)).toEqual(["provider", "apiKey", "model"]) - // A subscription provider swaps the paste for a sign-in, same three steps. + // A subscription provider swaps the paste for a name-then-sign-in pair, + // so it lands one step longer than a preset. expect(stepsFor(providerChoiceById("codex") ?? null)).toEqual([ "provider", + "name", "login", "model", ]) @@ -94,6 +100,26 @@ describe("provider setup pure helpers", () => { ]) }) + test("an OAuth account slug is lowercased and constrained to a settings-key-safe charset", () => { + expect(validateOAuthProfileSlug("Personal")).toEqual({ ok: true, slug: "personal" }) + expect(validateOAuthProfileSlug(" work ")).toEqual({ ok: true, slug: "work" }) + expect(validateOAuthProfileSlug("")).toEqual({ ok: false, error: "name cannot be empty" }) + expect(validateOAuthProfileSlug(" ")).toEqual({ ok: false, error: "name cannot be empty" }) + expect(validateOAuthProfileSlug("codex/personal").ok).toBe(false) + expect(validateOAuthProfileSlug("my account").ok).toBe(false) + expect(validateOAuthProfileSlug("-personal").ok).toBe(false) + expect(validateOAuthProfileSlug("personal-").ok).toBe(false) + expect(validateOAuthProfileSlug("a".repeat(64)).ok).toBe(true) + expect(validateOAuthProfileSlug("a".repeat(65)).ok).toBe(false) + }) + + test("a suggested OAuth slug auto-suffixes on collision", () => { + expect(suggestOAuthProfileSlug([])).toBe("default") + expect(suggestOAuthProfileSlug(["personal"])).toBe("default") + expect(suggestOAuthProfileSlug(["default"])).toBe("default-2") + expect(suggestOAuthProfileSlug(["default", "default-2"])).toBe("default-3") + }) + test("the pick-list carries known providers and ends with custom", () => { const choices = providerChoices() const ids = choices.map((c) => c.id) @@ -149,6 +175,14 @@ describe("provider setup pure helpers", () => { ) }) + test("the OAuth name step is headlined and summarized as an account name", () => { + const codex = providerChoiceById("codex") ?? null + const steps = stepsFor(codex) + expect(stepHeadline(steps, 1, codex)).toBe("step 2 of 4 · account name") + const rows = summaryRows(steps, 2, { ...EMPTY, oauthProfile: "work" }, codex) + expect(rows[1]).toMatchObject({ label: "account name", value: "work" }) + }) + test("summary rows mark done, current, and pending steps", () => { const values: ProviderFormValues = { ...EMPTY, name: "openai" } const choice = providerChoiceById("openai") ?? null @@ -250,12 +284,15 @@ async function flush(harness: Harness): Promise { /** * Mount with an injected login driver. No test may open a browser or bind a - * port, so the real PKCE/loopback path is never reached from here. + * port, so the real PKCE/loopback path is never reached from here. Also + * injects a profile lister so no test touches the real auth-store files; + * it defaults to reporting no existing profiles. */ async function mountLogin(opts: { start: OAuthLoginStarter onSubmit?: ProviderSetupSubmit loginTimeoutMs?: number + listOAuthProfiles?: OAuthProfileLister }): Promise<{ done: Promise; harness: Harness }> { const harness = await createHarness({ width: 80, height: 30 }) const done = runProviderSetup({ @@ -263,6 +300,7 @@ async function mountLogin(opts: { showTelemetryNotice: false, createRenderer: async () => harness.renderer, startLogin: opts.start, + listOAuthProfiles: opts.listOAuthProfiles ?? (async () => []), ...(opts.loginTimeoutMs !== undefined ? { loginTimeoutMs: opts.loginTimeoutMs } : {}), @@ -271,6 +309,35 @@ async function mountLogin(opts: { return { done, harness } } +/** + * Wait for a prefetched suggestion to land in the OAuth name field, then + * clear it. The input has no select-all-on-focus behavior, so typing over a + * prefilled suggestion would append rather than replace it; 80 backspaces is + * comfortably more than the field's 64-character cap, and backspacing an + * already-empty field is a no-op. + */ +async function clearOAuthNameField(harness: Harness): Promise { + await flush(harness) + for (let i = 0; i < 80; i++) harness.pressKey("Backspace") +} + +/** + * Accept the OAuth name step: type `name` when given (after clearing + * whatever suggestion was prefilled), otherwise accept the suggestion as is. + * The flush after Enter lets the submit-time collision re-check resolve + * before the caller inspects the result. + */ +async function nameOAuthAccount(harness: Harness, name?: string): Promise { + if (name === undefined) { + await flush(harness) + } else { + await clearOAuthNameField(harness) + type(harness, name) + } + harness.pressKey("Enter") + await flush(harness) +} + describe("runProviderSetup renderer ownership", () => { test("does not destroy a caller-supplied renderer on cancel", async () => { const { done, harness } = await mountSetup() @@ -307,16 +374,17 @@ describe("runProviderSetup sign-in", () => { }, }) await pickRow(harness, PROVIDER_IDS, "codex") - await flush(harness) + expect(harness.captureCharFrame()).toContain("step 2 of 4") + await nameOAuthAccount(harness) const waiting = harness.captureCharFrame() - expect(waiting).toContain("step 2 of 3") + expect(waiting).toContain("step 3 of 4") expect(waiting).toContain("sign in") expect(waiting).toContain("auth.example.com/authorize") expect(waiting).toContain("waiting for browser sign-in") complete({ profile: "default" }) await flush(harness) - expect(harness.captureCharFrame()).toContain("step 3 of 3") + expect(harness.captureCharFrame()).toContain("step 4 of 4") harness.pressKey("Enter") await harness.renderOnce() @@ -331,6 +399,140 @@ describe("runProviderSetup sign-in", () => { }) }) + test("the entered account name reaches startLogin as the profile slug", async () => { + const seenProfiles: string[] = [] + const { done, harness } = await mountLogin({ + start: async ({ profile }) => { + seenProfiles.push(profile) + return { + authorizeUrl: AUTHORIZE_URL, + completed: new Promise<{ profile: string }>(() => {}), + cancel: () => {}, + } + }, + }) + await pickRow(harness, PROVIDER_IDS, "codex") + await nameOAuthAccount(harness, "personal-account") + expect(seenProfiles).toEqual(["personal-account"]) + harness.pressKey("Ctrl+C") + expect(await done).toBe(false) + }) + + test("a suggested name auto-suffixes on collision with existing profiles", async () => { + const seenProfiles: string[] = [] + const { done, harness } = await mountLogin({ + listOAuthProfiles: async () => ["default"], + start: async ({ profile }) => { + seenProfiles.push(profile) + return { + authorizeUrl: AUTHORIZE_URL, + completed: new Promise<{ profile: string }>(() => {}), + cancel: () => {}, + } + }, + }) + await pickRow(harness, PROVIDER_IDS, "codex") + await flush(harness) + // The suggestion is visible before it is accepted, not just inferred + // from what startLogin later receives. + expect(harness.captureCharFrame()).toContain("default-2") + harness.pressKey("Enter") + await flush(harness) + expect(seenProfiles).toEqual(["default-2"]) + harness.pressKey("Ctrl+C") + expect(await done).toBe(false) + }) + + test("reusing a connected account's name asks to confirm before re-authorizing it", async () => { + const seenProfiles: string[] = [] + const { done, harness } = await mountLogin({ + listOAuthProfiles: async () => ["personal"], + start: async ({ profile }) => { + seenProfiles.push(profile) + return { + authorizeUrl: AUTHORIZE_URL, + completed: new Promise<{ profile: string }>(() => {}), + cancel: () => {}, + } + }, + }) + await pickRow(harness, PROVIDER_IDS, "codex") + await clearOAuthNameField(harness) + type(harness, "personal") + harness.pressKey("Enter") + await flush(harness) + // Still on the name step: the collision must be acknowledged, not just + // captioned, before the browser opens. + const confirming = harness.captureCharFrame() + expect(confirming).toContain("step 2 of 4") + expect(confirming).toContain("already connected") + expect(confirming).toContain("re-authorize") + expect(seenProfiles).toEqual([]) + + harness.pressKey("Enter") + await flush(harness) + expect(seenProfiles).toEqual(["personal"]) + expect(harness.captureCharFrame()).toContain("step 3 of 4") + harness.pressKey("Ctrl+C") + expect(await done).toBe(false) + }) + + test("editing the name after a collision confirm re-derives the check instead of reusing it", async () => { + let starts = 0 + const { done, harness } = await mountLogin({ + listOAuthProfiles: async () => ["personal"], + start: async () => { + starts += 1 + return { + authorizeUrl: AUTHORIZE_URL, + completed: new Promise<{ profile: string }>(() => {}), + cancel: () => {}, + } + }, + }) + await pickRow(harness, PROVIDER_IDS, "codex") + await clearOAuthNameField(harness) + type(harness, "personal") + harness.pressKey("Enter") + await flush(harness) + expect(harness.captureCharFrame()).toContain("already connected") + + // Editing the name invalidates the pending confirm — the next Enter must + // recheck rather than silently proceeding as if "personal2" were the + // account already confirmed. + type(harness, "2") + harness.pressKey("Enter") + await flush(harness) + expect(starts).toBe(1) + expect(harness.captureCharFrame()).toContain("step 3 of 4") + harness.pressKey("Ctrl+C") + expect(await done).toBe(false) + }) + + test("an invalid account name is rejected with a visible error and does not sign in", async () => { + let starts = 0 + const { done, harness } = await mountLogin({ + start: async () => { + starts += 1 + return { + authorizeUrl: AUTHORIZE_URL, + completed: new Promise<{ profile: string }>(() => {}), + cancel: () => {}, + } + }, + }) + await pickRow(harness, PROVIDER_IDS, "codex") + type(harness, "My Account!") + harness.pressKey("Enter") + await flush(harness) + const frame = harness.captureCharFrame() + expect(frame).toContain("step 2 of 4") + expect(frame).toContain("lowercase letters, numbers") + expect(starts).toBe(0) + harness.pressKey("Ctrl+C") + expect(await done).toBe(false) + }) + test("a denied sign-in says so and Enter retries it", async () => { let starts = 0 const { done, harness } = await mountLogin({ @@ -347,7 +549,7 @@ describe("runProviderSetup sign-in", () => { }, }) await pickRow(harness, PROVIDER_IDS, "codex") - await flush(harness) + await nameOAuthAccount(harness) const failed = harness.captureCharFrame() expect(failed).toContain("access denied by the user") expect(failed).toContain("enter to try signing in again") @@ -373,6 +575,7 @@ describe("runProviderSetup sign-in", () => { }), }) await pickRow(harness, PROVIDER_IDS, "codex") + await nameOAuthAccount(harness) await new Promise((r) => setTimeout(r, 30)) await harness.renderOnce() const frame = harness.captureCharFrame() @@ -383,7 +586,7 @@ describe("runProviderSetup sign-in", () => { expect(await done).toBe(false) }) - test("Escape abandons a sign-in and returns to the provider list", async () => { + test("Escape abandons a sign-in and returns to the name step for editing", async () => { let cancelled = 0 let aborted = false const { done, harness } = await mountLogin({ @@ -401,10 +604,10 @@ describe("runProviderSetup sign-in", () => { }, }) await pickRow(harness, PROVIDER_IDS, "codex") - await flush(harness) + await nameOAuthAccount(harness) await pressEscape(harness) const frame = harness.captureCharFrame() - expect(frame).toContain("step 1 of 3") + expect(frame).toContain("step 2 of 4") expect(frame).toContain(LOGIN_CANCELLED_MESSAGE) expect(frame).toContain("pick a provider to start over") expect(cancelled).toBeGreaterThan(0) @@ -413,6 +616,37 @@ describe("runProviderSetup sign-in", () => { expect(await done).toBe(false) }) + test("a failed sign-in can be retried under a different name after going back", async () => { + const seenProfiles: string[] = [] + const { done, harness } = await mountLogin({ + start: async ({ profile }) => { + seenProfiles.push(profile) + return { + authorizeUrl: AUTHORIZE_URL, + completed: + seenProfiles.length === 1 + ? Promise.reject(new Error("access denied by the user")) + : new Promise<{ profile: string }>(() => {}), + cancel: () => {}, + } + }, + }) + await pickRow(harness, PROVIDER_IDS, "codex") + await nameOAuthAccount(harness, "first-try") + expect(harness.captureCharFrame()).toContain("access denied by the user") + + await pressEscape(harness) + const backAtName = harness.captureCharFrame() + expect(backAtName).toContain("step 2 of 4") + expect(backAtName).toContain("first-try") + + await nameOAuthAccount(harness, "second-try") + expect(harness.captureCharFrame()).toContain("waiting for browser sign-in") + expect(seenProfiles).toEqual(["first-try", "second-try"]) + harness.pressKey("Ctrl+C") + expect(await done).toBe(false) + }) + test("a late resolution from an abandoned attempt cannot move the screen", async () => { let complete: (result: { profile: string }) => void = () => {} const { done, harness } = await mountLogin({ @@ -425,11 +659,11 @@ describe("runProviderSetup sign-in", () => { }), }) await pickRow(harness, PROVIDER_IDS, "codex") - await flush(harness) + await nameOAuthAccount(harness) await pressEscape(harness) complete({ profile: "default" }) await flush(harness) - expect(harness.captureCharFrame()).toContain("step 1 of 3") + expect(harness.captureCharFrame()).toContain("step 2 of 4") harness.pressKey("Ctrl+C") expect(await done).toBe(false) }) @@ -472,6 +706,7 @@ describe("runProviderSetup", () => { baseURL: "https://api.openai.com/v1", apiKey: "sk-key", model: openai?.defaultModel ?? "", + oauthProfile: "", }) expect(opts[0]?.preset?.id).toBe("openai") expect(opts[0]?.preset?.models.length).toBeGreaterThan(1) @@ -502,6 +737,7 @@ describe("runProviderSetup", () => { baseURL: "https://api.example.com", apiKey: "sk-key", model: "fp-small", + oauthProfile: "", }) expect(opts[0]?.preset).toBeUndefined() }) @@ -657,7 +893,7 @@ describe("runProviderSetup", () => { }), }) await pickRow(harness, PROVIDER_IDS, "codex") - await harness.renderOnce() + await nameOAuthAccount(harness) harness.pressKey("Ctrl+C") expect(await done).toBe(false) expect(cancelled).toBeGreaterThan(0) diff --git a/src/tui/provider-setup.ts b/src/tui/provider-setup.ts index b0c61f1bf..f108c7450 100644 --- a/src/tui/provider-setup.ts +++ b/src/tui/provider-setup.ts @@ -60,7 +60,23 @@ const PROVIDER_FIELD_HINTS: Record = { model: "gpt-4o", } -export type ProviderFormValues = Record +/** Placeholder for the OAuth account-name step, which edits `oauthProfile`. */ +const OAUTH_PROFILE_HINT = "default, personal, work, …" + +export type ProviderFormValues = { + name: string + baseURL: string + apiKey: string + model: string + /** + * Pre-login account slug for the OAuth path (e.g. "personal"). Kept apart + * from `name`, which for OAuth is only written once login succeeds and + * then carries the compound catalog name ("codex/personal") — reusing it + * for the slug would make the field mean two different things depending + * on where the operator is in the flow. + */ + oauthProfile: string +} /** One screen of the flow. `provider` and `model` can be pick-lists. */ export type SetupStep = @@ -74,8 +90,17 @@ export type SetupStep = /** Known-provider path: pick, paste key, pick model. */ export const PRESET_STEPS: readonly SetupStep[] = ["provider", "apiKey", "model"] -/** Subscription path: pick, sign in through the browser, pick model. */ -export const OAUTH_STEPS: readonly SetupStep[] = ["provider", "login", "model"] +/** + * Subscription path: pick, name the account (a suggested slug is prefilled; + * reusing an existing name asks for confirmation before re-authorizing it), + * sign in through the browser, pick a model. + */ +export const OAUTH_STEPS: readonly SetupStep[] = [ + "provider", + "name", + "login", + "model", +] /** Unknown endpoint: the full manual form, still preceded by the pick-list. */ export const CUSTOM_STEPS: readonly SetupStep[] = [ @@ -104,6 +129,12 @@ const STEP_PROMPTS: Record = { login: "authorize in the browser — this window waits for you", } +/** Instruction for the "name" step on the OAuth path, which names an account + * rather than a whole provider — reusing an existing name re-authorizes it. */ +function oauthNamePrompt(kind: OAuthKind): string { + return `name this account — stored as ${kind}/, and used again if you reconnect it` +} + // "testing" covers the connection-check call against the entered credentials; // "saving" covers the settings write that follows once the test succeeds. export type SubmitPhase = "testing" | "saving" @@ -141,9 +172,6 @@ export type ProviderChoice = { export type OAuthKind = FirstClassOAuthProvider -/** Profile name a first run authorizes under. `/model` can add more later. */ -export const DEFAULT_OAUTH_PROFILE = "default" - /** * What a signed-in subscription provider resolves to. The endpoint and model * list are the same constants the auth stack projects into the catalog, so a @@ -177,6 +205,66 @@ export function oauthProviderName(kind: OAuthKind, profile: string): string { return OAUTH_SURFACES[kind].providerName(profile) } +/** Longest slug the name step accepts, after normalization. */ +const OAUTH_PROFILE_MAX_LENGTH = 64 + +const OAUTH_PROFILE_CHARS = /^[a-z0-9._-]+$/ +const OAUTH_PROFILE_EDGE_SEPARATOR = /^[._-]|[._-]$/ + +export type OAuthProfileValidation = + | { readonly ok: true; readonly slug: string } + | { readonly ok: false; readonly error: string } + +/** + * Validate and lowercase-normalize an operator-entered account slug. This is + * the constraint owner for the slug shape — the auth store and the catalog + * projection (`oauthProviderName`) trust whatever they are handed, since a + * "/" here would silently join into the compound catalog name they build. + */ +export function validateOAuthProfileSlug(raw: string): OAuthProfileValidation { + const slug = raw.trim().toLowerCase() + if (slug.length === 0) return { ok: false, error: "name cannot be empty" } + if (slug.length > OAUTH_PROFILE_MAX_LENGTH) { + return { ok: false, error: `name must be ${String(OAUTH_PROFILE_MAX_LENGTH)} characters or fewer` } + } + if (!OAUTH_PROFILE_CHARS.test(slug)) { + return { ok: false, error: "use only lowercase letters, numbers, and . _ -" } + } + if (OAUTH_PROFILE_EDGE_SEPARATOR.test(slug)) { + return { ok: false, error: "name cannot start or end with . _ or -" } + } + return { ok: true, slug } +} + +/** + * A slug that does not collide with `existing`, so a first sign-in can + * default to something usable without asking the operator to invent a name. + * "default" first, then "default-2", "default-3", … on collision. + */ +export function suggestOAuthProfileSlug(existing: readonly string[]): string { + const taken = new Set(existing) + if (!taken.has("default")) return "default" + let n = 2 + while (taken.has(`default-${String(n)}`)) n += 1 + return `default-${String(n)}` +} + +/** Fetches the names of already-authorized profiles for a provider kind. */ +export type OAuthProfileLister = (kind: OAuthKind) => Promise + +/** + * Real lister, imported lazily per kind so mounting the surface never touches + * the auth-store files in a test that injects its own lister. + */ +const defaultProfileLister: OAuthProfileLister = async (kind) => { + if (kind === "codex") { + const { listCodexProfiles } = await import("../auth/codex/store.js") + return (await listCodexProfiles()).map((p) => p.name) + } + const { listXaiProfiles } = await import("../auth/xai/store.js") + return (await listXaiProfiles()).map((p) => p.name) +} + function oauthChoice( id: string, label: string, @@ -393,14 +481,22 @@ export function secretFromMaskedEdit(secret: string, displayed: string): string return [...secret].slice(0, keptLength).join("") + typed.join("") } +// The "name" step names a whole provider on the custom path but a single +// account on the OAuth path; the shared label would mislabel the latter. +function stepLabel(step: SetupStep, choice: ProviderChoice | null): string { + if (step === "name" && choice?.oauth != null) return "account name" + return STEP_LABELS[step] +} + /** `step 2 of 3 · api key` — always says where the operator is and what is left. */ export function stepHeadline( steps: readonly SetupStep[], index: number, + choice: ProviderChoice | null = null, ): string { const step = steps[Math.min(Math.max(index, 0), steps.length - 1)] if (step === undefined) return "" - return `step ${index + 1} of ${steps.length} · ${STEP_LABELS[step]}` + return `step ${index + 1} of ${steps.length} · ${stepLabel(step, choice)}` } export type SummaryRow = { @@ -419,7 +515,7 @@ export function summaryRows( return steps.map((step, i) => { const state = i < index ? "done" : i === index ? "current" : "pending" return { - label: STEP_LABELS[step], + label: stepLabel(step, choice), value: state === "done" ? settledValue(step, values, choice) : "—", state, } @@ -436,7 +532,7 @@ function settledValue( if (step === "apiKey") { return values.apiKey.length > 0 ? maskSecret(values.apiKey) : "keyless" } - if (step === "name") return values.name + if (step === "name") return choice?.oauth != null ? values.oauthProfile : values.name if (step === "baseURL") return values.baseURL return values.model } @@ -571,6 +667,8 @@ export type ProviderSetupConfig = { readonly createRenderer?: () => Promise /** Login driver override so tests need neither a browser nor a port. */ readonly startLogin?: OAuthLoginStarter + /** Profile lister override so tests need no auth-store files on disk. */ + readonly listOAuthProfiles?: OAuthProfileLister /** Sign-in deadline override, in milliseconds. */ readonly loginTimeoutMs?: number /** @@ -626,6 +724,7 @@ export async function runProviderSetup( baseURL: "", apiKey: "", model: "", + oauthProfile: "", } let choice: ProviderChoice | null = null let stepIndex = 0 @@ -638,6 +737,7 @@ export async function runProviderSetup( let rampTimer: ReturnType | null = null const startLogin = config.startLogin ?? defaultLoginStarter + const listOAuthProfiles = config.listOAuthProfiles ?? defaultProfileLister const loginTimeoutMs = config.loginTimeoutMs ?? LOGIN_TIMEOUT_MS let loginStatus: "idle" | "pending" | "failed" | "done" = "idle" let loginURL: string | null = null @@ -653,6 +753,19 @@ export async function runProviderSetup( // cancelled or superseded attempt can never move the screen. let loginAttempt = 0 + // The OAuth "name" step's own state: an inline error from the last + // validation, and a pending re-authorize confirmation for a name that + // collided with an existing profile. `confirmedSlug` is the exact slug the + // confirmation applies to, so an edit to the field (which invalidates it) + // is detected by comparison rather than a separate dirty flag. + let oauthProfileError: string | null = null + let oauthProfileConfirmPending = false + let confirmedSlug: string | null = null + // Bumped whenever the name step is (re-)entered, so a profile-list fetch + // left over from a step the operator has since navigated away from can + // never write into the wrong step's state. + let oauthNameAttempt = 0 + if (config.initialProviderId !== undefined) { const preselected = choices.find((c) => c.id === config.initialProviderId) if (preselected !== undefined) { @@ -694,6 +807,11 @@ export async function runProviderSetup( if (step === "provider") return true return step === "model" && choice !== null && !choice.custom && !typedModel } + // The "name" step means two different things depending on the path: a + // free-text provider name (custom) or an OAuth account slug with its own + // suggestion/collision machinery. Only the latter needs this branch. + const isOAuthNameStep = (): boolean => + currentStep() === "name" && choice !== null && choice.oauth !== null const root = new BoxRenderable(renderer, { id: "provider-setup", @@ -948,6 +1066,27 @@ export async function runProviderSetup( } const paintStatus = (): void => { + if (!submitting && isOAuthNameStep()) { + if (oauthProfileError !== null) { + const ramp = rampFor({ phase: "blocked", nowMs: 0 }) + statusLine.content = rampLine(ramp, oauthProfileError) + statusLine.fg = ramp.fg + guidance.content = "fix the name and press enter" + guidance.fg = UI.textDim + return + } + if (oauthProfileConfirmPending) { + const ramp = rampFor({ phase: "blocked", nowMs: 0 }) + statusLine.content = rampLine( + ramp, + `"${confirmedSlug ?? values.oauthProfile}" is already connected`, + ) + statusLine.fg = ramp.fg + guidance.content = "enter again to re-authorize this account · esc to cancel" + guidance.fg = UI.textDim + return + } + } if (!submitting && isLoginStep()) { if (loginStatus === "failed") { const ramp = rampFor({ phase: "blocked", nowMs: 0 }) @@ -961,7 +1100,7 @@ export async function runProviderSetup( const ramp = rampFor({ phase: "done", nowMs: 0 }) statusLine.content = rampLine( ramp, - `signed in as ${loginResult?.providerName ?? DEFAULT_OAUTH_PROFILE}`, + `signed in as ${loginResult?.providerName ?? "the account"}`, ) statusLine.fg = ramp.fg guidance.content = "enter to pick a model" @@ -1025,8 +1164,11 @@ export async function runProviderSetup( const paint = (): void => { const active = currentStep() - step.content = stepHeadline(steps(), stepIndex) - instruction.content = STEP_PROMPTS[active] + step.content = stepHeadline(steps(), stepIndex, choice) + instruction.content = + isOAuthNameStep() && choice?.oauth != null + ? oauthNamePrompt(choice.oauth) + : STEP_PROMPTS[active] paintSummary() paintList() paintLogin() @@ -1047,6 +1189,10 @@ export async function runProviderSetup( if (isLoginStep() && loginStatus === "idle") beginLogin() return } + if (isOAuthNameStep()) { + enterOAuthNameStep() + return + } const field = active as ProviderField input.placeholder = PROVIDER_FIELD_HINTS[field] input.value = field === "apiKey" ? maskEcho(values.apiKey) : values[field] @@ -1055,6 +1201,35 @@ export async function runProviderSetup( input.focus() } + /** + * Enter the OAuth "name" step: reset its per-visit state, show whatever + * slug is already typed, then fetch this provider's profiles fresh (never + * cached across visits — another sign-in could have landed between two + * visits to this step) to prefill a suggested, non-colliding slug when the + * field is still blank. + */ + const enterOAuthNameStep = (): void => { + oauthProfileError = null + oauthProfileConfirmPending = false + input.placeholder = OAUTH_PROFILE_HINT + input.value = values.oauthProfile + paint() + input.focus() + const kind = choice?.oauth ?? null + if (kind === null) return + const attempt = (oauthNameAttempt += 1) + listOAuthProfiles(kind) + .catch((): readonly string[] => []) + .then((names) => { + if (settled || attempt !== oauthNameAttempt) return + if (values.oauthProfile.trim().length === 0) { + values.oauthProfile = suggestOAuthProfileSlug(names) + input.value = values.oauthProfile + paint() + } + }) + } + let settled = false let resolveDone: (submitted: boolean) => void = () => {} const done = new Promise((resolve) => { @@ -1174,7 +1349,7 @@ export async function runProviderSetup( rampTimer = setInterval(paintStatus, RAMP_TICK_MS) paint() - startLogin({ kind, profile: DEFAULT_OAUTH_PROFILE, signal: abort.signal }).then( + startLogin({ kind, profile: values.oauthProfile, signal: abort.signal }).then( (handle) => { if (attempt !== loginAttempt) { handle.cancel() @@ -1264,10 +1439,21 @@ export async function runProviderSetup( if (picked === undefined) return choice = picked typedModel = false + oauthProfileError = null + oauthProfileConfirmPending = false + confirmedSlug = null if (picked.custom) { values.name = "" values.baseURL = "" values.model = "" + } else if (picked.oauth !== null) { + // Left blank until login succeeds — see the `oauthProfile` doc comment + // on `ProviderFormValues` for why the pre-login slug is a field of + // its own rather than a stand-in value here. + values.name = "" + values.baseURL = picked.baseURL + values.model = picked.defaultModel + values.oauthProfile = "" } else { values.name = picked.id values.baseURL = picked.baseURL @@ -1340,6 +1526,10 @@ export async function runProviderSetup( if (loginStatus !== "pending") beginLogin() return } + if (isOAuthNameStep()) { + advanceOAuthNameStep() + return + } const field = currentStep() as ProviderField if (!stepReady(field, values[field])) return @@ -1353,6 +1543,57 @@ export async function runProviderSetup( submit(false) } + /** + * Validate the entered slug, then re-derive the collision check against a + * fresh profile fetch rather than trusting the snapshot taken when the + * step was entered — a profile authorized elsewhere in the meantime must + * still be caught. A collision needs one more Enter to confirm before the + * step advances, worded as a re-authorization rather than a bare retry. + */ + const advanceOAuthNameStep = (): void => { + const kind = choice?.oauth ?? null + if (kind === null) return + const validated = validateOAuthProfileSlug(values.oauthProfile) + if (!validated.ok) { + oauthProfileError = validated.error + oauthProfileConfirmPending = false + confirmedSlug = null + paint() + return + } + const slug = validated.slug + // Already confirmed this exact slug on the previous Enter — proceed + // without another round-trip. Any edit since then cleared the flag (see + // onInput), so this only fires on a genuine second, unmodified Enter. + if (oauthProfileConfirmPending && confirmedSlug === slug) { + enterLoginStepWithSlug(slug) + return + } + const attempt = (oauthNameAttempt += 1) + listOAuthProfiles(kind) + .catch((): readonly string[] => []) + .then((names) => { + if (settled || attempt !== oauthNameAttempt) return + if (names.includes(slug)) { + oauthProfileError = null + oauthProfileConfirmPending = true + confirmedSlug = slug + paint() + return + } + enterLoginStepWithSlug(slug) + }) + } + + const enterLoginStepWithSlug = (slug: string): void => { + values.oauthProfile = slug + oauthProfileError = null + oauthProfileConfirmPending = false + confirmedSlug = null + stepIndex += 1 + showStep() + } + const back = (): void => { if (stepIndex === 0) return stepIndex -= 1 @@ -1364,6 +1605,18 @@ export async function runProviderSetup( function onInput(next: string): void { if (submitting || isListStep()) return + if (isOAuthNameStep()) { + values.oauthProfile = next + // An edit invalidates whatever the last submit attempt found — the + // confirm applies to one exact slug, and any inline error is stale + // the moment the text it described changes. + const hadFeedback = oauthProfileError !== null || oauthProfileConfirmPending + oauthProfileError = null + oauthProfileConfirmPending = false + confirmedSlug = null + if (hadFeedback) paint() + return + } const field = currentStep() as ProviderField if (field === "apiKey") { values.apiKey = secretFromMaskedEdit(values.apiKey, next) @@ -1402,8 +1655,17 @@ export async function runProviderSetup( } if (key.name === "escape") { key.preventDefault() - if (isLoginStep()) cancelLogin() - else back() + if (isLoginStep()) { + cancelLogin() + } else if (isOAuthNameStep() && oauthProfileConfirmPending) { + // Cancel the re-authorize confirm without leaving the step — the + // operator is about to edit the name, not abandon the provider. + oauthProfileConfirmPending = false + confirmedSlug = null + paint() + } else { + back() + } return } if (isLoginStep()) { From 247132ed1bd68f18ef5f30618ce618a589bca7bd Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Mon, 10 Aug 2026 09:13:19 -0700 Subject: [PATCH 3/8] Add an Alt+A add-provider selector to the model picker The flat /model list only shows a provider once it has zero connected accounts, so a second OAuth account (a second Codex or xAI login) had no path to a connect flow. Alt+A opens a fresh selector over every first-class provider kind, with a live connected-account count per row and no already-connected filtering, so a provider stays reachable no matter how many accounts it already has. Picking a row runs the same inline connect flow the flat list's connect rows use, and now reopens the model picker focused on the account's default model instead of leaving the operator to find it themselves. --- src/tui/overlay-paint.test.ts | 4 +- src/tui/overlays.ts | 36 ++++++++++ src/tui/product-host.test.ts | 128 +++++++++++++++++++++++++++++++++- src/tui/product-host.ts | 93 ++++++++++++++++++++---- src/tui/provider-setup.ts | 24 ++++--- src/tui/runner-host.test.ts | 36 ++++++++++ src/tui/runner-host.ts | 14 +++- src/tui/runner.ts | 27 ++++++- src/tui/shell.ts | 41 +++++++++-- 9 files changed, 373 insertions(+), 30 deletions(-) diff --git a/src/tui/overlay-paint.test.ts b/src/tui/overlay-paint.test.ts index e5e05ff73..7f8fac710 100644 --- a/src/tui/overlay-paint.test.ts +++ b/src/tui/overlay-paint.test.ts @@ -106,12 +106,14 @@ describe("overlay host never shares cells with the prompt border", () => { kind: "model_picker", title: "model", items: ITEMS, + // Mirror production /model, which always wires Alt+A. + addProviderHint: true, }), size, ) const expected = [ - " model · Esc cancel · Enter choose", + " model · Esc cancel · Enter choose · Alt+A add provider", ` > ${ITEMS[0]}`, ...ITEMS.slice(1).map((i) => ` ${i}`), ] diff --git a/src/tui/overlays.ts b/src/tui/overlays.ts index 5aae171a0..44fc5c5cd 100644 --- a/src/tui/overlays.ts +++ b/src/tui/overlays.ts @@ -195,6 +195,8 @@ export type OpenModelPickerOpts = { * as you type. Off by default so other list overlays keep j/k. */ readonly typeToFilter?: boolean + /** Advertise Alt+A in the footer — only when the caller wired the handler. */ + readonly addProviderHint?: boolean } export function openModelPickerOverlay( @@ -215,6 +217,40 @@ export function openModelPickerOverlay( ...(opts?.typeToFilter !== undefined ? { typeToFilter: opts.typeToFilter } : {}), + ...(opts?.addProviderHint !== undefined + ? { addProviderHint: opts.addProviderHint } + : {}), + }) +} + +export type OpenAddProviderOpts = { + readonly items?: readonly string[] + /** Stable provider ids aligned with `items`. */ + readonly itemIds?: readonly string[] + readonly activeIndex?: number + /** Per-open accept; host runs the connect flow for the chosen provider. */ + readonly onAccept?: (selection: OverlaySelection) => void + /** Description-zone source, keyed by the focused row's id. */ + readonly describe?: (itemId: string) => ItemDescription | null + /** Per-open Esc/dismiss — the caller returns to the model list. */ + readonly onCancel?: () => void +} + +/** Alt+A from the model picker: every first-class provider kind, no already-connected filtering. */ +export function openAddProviderOverlay( + shell: AppShell, + opts?: OpenAddProviderOpts, +): void { + openListOverlay(shell, { + kind: "add_provider", + title: "add provider", + items: opts?.items ?? [], + activeIndex: opts?.activeIndex ?? 0, + frameId: "overlay-add-provider", + ...(opts?.itemIds !== undefined ? { itemIds: opts.itemIds } : {}), + ...(opts?.onAccept !== undefined ? { onAccept: opts.onAccept } : {}), + ...(opts?.describe !== undefined ? { describe: opts.describe } : {}), + ...(opts?.onCancel !== undefined ? { onCancel: opts.onCancel } : {}), }) } diff --git a/src/tui/product-host.test.ts b/src/tui/product-host.test.ts index b233baa02..527ab95de 100644 --- a/src/tui/product-host.test.ts +++ b/src/tui/product-host.test.ts @@ -4,9 +4,10 @@ */ import { EventEmitter } from "node:events" import { describe, expect, test } from "bun:test" +import type { KeyEvent } from "@opentui/core" import type { PermissionRequest } from "../permission/types.js" import { createHarness } from "./harness.js" -import { acceptOverlaySelection, moveOverlaySelection } from "./shell.js" +import { acceptOverlaySelection, closeInsetOverlay, moveOverlaySelection, runOverlayAction } from "./shell.js" import { mountProductHost, operatorResultFromSelection, @@ -585,6 +586,131 @@ describe("flat type-to-filter model picker", () => { harness.destroy() } }) + + const altA = { name: "a", ctrl: false, meta: false, option: true } as KeyEvent + + test("the model picker footer advertises Alt+A", async () => { + const { harness, host } = await mountPicker({ + // The hint requires the full wiring — choices AND the connect handler — + // because that is exactly when the key actually works. + onConnectProvider: () => {}, + addProviderChoices: () => [{ id: "codex", label: "Codex", hint: "", accountCount: 0 }], + }) + try { + host.openModels?.() + await harness.renderOnce() + expect(harness.captureCharFrame()).toContain("Alt+A") + } finally { + host.dispose() + harness.destroy() + } + }) + + test("Alt+A opens the add-provider selector listing every provider kind and its account count", async () => { + const { harness, host } = await mountPicker({ + onConnectProvider: () => {}, + addProviderChoices: () => [ + { id: "codex", label: "Codex", hint: "ChatGPT subscription", accountCount: 2 }, + { id: "openai", label: "OpenAI", hint: "", accountCount: 0 }, + ], + }) + try { + host.openModels?.() + await harness.renderOnce() + expect(runOverlayAction(host.shell, altA)).toBe(true) + await harness.renderOnce() + expect(host.shell.overlayKind).toBe("add_provider") + expect(host.shell.overlayItems).toEqual([ + "Codex — 2 accounts", + "OpenAI — 0 accounts", + ]) + } finally { + host.dispose() + harness.destroy() + } + }) + + test("Esc from the add-provider selector returns to the model list", async () => { + const { harness, host } = await mountPicker({ + onConnectProvider: () => {}, + addProviderChoices: () => [{ id: "codex", label: "Codex", hint: "", accountCount: 1 }], + }) + try { + host.openModels?.() + await harness.renderOnce() + const modelItems = host.shell.overlayItems + runOverlayAction(host.shell, altA) + await harness.renderOnce() + expect(host.shell.overlayKind).toBe("add_provider") + closeInsetOverlay(host.shell) + await harness.renderOnce() + expect(host.shell.overlayKind).toBe("model_picker") + expect(host.shell.overlayItems).toEqual(modelItems) + } finally { + host.dispose() + harness.destroy() + } + }) + + test("Enter on an add-provider row runs the connect flow for that provider", async () => { + const connected: string[] = [] + const { harness, host } = await mountPicker({ + onConnectProvider: (name) => connected.push(name), + addProviderChoices: () => [{ id: "codex", label: "Codex", hint: "", accountCount: 0 }], + }) + try { + host.openModels?.() + await harness.renderOnce() + runOverlayAction(host.shell, altA) + await harness.renderOnce() + acceptOverlaySelection(host.shell) + expect(connected).toEqual(["codex"]) + } finally { + host.dispose() + harness.destroy() + } + }) + + test("without addProviderChoices, Alt+A is not claimed", async () => { + const { harness, host } = await mountPicker() + try { + host.openModels?.() + await harness.renderOnce() + expect(runOverlayAction(host.shell, altA)).toBe(false) + expect(host.shell.overlayKind).toBe("model_picker") + } finally { + host.dispose() + harness.destroy() + } + }) + + test("without addProviderChoices, the footer never advertises Alt+A", async () => { + // The hint and the key claim must move together: a host that omits + // addProviderChoices gets neither, so the footer never names a dead key. + const { harness, host } = await mountPicker() + try { + host.openModels?.() + await harness.renderOnce() + expect(harness.captureCharFrame()).not.toContain("Alt+A") + } finally { + host.dispose() + harness.destroy() + } + }) + + test("openModels(focusId) preselects the given row instead of the top of the list", async () => { + const { harness, host } = await mountPicker() + try { + host.openModels?.("codex/abk-labs:gpt-5.6-sol") + await harness.renderOnce() + const idx = host.shell.overlayItems.findIndex((label) => label.includes("gpt-5.6-sol")) + expect(idx).toBeGreaterThanOrEqual(0) + expect(host.shell.overlayList?.activeIndex).toBe(idx) + } finally { + host.dispose() + harness.destroy() + } + }) }) describe("mount failure", () => { diff --git a/src/tui/product-host.ts b/src/tui/product-host.ts index 14de84619..c2ba9ddd2 100644 --- a/src/tui/product-host.ts +++ b/src/tui/product-host.ts @@ -16,7 +16,7 @@ import { type TaskProgressSession, type TurnMonitorOptions, } from "./runtime-bridge.js" -import { openModelPickerOverlay } from "./overlays.js" +import { openAddProviderOverlay, openModelPickerOverlay } from "./overlays.js" import { wireGates } from "./gate-wire.js" import { createSystemClipboard } from "./system-clipboard.js" import { @@ -40,6 +40,7 @@ import { appendObserveStreamRow, appendStreamRow, clearTranscript, + closeInsetOverlay, createAppShell, paintChrome, setChromeZones, @@ -96,6 +97,15 @@ export type ProductHostModelOption = { readonly section?: "recent" | "favorites" | "provider" | "unconnected" } +/** A first-class provider kind offered by the Alt+A add-provider selector. */ +export type ProductHostAddProviderChoice = { + readonly id: string + readonly label: string + readonly hint: string + /** Live count of connected accounts for this provider kind (0 or more). */ + readonly accountCount: number +} + export type ProductHostConfig = { readonly title: string /** Working directory carried by the prompt box's bottom border. */ @@ -129,6 +139,12 @@ export type ProductHostConfig = { readonly onConnectProvider?: (providerName: string) => void /** Alt+F on a focused model row; connect rows are skipped by the caller. Bare `f` is claimed by type-to-filter. */ readonly onFavoriteToggle?: (itemId: string) => void + /** + * Every first-class provider kind, read fresh on each Alt+A open so a + * just-connected account's count is current. Omitted hosts get no Alt+A + * hint and no add-provider selector. + */ + readonly addProviderChoices?: () => readonly ProductHostAddProviderChoice[] /** Command palette catalog (registry-backed). */ readonly commands?: readonly PaletteCommand[] readonly onCommand?: (name: string) => void @@ -179,8 +195,12 @@ export type ProductHost = { * No-op (returns false) when observe is not active. */ readonly pushObserveRow: (row: StreamRow) => boolean - /** Opens the model/provider picker; absent when no models were supplied. */ - readonly openModels?: () => void + /** + * Opens the model/provider picker; absent when no models were supplied. + * `focusId` selects an initial row (e.g. a just-connected account's + * default model) instead of the top of the list. + */ + readonly openModels?: (focusId?: string) => void /** Swap the picker's rows/descriptions in place (e.g. after a provider connects). */ readonly setModels?: ( models: readonly ProductHostModelOption[], @@ -500,20 +520,62 @@ export async function mountProductHost( let currentModels = config.models ?? [] let currentDescribeModel = config.describeModel - let openModels: (() => void) | undefined + let openModels: ((focusId?: string) => void) | undefined if (config.onModelSelect) { const onSelect = config.onModelSelect const onConnect = config.onConnectProvider const onFavoriteToggle = config.onFavoriteToggle + const addProviderChoices = config.addProviderChoices + + // Alt+A from the model picker: close it and open a fresh selector over + // every first-class provider kind, no already-connected filtering. This + // gets its own PrimaryOverlayKind opened through the same close-then-open + // path openModels itself uses, rather than the palette's priorOverlay + // stack — that stack exists so the palette can float over a permission or + // operator question without dropping the awaited promise underneath it, + // which does not apply here. + const openAddProvider = + addProviderChoices !== undefined && onConnect !== undefined + ? (): void => { + const rows = addProviderChoices() + closeInsetOverlay(shell) + openAddProviderOverlay(shell, { + items: rows.map( + (r) => `${r.label} — ${r.accountCount} account${r.accountCount === 1 ? "" : "s"}`, + ), + itemIds: rows.map((r) => r.id), + onAccept: (sel) => { + const id = sel.id + if (id === undefined || id.length === 0) return + onConnect(id) + }, + describe: (itemId) => { + const row = rows.find((r) => r.id === itemId) + if (row === undefined) return null + return { + what: row.hint.length > 0 ? row.hint : "Opens the connect flow for this provider.", + impact: `${row.accountCount} account${row.accountCount === 1 ? "" : "s"} connected today.`, + tone: "plain", + } + }, + // Esc returns to the model list through the same entry point + // Alt+A itself, /model, and a completed connect all use. + onCancel: () => openModels?.(), + }) + } + : undefined - openModels = (): void => { + openModels = (focusId?: string): void => { const activeId = config.activeModelId?.() const items = annotateCurrent(currentModels, activeId) + const focusIndex = focusId !== undefined ? items.findIndex((m) => m.id === focusId) : -1 openModelPickerOverlay(shell, { items: items.map((m) => m.label), itemIds: items.map((m) => m.id), // Flat list: type to narrow rather than drill into a provider pane. typeToFilter: true, + addProviderHint: openAddProvider !== undefined, + ...(focusIndex >= 0 ? { activeIndex: focusIndex } : {}), onAccept: (sel) => { // Prefer the stable id from the (possibly filtered) row. Do not fall // back to `items[sel.index]` — that index is into the filtered list, @@ -528,16 +590,23 @@ export async function mountProductHost( onSelect(id) }, describe: (itemId) => currentDescribeModel?.(itemId) ?? null, - ...(onFavoriteToggle !== undefined + ...(onFavoriteToggle !== undefined || openAddProvider !== undefined ? { onAction: (itemId, key) => { - // Alt+F, never bare f — type-to-filter claims printable keys. + if (key.ctrl || !(key.meta || key.option)) return false const name = typeof key.name === "string" ? key.name.toLowerCase() : "" - if (name !== "f" || key.ctrl || !(key.meta || key.option)) return false - // Empty id is the "(no matches)" filter sentinel — not a model. - if (itemId.length === 0 || itemId.startsWith("connect:")) return false - onFavoriteToggle(itemId) - return true + // Alt+A / Alt+F, never bare — type-to-filter claims printable keys. + if (name === "a" && openAddProvider !== undefined) { + openAddProvider() + return true + } + if (name === "f" && onFavoriteToggle !== undefined) { + // Empty id is the "(no matches)" filter sentinel — not a model. + if (itemId.length === 0 || itemId.startsWith("connect:")) return false + onFavoriteToggle(itemId) + return true + } + return false }, } : {}), diff --git a/src/tui/provider-setup.ts b/src/tui/provider-setup.ts index f108c7450..1728dedeb 100644 --- a/src/tui/provider-setup.ts +++ b/src/tui/provider-setup.ts @@ -366,19 +366,27 @@ export function providerChoiceById(id: string): ProviderChoice | undefined { } /** - * Whether `choice` already has a connected provider in `providers`. OAuth - * choices (`codex`, `xai`) are keyed by vendor id, but a signed-in account - * lands in the catalog as `codex/` / `xai/` — one row per - * account — so exact-id matching alone never clears the "connect" row after - * a successful browser login. Key-based choices still match by exact id. + * How many connected accounts `choice` has in `providers`. OAuth choices + * (`codex`, `xai`) are keyed by vendor id, but a signed-in account lands in + * the catalog as `codex/` / `xai/` — one row per account — + * so exact-id matching alone can only ever find zero or one. Key-based + * choices still match by exact id, which is also at most one. */ +export function connectedAccountCount( + choice: ProviderChoice, + providers: readonly { readonly name: string }[], +): number { + return providers.filter( + (p) => p.name === choice.id || (choice.oauth !== null && p.name.startsWith(`${choice.id}/`)), + ).length +} + +/** Whether `choice` already has a connected provider in `providers`. */ export function isChoiceConnected( choice: ProviderChoice, providers: readonly { readonly name: string }[], ): boolean { - return providers.some( - (p) => p.name === choice.id || (choice.oauth !== null && p.name.startsWith(`${choice.id}/`)), - ) + return connectedAccountCount(choice, providers) > 0 } /** Known choices with no connected provider yet — the picker's "connect →" rows. */ diff --git a/src/tui/runner-host.test.ts b/src/tui/runner-host.test.ts index d9ce3e378..f83f2b9a5 100644 --- a/src/tui/runner-host.test.ts +++ b/src/tui/runner-host.test.ts @@ -364,6 +364,42 @@ describe("mountRunnerHost model picker", () => { harness.destroy() } }) + + test("Alt+A opens the add-provider selector built from addProviderChoices", async () => { + const harness = await createHarness({ width: 80, height: 24 }) + const connected: string[] = [] + const host = await mountRunnerHost({ + title: "test", + eventEmitter: new EventEmitter(), + send: () => {}, + interrupt: () => {}, + providers: { xai: { models: ["grok-4"] } }, + onModelSelect: () => {}, + onConnectProvider: (name) => connected.push(name), + addProviderChoices: () => [ + { id: "codex", label: "Codex", hint: "", accountCount: 1 }, + { id: "openai", label: "OpenAI", hint: "", accountCount: 0 }, + ], + commands: [], + onCommand: () => {}, + chrome: () => ({ agents: [] }), + subscribeChrome: () => () => {}, + subAgentSessions: () => [], + createRenderer: async () => harness.renderer, + }) + try { + expect(host.openSurface("models")).toBe(true) + const altA = { name: "a", ctrl: false, meta: false, option: true } as KeyEvent + expect(runOverlayAction(host.shell, altA)).toBe(true) + expect(host.shell.overlayKind).toBe("add_provider") + expect(host.shell.overlayItems).toEqual(["Codex — 1 account", "OpenAI — 0 accounts"]) + acceptOverlaySelection(host.shell) + expect(connected).toEqual(["codex"]) + } finally { + host.dispose() + harness.destroy() + } + }) }) describe("bottom border cost run", () => { diff --git a/src/tui/runner-host.ts b/src/tui/runner-host.ts index 424001c7f..9d20a5c84 100644 --- a/src/tui/runner-host.ts +++ b/src/tui/runner-host.ts @@ -28,7 +28,11 @@ import { type ModelCatalogUnconnectedProvider, } from "./model-catalog.js" import type { ItemDescription } from "./shell.js" -import { mountProductHost, type ProductHost } from "./product-host.js" +import { + mountProductHost, + type ProductHost, + type ProductHostAddProviderChoice, +} from "./product-host.js" import { onTurnBoundary } from "../agent/reactor-events.js" import { clearShellExitHandler, @@ -88,6 +92,11 @@ export type RunnerHostDeps = { readonly onConnectProvider?: (providerName: string) => void /** `f` on a focused model row; runner owns the favorite persist + refresh. */ readonly onFavoriteToggle?: (id: string) => void + /** + * Alt+A from the model picker: every first-class provider kind, read fresh + * on each open so a just-connected account's count is current. + */ + readonly addProviderChoices?: () => readonly ProductHostAddProviderChoice[] /** Working directory carried by the prompt box's bottom border. */ readonly cwd?: string /** Branch lookup override for tests; defaults to a real `git rev-parse`. */ @@ -267,6 +276,9 @@ export async function mountRunnerHost(deps: RunnerHostDeps): Promise ...(deps.onFavoriteToggle !== undefined ? { onFavoriteToggle: deps.onFavoriteToggle } : {}), + ...(deps.addProviderChoices !== undefined + ? { addProviderChoices: deps.addProviderChoices } + : {}), models: catalog, activeModelId: () => { const active = deps.activeModel?.() diff --git a/src/tui/runner.ts b/src/tui/runner.ts index bc6bc6f4b..cb22c99b8 100644 --- a/src/tui/runner.ts +++ b/src/tui/runner.ts @@ -42,8 +42,9 @@ import { type LocalSettings, type PluginConfig, } from "../config/settings.js"; -import { unconnectedProviderChoices } from "./provider-setup.js"; +import { connectedAccountCount, providerChoices, unconnectedProviderChoices } from "./provider-setup.js"; import { connectProviderInline } from "./provider-connect.js"; +import { modelOptionId } from "./model-catalog.js"; import type { SessionModeScope } from "./command-surfaces.js"; import { resolveWaitForApproval, type ToolWatchdogConfig } from "./tool-execution-watchdog.js"; import { attachApprovalBudget, createGateRequestApproval } from "./request-approval.js"; @@ -2052,6 +2053,20 @@ export async function runTUI(initialConfig: Config): Promise { authKind: choice.oauth !== null ? ("oauth" as const) : ("key" as const), })); + // Alt+A add-provider selector rows: every first-class provider kind, no + // already-connected filtering (unlike the flat list's connect rows above), + // so a second OAuth account is reachable once the first is already + // connected. Read fresh on each open against the live catalog. + const computeAddProviderChoices = () => + providerChoices() + .filter((choice) => !choice.custom) + .map((choice) => ({ + id: choice.id, + label: choice.label, + hint: choice.hint, + accountCount: connectedAccountCount(choice, config.providers), + })); + const host = await mountRunnerHost({ // An unnamed session shows nothing rather than a placeholder. title: runTaskTitle, @@ -2094,6 +2109,7 @@ export async function runTUI(initialConfig: Config): Promise { recentModels: listRecentModels(config.settings ?? { providers: {} }), favoriteModels: listFavoriteModels(config.settings ?? { providers: {} }), unconnectedProviders: computeUnconnectedProviders(config.providers), + addProviderChoices: computeAddProviderChoices, onConnectProvider: (providerName) => { void (async () => { let result: Awaited>; @@ -2131,7 +2147,14 @@ export async function runTUI(initialConfig: Config): Promise { providers, computeUnconnectedProviders(providers), ); - systemNotice(`Connected ${result.providerName ?? providerName}. Open /model to pick a model.`); + // Reopen positioned at the account just connected — the picker's + // default open (top of list) would otherwise leave the operator to + // hunt for the row they just authorized. + const connectedName = result.providerName ?? providerName; + host.openModels?.( + result.model !== undefined ? modelOptionId(connectedName, result.model) : undefined, + ); + systemNotice(`Connected ${connectedName}. Open /model to pick a model.`); })().catch((err: unknown) => { tuiLogger.debug("provider connect failed: {error}", { error: err instanceof Error ? err.message : String(err), diff --git a/src/tui/shell.ts b/src/tui/shell.ts index 04bb1ccc9..e9222525c 100644 --- a/src/tui/shell.ts +++ b/src/tui/shell.ts @@ -714,6 +714,7 @@ export type PrimaryOverlayKind = | "permissions" | "operator" | "model_picker" + | "add_provider" | "demo" | "palette" | "settings" @@ -1300,12 +1301,25 @@ const DEFAULT_OVERLAY_HINTS = [ * Never promises "Enter choose" when there is nothing to choose: an overlay * with no rows says what the operator can actually do instead. */ +/** Model picker only: same three-tier fallback shape as DEFAULT_OVERLAY_HINTS. */ +const MODEL_PICKER_HINTS = [ + "Esc cancel · Enter choose · Alt+A add provider", + "Esc · Enter · Alt+A add", + "Esc · Enter", +] as const + function overlayHints(shell: AppShell): readonly string[] { const answer = overlayAnswerState(shell) const hasChoices = shell.overlayItems.length > 0 if (answer === null) { - if (hasChoices) return DEFAULT_OVERLAY_HINTS - return ["Esc dismiss"] + if (!hasChoices) return ["Esc dismiss"] + if ( + shell.overlayKind === "model_picker" && + internals.get(shell)?.overlayAddProviderHint === true + ) { + return MODEL_PICKER_HINTS + } + return DEFAULT_OVERLAY_HINTS } if (answer.active) { return hasChoices @@ -1864,6 +1878,7 @@ type PriorOverlaySnapshot = { readonly answer: OverlayAnswerState | null readonly titleText: string readonly onCancel: (() => void) | null + readonly addProviderHint: boolean } type ShellInternals = { @@ -1890,6 +1905,8 @@ type ShellInternals = { overlayDescribe: ((itemId: string) => ItemDescription | null) | null /** Per-open bare-key claim for the open primary overlay. */ overlayOnAction: ((itemId: string, key: KeyEvent) => boolean) | null + /** Whether the open primary advertises Alt+A in the footer hints. */ + overlayAddProviderHint: boolean /** Per-open free-text answer field, when the overlay opted into one. */ overlayAnswer: OverlayAnswerState | null /** Bare title of the open overlay, so its key hints can be re-composed. */ @@ -3416,6 +3433,12 @@ export type OpenListOverlayOpts = { * navigation; with it, j/k type into the filter and arrows still navigate. */ readonly typeToFilter?: boolean + /** + * Advertise the Alt+A add-provider hint in the footer for this open. Set + * only when the caller actually wired an Alt+A handler via `onAction`, so + * the hint can never name a key that is a dead end. + */ + readonly addProviderHint?: boolean } /** @@ -3454,6 +3477,7 @@ export function openListOverlay( answer: bag.overlayAnswer, titleText: bag.overlayTitleText, onCancel: bag.overlayOnCancel, + addProviderHint: bag.overlayAddProviderHint, } } // Leave prior overlay focus frame; palette will stack above it. @@ -3484,6 +3508,7 @@ export function openListOverlay( bag.overlayDescribe = opts?.describe ?? null bag.overlayOnAction = opts?.onAction ?? null bag.overlayOnCancel = opts?.onCancel ?? null + bag.overlayAddProviderHint = opts?.addProviderHint ?? false // Capture the full unfiltered set so typing can re-narrow in place. bag.listFilter = opts?.typeToFilter === true @@ -3505,6 +3530,7 @@ export function openListOverlay( bag.overlayDescribe = opts?.describe ?? null bag.overlayOnAction = opts?.onAction ?? null bag.overlayOnCancel = opts?.onCancel ?? null + bag.overlayAddProviderHint = opts?.addProviderHint ?? false bag.listFilter = null } if (!isPalette) { @@ -3919,13 +3945,15 @@ export function closeInsetOverlay(shell: AppShell): void { const prior = wasPalette ? bag?.priorOverlay ?? null : null // Permissions/operator overlays back a caller awaiting ev.resolve — Esc must // still settle that promise (as a deny/cancel) or the caller hangs forever. - // model_picker onCancel is optional back-navigation for callers that set one; - // palette/mentions/copy have no awaited caller and drop silently. + // model_picker/add_provider onCancel is optional back-navigation for + // callers that set one; palette/mentions/copy have no awaited caller and + // drop silently. const cancelable = !prior && (shell.overlayKind === "permissions" || shell.overlayKind === "operator" || - shell.overlayKind === "model_picker") + shell.overlayKind === "model_picker" || + shell.overlayKind === "add_provider") const onCancel = cancelable ? bag?.overlayOnCancel ?? null : null shell.overlayList = null @@ -3945,6 +3973,7 @@ export function closeInsetOverlay(shell: AppShell): void { bag.overlayOnCycle = null bag.overlayDescribe = null bag.overlayOnAction = null + bag.overlayAddProviderHint = false bag.overlayAnswer = null bag.overlayOnCancel = null } @@ -3978,6 +4007,7 @@ export function closeInsetOverlay(shell: AppShell): void { bag.overlayAnswer = prior.answer bag.overlayTitleText = prior.titleText bag.overlayOnCancel = prior.onCancel + bag.overlayAddProviderHint = prior.addProviderHint // If focus was not stacked (edge case), re-open overlay frame. if (focusOwner(shell.focus) !== "overlay") { shell.focus = openOverlay(shell.focus, OVERLAY_FRAME_ID, { @@ -5995,6 +6025,7 @@ export function createAppShell( overlayOnCycle: null, overlayDescribe: null, overlayOnAction: null, + overlayAddProviderHint: false, overlayAnswer: null, overlayTitleText: "", overlayOnCancel: null, From 90800ea9aacad6cd76a399e4d82ef410ea10222f Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Mon, 10 Aug 2026 09:19:59 -0700 Subject: [PATCH 4/8] Remove the flat model list's per-provider connect rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Alt+A's add-provider selector is now the only path to connecting a provider, so the flat list's "connect →" rows and the filtering that fed them are dead weight: the unconnected-providers plumbing, the unconnected section of the models-first catalog, and the connect-row id helpers all go. The connected-choice predicates were only ever called to build that filtering, so they go too — their matching logic already lives on in the account-count helper. --- docs/TUI.md | 31 +++++++++++++++------ src/tui/model-catalog.test.ts | 25 ----------------- src/tui/model-catalog.ts | 50 ++-------------------------------- src/tui/product-host.ts | 21 ++++++-------- src/tui/provider-connect.ts | 2 +- src/tui/provider-setup.test.ts | 24 ++++++++-------- src/tui/provider-setup.ts | 19 ++----------- src/tui/runner-host.test.ts | 48 ++++---------------------------- src/tui/runner-host.ts | 22 ++++----------- src/tui/runner.ts | 21 +++----------- 10 files changed, 65 insertions(+), 198 deletions(-) diff --git a/docs/TUI.md b/docs/TUI.md index a308e958f..bac6a0c3d 100644 --- a/docs/TUI.md +++ b/docs/TUI.md @@ -419,14 +419,29 @@ known, accepted cost of the badge rather than an oversight — see The model/provider picker is one flat, type-to-filter list (`src/tui/product-host.ts` + `openModelPickerOverlay({ typeToFilter: true })`): recent and favorite provider+model pairs sit at the top, then every -`provider / model` leaf from the catalog — no nested provider pane. Typing -narrows the list in place (printable keys claimed by the filter row, same -pattern as the command palette); Enter selects. Escape closes the picker. -The row matching the session's live active model gets a `(current)` suffix. -Alt+F on a model row still toggles favorite when a favorite hook is wired. -While type-to-filter is active, bare `j`/`k` type into the filter rather than -moving the highlight — use arrow keys (or the filtered list's navigation) to -move. +`provider / model` leaf from the catalog. Typing narrows the list in place +(printable keys claimed by the filter row, same pattern as the command +palette); Enter selects. Escape closes the picker. The row matching the +session's live active model gets a `(current)` suffix. Alt+F on a model row +still toggles favorite when a favorite hook is wired. While type-to-filter is +active, bare `j`/`k` type into the filter rather than moving the highlight — +use arrow keys (or the filtered list's navigation) to move. + +The list itself never nests by provider, but connecting a new provider is not +a flat-list row either: the picker used to grow a "connect →" row per +not-yet-configured provider kind, filtered out once that kind had any +connected account. That filtering made a second OAuth account (a second +Codex or xAI login) unreachable — OAuth accounts are per-profile, so +kind-level "already connected" filtering hid the connect path the moment the +first profile existed. **Alt+A** now opens `add_provider` +(`src/tui/overlays.ts:openAddProviderOverlay`), a separate `PrimaryOverlayKind` +listing every first-class provider kind from `providerChoices()` — OAuth and +API-key alike — each annotated with its live connected-account count and none +of them filtered out. Esc returns to the model list through the same +`openModels()` entry point the picker itself uses. Picking a row runs the +existing inline connect flow (`provider-connect.ts`); on success the picker +reopens focused on the new account's default model instead of the top of the +list. Onboarding (the standalone provider-setup screen, `provider-setup.ts`) and the satellite pickers used for session resume and session-mode selection diff --git a/src/tui/model-catalog.test.ts b/src/tui/model-catalog.test.ts index 5b54e49de..d61d659c5 100644 --- a/src/tui/model-catalog.test.ts +++ b/src/tui/model-catalog.test.ts @@ -2,7 +2,6 @@ import { describe, expect, test } from "bun:test" import { buildModelCatalog, buildModelsFirstCatalog, - connectRowId, describeModelCatalogOption, modelOptionId, type ModelCatalogProvider, @@ -196,33 +195,9 @@ describe("buildModelsFirstCatalog", () => { }) expect(list[0]?.label).toBe("custom / m1") }) - - test("appends a not-connected connect row for each unconnected provider", () => { - const list = buildModelsFirstCatalog({ - providers: [xai], - recent: [], - favorites: [], - unconnected: [ - { name: "openai", label: "OpenAI", modelCount: 4, authKind: "key" }, - ], - }) - - const row = list.find((r) => r.section === "unconnected") - expect(row?.id).toBe(connectRowId("openai")) - expect(row?.label).toBe("OpenAI — connect →") - }) }) describe("describeModelCatalogOption", () => { - test("describes an unconnected provider's connect row", () => { - const description = describeModelCatalogOption( - { id: connectRowId("openai"), label: "OpenAI — connect →", section: "unconnected" }, - { unconnected: [{ name: "openai", label: "OpenAI", modelCount: 4, authKind: "key" }] }, - ) - expect(description?.what).toMatch(/not set up yet/i) - expect(description?.impact).toMatch(/4 models become available/) - }) - test("surfaces the Go-on-Zen billing warning as a consequence-toned impact, not the label", () => { const description = describeModelCatalogOption( { id: "zen:kimi-k2.7-code", label: "OpenCode Zen / kimi-k2.7-code", warning: "Go model on Zen path" }, diff --git a/src/tui/model-catalog.ts b/src/tui/model-catalog.ts index d4d0f4f6b..57fa718d9 100644 --- a/src/tui/model-catalog.ts +++ b/src/tui/model-catalog.ts @@ -16,7 +16,7 @@ import { contextWindowFor, hasContextWindowFor } from "../provider/context-windo import { modelReasoningCapability } from "../provider/reasoning-effort.js" import type { ItemDescription } from "./shell.js" -export type ModelCatalogSection = "recent" | "favorites" | "provider" | "unconnected" +export type ModelCatalogSection = "recent" | "favorites" | "provider" /** Picker row — superset of ProductHostModelOption (`id`, `label`). */ export type ModelCatalogOption = { @@ -129,16 +129,6 @@ const GO_ON_ZEN_WARNING = "Go model on Zen path — billed as Zen credits" /** Default recent-section cap (mirrors config/settings.js DEFAULT_RECENT_MODELS_SHOWN). */ const DEFAULT_RECENT_MAX = 5 -/** Known-but-unconfigured provider, surfaced as a "connect →" row. */ -export type ModelCatalogUnconnectedProvider = { - readonly name: string - readonly label?: string - /** How many models become selectable once this provider is connected. */ - readonly modelCount: number - /** "key" prompts for an API key; "oauth" runs the authorize-link flow. */ - readonly authKind: "key" | "oauth" -} - export type BuildModelsFirstCatalogArgs = { readonly providers: ModelCatalogProvidersInput readonly recent?: readonly ModelCatalogRef[] @@ -151,8 +141,6 @@ export type BuildModelsFirstCatalogArgs = { * billing-product detector; override in tests. */ readonly isGoModelOnZenPath?: (model: string, provider: ModelCatalogProvider) => boolean - /** Known providers with no stored credentials yet — rendered as "connect →" rows. */ - readonly unconnected?: readonly ModelCatalogUnconnectedProvider[] } function providerLabelOf(p: ModelCatalogProvider): string { @@ -224,29 +212,9 @@ export function buildModelsFirstCatalog( } } - for (const provider of args.unconnected ?? []) { - const id = connectRowId(provider.name) - if (seen.has(id)) continue - seen.add(id) - const label = provider.label !== undefined && provider.label.trim().length > 0 - ? provider.label.trim() - : provider.name - out.push({ id, label: `${label} — connect →`, section: "unconnected" }) - } - return out } -/** Stable id for an unconnected-provider "connect" row. */ -export function connectRowId(providerName: string): string { - return `connect:${providerName}` -} - -/** Provider name a connect-row id refers to, or null when `id` is not a connect row. */ -export function providerFromConnectRowId(id: string): string | null { - return id.startsWith("connect:") ? id.slice("connect:".length) : null -} - function formatPrice(perToken: number): string { const perMtok = perToken * 1_000_000 return `$${perMtok % 1 === 0 ? perMtok.toFixed(0) : perMtok.toFixed(2)}` @@ -282,27 +250,15 @@ function whatLine(model: string): string { /** * Description-zone content for a picker row. `pricing` defaults to the live - * models.dev cache; override in tests. Unconnected "connect →" rows and rows - * with a billing warning override the plain what/impact pair. + * models.dev cache; override in tests. Rows with a billing warning override + * the plain what/impact pair. */ export function describeModelCatalogOption( option: ModelCatalogOption, args?: { readonly pricing?: PricingCache | null - readonly unconnected?: readonly ModelCatalogUnconnectedProvider[] }, ): ItemDescription | null { - const providerName = providerFromConnectRowId(option.id) - if (providerName !== null) { - const provider = (args?.unconnected ?? []).find((p) => p.name === providerName) - const count = provider?.modelCount ?? 0 - return { - what: "Not set up yet. Connecting asks for an API key and stores it in your global settings.", - impact: `${count} model${count === 1 ? "" : "s"} become available. Nothing is sent until you send a message.`, - tone: "plain", - } - } - const model = option.id.slice(option.id.indexOf(":") + 1) const pricing = args?.pricing !== undefined ? args.pricing : getActivePricingCache() diff --git a/src/tui/product-host.ts b/src/tui/product-host.ts index c2ba9ddd2..976bfffa3 100644 --- a/src/tui/product-host.ts +++ b/src/tui/product-host.ts @@ -88,13 +88,13 @@ export type ProductHostDeliver = ( /** * `section` tags catalog rows for grouping/ordering in `buildModelsFirstCatalog` - * (recent and favorites first, then provider models, then unconnected connect - * rows). The live picker is flat + type-to-filter — it does not nest by section. + * (recent and favorites first, then provider models). The live picker is flat + * + type-to-filter — it does not nest by section. */ export type ProductHostModelOption = { readonly id: string readonly label: string - readonly section?: "recent" | "favorites" | "provider" | "unconnected" + readonly section?: "recent" | "favorites" | "provider" } /** A first-class provider kind offered by the Alt+A add-provider selector. */ @@ -132,12 +132,12 @@ export type ProductHostConfig = { /** Description-zone source for the model picker, keyed by row id. */ readonly describeModel?: (itemId: string) => ItemDescription | null /** - * Selecting a "connect →" row (id `connect:`) calls this instead - * of `onModelSelect`. Caller runs the connect flow and, on success, updates - * `models`/`describeModel` via `setModels` and reopens the picker. + * Picking a provider in the Alt+A add-provider selector calls this. Caller + * runs the connect flow and, on success, updates `models`/`describeModel` + * via `setModels` and reopens the picker. */ readonly onConnectProvider?: (providerName: string) => void - /** Alt+F on a focused model row; connect rows are skipped by the caller. Bare `f` is claimed by type-to-filter. */ + /** Alt+F on a focused model row. Bare `f` is claimed by type-to-filter. */ readonly onFavoriteToggle?: (itemId: string) => void /** * Every first-class provider kind, read fresh on each Alt+A open so a @@ -582,11 +582,6 @@ export async function mountProductHost( // not the unfiltered catalog, so it would pick the wrong model. const id = sel.id if (id === undefined || id.length === 0) return - const providerName = id.startsWith("connect:") ? id.slice("connect:".length) : null - if (providerName !== null) { - onConnect?.(providerName) - return - } onSelect(id) }, describe: (itemId) => currentDescribeModel?.(itemId) ?? null, @@ -602,7 +597,7 @@ export async function mountProductHost( } if (name === "f" && onFavoriteToggle !== undefined) { // Empty id is the "(no matches)" filter sentinel — not a model. - if (itemId.length === 0 || itemId.startsWith("connect:")) return false + if (itemId.length === 0) return false onFavoriteToggle(itemId) return true } diff --git a/src/tui/provider-connect.ts b/src/tui/provider-connect.ts index 9755a6768..602fe7dd3 100644 --- a/src/tui/provider-connect.ts +++ b/src/tui/provider-connect.ts @@ -1,5 +1,5 @@ /** - * Inline "connect →" flow for the model picker's not-connected providers. + * Inline connect flow for the model picker's Alt+A add-provider selector. * Extracted wiring around provider-setup's existing full-screen setup surface * (key entry + OAuth login, with its timeout/cancel/failure handling already * implemented there) — reused via `initialProviderId`, not reimplemented. diff --git a/src/tui/provider-setup.test.ts b/src/tui/provider-setup.test.ts index 3851705c0..278714e7d 100644 --- a/src/tui/provider-setup.test.ts +++ b/src/tui/provider-setup.test.ts @@ -2,9 +2,9 @@ import { describe, expect, test } from "bun:test" import { createHarness, type Harness } from "./harness.js" import { + connectedAccountCount, CUSTOM_CHOICE_ID, failureGuidance, - isChoiceConnected, LOGIN_CANCELLED_MESSAGE, LOGIN_TIMEOUT_MESSAGE, maskEcho, @@ -22,7 +22,6 @@ import { suggestOAuthProfileSlug, summaryRows, TYPE_MODEL_ID, - unconnectedProviderChoices, validateOAuthProfileSlug, type OAuthLoginStarter, type OAuthProfileLister, @@ -139,21 +138,24 @@ describe("provider setup pure helpers", () => { expect(providerChoiceRows(choices)[0]?.label).toContain("OpenAI") }) - test("a connected Codex account clears the ChatGPT connect row (CL-5606)", () => { + 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 - // account. Exact-id matching alone would leave the connect row stuck - // forever after a successful login. - const connected = [{ name: "codex/default" }] - expect(unconnectedProviderChoices(connected).map((c) => c.id)).not.toContain("codex") - expect(unconnectedProviderChoices([]).map((c) => c.id)).toContain("codex") + // account. Exact-id matching alone would only ever find zero or one. + const codexChoice = providerChoiceById("codex") + if (codexChoice === undefined) throw new Error("expected a codex choice") + expect(connectedAccountCount(codexChoice, [{ name: "codex/default" }])).toBe(1) + expect( + connectedAccountCount(codexChoice, [{ name: "codex/default" }, { name: "codex/work" }]), + ).toBe(2) + expect(connectedAccountCount(codexChoice, [])).toBe(0) }) - test("isChoiceConnected does not prefix-match key-based providers", () => { + test("connectedAccountCount does not prefix-match key-based providers", () => { const openaiChoice = providerChoiceById("openai") if (openaiChoice === undefined) throw new Error("expected an openai choice") - expect(isChoiceConnected(openaiChoice, [{ name: "openai-eu" }])).toBe(false) - expect(isChoiceConnected(openaiChoice, [{ name: "openai" }])).toBe(true) + expect(connectedAccountCount(openaiChoice, [{ name: "openai-eu" }])).toBe(0) + expect(connectedAccountCount(openaiChoice, [{ name: "openai" }])).toBe(1) }) test("model rows come from the provider catalog plus a free-text escape", () => { diff --git a/src/tui/provider-setup.ts b/src/tui/provider-setup.ts index 1728dedeb..fb24f50a9 100644 --- a/src/tui/provider-setup.ts +++ b/src/tui/provider-setup.ts @@ -381,21 +381,6 @@ export function connectedAccountCount( ).length } -/** Whether `choice` already has a connected provider in `providers`. */ -export function isChoiceConnected( - choice: ProviderChoice, - providers: readonly { readonly name: string }[], -): boolean { - return connectedAccountCount(choice, providers) > 0 -} - -/** Known choices with no connected provider yet — the picker's "connect →" rows. */ -export function unconnectedProviderChoices( - providers: readonly { readonly name: string }[], - choices: readonly ProviderChoice[] = providerChoices(), -): readonly ProviderChoice[] { - return choices.filter((choice) => !choice.custom && !isChoiceConnected(choice, providers)) -} /** Pick-list rows for the provider step. */ export function providerChoiceRows( @@ -681,8 +666,8 @@ export type ProviderSetupConfig = { readonly loginTimeoutMs?: number /** * Skip the provider pick-list and start directly on that provider's - * apiKey/login step — the inline "connect →" path from the model picker - * already knows which provider it wants. + * apiKey/login step — the inline connect path from the model picker's + * add-provider selector already knows which provider it wants. */ readonly initialProviderId?: string } diff --git a/src/tui/runner-host.test.ts b/src/tui/runner-host.test.ts index f83f2b9a5..1d09aea09 100644 --- a/src/tui/runner-host.test.ts +++ b/src/tui/runner-host.test.ts @@ -223,8 +223,8 @@ describe("mountRunnerHost command surfaces", () => { expect(host.openSurface("settings")).toBe(true) expect(host.shell.overlayKind).toBe("settings") closeInsetOverlay(host.shell) - // onModelSelect is wired even with an empty catalog, since the "not - // connected" section can populate the picker on its own. + // onModelSelect being wired is enough to open the picker, even with an + // empty catalog (nothing to pick yet, but the surface itself opens). expect(host.openSurface("models")).toBe(true) } finally { host.dispose() @@ -234,37 +234,6 @@ describe("mountRunnerHost command surfaces", () => { }) describe("mountRunnerHost model picker", () => { - test("lists a connect row for each unconnected provider, described in the connect copy", async () => { - const harness = await createHarness({ width: 80, height: 24 }) - const host = await mountRunnerHost({ - title: "test", - eventEmitter: new EventEmitter(), - send: () => {}, - interrupt: () => {}, - providers: { xai: { models: ["grok-4"] } }, - onModelSelect: () => {}, - unconnectedProviders: [ - { name: "openai", label: "OpenAI", modelCount: 4, authKind: "key" }, - ], - commands: [], - onCommand: () => {}, - chrome: () => ({ agents: [] }), - subscribeChrome: () => () => {}, - subAgentSessions: () => [], - createRenderer: async () => harness.renderer, - }) - try { - expect(host.openSurface("models")).toBe(true) - expect(host.shell.overlayItems).toContain("OpenAI — connect →") - expect(host.shell.overlayItems.some((i) => i.includes("Go model on Zen path"))).toBe( - false, - ) - } finally { - host.dispose() - harness.destroy() - } - }) - test("refreshModels moves a selected pair into the Recent section", async () => { const harness = await createHarness({ width: 80, height: 24 }) const host = await mountRunnerHost({ @@ -293,10 +262,10 @@ describe("mountRunnerHost model picker", () => { } }) - test("refreshModels swaps in a freshly connected provider and drops its connect row", async () => { + test("refreshModels swaps in a freshly connected provider's models without a remount", async () => { // Mount-time deps are a snapshot; a live provider connect (CL-5602) must be // able to replace them without remounting the host, or the newly connected - // provider's models never appear and its "connect →" row never clears. + // provider's models never appear. const harness = await createHarness({ width: 80, height: 24 }) const host = await mountRunnerHost({ title: "test", @@ -305,9 +274,6 @@ describe("mountRunnerHost model picker", () => { interrupt: () => {}, providers: { xai: { models: ["grok-4"] } }, onModelSelect: () => {}, - unconnectedProviders: [ - { name: "openai", label: "OpenAI", modelCount: 1, authKind: "key" }, - ], commands: [], onCommand: () => {}, chrome: () => ({ agents: [] }), @@ -320,14 +286,12 @@ describe("mountRunnerHost model picker", () => { [], [], { xai: { models: ["grok-4"] }, openai: { models: ["gpt-5"] } }, - [], ) expect(host.openSurface("models")).toBe(true) - // Flat list: connect row is gone; the new provider appears as a leaf - // `provider / model` row, not a nested group to drill into. + // Flat list: the new provider appears as a leaf `provider / model` row, + // not a nested group to drill into. expect(host.shell.overlayItems.some((label) => label.includes("openai"))).toBe(true) expect(host.shell.overlayItems.some((label) => label.includes("gpt-5"))).toBe(true) - expect(host.shell.overlayItems).not.toContain("OpenAI — connect →") } finally { host.dispose() harness.destroy() diff --git a/src/tui/runner-host.ts b/src/tui/runner-host.ts index 9d20a5c84..45b381f55 100644 --- a/src/tui/runner-host.ts +++ b/src/tui/runner-host.ts @@ -25,7 +25,6 @@ import { type ModelCatalogOption, type ModelCatalogProvidersInput, type ModelCatalogRef, - type ModelCatalogUnconnectedProvider, } from "./model-catalog.js" import type { ItemDescription } from "./shell.js" import { @@ -79,8 +78,6 @@ export type RunnerHostDeps = { readonly recentModels?: readonly ModelCatalogRef[] /** Favorited provider+model pairs (settings.favoriteModels). */ readonly favoriteModels?: readonly ModelCatalogRef[] - /** Known providers with no stored credentials yet — rendered as "connect →" rows. */ - readonly unconnectedProviders?: readonly ModelCatalogUnconnectedProvider[] /** * Provider+model the session is actually running, read live on every * picker open. Marks that row "(current)" — independent of recents, which @@ -88,7 +85,7 @@ export type RunnerHostDeps = { */ readonly activeModel?: () => ModelCatalogRef | undefined readonly onModelSelect: (id: string) => void - /** Selecting a "connect →" row; runner owns the actual connect flow. */ + /** Picking a row in the Alt+A add-provider selector; runner owns the connect flow. */ readonly onConnectProvider?: (providerName: string) => void /** `f` on a focused model row; runner owns the favorite persist + refresh. */ readonly onFavoriteToggle?: (id: string) => void @@ -156,15 +153,14 @@ export type RunnerHost = ProductHost & { * push it into the already-open host — the picker's Recent/Favorites * sections would otherwise never reflect a same-session selection. * - * `providers`/`unconnected` default to the values last passed here (or the - * mount-time deps) — pass fresh ones after a live provider connect so a - * newly authorized provider's models appear without a restart. + * `providers` defaults to the value last passed here (or the mount-time + * deps) — pass a fresh one after a live provider connect so a newly + * authorized provider's models appear without a restart. */ readonly refreshModels: ( recentModels: readonly ModelCatalogRef[], favoriteModels: readonly ModelCatalogRef[], providers?: ModelCatalogProvidersInput, - unconnected?: readonly ModelCatalogUnconnectedProvider[], ) => void /** Re-reads `showPromptCost` and cost/context state, repainting the border immediately. */ readonly refreshCostContext: () => void @@ -242,18 +238,13 @@ export async function mountRunnerHost(deps: RunnerHostDeps): Promise // Mutable so a live provider connect (see refreshModels below) can replace // the catalog source without remounting the host. let liveProviders = deps.providers - let liveUnconnected = deps.unconnectedProviders ?? [] let catalog: readonly ModelCatalogOption[] = buildModelsFirstCatalog({ providers: liveProviders, recent: deps.recentModels ?? [], favorites: deps.favoriteModels ?? [], - unconnected: liveUnconnected, }) const describeModel = (itemId: string): ItemDescription | null => - describeModelCatalogOption( - catalog.find((o) => o.id === itemId) ?? { id: itemId, label: itemId }, - { unconnected: liveUnconnected }, - ) + describeModelCatalogOption(catalog.find((o) => o.id === itemId) ?? { id: itemId, label: itemId }) const readModelLabel = deps.modelLabel const onModelSelect = (id: string): void => { deps.onModelSelect(id) @@ -364,15 +355,12 @@ export async function mountRunnerHost(deps: RunnerHostDeps): Promise recentModels: readonly ModelCatalogRef[], favoriteModels: readonly ModelCatalogRef[], providers?: ModelCatalogProvidersInput, - unconnected?: readonly ModelCatalogUnconnectedProvider[], ): void => { if (providers !== undefined) liveProviders = providers - if (unconnected !== undefined) liveUnconnected = unconnected catalog = buildModelsFirstCatalog({ providers: liveProviders, recent: recentModels, favorites: favoriteModels, - unconnected: liveUnconnected, }) host.setModels?.(catalog, describeModel) } diff --git a/src/tui/runner.ts b/src/tui/runner.ts index cb22c99b8..a35955be6 100644 --- a/src/tui/runner.ts +++ b/src/tui/runner.ts @@ -42,7 +42,7 @@ import { type LocalSettings, type PluginConfig, } from "../config/settings.js"; -import { connectedAccountCount, providerChoices, unconnectedProviderChoices } from "./provider-setup.js"; +import { connectedAccountCount, providerChoices } from "./provider-setup.js"; import { connectProviderInline } from "./provider-connect.js"; import { modelOptionId } from "./model-catalog.js"; import type { SessionModeScope } from "./command-surfaces.js"; @@ -2042,21 +2042,10 @@ export async function runTUI(initialConfig: Config): Promise { // Mount OpenTUI before the initial task is sent so gate and stream listeners // are registered first. Ctrl+C stays with the shell (interrupt the run); // OpenTUI owns the alternate screen and mouse reporting itself. - // Providers the picker offers as "connect →" rows: known choices minus - // whatever is already in the live catalog. Recomputed after a live connect - // so a newly authorized provider drops out of this list immediately. - const computeUnconnectedProviders = (providers: Config["providers"]) => - unconnectedProviderChoices(providers).map((choice) => ({ - name: choice.id, - label: choice.label, - modelCount: choice.models.length, - authKind: choice.oauth !== null ? ("oauth" as const) : ("key" as const), - })); - // Alt+A add-provider selector rows: every first-class provider kind, no - // already-connected filtering (unlike the flat list's connect rows above), - // so a second OAuth account is reachable once the first is already - // connected. Read fresh on each open against the live catalog. + // already-connected filtering, so a second OAuth account is reachable once + // the first is already connected. Read fresh on each open against the live + // catalog. const computeAddProviderChoices = () => providerChoices() .filter((choice) => !choice.custom) @@ -2108,7 +2097,6 @@ export async function runTUI(initialConfig: Config): Promise { providers: config.providers, recentModels: listRecentModels(config.settings ?? { providers: {} }), favoriteModels: listFavoriteModels(config.settings ?? { providers: {} }), - unconnectedProviders: computeUnconnectedProviders(config.providers), addProviderChoices: computeAddProviderChoices, onConnectProvider: (providerName) => { void (async () => { @@ -2145,7 +2133,6 @@ export async function runTUI(initialConfig: Config): Promise { listRecentModels(config.settings ?? { providers: {} }), listFavoriteModels(config.settings ?? { providers: {} }), providers, - computeUnconnectedProviders(providers), ); // Reopen positioned at the account just connected — the picker's // default open (top of list) would otherwise leave the operator to From 9765b5928c918e19746d8f629b6ae9d28e9ac1ef Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Mon, 10 Aug 2026 09:44:25 -0700 Subject: [PATCH 5/8] Correct the connect-flow comment to name the Alt+A selector --- src/tui/commands/built-in.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tui/commands/built-in.ts b/src/tui/commands/built-in.ts index fb336099d..ea31c2255 100644 --- a/src/tui/commands/built-in.ts +++ b/src/tui/commands/built-in.ts @@ -51,9 +51,9 @@ export function registerBuiltInCommands(): void { handler: (_args, _ctx) => ({ type: "overlay", overlay: "hooks" }), }); - // Models-first connect: providers are connected from /model (Ctrl+A / c), not a - // standalone /login picker. Keep codex/xai login modals reachable only via - // Connect or re-auth on an expired profile. + // Models-first connect: providers are connected from /model via the Alt+A + // add-provider selector, not a standalone /login picker. The OAuth sign-in + // surface is reachable only through that connect flow. // signalClear rotates to a fresh session: the on-screen transcript and run // telemetry are reset and the agent is rebuilt against a new state directory, From 50d0409589b8b67505984404fdcfc972ba77ff1a Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Mon, 10 Aug 2026 09:53:25 -0700 Subject: [PATCH 6/8] Reuse the live renderer for the inline provider connect surface Choosing a provider from Alt+A crashed with "Cannot create CliRenderer: stdin is already used by another CliRenderer": the inline connect flow asked the setup surface to create its own renderer while the session's TUI still owned stdin. The surface already supports a caller-owned renderer, so the session now lends it its own, suspends the shell's key/paste/submit handling for the surface's lifetime (Ctrl+C during sign-in must cancel the sign-in, not interrupt the running agent), and restores prompt focus after. --- src/tui/runner.ts | 13 +++++++++++++ src/tui/shell.ts | 22 ++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/src/tui/runner.ts b/src/tui/runner.ts index a35955be6..de1ee6220 100644 --- a/src/tui/runner.ts +++ b/src/tui/runner.ts @@ -163,10 +163,12 @@ import { consumeStream } from "../session/stream-consumer.js"; import { createCycleTextRecorder } from "../session/stream-journal.js"; import { mountRunnerHost } from "./runner-host.js"; import { + applyFocus, attachClipboardImage, setMentionSuggestionSource, setPromptRecognitionSource, setSentMessageHistory, + setShellInputSuspended, setShellRunState, surfaceSystemNotice, } from "./shell.js"; @@ -2101,6 +2103,11 @@ export async function runTUI(initialConfig: Config): Promise { onConnectProvider: (providerName) => { void (async () => { let result: Awaited>; + // The setup surface shares the live session's renderer — a second + // CliRenderer cannot exist on the same stdin. Shell input stays + // suspended for the surface's lifetime so its keystrokes (including + // Ctrl+C to cancel the sign-in) never also reach the shell. + setShellInputSuspended(host.shell, true); try { result = await connectProviderInline({ providerId: providerName, @@ -2108,12 +2115,18 @@ export async function runTUI(initialConfig: Config): Promise { localSettingsPath: localSettingsFile, cwd: config.cwd, existing: config.settings ?? null, + createRenderer: () => Promise.resolve(host.renderer), }); } catch (err) { systemNotice( `Connecting ${providerName} failed: ${err instanceof Error ? err.message : String(err)}`, ); return; + } finally { + setShellInputSuspended(host.shell, false); + // The setup surface focused its own input; hand focus back to + // whatever shell zone owned it before the surface mounted. + applyFocus(host.shell); } if (!result.connected) return; diff --git a/src/tui/shell.ts b/src/tui/shell.ts index e9222525c..5665ee4c6 100644 --- a/src/tui/shell.ts +++ b/src/tui/shell.ts @@ -1907,6 +1907,13 @@ type ShellInternals = { overlayOnAction: ((itemId: string, key: KeyEvent) => boolean) | null /** Whether the open primary advertises Alt+A in the footer hints. */ overlayAddProviderHint: boolean + /** + * While true the shell ignores its own key/paste/submit handlers. Set for + * the lifetime of a full-screen surface (inline provider connect) that + * shares this renderer — two live key handlers on one stdin would both + * act on every keystroke. + */ + inputSuspended: boolean /** Per-open free-text answer field, when the overlay opted into one. */ overlayAnswer: OverlayAnswerState | null /** Bare title of the open overlay, so its key hints can be re-composed. */ @@ -2236,6 +2243,17 @@ function evictedRowsNotice(evicted: number): string { * and the plugin producer kept the defect, which is what per-call-site rules * buy you. Reaching for `appendStreamRow` directly is the bug. */ +/** + * Suspend or resume the shell's own key/paste/submit handling. A full-screen + * surface that borrows this renderer (the inline provider connect) owns the + * keyboard for its lifetime; without this, Ctrl+C during a sign-in would + * also reach the shell and interrupt the running agent. + */ +export function setShellInputSuspended(shell: AppShell, suspended: boolean): void { + const bag = internals.get(shell) + if (bag !== undefined) bag.inputSuspended = suspended +} + export function surfaceSystemNotice(shell: AppShell, text: string): void { if (isLanding(shell)) { const bag = internals.get(shell) @@ -5441,11 +5459,13 @@ export function createAppShell( let lastKeyWasPrintable = false let suppressNextLinefeed = false const onPaste = (): void => { + if (internals.get(shell)?.inputSuspended === true) return sawBracketedPaste = true } const onKey = (key: KeyEvent): void => { if (disposed) return + if (internals.get(shell)?.inputSuspended === true) return if (key.name === "escape") { if (exitOverlayAnswerMode(shell)) { @@ -5874,6 +5894,7 @@ export function createAppShell( const onEnter = (): void => { if (disposed || shell.overlayList) return + if (internals.get(shell)?.inputSuspended === true) return // Every mid-run send steers — there is no longer a plain "queue and wait // quietly" gesture distinct from it (that's what collapsed into Alt+Enter // stop-and-reinject instead). Idle sends ignore "kind" entirely. @@ -6026,6 +6047,7 @@ export function createAppShell( overlayDescribe: null, overlayOnAction: null, overlayAddProviderHint: false, + inputSuspended: false, overlayAnswer: null, overlayTitleText: "", overlayOnCancel: null, From 4d40cd36ddb2491d3b6fd1f024b7ad247caf4670 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Mon, 10 Aug 2026 10:10:31 -0700 Subject: [PATCH 7/8] Clear stale float offsets when the overlay host returns to flow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The overlay host floats absolutely over the landing screen, and the un-float path only flipped position back to relative. Yoga treats the leftover top inset as a relative offset, so after any overlay had opened on the landing, every later in-session overlay — the slash popup, the model picker — rendered that many rows below its flow slot, ending up under the prompt and clipped off the screen. --- src/tui/overlay-float-reset.test.ts | 56 +++++++++++++++++++++++++++++ src/tui/shell.ts | 7 ++++ 2 files changed, 63 insertions(+) create mode 100644 src/tui/overlay-float-reset.test.ts diff --git a/src/tui/overlay-float-reset.test.ts b/src/tui/overlay-float-reset.test.ts new file mode 100644 index 000000000..dab43f1ac --- /dev/null +++ b/src/tui/overlay-float-reset.test.ts @@ -0,0 +1,56 @@ +import { expect, test } from "bun:test" + +import { withTestRenderer } from "./harness" +import { appendStreamRow, closeInsetOverlay, createAppShell } from "./shell" +import { openModelPickerOverlay } from "./overlays" +import type { PaletteCommand } from "./command-catalog" + +const CATALOG: readonly PaletteCommand[] = [ + { id: "model", label: "/model" }, + { id: "mcp", label: "/mcp" }, +] + +// Regression: the overlay host floats absolutely over the landing (top set to +// a large row offset) but is an in-flow band once a transcript exists. +// Un-floating used to leave the absolute insets behind, and under relative +// positioning a stale top offsets the band downward — the slash popup rendered +// below the prompt, clipped off the bottom of the screen. +test("slash popup stays above the prompt after a landing-floated overlay", async () => { + await withTestRenderer( + async (h) => { + const shell = createAppShell(h.renderer, { + terminal: { columns: 120, rows: 50 }, + wireKeys: true, + run: "idle", + paletteCatalog: CATALOG, + }) + try { + // Landing: overlay floats absolutely with a large top offset. + openModelPickerOverlay(shell, { items: ["codex/def / gpt-5.6-sol"] }) + await h.renderOnce() + closeInsetOverlay(shell) + await h.renderOnce() + + // Transcript starts; overlays are in-flow bands from here on. + appendStreamRow(shell, { role: "user", text: "hi" }) + appendStreamRow(shell, { role: "assistant", text: "Hi! What can I help you with?" }) + await h.renderOnce() + + h.pressKey("/") + h.pressKey("m") + h.pressKey("o") + await h.renderOnce() + const frame = h.captureCharFrame() + const lines = frame.split("\n") + const popupRow = lines.findIndex((l) => l.includes("/model")) + const promptRow = lines.findIndex((l) => l.includes("/mo") && !l.includes("/model")) + expect(popupRow).toBeGreaterThanOrEqual(0) + expect(promptRow).toBeGreaterThanOrEqual(0) + expect(popupRow).toBeLessThan(promptRow) + } finally { + shell.dispose() + } + }, + { width: 120, height: 50 }, + ) +}) diff --git a/src/tui/shell.ts b/src/tui/shell.ts index 5665ee4c6..024072720 100644 --- a/src/tui/shell.ts +++ b/src/tui/shell.ts @@ -1193,6 +1193,13 @@ function floatOverlayHost( if (!floating) { host.position = "relative" host.zIndex = 0 + // A previous landing float left absolute insets behind. Under relative + // positioning those same values act as offsets from the in-flow slot, so + // a stale top pushes the band that many rows below the prompt — clear + // them so the band sits where the flow put it. + host.top = 0 + host.left = 0 + host.width = "100%" return } host.position = "absolute" From ae4c34963a165fb16645cf17ce4b6af88d65b984 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Mon, 10 Aug 2026 10:14:04 -0700 Subject: [PATCH 8/8] Note the Codex connect fixes in the changelog --- CHANGELOG.md | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f819d50e9..cb01d9c2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,39 @@ matching `## [X.Y.Z]` section (plus install instructions). Do not maintain parallel copies under `docs/` or `scripts/notes/`. At cut time: rename `## [Unreleased]` to `## [X.Y.Z] - YYYY-MM-DD`, then run the release script. +## [Unreleased] + +Codex connect works again: streaming responses no longer die on a missing +header, multiple ChatGPT accounts can be connected by name, and providers are +added from the model picker with Alt+A. + +### Providers + +- **Codex streaming repaired.** Some Codex models (the gpt-5.6 family) stream + valid responses with no Content-Type header, which failed every turn with + "Cannot detect response kind". The response protocol is now recovered from + what the request asked for, so those models work; genuinely malformed + responses still fail loudly. +- **Named accounts with re-auth.** Browser sign-in asks for an account name + first, so any number of ChatGPT or Grok accounts can be connected side by + side (`codex/work`, `codex/personal`, …). Reusing an existing name + re-authorizes that account after an explicit confirmation — the recovery + path for expired sign-ins. Second sign-ins can no longer silently overwrite + an existing account's credentials. + +### TUI + +- **Alt+A adds providers.** The model picker lists only connected accounts + and their models; Alt+A opens an add-provider selector that always shows + every provider with its connected-account count, so adding a second account + is never blocked. After connecting, the picker reopens focused on the new + account. +- **Connect works mid-session.** Adding a provider from a running session no + longer crashes with a renderer conflict; the sign-in surface shares the + session's screen and hands control back when done. +- **Pickers stay on screen.** Overlays opened after using one on the launch + screen no longer render below the prompt box. + ## [0.2.96] - 2026-09-08 Drag-select auto-copy, a flat type-to-filter model picker, install-aware upgrade