Merge pull request #1535 from corbitsdev/cl-10812-boot-overlap-on-main

perf(boot): overlap independent startup file reads

Author: TheGreatAxios

src/config.test.ts

@@ -37,16 +37,19 @@ import {
   clearSourceCredentials,
   peekSourceCredentialSecret,
 } from "./config/source-credentials.js";
-import type {
-  Config,
-  LoadConfigOptions,
-  UnconfiguredConfig,
-} from "./config/index.js";
 import {
+  ensureTelemetrySettings,
+  resetSettingsDocumentReadCountForTests,
+  settingsDocumentReadCountForTests,
   mergeProviderIntoSettings,
   type ResolvedProvider,
   type Settings,
 } from "./config/settings.js";
+import type {
+  Config,
+  LoadConfigOptions,
+  UnconfiguredConfig,
+} from "./config/index.js";
 import {
   OPENCODE_GO_BASE_URL,
   OPENCODE_GO_MODEL_IDS,
@@ -1583,6 +1586,73 @@ describe("loadConfig", () => {
     expect(config.apiKey).toBe("b-key");
   });
 
+  test("CLI selection wins over local, and local wins over global", async () => {
+    const cwd = await emptyCwd();
+    await mkdir(join(cwd, ".corbits"), { recursive: true });
+    await writeFile(
+      join(cwd, ".corbits", "settings.json"),
+      JSON.stringify({ provider: "local-provider", model: "local-model" }),
+    );
+    const globalPath = join(cwd, "global.json");
+    await writeFile(
+      globalPath,
+      JSON.stringify({
+        defaultProvider: "global-provider",
+        providers: {
+          "global-provider": {
+            baseURL: "https://global.example/v1",
+            apiKey: "global-key",
+            models: ["global-model"],
+          },
+          "local-provider": {
+            baseURL: "https://local.example/v1",
+            apiKey: "local-key",
+            models: ["local-model"],
+          },
+        },
+      }),
+    );
+    const localWins = await loadConfig(["--cwd", cwd, "task"], {
+      globalSettingsPath: globalPath,
+    });
+    assertConfigured(localWins);
+    expect(localWins.providerName).toBe("local-provider");
+    expect(localWins.model).toBe("local-model");
+
+    const cliWins = await loadConfig(
+      [
+        "--cwd",
+        cwd,
+        "--provider",
+        "global-provider",
+        "--model",
+        "global-model",
+        "task",
+      ],
+      { globalSettingsPath: globalPath },
+    );
+    assertConfigured(cliWins);
+    expect(cliWins.providerName).toBe("global-provider");
+    expect(cliWins.model).toBe("global-model");
+  });
+
+  test("boot reuses the settings document instead of reading it again", async () => {
+    const cwd = await emptyCwd();
+    const globalPath = await writeGlobalSettings(cwd, {
+      telemetry: { installationId: "boot-id" },
+    });
+    resetSettingsDocumentReadCountForTests();
+    const config = await loadConfig(["--cwd", cwd, "task"], {
+      globalSettingsPath: globalPath,
+    });
+    assertConfigured(config);
+    const settings = await ensureTelemetrySettings(globalPath, {
+      reuseBootSnapshot: true,
+    });
+    expect(settingsDocumentReadCountForTests()).toBe(1);
+    expect(settings.telemetry?.installationId).toBe("boot-id");
+  });
+
   test("rejects a local reasoningEffort unsupported by the selected model", async () => {
     const cwd = await emptyCwd();
     await mkdir(join(cwd, ".corbits"), { recursive: true });

src/config/index.ts

@@ -24,7 +24,10 @@ import {
   setProviderContextWindowOverrides,
   setReservedCompletionTokens,
 } from "../provider/context-window.js";
-import { bootstrapPricingMetadata } from "../cost/pricing-metadata.js";
+import {
+  bootstrapPricingMetadata,
+  startPricingCacheRead,
+} from "../cost/pricing-metadata.js";
 import {
   defaultPricingCachePath,
   type PricingFetcherOptions,
@@ -89,6 +92,8 @@ import {
   loadLocalSettingsResult,
   type SettingsLoadDiagnostic,
   loadSettingsRecoveringClobberedOAuthSelection,
+  readSettingsDocument,
+  rememberBootGlobalSettings,
   resolveLocalSettingsPath,
   normalizeOpenAICompatibleBaseURL,
   resolveProvider,
@@ -1075,21 +1080,6 @@ export async function loadConfig(
     positional.push(arg);
   }
 
-  if (permissionsPath !== undefined) {
-    try {
-      await loadPermissionsFile(permissionsPath);
-    } catch (err) {
-      throw new CliUserError(err instanceof Error ? err.message : String(err));
-    }
-  }
-
-  const pricingCachePath = defaultPricingCachePath();
-
-  await bootstrapPricingMetadata({
-    cachePath: pricingCachePath,
-    ...options.pricing,
-  });
-
   // Resolve both settings targets from the same active source. The local schema
   // must never be read from or written to the active global-schema target.
   const effectiveSettingsPath =
@@ -1106,22 +1096,74 @@ export async function loadConfig(
   // unauthenticated. Only the programmatic `globalSettingsPath` test override
   // (never a CLI flag) opts out — tests want no home-directory reads.
   const useOAuthProfiles = options.globalSettingsPath === undefined;
-  const [codexProfiles, xaiProfiles]: [CodexProfile[], XaiProfile[]] =
+
+  // These files do not decide precedence for each other. Overlap the reads,
+  // then apply results in the previous order: permissions error, pricing
+  // install, OAuth error, settings warnings, local settings, profile.
+  const permissionsTask: Promise<void> =
+    permissionsPath !== undefined
+      ? loadPermissionsFile(permissionsPath).then(
+          () => undefined,
+          (err: unknown) => {
+            throw new CliUserError(
+              err instanceof Error ? err.message : String(err),
+            );
+          },
+        )
+      : Promise.resolve();
+  const pricingOptions: PricingFetcherOptions = {
+    cachePath: defaultPricingCachePath(),
+    ...options.pricing,
+  };
+  startPricingCacheRead(pricingOptions.cachePath);
+  const oauthTask: Promise<readonly [CodexProfile[], XaiProfile[]]> =
     useOAuthProfiles
-      ? await Promise.all([listCodexProfiles(), listXaiProfiles()])
-      : [[], []];
+      ? Promise.all([listCodexProfiles(), listXaiProfiles()])
+      : Promise.resolve([[], []]);
+  const settingsPath = configPath ?? effectiveSettingsPath;
+  const settingsDocumentTask = readSettingsDocument(settingsPath);
+  const localTask =
+    localSettingsFile === null
+      ? Promise.resolve({
+          settings: null,
+          diagnostics: [] as SettingsLoadDiagnostic[],
+        })
+      : loadLocalSettingsResult(localSettingsFile);
+  const profileTask = resolveProfile(cwd, profileFlag);
+
+  const [
+    permissionsResult,
+    oauthResult,
+    settingsDocumentResult,
+    localResultSettled,
+    profileResult,
+  ] = await Promise.allSettled([
+    permissionsTask,
+    oauthTask,
+    settingsDocumentTask,
+    localTask,
+    profileTask,
+  ]);
+
+  if (permissionsResult.status === "rejected") throw permissionsResult.reason;
+  await bootstrapPricingMetadata(pricingOptions);
+  if (oauthResult.status === "rejected") throw oauthResult.reason;
+  if (settingsDocumentResult.status === "rejected") {
+    throw settingsDocumentResult.reason;
+  }
+
+  const [codexProfiles, xaiProfiles] = oauthResult.value;
   let projectedOAuthProviders = {
     ...codexProvidersAsSettings(codexProfiles),
     ...xaiProvidersAsSettings(xaiProfiles),
   };
+  const parsedDocument = { value: settingsDocumentResult.value };
   const settings =
     configPath !== undefined
       ? await loadSettingsRecoveringClobberedOAuthSelection(
           configPath,
           projectedOAuthProviders,
-          {
-            persist: false,
-          },
+          { persist: false, parsedDocument },
         ).then((s) => {
           if (s === null)
             throw new Error(`--config file not found or empty: ${configPath}`);
@@ -1130,8 +1172,15 @@ export async function loadConfig(
       : await loadSettingsRecoveringClobberedOAuthSelection(
           effectiveSettingsPath,
           projectedOAuthProviders,
-          { persist: true },
+          { persist: true, parsedDocument },
         );
+  // --config does not load the global file, so do not reuse that document for
+  // the later global telemetry read. Do not overlap that read with the heal
+  // write above: both target settings.json and the later read must see the
+  // persisted document, which is what this snapshot is.
+  if (configPath === undefined) {
+    rememberBootGlobalSettings(effectiveSettingsPath, settings);
+  }
 
   // True when the value came from settings, not this invocation's
   // --dangerously-skip-permissions flag (Config.skipPermissionsFromSettings).
@@ -1156,14 +1205,12 @@ export async function loadConfig(
   // supplies provider definitions, .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 =
-    localSettingsFile === null
-      ? { settings: null, diagnostics: [] }
-      : await loadLocalSettingsResult(localSettingsFile);
+  if (localResultSettled.status === "rejected") throw localResultSettled.reason;
+  if (profileResult.status === "rejected") throw profileResult.reason;
+  const localResult = localResultSettled.value;
   const local = localResult.settings;
   const settingsDiagnostics = localResult.diagnostics;
-
-  const profile = await resolveProfile(cwd, profileFlag);
+  const profile = profileResult.value;
 
   // Apply profile as a fallback layer: profile.model fills in when neither CLI
   // nor local settings specify a model. This sits below local in precedence.

src/config/profiles.ts

@@ -82,14 +82,34 @@ export async function resolveProfile(
   profileName?: string,
   home: string = homedir(),
 ): Promise<ProfileConfig> {
-  const projectProfile = await loadProfile(projectProfilePath(cwd));
+  const projectPromise = loadProfile(projectProfilePath(cwd));
+  // The named file is known only once the CLI flag or the project profile
+  // says so. Overlap the named read when the flag already names it. When the
+  // name comes from project profile.json, reading a guessed file would not
+  // change the winner — there is nothing to guess — so that read stays after
+  // the project file.
+  const namedPromise =
+    profileName !== undefined
+      ? loadProfile(join(profilesDir(home), `${profileName}.json`))
+      : undefined;
+
+  let projectProfile: ProfileConfig | null;
+  try {
+    projectProfile = await projectPromise;
+  } catch (err) {
+    if (namedPromise !== undefined) await namedPromise.catch(() => undefined);
+    throw err;
+  }
 
   const namedProfileName = profileName ?? projectProfile?.profile;
 
   let namedProfile: ProfileConfig | null = null;
   if (namedProfileName !== undefined) {
     const namedPath = join(profilesDir(home), `${namedProfileName}.json`);
-    namedProfile = await loadProfile(namedPath);
+    namedProfile =
+      namedPromise !== undefined
+        ? await namedPromise
+        : await loadProfile(namedPath);
     if (namedProfile === null) {
       throw new Error(`Profile not found: ${namedPath}`);
     }

src/config/settings.ts

@@ -986,7 +986,22 @@ function recoverClobberedOAuthSelection(
   };
 }
 
+let settingsDocumentReads = 0;
+let bootGlobalSettings: { path: string; settings: Settings | null } | undefined;
+
+/** How many times settings JSON has been read from disk. */
+export function settingsDocumentReadCountForTests(): number {
+  return settingsDocumentReads;
+}
+
+/** Test-only: clear the boot settings snapshot and the read counter. */
+export function resetSettingsDocumentReadCountForTests(): void {
+  settingsDocumentReads = 0;
+  bootGlobalSettings = undefined;
+}
+
 async function loadSettingsJSON(path: string): Promise<unknown | null> {
+  settingsDocumentReads += 1;
   let raw: string;
   try {
     raw = await readFile(path, "utf8");
@@ -1001,6 +1016,37 @@ async function loadSettingsJSON(path: string): Promise<unknown | null> {
   }
 }
 
+/** Read a settings file once so boot can overlap the disk read with other I/O. */
+export async function readSettingsDocument(
+  path: string,
+): Promise<unknown | null> {
+  return loadSettingsJSON(path);
+}
+
+/**
+ * Remember the on-disk settings document loadConfig just resolved so startup
+ * telemetry does not read settings.json again. The value is the persisted
+ * document, not the OAuth overlay.
+ */
+export function rememberBootGlobalSettings(
+  path: string,
+  settings: Settings | null,
+): void {
+  bootGlobalSettings = {
+    path,
+    settings: settings === null ? null : structuredClone(settings),
+  };
+}
+
+function takeBootGlobalSettings(path: string): Settings | null | undefined {
+  if (bootGlobalSettings === undefined || bootGlobalSettings.path !== path) {
+    return undefined;
+  }
+  const settings = bootGlobalSettings.settings;
+  bootGlobalSettings = undefined;
+  return settings;
+}
+
 function settingsSchemaError(path: string): Error {
   return new Error(
     `Invalid settings schema in ${path}: expected { providers: { <name>: { baseURL, apiKey, models: [...] } } }`,
@@ -1160,10 +1206,13 @@ export async function loadSettings(path: string): Promise<Settings | null> {
 export async function loadSettingsRecoveringClobberedOAuthSelection(
   path: string,
   recoverableOAuthProviders: Record<string, ProviderSettings>,
-  options: { persist: boolean },
+  options: { persist: boolean; parsedDocument?: { value: unknown | null } },
 ): Promise<Settings | null> {
-  const parsed = await loadSettingsJSON(path);
-  if (parsed === null) return null;
+  const parsed =
+    options.parsedDocument !== undefined
+      ? options.parsedDocument.value
+      : await loadSettingsJSON(path);
+  if (parsed === null || parsed === undefined) return null;
   if (isClobberedLocalSelection(parsed)) {
     const recovered = recoverClobberedOAuthSelection(
       parsed,
@@ -1466,9 +1515,18 @@ export async function markLastChangelogVersion(
 }
 
 // Ensure a persisted telemetry installationId exists, generating one on first
-// use. Reads on-disk settings fresh (same rationale as markOnboarded).
-export async function ensureTelemetrySettings(path: string): Promise<Settings> {
-  const onDisk = await loadSettings(path);
+// use. Callers that already loaded this path at boot pass reuseBootSnapshot so
+// the file is not read again. The snapshot is the persisted document, never the
+// OAuth overlay. Other callers still read disk (same rationale as markOnboarded).
+export async function ensureTelemetrySettings(
+  path: string,
+  options?: { reuseBootSnapshot?: boolean },
+): Promise<Settings> {
+  const cached =
+    options?.reuseBootSnapshot === true
+      ? takeBootGlobalSettings(path)
+      : undefined;
+  const onDisk = cached !== undefined ? cached : await loadSettings(path);
   const base: Settings = onDisk ?? { providers: {} };
   // Read-then-write, not read-then-lock: concurrent first launches could each
   // generate a different installationId and the second save wins. Accepted —