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 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/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/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, 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/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/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..976bfffa3 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, @@ -87,13 +88,22 @@ 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. */ +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 = { @@ -122,13 +132,19 @@ 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 + * 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,44 +520,88 @@ 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, // 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, - ...(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) return false + onFavoriteToggle(itemId) + return true + } + return false }, } : {}), 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-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..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, @@ -19,10 +19,12 @@ import { stepHeadline, stepReady, stepsFor, + suggestOAuthProfileSlug, summaryRows, TYPE_MODEL_ID, - unconnectedProviderChoices, + validateOAuthProfileSlug, type OAuthLoginStarter, + type OAuthProfileLister, type ProviderFormValues, type ProviderSetupSubmit, type SubmitOpts, @@ -33,6 +35,7 @@ const EMPTY: ProviderFormValues = { baseURL: "", apiKey: "", model: "", + oauthProfile: "", } describe("provider setup pure helpers", () => { @@ -75,13 +78,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 +99,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) @@ -113,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", () => { @@ -149,6 +177,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 +286,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 +302,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 +311,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 +376,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 +401,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 +551,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 +577,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 +588,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 +606,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 +618,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 +661,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 +708,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 +739,7 @@ describe("runProviderSetup", () => { baseURL: "https://api.example.com", apiKey: "sk-key", model: "fp-small", + oauthProfile: "", }) expect(opts[0]?.preset).toBeUndefined() }) @@ -657,7 +895,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..fb24f50a9 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, @@ -278,28 +366,21 @@ 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 isChoiceConnected( +export function connectedAccountCount( choice: ProviderChoice, providers: readonly { readonly name: string }[], -): boolean { - return providers.some( +): number { + return providers.filter( (p) => p.name === choice.id || (choice.oauth !== null && p.name.startsWith(`${choice.id}/`)), - ) + ).length } -/** 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( @@ -393,14 +474,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 +508,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 +525,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,12 +660,14 @@ 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 /** * 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 } @@ -626,6 +717,7 @@ export async function runProviderSetup( baseURL: "", apiKey: "", model: "", + oauthProfile: "", } let choice: ProviderChoice | null = null let stepIndex = 0 @@ -638,6 +730,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 +746,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 +800,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 +1059,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 +1093,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 +1157,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 +1182,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 +1194,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 +1342,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 +1432,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 +1519,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 +1536,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 +1598,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 +1648,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()) { diff --git a/src/tui/runner-host.test.ts b/src/tui/runner-host.test.ts index d9ce3e378..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() @@ -364,6 +328,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..45b381f55 100644 --- a/src/tui/runner-host.ts +++ b/src/tui/runner-host.ts @@ -25,10 +25,13 @@ import { type ModelCatalogOption, type ModelCatalogProvidersInput, type ModelCatalogRef, - 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, @@ -75,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 @@ -84,10 +85,15 @@ 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 + /** + * 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`. */ @@ -147,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 @@ -233,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) @@ -267,6 +267,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?.() @@ -352,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 bc6bc6f4b..de1ee6220 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 } 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"; @@ -162,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"; @@ -2041,16 +2044,19 @@ 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, 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. @@ -2093,10 +2099,15 @@ 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 () => { 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, @@ -2104,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; @@ -2129,9 +2146,15 @@ export async function runTUI(initialConfig: Config): Promise { listRecentModels(config.settings ?? { providers: {} }), listFavoriteModels(config.settings ?? { providers: {} }), 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..024072720 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" @@ -1192,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" @@ -1300,12 +1308,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 +1885,7 @@ type PriorOverlaySnapshot = { readonly answer: OverlayAnswerState | null readonly titleText: string readonly onCancel: (() => void) | null + readonly addProviderHint: boolean } type ShellInternals = { @@ -1890,6 +1912,15 @@ 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 + /** + * 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. */ @@ -2219,6 +2250,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) @@ -3416,6 +3458,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 +3502,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 +3533,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 +3555,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 +3970,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 +3998,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 +4032,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, { @@ -5411,11 +5466,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)) { @@ -5844,6 +5901,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. @@ -5995,6 +6053,8 @@ export function createAppShell( overlayOnCycle: null, overlayDescribe: null, overlayOnAction: null, + overlayAddProviderHint: false, + inputSuspended: false, overlayAnswer: null, overlayTitleText: "", overlayOnCancel: null, 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); + }); +});