Merge pull request #1007 from corbitsdev/cl-6728-do-not-let-oauth-projections-overwrite-hand-named-provider

Do not let OAuth projections overwrite hand-named provider entries

Author: TheGreatAxios

src/config/index.ts

@@ -135,16 +135,17 @@ function applyPersistedOAuthDefaults(
 // 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(
+export function dropOrphanedOAuthEntries(
   settings: Settings | null,
   projected: Record<string, ProviderSettings>,
 ): Settings | null {
   if (settings === null) return null;
   const providers = Object.fromEntries(
     Object.entries(settings.providers).filter(
-      ([name]) =>
+      ([name, provider]) =>
         (!isCodexProviderName(name) && !isXaiProviderName(name)) ||
-        projected[name] !== undefined,
+        projected[name] !== undefined ||
+        isHandNamedProviderEntry(provider),
     ),
   );
   const { defaultProvider, ...rest } = settings;
@@ -158,6 +159,35 @@ function dropOrphanedOAuthEntries(
   };
 }
 
+// A codex/<slug> or xai/<slug> settings row carrying its own credential is the
+// operator's explicit config, not an OAuth placeholder: OAuth profile
+// projections must never overwrite it, orphan-sweep it, or drop it from the
+// catalog. Credential-less namespaced rows stay placeholders (CL-6728).
+function isHandNamedProviderEntry(
+  entry: Pick<ProviderSettings, "apiKey" | "keyless"> | undefined,
+): boolean {
+  if (entry === undefined) return false;
+  if (entry.keyless === true) return true;
+  return typeof entry.apiKey === "string" && entry.apiKey.length > 0;
+}
+
+// Overlay live OAuth profile projections onto settings for runtime provider
+// resolution. Exported for tests; loadConfig is the only production caller.
+export function overlayOAuthProjections(
+  settings: Settings | null,
+  projected: Record<string, ProviderSettings>,
+): Settings | null {
+  if (Object.keys(projected).length === 0) return settings;
+  const providers = { ...(settings?.providers ?? {}) };
+  for (const [name, entry] of Object.entries(projected)) {
+    if (!isHandNamedProviderEntry(providers[name])) providers[name] = entry;
+  }
+  return {
+    ...(settings ?? { providers: {} }),
+    providers,
+  };
+}
+
 function hasExaEntry(servers: MCPServerSettingsEntry[] | undefined): boolean {
   return (
     servers?.some((server) => server.name === EXA_MCP_SERVER_NAME) === true
@@ -936,16 +966,10 @@ export async function loadConfig(
   const liveSettings = useOAuthProfiles
     ? dropOrphanedOAuthEntries(settings, projectedOAuthProviders)
     : settings;
-  const settingsForResolution: Settings | null =
-    Object.keys(projectedOAuthProviders).length > 0
-      ? {
-          ...(liveSettings ?? { providers: {} }),
-          providers: {
-            ...(liveSettings?.providers ?? {}),
-            ...projectedOAuthProviders,
-          },
-        }
-      : liveSettings;
+  const settingsForResolution: Settings | null = overlayOAuthProjections(
+    liveSettings,
+    projectedOAuthProviders,
+  );
 
   // The per-repo selection file still applies on top of a --config source: that
   // file supplies provider definitions, while .corbits/settings.json supplies
@@ -1194,15 +1218,33 @@ export function mergeOAuthCatalog(
       ? ["xai"]
       : []),
   ]);
+  const settingsRows = buildProviderCatalog(settings, resolved);
+  // A hand-named codex/<slug> or xai/<slug> API-key row is the operator's
+  // explicit config, not an OAuth placeholder: keep it and skip the colliding
+  // live profile projection instead of overwriting it (CL-6728). Read the raw
+  // settings rows only: buildProviderCatalog synthesizes a [resolved] row when
+  // settings is null/empty, and when resolved is itself codex/<slug> that row
+  // carries the live apiKey with no profile marker — treating it as hand-named
+  // would eject the real marked entry for a stale token snapshot.
+  const handNamed = new Set(
+    Object.entries(settings?.providers ?? {})
+      .filter(
+        ([name, provider]) =>
+          (isCodexProviderName(name) || isXaiProviderName(name)) &&
+          isHandNamedProviderEntry(provider),
+      )
+      .map(([name]) => name),
+  );
   return [
-    ...buildProviderCatalog(settings, resolved).filter(
+    ...settingsRows.filter(
       (e) =>
-        !isCodexProviderName(e.name) &&
-        !isXaiProviderName(e.name) &&
-        !dropBare.has(e.name),
+        handNamed.has(e.name) ||
+        (!isCodexProviderName(e.name) &&
+          !isXaiProviderName(e.name) &&
+          !dropBare.has(e.name)),
     ),
-    ...codexEntries,
-    ...xaiEntries,
+    ...codexEntries.filter((e) => !handNamed.has(e.name)),
+    ...xaiEntries.filter((e) => !handNamed.has(e.name)),
   ].map((entry) =>
     isOpenCodeGoProvider(entry)
       ? { ...entry, models: [...selectableGoModelIds()] }
@@ -1272,11 +1314,20 @@ export function runtimeSettingsWithCatalog(
   if (settings === undefined) {
     return { providers: fromCatalog };
   }
+  // OAuth-marked catalog rows carry live profile tokens; they overlay
+  // credential-less placeholders but never a hand-named API-key row (CL-6728).
+  const overlaid = Object.fromEntries(
+    Object.entries(fromCatalog).filter(
+      ([name]) =>
+        (!isCodexProviderName(name) && !isXaiProviderName(name)) ||
+        !isHandNamedProviderEntry(settings.providers[name]),
+    ),
+  );
   return {
     ...settings,
     providers: {
       ...settings.providers,
-      ...fromCatalog,
+      ...overlaid,
     },
   };
 }