diff --git a/src/config/index.ts b/src/config/index.ts index cc78dcf9a..492cd3d83 100644 --- a/src/config/index.ts +++ b/src/config/index.ts @@ -52,8 +52,8 @@ import { globalSettingsPath, loadLocalSettingsResult, type SettingsLoadDiagnostic, - loadSettings, - localSettingsPath, + loadSettingsRecoveringClobberedOAuthSelection, + resolveLocalSettingsPath, normalizeOpenAICompatibleBaseURL, resolveProvider, type MCPServerSettingsEntry, @@ -80,6 +80,51 @@ export const SOURCE_MAX_TOKENS = 16384; // keyless servers ignore it entirely. export const KEYLESS_API_KEY = "keyless"; +function applyPersistedOAuthDefaults( + settings: Settings | null, + projected: Record, +): Record { + const merged: Record = {}; + for (const [name, provider] of Object.entries(projected)) { + const defaultModel = settings?.providers[name]?.defaultModel; + merged[name] = + defaultModel !== undefined && defaultModel.length > 0 + ? { + ...provider, + models: provider.models.includes(defaultModel) + ? provider.models + : [defaultModel, ...provider.models], + defaultModel, + } + : provider; + } + return merged; +} + +// OAuth entries in settings.json carry no credentials; they are only usable +// while a matching auth-store profile exists. Drop orphans in memory so a +// removed profile does not pin resolution to an unauthenticatable provider. +function dropOrphanedOAuthEntries( + settings: Settings | null, + projected: Record, +): Settings | null { + if (settings === null) return null; + const providers = Object.fromEntries( + Object.entries(settings.providers).filter( + ([name]) => + (!isCodexProviderName(name) && !isXaiProviderName(name)) || projected[name] !== undefined, + ), + ); + const { defaultProvider, ...rest } = settings; + return { + ...rest, + providers, + ...(defaultProvider !== undefined && providers[defaultProvider] !== undefined + ? { defaultProvider } + : {}), + }; +} + function hasExaEntry(servers: MCPServerSettingsEntry[] | undefined): boolean { return servers?.some((server) => server.name === EXA_MCP_SERVER_NAME) === true; } @@ -655,21 +700,10 @@ export async function loadConfig( await bootstrapPricingMetadata({ cachePath: pricingCachePath, ...options.pricing }); - const settings = - configPath !== undefined - ? await loadSettings(configPath).then((s) => { - if (s === null) throw new Error(`--config file not found or empty: ${configPath}`); - return s; - }) - : await loadSettings(options.globalSettingsPath ?? globalSettingsPath()); - - // Track whether the effective value came from the persisted global default - // rather than this invocation's --dangerously-skip-permissions flag, so the - // TUI/exec entry points can surface a startup notice for the silent case. - const skipPermissionsFromSettings = - !dangerouslySkipPermissions && settings?.dangerouslySkipPermissions === true; - dangerouslySkipPermissions = - dangerouslySkipPermissions || settings?.dangerouslySkipPermissions === true; + // Resolve both settings targets from the same effective global path. The + // local schema must never be read from or written to that global target. + const effectiveSettingsPath = configPath ?? options.globalSettingsPath ?? globalSettingsPath(); + const localSettingsFile = resolveLocalSettingsPath(cwd, effectiveSettingsPath); // OAuth profiles live in home-level auth stores (~/.corbits/codex-auth.json, // xai-auth.json), entirely separate from settings.json. --config only @@ -683,22 +717,51 @@ export async function loadConfig( const [codexProfiles, xaiProfiles]: [CodexProfile[], XaiProfile[]] = useOAuthProfiles ? await Promise.all([listCodexProfiles(), listXaiProfiles()]) : [[], []]; - const codexProviderSettings = codexProvidersAsSettings(codexProfiles); - const xaiProviderSettings = xaiProvidersAsSettings(xaiProfiles); - const oauthProviderSettings = { ...codexProviderSettings, ...xaiProviderSettings }; + let projectedOAuthProviders = { + ...codexProvidersAsSettings(codexProfiles), + ...xaiProvidersAsSettings(xaiProfiles), + }; + const settings = + configPath !== undefined + ? await loadSettingsRecoveringClobberedOAuthSelection(configPath, projectedOAuthProviders, { + persist: false, + }).then((s) => { + if (s === null) throw new Error(`--config file not found or empty: ${configPath}`); + return s; + }) + : await loadSettingsRecoveringClobberedOAuthSelection( + effectiveSettingsPath, + projectedOAuthProviders, + { persist: true }, + ); + + // Track whether the effective value came from the persisted global default + // rather than this invocation's --dangerously-skip-permissions flag, so the + // TUI/exec entry points can surface a startup notice for the silent case. + const skipPermissionsFromSettings = + !dangerouslySkipPermissions && settings?.dangerouslySkipPermissions === true; + dangerouslySkipPermissions = + dangerouslySkipPermissions || settings?.dangerouslySkipPermissions === true; + projectedOAuthProviders = applyPersistedOAuthDefaults(settings, projectedOAuthProviders); + const liveSettings = useOAuthProfiles + ? dropOrphanedOAuthEntries(settings, projectedOAuthProviders) + : settings; const settingsForResolution: Settings | null = - Object.keys(oauthProviderSettings).length > 0 + Object.keys(projectedOAuthProviders).length > 0 ? { - ...(settings ?? { providers: {} }), - providers: { ...(settings?.providers ?? {}), ...oauthProviderSettings }, + ...(liveSettings ?? { providers: {} }), + providers: { ...(liveSettings?.providers ?? {}), ...projectedOAuthProviders }, } - : settings; + : liveSettings; // The per-repo selection file still applies on top of a --config source: that // file supplies provider definitions, while .corbits/settings.json supplies // the provider/model selection. CLI --provider/--model override both. // Fail open on unknown/invalid local keys — never crash startup. - const localResult = await loadLocalSettingsResult(localSettingsPath(cwd)); + const localResult = + localSettingsFile === null + ? { settings: null, diagnostics: [] } + : await loadLocalSettingsResult(localSettingsFile); const local = localResult.settings; const settingsDiagnostics = localResult.diagnostics; @@ -717,10 +780,6 @@ export async function loadConfig( if (provider !== undefined) cli.provider = provider; if (model !== undefined) cli.model = model; - // When --config is given, onboarding must write to and reload from that - // same file, not the global default. Prefer configPath, then the caller - // override, then the real global default. - const effectiveSettingsPath = configPath ?? options.globalSettingsPath ?? globalSettingsPath(); const task = positional.join(" ").trim(); let resolved: ResolvedProvider; diff --git a/src/config/settings.ts b/src/config/settings.ts index 775dd42c7..0e593af32 100644 --- a/src/config/settings.ts +++ b/src/config/settings.ts @@ -1,7 +1,8 @@ import { randomUUID } from "node:crypto"; +import { realpathSync } from "node:fs"; import { mkdir, readFile, rename, writeFile } from "node:fs/promises"; import { homedir } from "node:os"; -import { dirname, join } from "node:path"; +import { basename, dirname, join, resolve } from "node:path"; import { type } from "arktype"; @@ -212,20 +213,44 @@ export function toggleFavoriteModel(settings: Settings, ref: ModelRef): Settings }; } -export function setDefaultModel(settings: Settings, ref: ModelRef): Settings { +function providerSelectionMetadata(provider: ProviderSettings, model: string): ProviderSettings { + return { + ...(provider.name !== undefined ? { name: provider.name } : {}), + baseURL: provider.baseURL, + models: [model], + defaultModel: model, + ...(provider.keyless === true ? { keyless: true } : {}), + ...(provider.free === true ? { free: true } : {}), + ...(provider.contextWindow !== undefined ? { contextWindow: provider.contextWindow } : {}), + ...(provider.bifrostVirtualKey === true ? { bifrostVirtualKey: true } : {}), + ...(provider.anthropic === true ? { anthropic: true } : {}), + ...(provider.opencodeGo === true ? { opencodeGo: true } : {}), + ...(provider.verified !== undefined ? { verified: provider.verified } : {}), + }; +} + +export function setDefaultModel( + settings: Settings, + ref: ModelRef, + projectedProvider?: ProviderSettings, +): Settings { const next: ModelRef = { provider: ref.provider, model: ref.model }; const existing = settings.providers[next.provider]; + const provider = existing ?? projectedProvider; + if (provider === undefined) { + return { ...settings, defaultProvider: next.provider }; + } + const persistedProvider = + existing !== undefined + ? { ...existing, defaultModel: next.model } + : providerSelectionMetadata(provider, next.model); return { ...settings, defaultProvider: next.provider, - ...(existing !== undefined - ? { - providers: { - ...settings.providers, - [next.provider]: { ...existing, defaultModel: next.model }, - }, - } - : {}), + providers: { + ...settings.providers, + [next.provider]: persistedProvider, + }, }; } @@ -372,6 +397,40 @@ export function localSettingsPath(cwd: string): string { return join(cwd, SETTINGS_DIR_NAME, "settings.json"); } +function physicalPathIdentity(path: string): string { + let candidate = resolve(path); + const missingSegments: string[] = []; + + while (true) { + try { + return join(realpathSync.native(candidate), ...missingSegments.reverse()); + } catch (err) { + // Anything but a missing segment (ENOTDIR, EACCES, ...) is not aliasable; + // fall back to the lexical path so the fail-open loader sees it. + if (!isENOENT(err)) return resolve(path); + const parent = dirname(candidate); + if (parent === candidate) return resolve(path); + missingSegments.push(basename(candidate)); + candidate = parent; + } + } +} + +export function resolveLocalSettingsPath(cwd: string, globalPath: string): string | null { + const localPath = localSettingsPath(cwd); + return physicalPathIdentity(localPath) === physicalPathIdentity(globalPath) ? null : localPath; +} + +// True when `settingsPath` is a distinct settings file from the default home +// path. Symlink and lexical aliases of the default path are not overrides — +// treating them as such would suppress OAuth profile projection after setup. +export function isProgrammaticSettingsOverride( + settingsPath: string, + defaultGlobalPath: string = globalSettingsPath(), +): boolean { + return physicalPathIdentity(settingsPath) !== physicalPathIdentity(defaultGlobalPath); +} + function isENOENT(err: unknown): boolean { return ( typeof err === "object" && @@ -688,7 +747,33 @@ export function healOpenCodeGoProviders(settings: Settings): string[] { return healed; } -export async function loadSettings(path: string): Promise { +const ClobberedLocalSelectionSchema = type({ + provider: "string>0", + model: "string>0", + "+": "reject", +}); + +function isClobberedLocalSelection(value: unknown): value is { provider: string; model: string } { + return ClobberedLocalSelectionSchema.allows(value); +} + +function recoverClobberedOAuthSelection( + selection: { provider: string; model: string }, + projected: Record, +): Settings | undefined { + const provider = projected[selection.provider]; + // Auth-profile presence is enough: the selected model may be outside the + // projected fallback catalog (CODEX_DEFAULT_MODELS / xAI equivalents). + if (provider === undefined) return undefined; + return { + defaultProvider: selection.provider, + providers: { + [selection.provider]: providerSelectionMetadata(provider, selection.model), + }, + }; +} + +async function loadSettingsJSON(path: string): Promise { let raw: string; try { raw = await readFile(path, "utf8"); @@ -696,16 +781,22 @@ export async function loadSettings(path: string): Promise { if (isENOENT(err)) return null; throw err; } - let parsed: unknown; try { - parsed = JSON.parse(raw); + return JSON.parse(raw); } catch { throw new Error(`Invalid JSON in settings file: ${path}`); } +} + +function settingsSchemaError(path: string): Error { + return new Error( + `Invalid settings schema in ${path}: expected { providers: { : { baseURL, apiKey, models: [...] } } }`, + ); +} + +function normalizeParsedSettings(path: string, parsed: unknown): Settings { if (!isSettings(parsed)) { - throw new Error( - `Invalid settings schema in ${path}: expected { providers: { : { baseURL, apiKey, models: [...] } } }`, - ); + throw settingsSchemaError(path); } const s = parsed as unknown as Record; // These keys were removed when plugins moved to discovery; they are now @@ -759,10 +850,14 @@ export async function loadSettings(path: string): Promise { ? Boolean(s.dangerouslySkipPermissions) : undefined, }; - const settings: Settings = { + return { providers: s.providers as Settings["providers"], ...pickDefined(optional), }; +} + +async function loadStrictSettings(path: string, parsed: unknown): Promise { + const settings = normalizeParsedSettings(path, parsed); // Hard cutover: pin Go flag + canonical baseURL on disk when any Go signal matches. // Only rewrite disk when heal actually mutates (no write-on-read for no-op reloads). // Fail open on save: keep the in-memory heal so startup is not bricked by a @@ -783,6 +878,27 @@ export async function loadSettings(path: string): Promise { return settings; } +export async function loadSettings(path: string): Promise { + const parsed = await loadSettingsJSON(path); + return parsed === null ? null : await loadStrictSettings(path, parsed); +} + +export async function loadSettingsRecoveringClobberedOAuthSelection( + path: string, + recoverableOAuthProviders: Record, + options: { persist: boolean }, +): Promise { + const parsed = await loadSettingsJSON(path); + if (parsed === null) return null; + if (isClobberedLocalSelection(parsed)) { + const recovered = recoverClobberedOAuthSelection(parsed, recoverableOAuthProviders); + if (recovered === undefined) throw settingsSchemaError(path); + if (options.persist) await saveGlobalSettings(path, recovered); + return recovered; + } + return loadStrictSettings(path, parsed); +} + /** Diagnostic produced when settings fail open instead of crashing startup. */ export interface SettingsLoadDiagnostic { path: string; diff --git a/src/exec/runner.ts b/src/exec/runner.ts index cd6c21d06..908209084 100644 --- a/src/exec/runner.ts +++ b/src/exec/runner.ts @@ -21,7 +21,7 @@ import { } from "../config/index.js"; import { loadLocalSettings, - localSettingsPath, + resolveLocalSettingsPath, shellTimeoutFromSettings, toolWatchdogFromSettings, } from "../config/settings.js"; @@ -341,14 +341,19 @@ export async function runExec(config: Config): Promise { const liveAgentProfiles = await loadAgentProfiles(profilesDir); // loadLocalSettings maps ENOENT → null; a throw is real I/O or schema failure. - const localSettingsForMode = await loadLocalSettings(localSettingsPath(config.cwd)).catch( - (err: unknown) => { - logger.warn("Failed to load local settings: {error}", { - error: formatCaughtError(err), - }); - return null; - }, + const localSettingsForModePath = resolveLocalSettingsPath( + config.cwd, + config.globalSettingsPath, ); + const localSettingsForMode = + localSettingsForModePath === null + ? null + : await loadLocalSettings(localSettingsForModePath).catch((err: unknown) => { + logger.warn("Failed to load local settings: {error}", { + error: formatCaughtError(err), + }); + return null; + }); const sessionMode: SessionMode = resolveSessionMode(config.settings, localSettingsForMode) ?? "orchestrator"; diff --git a/src/settings.test.ts b/src/settings.test.ts index d4a57f55b..557842731 100644 --- a/src/settings.test.ts +++ b/src/settings.test.ts @@ -11,10 +11,12 @@ import { loadLocalSettings, loadLocalSettingsWriteBase, loadSettings, + loadSettingsRecoveringClobberedOAuthSelection, normalizeOpenAICompatibleBaseURL, resolveProvider, saveGlobalSettings, saveLocalSettings, + type ProviderSettings, type Settings, toolWatchdogFromSettings, loadGlobalSettingsWriteBase, @@ -593,6 +595,172 @@ describe("loaders", () => { } }); + test("loadSettings keeps local selection recovery out of the strict loader", async () => { + const dir = await mkdtemp(join(tmpdir(), "ic-settings-")); + try { + const path = join(dir, "settings.json"); + await writeFile(path, JSON.stringify({ provider: "codex/work", model: "gpt-5.1-codex" })); + await expect(loadSettings(path)).rejects.toThrow(/Invalid settings schema/); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + test.each([ + ["one profile", ["codex/work"]], + ["multiple profiles", ["codex/personal", "codex/work"]], + ])( + "loadSettingsRecoveringClobberedOAuthSelection recovers an exact OAuth selection with %s", + async (_name, providerNames) => { + const dir = await mkdtemp(join(tmpdir(), "ic-settings-")); + try { + const path = join(dir, "settings.json"); + await writeFile(path, JSON.stringify({ provider: "codex/work", model: "gpt-5.1-codex" })); + const projected = Object.fromEntries( + providerNames.map((name) => [ + name, + { + baseURL: "https://chatgpt.com/backend-api", + apiKey: "oauth-token", + models: ["gpt-5.2-codex", "gpt-5.1-codex"], + defaultModel: "gpt-5.2-codex", + } satisfies ProviderSettings, + ]), + ); + + const recovered = await loadSettingsRecoveringClobberedOAuthSelection(path, projected, { + persist: true, + }); + expect(recovered).toEqual({ + defaultProvider: "codex/work", + providers: { + "codex/work": { + baseURL: "https://chatgpt.com/backend-api", + models: ["gpt-5.1-codex"], + defaultModel: "gpt-5.1-codex", + }, + }, + }); + expect(JSON.stringify(recovered)).not.toContain("oauth-token"); + expect(await loadSettings(path)).toEqual(recovered); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }, + ); + + test("loadSettingsRecoveringClobberedOAuthSelection recovers a non-catalog OAuth model when the auth profile exists", async () => { + const dir = await mkdtemp(join(tmpdir(), "ic-settings-")); + try { + const path = join(dir, "settings.json"); + await writeFile( + path, + JSON.stringify({ provider: "codex/work", model: "gpt-special-custom" }), + ); + const recovered = await loadSettingsRecoveringClobberedOAuthSelection( + path, + { + "codex/work": { + baseURL: "https://chatgpt.com/backend-api", + apiKey: "oauth-token", + models: ["gpt-5.2-codex", "gpt-5.1-codex"], + defaultModel: "gpt-5.2-codex", + }, + }, + { persist: true }, + ); + expect(recovered).toEqual({ + defaultProvider: "codex/work", + providers: { + "codex/work": { + baseURL: "https://chatgpt.com/backend-api", + models: ["gpt-special-custom"], + defaultModel: "gpt-special-custom", + }, + }, + }); + expect(JSON.stringify(recovered)).not.toContain("oauth-token"); + expect(await loadSettings(path)).toEqual(recovered); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + test("loadSettings keeps malformed clobber documents strict", async () => { + const dir = await mkdtemp(join(tmpdir(), "ic-settings-")); + try { + const path = join(dir, "settings.json"); + await writeFile( + path, + JSON.stringify({ provider: "codex/work", model: "gpt-5.1-codex", apiKey: "nope" }), + ); + await expect( + loadSettingsRecoveringClobberedOAuthSelection( + path, + { + "codex/work": { + baseURL: "https://chatgpt.com/backend-api", + apiKey: "oauth-token", + models: ["gpt-5.1-codex"], + }, + }, + { persist: true }, + ), + ).rejects.toThrow(/Invalid settings schema/); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + test("loadSettings fails closed on unmatched OAuth selections without touching the file", async () => { + const dir = await mkdtemp(join(tmpdir(), "ic-settings-")); + try { + const path = join(dir, "settings.json"); + const original = JSON.stringify({ provider: "codex/missing", model: "gpt-5.1-codex" }); + await writeFile(path, original); + await expect( + loadSettingsRecoveringClobberedOAuthSelection( + path, + { + "codex/work": { + baseURL: "https://chatgpt.com/backend-api", + apiKey: "oauth-token", + models: ["gpt-5.1-codex"], + }, + }, + { persist: true }, + ), + ).rejects.toThrow(/Invalid settings schema/); + expect(await readFile(path, "utf8")).toBe(original); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + test("loadSettingsRecoveringClobberedOAuthSelection leaves the file unchanged when persist is false", async () => { + const dir = await mkdtemp(join(tmpdir(), "ic-settings-")); + try { + const path = join(dir, "settings.json"); + const original = JSON.stringify({ provider: "codex/work", model: "gpt-5.1-codex" }); + await writeFile(path, original); + const recovered = await loadSettingsRecoveringClobberedOAuthSelection( + path, + { + "codex/work": { + baseURL: "https://chatgpt.com/backend-api", + apiKey: "oauth-token", + models: ["gpt-5.1-codex"], + }, + }, + { persist: false }, + ); + expect(recovered?.defaultProvider).toBe("codex/work"); + expect(await readFile(path, "utf8")).toBe(original); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + test("loadSettings preserves bifrostVirtualKey and agentModelFallback", async () => { const dir = await mkdtemp(join(tmpdir(), "ic-settings-")); try { @@ -1138,6 +1306,37 @@ describe("recent and favorite model helpers", () => { expect(next.providers.missing).toBeUndefined(); }); + test("setDefaultModel persists credential-free projected OAuth metadata", async () => { + const dir = await mkdtemp(join(tmpdir(), "ic-settings-")); + try { + const path = join(dir, "settings.json"); + const projected: ProviderSettings = { + baseURL: "https://chatgpt.com/backend-api", + apiKey: "oauth-token", + models: ["gpt-5.2-codex", "gpt-5.1-codex"], + }; + const next = setDefaultModel( + { providers: {} }, + { provider: "codex/work", model: "gpt-5.1-codex" }, + projected, + ); + await saveGlobalSettings(path, next); + + expect(await loadSettings(path)).toEqual({ + defaultProvider: "codex/work", + providers: { + "codex/work": { + baseURL: projected.baseURL, + models: ["gpt-5.1-codex"], + defaultModel: "gpt-5.1-codex", + }, + }, + }); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + test("listRecentModels respects max (default 5)", () => { const recent = Array.from({ length: 8 }, (_, i) => ({ provider: "a", diff --git a/src/tui/onboarding.test.ts b/src/tui/onboarding.test.ts index af7435cb3..ede98ea65 100644 --- a/src/tui/onboarding.test.ts +++ b/src/tui/onboarding.test.ts @@ -119,7 +119,10 @@ describe("runOnboarding settings source", () => { providers?: Record; }; expect(persisted.defaultProvider).toBe("xai/work"); - expect(persisted.providers).toEqual({}); + expect(persisted.providers).toEqual({ + "xai/work": { baseURL: "https://api.x.ai/v1", models: ["grok-4"], defaultModel: "grok-4" }, + }); + expect(JSON.stringify(persisted)).not.toContain("apiKey"); } finally { await rm(testHome, { recursive: true, force: true }); await rm(cwd, { recursive: true, force: true }); diff --git a/src/tui/onboarding.ts b/src/tui/onboarding.ts index 9d3416672..47b8d4aa2 100644 --- a/src/tui/onboarding.ts +++ b/src/tui/onboarding.ts @@ -1,7 +1,7 @@ import { runTUI } from "./runner.js"; import { buildProviderSubmitHandler } from "./provider-setup-submit.js"; import { loadConfig, type UnconfiguredConfig } from "../config/index.js"; -import { globalSettingsPath, loadSettings, localSettingsPath } from "../config/settings.js"; +import { globalSettingsPath, loadSettings, resolveLocalSettingsPath } from "../config/settings.js"; import { activateHeldTelemetry, telemetryFirstRunPending } from "../telemetry/first-run.js"; import { runProviderSetup } from "./provider-setup.js"; @@ -20,7 +20,11 @@ export async function runOnboarding(config: UnconfiguredConfig): Promise const submitted = await runProviderSetup({ showTelemetryNotice, existingProviderNames: Object.keys(existing?.providers ?? {}), - onSubmit: buildProviderSubmitHandler(settingsPath, existing, localSettingsPath(config.cwd)), + onSubmit: buildProviderSubmitHandler( + settingsPath, + existing, + resolveLocalSettingsPath(config.cwd, settingsPath), + ), }); // If the user cancelled (Ctrl+C) onSubmit was never called and settings were diff --git a/src/tui/provider-connect.ts b/src/tui/provider-connect.ts index afe0ec3c0..2cda8b5a9 100644 --- a/src/tui/provider-connect.ts +++ b/src/tui/provider-connect.ts @@ -12,8 +12,8 @@ import { runProviderSetup, type ProviderSetupConfig } from "./provider-setup.js" export interface ConnectProviderInput { readonly providerId: string; readonly settingsPath: string; - /** Project-local selection file; written after a successful connect. */ - readonly localSettingsPath: string; + /** Project-local selection file, or null when it aliases global settings. */ + readonly localSettingsPath: string | null; readonly existing: Settings | null; readonly createRenderer?: ProviderSetupConfig["createRenderer"]; readonly startLogin?: ProviderSetupConfig["startLogin"]; diff --git a/src/tui/provider-setup-submit.test.ts b/src/tui/provider-setup-submit.test.ts index db51e9bc7..4786aeb0d 100644 --- a/src/tui/provider-setup-submit.test.ts +++ b/src/tui/provider-setup-submit.test.ts @@ -19,7 +19,7 @@ await withMockedModule( ); const { buildProviderSubmitHandler } = await import("./provider-setup-submit.js"); -const { loadLocalSettings, loadSettings, localSettingsPath } = +const { loadLocalSettings, loadSettings, localSettingsPath, resolveLocalSettingsPath } = await import("../config/settings.js"); import type { ProviderFormValues, SubmitPhase } from "./provider-setup.js"; @@ -98,6 +98,79 @@ describe("buildProviderSubmitHandler", () => { }); }); + test.each([ + { + name: "API-key preset", + values: { + name: "openai", + baseURL: "https://api.openai.com/v1", + apiKey: "sk-test-fake", + model: "gpt-5", + oauthProfile: "", + }, + options: { + skipValidation: true, + preset: { id: "openai", models: ["gpt-5"], anthropic: false, opencodeGo: false }, + }, + provider: "openai", + }, + { + name: "custom provider", + values: { + name: "ollama", + baseURL: "http://localhost:11434/v1", + apiKey: "", + model: "llama3", + oauthProfile: "", + }, + options: { skipValidation: true }, + provider: "ollama", + }, + { + name: "OAuth provider", + values: { + name: "", + baseURL: "https://chatgpt.com/backend-api", + apiKey: "", + model: "gpt-5", + oauthProfile: "work", + }, + options: { + skipValidation: true, + oauth: { kind: "codex" as const, providerName: "codex/work", profile: "work" }, + }, + provider: "codex/work", + }, + ])("$name setup preserves global settings when local path aliases it", async (testCase) => { + await withTempDir(async (home) => { + const settingsPath = localSettingsPath(home); + const localTarget = resolveLocalSettingsPath(home, settingsPath); + const existing = { + defaultProvider: "existing", + providers: { + existing: { + baseURL: "https://example.test/v1", + apiKey: "existing-key", + models: ["existing-model"], + }, + }, + }; + const submit = buildProviderSubmitHandler(settingsPath, existing, localTarget); + + await submit(testCase.values, noopSetPhase, testCase.options); + + const settings = await loadSettings(settingsPath); + expect(settings?.defaultProvider).toBe(testCase.provider); + expect(settings?.providers.existing?.apiKey).toBe("existing-key"); + if (testCase.provider === "codex/work") { + expect(settings?.providers[testCase.provider]?.defaultModel).toBe(testCase.values.model); + expect(settings?.providers[testCase.provider]?.apiKey).toBeUndefined(); + } else { + expect(settings?.providers[testCase.provider]).toBeDefined(); + } + }); + }); + test("API-key connect persists project-local selection like OAuth", async () => { // CL-5900: API-key path must write the same local selection OAuth writes, // so a restart in this repo resolves to the connected provider/model. diff --git a/src/tui/provider-setup-submit.ts b/src/tui/provider-setup-submit.ts index 4d8e409b1..d8a9d5425 100644 --- a/src/tui/provider-setup-submit.ts +++ b/src/tui/provider-setup-submit.ts @@ -15,10 +15,11 @@ import type { ProviderSetupSubmit } from "./provider-setup.js"; * local selection only (never secrets). */ export async function persistConnectedSelection( - localSettingsFile: string, + localSettingsFile: string | null, provider: string, model: string, ): Promise { + if (localSettingsFile === null) return; await saveLocalSettings(localSettingsFile, { provider, model, @@ -38,7 +39,7 @@ export async function persistConnectedSelection( export function buildProviderSubmitHandler( settingsPath: string, existing: Settings | null, - localSettingsFile: string, + localSettingsFile: string | null, ): ProviderSetupSubmit { return async (values, setPhase, { skipValidation, preset, oauth }) => { const { name, baseURL, apiKey, model } = values; @@ -49,8 +50,9 @@ export function buildProviderSubmitHandler( // A signed-in subscription provider has no key to test or store: the // tokens are already in the home-level auth store, and config load - // projects that store into the provider catalog. Only the selection is - // persisted here — the same two files /model writes when switching. + // projects that store into the provider catalog. Persist only non-secret + // provider/model metadata globally so the selection survives when a local + // settings target would alias this file. // // Unlike a pasted key, this credential was just issued by the real // provider's own OAuth server completing a PKCE round-trip — so the @@ -74,6 +76,14 @@ export function buildProviderSubmitHandler( await saveGlobalSettings(settingsPath, { ...base, defaultProvider: oauth.providerName, + providers: { + ...base.providers, + [oauth.providerName]: { + baseURL: trimmedBaseURL, + models: [selectedModel], + defaultModel: selectedModel, + }, + }, }); await persistConnectedSelection(localSettingsFile, oauth.providerName, selectedModel); return; diff --git a/src/tui/runner-exit-code.test.ts b/src/tui/runner-exit-code.test.ts index 417ea6d8f..c78d267af 100644 --- a/src/tui/runner-exit-code.test.ts +++ b/src/tui/runner-exit-code.test.ts @@ -1,4 +1,5 @@ import { describe, test, expect } from "bun:test"; +import { resolveLocalSettingsPath } from "../config/settings.js"; import { resolveExitCode } from "./runner.js"; describe("resolveExitCode", () => { @@ -65,3 +66,15 @@ describe("resolveExitCode", () => { expect(code).toBe(0); }); }); + +describe("resolveLocalSettingsPath", () => { + test("treats an aliased --config path as the global settings target", () => { + expect(resolveLocalSettingsPath("/repo", "/repo/.corbits/settings.json")).toBeNull(); + }); + + test("preserves the normal distinct global and project settings paths", () => { + expect(resolveLocalSettingsPath("/tmp/repo", "/tmp/home/user/.corbits/settings.json")).toBe( + "/tmp/repo/.corbits/settings.json", + ); + }); +}); diff --git a/src/tui/runner.ts b/src/tui/runner.ts index 16f343442..b2f8178b0 100644 --- a/src/tui/runner.ts +++ b/src/tui/runner.ts @@ -30,7 +30,7 @@ import { listFavoriteModels, listRecentModels, loadSettings, - localSettingsPath, + resolveLocalSettingsPath, markTelemetryNoticeShown, persistSkipPermissionsDefault, pushRecentModel, @@ -1239,9 +1239,11 @@ export async function runTUI(initialConfig: Config): Promise { // CL-5814: orchestrator is the only product path — no first-run mode picker. const liveSessionMode: SessionMode = "orchestrator"; // Local settings still supply shell env; sessionMode is ignored if present. - const localSettingsForEnv = await loadLocalSettings(localSettingsPath(config.cwd)).catch( - () => null, - ); + const localSettingsForEnvPath = resolveLocalSettingsPath(config.cwd, config.globalSettingsPath); + const localSettingsForEnv = + localSettingsForEnvPath === null + ? null + : await loadLocalSettings(localSettingsForEnvPath).catch(() => null); const toolAvailability: ToolAvailability = { languageServerAvailable: detectLanguageServerAvailable(config.cwd), }; @@ -2138,7 +2140,7 @@ export async function runTUI(initialConfig: Config): Promise { // listing, so revoke resolves against the same snapshot the operator saw. let listedGrants: readonly ScopedApproval[] = []; - const localSettingsFile = localSettingsPath(config.cwd); + const localSettingsFile = resolveLocalSettingsPath(config.cwd, config.globalSettingsPath); const applyCommandResult = (result: CommandResult): void => { switch (result.type) { @@ -2387,7 +2389,11 @@ export async function runTUI(initialConfig: Config): Promise { const onDisk = (await loadGlobalSettingsWriteBase(trueGlobalSettingsPath)) ?? { providers: {}, }; - const next = setDefaultModel(onDisk, ref); + const next = setDefaultModel( + onDisk, + ref, + config.providers.find((provider) => provider.name === ref.provider), + ); await saveGlobalSettings(trueGlobalSettingsPath, next); await persistConnectedSelection(localSettingsFile, ref.provider, ref.model); config = { ...config, settings: next }; diff --git a/tests/unit/config.test.ts b/tests/unit/config.test.ts index 7385600b5..aa46a6285 100644 --- a/tests/unit/config.test.ts +++ b/tests/unit/config.test.ts @@ -1,5 +1,5 @@ import { test, expect } from "bun:test"; -import { mkdtemp, mkdir, writeFile, rm } from "node:fs/promises"; +import { mkdtemp, mkdir, readFile, writeFile, rm, symlink } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { loadConfig } from "../../src/config/index.js"; @@ -82,6 +82,133 @@ test("loadConfig uses the injected pricing fetchImpl instead of the network", as }); }); +test("local settings target is omitted when it aliases global settings", async () => { + const { globalSettingsPath, resolveLocalSettingsPath } = + await import("../../src/config/settings.js"); + const home = await mkdtemp(join(tmpdir(), "ic-unit-config-home-alias-")); + try { + const globalPath = globalSettingsPath(home); + expect(resolveLocalSettingsPath(home, globalPath)).toBeNull(); + expect( + resolveLocalSettingsPath(home, join(home, "nested", "..", ".corbits", "settings.json")), + ).toBeNull(); + expect(resolveLocalSettingsPath(home, join(home, "explicit.json"))).toBe( + join(home, ".corbits", "settings.json"), + ); + expect(resolveLocalSettingsPath(join(home, "repo"), globalPath)).toBe( + join(home, "repo", ".corbits", "settings.json"), + ); + } finally { + await rm(home, { recursive: true, force: true }); + } +}); + +test("local settings target falls back to the lexical path when .corbits is a regular file", async () => { + const { globalSettingsPath, resolveLocalSettingsPath } = + await import("../../src/config/settings.js"); + const root = await mkdtemp(join(tmpdir(), "ic-unit-config-notdir-")); + try { + await writeFile(join(root, ".corbits"), ""); + expect(resolveLocalSettingsPath(root, globalSettingsPath(join(root, "home")))).toBe( + join(root, ".corbits", "settings.json"), + ); + } finally { + await rm(root, { recursive: true, force: true }); + } +}); + +test("local settings target detects a symlink alias before the settings file exists", async () => { + const { globalSettingsPath, resolveLocalSettingsPath } = + await import("../../src/config/settings.js"); + const root = await mkdtemp(join(tmpdir(), "ic-unit-config-symlink-alias-")); + const home = join(root, "home"); + const linkedHome = join(root, "linked-home"); + try { + await mkdir(home); + await symlink(home, linkedHome, "dir"); + const globalPath = globalSettingsPath(home); + expect(resolveLocalSettingsPath(linkedHome, globalPath)).toBeNull(); + } finally { + await rm(root, { recursive: true, force: true }); + } +}); + +test("symlink alias of the default global settings path is not a programmatic override", async () => { + const { globalSettingsPath, isProgrammaticSettingsOverride } = + await import("../../src/config/settings.js"); + const root = await mkdtemp(join(tmpdir(), "ic-unit-config-global-symlink-")); + const home = join(root, "home"); + try { + await mkdir(join(home, ".corbits"), { recursive: true }); + const realPath = globalSettingsPath(home); + await writeFile(realPath, JSON.stringify({ providers: {} })); + const aliasPath = join(root, "alias-settings.json"); + await symlink(realPath, aliasPath); + + expect(isProgrammaticSettingsOverride(aliasPath, realPath)).toBe(false); + expect(isProgrammaticSettingsOverride(join(root, "other.json"), realPath)).toBe(true); + } finally { + await rm(root, { recursive: true, force: true }); + } +}); + +test("loadSettings recovery helper recovers only an exact clobbered local selection", async () => { + const { loadSettingsRecoveringClobberedOAuthSelection } = + await import("../../src/config/settings.js"); + const cwd = await mkdtemp(join(tmpdir(), "ic-unit-config-recovery-")); + try { + const clobberedPath = join(cwd, "clobbered.json"); + const clobbered = JSON.stringify({ provider: "openai", model: "gpt-5" }); + await writeFile(clobberedPath, clobbered); + await expect( + loadSettingsRecoveringClobberedOAuthSelection(clobberedPath, {}, { persist: true }), + ).rejects.toThrow(/Invalid settings schema/); + expect(await readFile(clobberedPath, "utf8")).toBe(clobbered); + + const malformedPath = join(cwd, "malformed.json"); + await writeFile( + malformedPath, + JSON.stringify({ provider: "openai", model: "gpt-5", apiKey: "not-recoverable" }), + ); + await expect( + loadSettingsRecoveringClobberedOAuthSelection(malformedPath, {}, { persist: true }), + ).rejects.toThrow(/Invalid settings schema/); + } finally { + await rm(cwd, { recursive: true, force: true }); + } +}); + +test("loadConfig does not load an aliased global file as local settings", async () => { + const { globalSettingsPath } = await import("../../src/config/settings.js"); + const home = await mkdtemp(join(tmpdir(), "ic-unit-config-load-alias-")); + const path = globalSettingsPath(home); + try { + await mkdir(join(home, ".corbits"), { recursive: true }); + await writeFile( + path, + JSON.stringify({ + defaultProvider: "openai", + providers: { + openai: { + baseURL: "https://api.openai.com/v1", + apiKey: "test-key", + models: ["gpt-5"], + }, + }, + }), + ); + const { impl } = offlineFetch(); + const config = await loadConfig(["--cwd", home, "hello"], { + globalSettingsPath: path, + pricing: { fetchImpl: impl }, + }); + expect(config.settingsDiagnostics).toBeUndefined(); + expect(config.providerName).toBe("openai"); + } finally { + await rm(home, { recursive: true, force: true }); + } +}); + test("global settings round-trip the telemetry block", async () => { const { loadSettings, saveGlobalSettings } = await import("../../src/config/settings.js"); const cwd = await mkdtemp(join(tmpdir(), "ic-unit-config-telemetry-")); @@ -192,8 +319,108 @@ test("loadSettings cannot silently drop a known optional key", async () => { } }); -// Regression: an OAuth-profile provider (xai/) is never written to -// settings.json — home-level auth stores are the source of truth, and +test("aliased-home restart preserves a non-default OAuth model", async () => { + const fakeHome = await mkdtemp(join(tmpdir(), "ic-unit-config-oauth-alias-home-")); + try { + const { XAI_DEFAULT_MODELS } = await import("../../src/auth/xai/constants.js"); + const selectedModel = XAI_DEFAULT_MODELS[1]; + if (selectedModel === undefined) throw new Error("Expected a non-default xAI model fixture"); + await mkdir(join(fakeHome, ".corbits"), { recursive: true }); + await writeFile( + join(fakeHome, ".corbits", "settings.json"), + JSON.stringify({ + defaultProvider: "xai/synthetic", + providers: { + "xai/synthetic": { + baseURL: "https://api.x.ai/v1", + models: [selectedModel], + defaultModel: selectedModel, + }, + }, + }), + ); + await writeFile( + join(fakeHome, ".corbits", "xai-auth.json"), + JSON.stringify({ + profiles: { + synthetic: { + name: "synthetic", + tokens: { + access: "test-access-token", + refresh: "test-refresh", + expiresAt: Date.now() + 3_600_000, + }, + createdAt: Date.now(), + }, + }, + }), + ); + + await withMockedModuleDuring( + import.meta.resolve("node:os"), + (real: typeof import("node:os")) => ({ ...real, homedir: () => fakeHome }), + async () => { + const { impl } = offlineFetch(); + const config = await loadConfig(["--cwd", fakeHome, "do something"], { + pricing: { fetchImpl: impl }, + }); + expect(config.configured).toBe(true); + if (config.configured) { + expect(config.providerName).toBe("xai/synthetic"); + expect(config.model).toBe(selectedModel); + } + }, + ); + } finally { + await rm(fakeHome, { recursive: true, force: true }); + } +}); + +test("loadConfig ignores a persisted OAuth entry whose auth profile is gone", async () => { + const fakeHome = await mkdtemp(join(tmpdir(), "ic-unit-config-oauth-orphan-home-")); + try { + await mkdir(join(fakeHome, ".corbits"), { recursive: true }); + const settingsPath = join(fakeHome, ".corbits", "settings.json"); + const original = JSON.stringify({ + defaultProvider: "xai/gone", + providers: { + "xai/gone": { + baseURL: "https://api.x.ai/v1", + models: ["grok-4"], + defaultModel: "grok-4", + }, + openai: { + baseURL: "https://api.openai.com/v1", + apiKey: "test-key", + models: ["gpt-5"], + }, + }, + }); + await writeFile(settingsPath, original); + + await withMockedModuleDuring( + import.meta.resolve("node:os"), + (real: typeof import("node:os")) => ({ ...real, homedir: () => fakeHome }), + async () => { + const { impl } = offlineFetch(); + const config = await loadConfig(["--cwd", fakeHome, "do something"], { + pricing: { fetchImpl: impl }, + }); + expect(config.configured).toBe(true); + if (config.configured) { + expect(config.providerName).toBe("openai"); + expect(config.providers.some((p) => p.name === "xai/gone")).toBe(false); + } + }, + ); + expect(await readFile(settingsPath, "utf8")).toBe(original); + } finally { + await rm(fakeHome, { recursive: true, force: true }); + } +}); + +// Regression: OAuth credentials for an xai/ provider are never read +// from settings.json — home-level auth stores are the source of truth, and // loadConfig merges them into the catalog it hands to resolveProvider (see // "OAuth profiles live in home-level auth stores" in src/config/index.ts). // --config only overrides where provider *definitions* come from (CL-6973);