Skip to content

Commit 7b529eb

Browse files
committed
Move settings recovery out of generic loader
1 parent 8e232b3 commit 7b529eb

6 files changed

Lines changed: 140 additions & 130 deletions

File tree

‎src/config/index.ts‎

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ import {
5252
globalSettingsPath,
5353
loadLocalSettingsResult,
5454
type SettingsLoadDiagnostic,
55-
loadSettings,
55+
loadSettingsRecoveringClobberedOAuthSelection,
5656
resolveLocalSettingsPath,
5757
normalizeOpenAICompatibleBaseURL,
5858
resolveProvider,
@@ -689,19 +689,23 @@ export async function loadConfig(
689689
const [codexProfiles, xaiProfiles]: [CodexProfile[], XaiProfile[]] = useOAuthProfiles
690690
? await Promise.all([listCodexProfiles(), listXaiProfiles()])
691691
: [[], []];
692-
const codexProviderSettings = codexProvidersAsSettings(codexProfiles);
693-
const xaiProviderSettings = xaiProvidersAsSettings(xaiProfiles);
694-
const recoverableOAuthProviders = {
695-
...codexProviderSettings,
696-
...xaiProviderSettings,
692+
let projectedOAuthProviders = {
693+
...codexProvidersAsSettings(codexProfiles),
694+
...xaiProvidersAsSettings(xaiProfiles),
697695
};
698696
const settings =
699697
configPath !== undefined
700-
? await loadSettings(configPath, { recoverableOAuthProviders }).then((s) => {
698+
? await loadSettingsRecoveringClobberedOAuthSelection(
699+
configPath,
700+
projectedOAuthProviders,
701+
).then((s) => {
701702
if (s === null) throw new Error(`--config file not found or empty: ${configPath}`);
702703
return s;
703704
})
704-
: await loadSettings(effectiveSettingsPath, { recoverableOAuthProviders });
705+
: await loadSettingsRecoveringClobberedOAuthSelection(
706+
effectiveSettingsPath,
707+
projectedOAuthProviders,
708+
);
705709

706710
// Track whether the effective value came from the persisted global default
707711
// rather than this invocation's --dangerously-skip-permissions flag, so the
@@ -710,15 +714,12 @@ export async function loadConfig(
710714
!dangerouslySkipPermissions && settings?.dangerouslySkipPermissions === true;
711715
dangerouslySkipPermissions =
712716
dangerouslySkipPermissions || settings?.dangerouslySkipPermissions === true;
713-
const oauthProviderSettings = applyPersistedOAuthDefaults(settings, {
714-
...codexProviderSettings,
715-
...xaiProviderSettings,
716-
});
717+
projectedOAuthProviders = applyPersistedOAuthDefaults(settings, projectedOAuthProviders);
717718
const settingsForResolution: Settings | null =
718-
Object.keys(oauthProviderSettings).length > 0
719+
Object.keys(projectedOAuthProviders).length > 0
719720
? {
720721
...(settings ?? { providers: {} }),
721-
providers: { ...(settings?.providers ?? {}), ...oauthProviderSettings },
722+
providers: { ...(settings?.providers ?? {}), ...projectedOAuthProviders },
722723
}
723724
: settings;
724725

‎src/config/settings.ts‎

Lines changed: 47 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -745,29 +745,21 @@ export function healOpenCodeGoProviders(settings: Settings): string[] {
745745
return healed;
746746
}
747747

748-
function isClobberedLocalSelection(value: unknown): value is { provider: string; model: string } {
749-
if (!LocalSettingsSchema.allows(value) || typeof value !== "object" || value === null) {
750-
return false;
751-
}
752-
const selection = value as Record<string, unknown>;
753-
return (
754-
Object.keys(selection).length === 2 &&
755-
typeof selection.provider === "string" &&
756-
selection.provider.length > 0 &&
757-
typeof selection.model === "string" &&
758-
selection.model.length > 0
759-
);
760-
}
748+
const ClobberedLocalSelectionSchema = type({
749+
provider: "string>0",
750+
model: "string>0",
751+
"+": "reject",
752+
});
761753

762-
interface LoadSettingsOptions {
763-
recoverableOAuthProviders?: Record<string, ProviderSettings>;
754+
function isClobberedLocalSelection(value: unknown): value is { provider: string; model: string } {
755+
return ClobberedLocalSelectionSchema.allows(value);
764756
}
765757

766758
function recoverClobberedOAuthSelection(
767759
selection: { provider: string; model: string },
768-
projected: Record<string, ProviderSettings> | undefined,
760+
projected: Record<string, ProviderSettings>,
769761
): Settings | undefined {
770-
const provider = projected?.[selection.provider];
762+
const provider = projected[selection.provider];
771763
// Auth-profile presence is enough: the selected model may be outside the
772764
// projected fallback catalog (CODEX_DEFAULT_MODELS / xAI equivalents).
773765
if (provider === undefined) return undefined;
@@ -779,35 +771,30 @@ function recoverClobberedOAuthSelection(
779771
};
780772
}
781773

782-
export async function loadSettings(
783-
path: string,
784-
options: LoadSettingsOptions = {},
785-
): Promise<Settings | null> {
774+
async function loadSettingsJSON(path: string): Promise<unknown | null> {
786775
let raw: string;
787776
try {
788777
raw = await readFile(path, "utf8");
789778
} catch (err) {
790779
if (isENOENT(err)) return null;
791780
throw err;
792781
}
793-
let parsed: unknown;
794782
try {
795-
parsed = JSON.parse(raw);
783+
return JSON.parse(raw);
796784
} catch {
797785
throw new Error(`Invalid JSON in settings file: ${path}`);
798786
}
787+
}
788+
789+
function settingsSchemaError(path: string): Error {
790+
return new Error(
791+
`Invalid settings schema in ${path}: expected { providers: { <name>: { baseURL, apiKey, models: [...] } } }`,
792+
);
793+
}
794+
795+
function normalizeParsedSettings(path: string, parsed: unknown): Settings {
799796
if (!isSettings(parsed)) {
800-
if (isClobberedLocalSelection(parsed)) {
801-
const recovered = recoverClobberedOAuthSelection(
802-
parsed,
803-
options.recoverableOAuthProviders,
804-
) ?? { providers: {} };
805-
await saveGlobalSettings(path, recovered);
806-
return recovered;
807-
}
808-
throw new Error(
809-
`Invalid settings schema in ${path}: expected { providers: { <name>: { baseURL, apiKey, models: [...] } } }`,
810-
);
797+
throw settingsSchemaError(path);
811798
}
812799
const s = parsed as unknown as Record<string, unknown>;
813800
// These keys were removed when plugins moved to discovery; they are now
@@ -861,10 +848,14 @@ export async function loadSettings(
861848
? Boolean(s.dangerouslySkipPermissions)
862849
: undefined,
863850
};
864-
const settings: Settings = {
851+
return {
865852
providers: s.providers as Settings["providers"],
866853
...pickDefined(optional),
867854
};
855+
}
856+
857+
async function loadStrictSettings(path: string, parsed: unknown): Promise<Settings> {
858+
const settings = normalizeParsedSettings(path, parsed);
868859
// Hard cutover: pin Go flag + canonical baseURL on disk when any Go signal matches.
869860
// Only rewrite disk when heal actually mutates (no write-on-read for no-op reloads).
870861
// Fail open on save: keep the in-memory heal so startup is not bricked by a
@@ -885,6 +876,27 @@ export async function loadSettings(
885876
return settings;
886877
}
887878

879+
export async function loadSettings(path: string): Promise<Settings | null> {
880+
const parsed = await loadSettingsJSON(path);
881+
return parsed === null ? null : await loadStrictSettings(path, parsed);
882+
}
883+
884+
export async function loadSettingsRecoveringClobberedOAuthSelection(
885+
path: string,
886+
recoverableOAuthProviders: Record<string, ProviderSettings>,
887+
): Promise<Settings | null> {
888+
const parsed = await loadSettingsJSON(path);
889+
if (parsed === null) return null;
890+
if (isClobberedLocalSelection(parsed)) {
891+
const recovered = recoverClobberedOAuthSelection(parsed, recoverableOAuthProviders) ?? {
892+
providers: {},
893+
};
894+
await saveGlobalSettings(path, recovered);
895+
return recovered;
896+
}
897+
return loadStrictSettings(path, parsed);
898+
}
899+
888900
/** Diagnostic produced when settings fail open instead of crashing startup. */
889901
export interface SettingsLoadDiagnostic {
890902
path: string;

‎src/settings.test.ts‎

Lines changed: 61 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
loadLocalSettings,
1212
loadLocalSettingsWriteBase,
1313
loadSettings,
14+
loadSettingsRecoveringClobberedOAuthSelection,
1415
normalizeOpenAICompatibleBaseURL,
1516
resolveProvider,
1617
saveGlobalSettings,
@@ -594,60 +595,72 @@ describe("loaders", () => {
594595
}
595596
});
596597

597-
test.each([
598-
["one profile", ["codex/work"]],
599-
["multiple profiles", ["codex/personal", "codex/work"]],
600-
])("loadSettings recovers an exact OAuth selection with %s", async (_name, providerNames) => {
598+
test("loadSettings keeps local selection recovery out of the strict loader", async () => {
601599
const dir = await mkdtemp(join(tmpdir(), "ic-settings-"));
602600
try {
603601
const path = join(dir, "settings.json");
604602
await writeFile(path, JSON.stringify({ provider: "codex/work", model: "gpt-5.1-codex" }));
605-
const projected = Object.fromEntries(
606-
providerNames.map((name) => [
607-
name,
608-
{
609-
baseURL: "https://chatgpt.com/backend-api",
610-
apiKey: "oauth-token",
611-
models: ["gpt-5.2-codex", "gpt-5.1-codex"],
612-
defaultModel: "gpt-5.2-codex",
613-
} satisfies ProviderSettings,
614-
]),
615-
);
616-
617-
const recovered = await loadSettings(path, { recoverableOAuthProviders: projected });
618-
expect(recovered).toEqual({
619-
defaultProvider: "codex/work",
620-
providers: {
621-
"codex/work": {
622-
baseURL: "https://chatgpt.com/backend-api",
623-
models: ["gpt-5.1-codex"],
624-
defaultModel: "gpt-5.1-codex",
625-
},
626-
},
627-
});
628-
expect(JSON.stringify(recovered)).not.toContain("oauth-token");
629-
expect(await loadSettings(path)).toEqual(recovered);
603+
await expect(loadSettings(path)).rejects.toThrow(/Invalid settings schema/);
630604
} finally {
631605
await rm(dir, { recursive: true, force: true });
632606
}
633607
});
634608

635-
test("loadSettings recovers a non-catalog OAuth model when the auth profile exists", async () => {
609+
test.each([
610+
["one profile", ["codex/work"]],
611+
["multiple profiles", ["codex/personal", "codex/work"]],
612+
])(
613+
"loadSettingsRecoveringClobberedOAuthSelection recovers an exact OAuth selection with %s",
614+
async (_name, providerNames) => {
615+
const dir = await mkdtemp(join(tmpdir(), "ic-settings-"));
616+
try {
617+
const path = join(dir, "settings.json");
618+
await writeFile(path, JSON.stringify({ provider: "codex/work", model: "gpt-5.1-codex" }));
619+
const projected = Object.fromEntries(
620+
providerNames.map((name) => [
621+
name,
622+
{
623+
baseURL: "https://chatgpt.com/backend-api",
624+
apiKey: "oauth-token",
625+
models: ["gpt-5.2-codex", "gpt-5.1-codex"],
626+
defaultModel: "gpt-5.2-codex",
627+
} satisfies ProviderSettings,
628+
]),
629+
);
630+
631+
const recovered = await loadSettingsRecoveringClobberedOAuthSelection(path, projected);
632+
expect(recovered).toEqual({
633+
defaultProvider: "codex/work",
634+
providers: {
635+
"codex/work": {
636+
baseURL: "https://chatgpt.com/backend-api",
637+
models: ["gpt-5.1-codex"],
638+
defaultModel: "gpt-5.1-codex",
639+
},
640+
},
641+
});
642+
expect(JSON.stringify(recovered)).not.toContain("oauth-token");
643+
expect(await loadSettings(path)).toEqual(recovered);
644+
} finally {
645+
await rm(dir, { recursive: true, force: true });
646+
}
647+
},
648+
);
649+
650+
test("loadSettingsRecoveringClobberedOAuthSelection recovers a non-catalog OAuth model when the auth profile exists", async () => {
636651
const dir = await mkdtemp(join(tmpdir(), "ic-settings-"));
637652
try {
638653
const path = join(dir, "settings.json");
639654
await writeFile(
640655
path,
641656
JSON.stringify({ provider: "codex/work", model: "gpt-special-custom" }),
642657
);
643-
const recovered = await loadSettings(path, {
644-
recoverableOAuthProviders: {
645-
"codex/work": {
646-
baseURL: "https://chatgpt.com/backend-api",
647-
apiKey: "oauth-token",
648-
models: ["gpt-5.2-codex", "gpt-5.1-codex"],
649-
defaultModel: "gpt-5.2-codex",
650-
},
658+
const recovered = await loadSettingsRecoveringClobberedOAuthSelection(path, {
659+
"codex/work": {
660+
baseURL: "https://chatgpt.com/backend-api",
661+
apiKey: "oauth-token",
662+
models: ["gpt-5.2-codex", "gpt-5.1-codex"],
663+
defaultModel: "gpt-5.2-codex",
651664
},
652665
});
653666
expect(recovered).toEqual({
@@ -676,13 +689,11 @@ describe("loaders", () => {
676689
JSON.stringify({ provider: "codex/work", model: "gpt-5.1-codex", apiKey: "nope" }),
677690
);
678691
await expect(
679-
loadSettings(path, {
680-
recoverableOAuthProviders: {
681-
"codex/work": {
682-
baseURL: "https://chatgpt.com/backend-api",
683-
apiKey: "oauth-token",
684-
models: ["gpt-5.1-codex"],
685-
},
692+
loadSettingsRecoveringClobberedOAuthSelection(path, {
693+
"codex/work": {
694+
baseURL: "https://chatgpt.com/backend-api",
695+
apiKey: "oauth-token",
696+
models: ["gpt-5.1-codex"],
686697
},
687698
}),
688699
).rejects.toThrow(/Invalid settings schema/);
@@ -696,13 +707,11 @@ describe("loaders", () => {
696707
try {
697708
const path = join(dir, "settings.json");
698709
await writeFile(path, JSON.stringify({ provider: "codex/missing", model: "gpt-5.1-codex" }));
699-
const recovered = await loadSettings(path, {
700-
recoverableOAuthProviders: {
701-
"codex/work": {
702-
baseURL: "https://chatgpt.com/backend-api",
703-
apiKey: "oauth-token",
704-
models: ["gpt-5.1-codex"],
705-
},
710+
const recovered = await loadSettingsRecoveringClobberedOAuthSelection(path, {
711+
"codex/work": {
712+
baseURL: "https://chatgpt.com/backend-api",
713+
apiKey: "oauth-token",
714+
models: ["gpt-5.1-codex"],
706715
},
707716
});
708717
expect(recovered).toEqual({ providers: {} });

‎src/tui/runner-exit-code.test.ts‎

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { describe, test, expect } from "bun:test";
2-
import { resolveExitCode, resolveTUILocalSettingsPath } from "./runner.js";
2+
import { resolveLocalSettingsPath } from "../config/settings.js";
3+
import { resolveExitCode } from "./runner.js";
34

45
describe("resolveExitCode", () => {
56
test("returns 0 when run completes successfully with no errors", () => {
@@ -66,22 +67,14 @@ describe("resolveExitCode", () => {
6667
});
6768
});
6869

69-
describe("resolveTUILocalSettingsPath", () => {
70+
describe("resolveLocalSettingsPath", () => {
7071
test("treats an aliased --config path as the global settings target", () => {
71-
expect(
72-
resolveTUILocalSettingsPath({
73-
cwd: "/repo",
74-
globalSettingsPath: "/repo/.corbits/settings.json",
75-
}),
76-
).toBeNull();
72+
expect(resolveLocalSettingsPath("/repo", "/repo/.corbits/settings.json")).toBeNull();
7773
});
7874

7975
test("preserves the normal distinct global and project settings paths", () => {
80-
expect(
81-
resolveTUILocalSettingsPath({
82-
cwd: "/tmp/repo",
83-
globalSettingsPath: "/tmp/home/user/.corbits/settings.json",
84-
}),
85-
).toBe("/tmp/repo/.corbits/settings.json");
76+
expect(resolveLocalSettingsPath("/tmp/repo", "/tmp/home/user/.corbits/settings.json")).toBe(
77+
"/tmp/repo/.corbits/settings.json",
78+
);
8679
});
8780
});

0 commit comments

Comments
 (0)