refactor(config): drop the OAuth projection wrapper and flag narration

Inline applyOAuthProviderProjections back into loadConfig and remove comments that only restate alias spellings. Flag parse results are unchanged.

Author: TheGreatAxios

docs/IMPLEMENTATION.md

@@ -66,11 +66,8 @@ Precedence:
   `resourceAttributes["service.name"]` > `corbits-code`. After merge,
   `resourceAttributes["service.name"]` is always set to the resolved name so
   the two never diverge.
-- **resourceAttributes:** env list replaces settings wholesale when
-  `OTEL_RESOURCE_ATTRIBUTES` is set (no per-key merge); otherwise settings
-  apply alone. Set-but-empty counts as set: `OTEL_RESOURCE_ATTRIBUTES=""`
-  replaces the settings attributes with nothing (and does not re-apply the
-  settings map) — the same set-but-empty rule as `OTEL_EXPORTER_OTLP_HEADERS`.
+- **resourceAttributes:** when `OTEL_RESOURCE_ATTRIBUTES` is set, it fully
+  replaces settings attributes (including set-but-empty)
 - **enabling export:** setting an endpoint (settings or env) enables export;
   `otel.enabled` need not be set. Service name or resource attributes alone do
   not enable export.

docs/PERFTRACE.md

@@ -572,8 +572,6 @@ describe("loadConfig", () => {
     { argv: ["resume", "--pick"] },
     { argv: ["resume"] },
     { argv: ["continue", "--list"] },
-    // CL-10672: --pick/--list fold into the single --resume picker verb; both
-    // aliases remain accepted in any resume-flow position.
     { argv: ["resume", "--list"] },
     { argv: ["resume", "--pick", "--list"] },
   ] as const)("$argv opens the session picker", async ({ argv }) => {
@@ -586,8 +584,6 @@ describe("loadConfig", () => {
   });
 
   test("CL-10672 keeps --pick/--list out of flow as unrecognized flags", async () => {
-    // Without the resume verb these spellings were never recognized; the
-    // alias fold preserves that fallthrough instead of quietly opening a picker.
     await expect(
       loadConfig(["--pick"], { globalSettingsPath: NO_SETTINGS }),
     ).rejects.toThrow("unrecognized flag: --pick");

src/config.test.ts

@@ -109,9 +109,7 @@ import { resolveProfile } from "./profiles.js";
 
 // Per-call token ceiling for inference sources, shared by agent creation
 // (runner.ts) and live provider switching (the /agent modal) so a live switch
-// cannot silently revert it. CL-10672 review: kept as the shared fallback —
-// it is NOT an always-overridden default; ProviderSettings.maxTokens is the
-// optional override and an unset value resolves to this ceiling.
+// cannot silently revert it.
 export const SOURCE_MAX_TOKENS = 16384;
 
 // Credential-cell placeholder for keyless local providers (e.g. Ollama). The
@@ -162,9 +160,8 @@ function applyPersistedOAuthDefaults(
 
 // OAuth entries in settings.json carry no credentials; they work only while a
 // matching auth-store profile exists. Drop orphans in memory so a removed
-// profile does not pin resolution to an unauthenticatable provider. Called
-// only from applyOAuthProviderProjections (plus its tests).
-function dropOrphanedOAuthEntries(
+// profile does not pin resolution to an unauthenticatable provider.
+export function dropOrphanedOAuthEntries(
   settings: Settings | null,
   projected: Record<string, ProviderSettings>,
 ): Settings | null {
@@ -200,8 +197,8 @@ function isHandNamedProviderEntry(
 }
 
 // Overlay live OAuth profile projections onto settings for runtime provider
-// resolution. Called only from applyOAuthProviderProjections (plus its tests).
-function overlayOAuthProjections(
+// resolution. Exported for tests; loadConfig is the only production caller.
+export function overlayOAuthProjections(
   settings: Settings | null,
   projected: Record<string, ProviderSettings>,
 ): Settings | null {
@@ -216,28 +213,6 @@ function overlayOAuthProjections(
   };
 }
 
-// The one ordered OAuth projection chain for runtime provider resolution,
-// folding the three dedicated helpers at the layer that owns them
-// (loadConfig): (1) persisted defaultModel defaults merge into the live
-// projections, (2) credential-less orphan entries are dropped from settings
-// (only when the projections came from the home auth stores), (3) the
-// projections overlay onto the surviving settings without overwriting
-// hand-named API-key rows. Exposed for tests.
-export function applyOAuthProviderProjections(
-  settings: Settings | null,
-  projectedProviderSettings: Record<string, ProviderSettings>,
-  options: { dropOrphans: boolean },
-): Settings | null {
-  const projected = applyPersistedOAuthDefaults(
-    settings,
-    projectedProviderSettings,
-  );
-  const liveSettings = options.dropOrphans
-    ? dropOrphanedOAuthEntries(settings, projected)
-    : settings;
-  return overlayOAuthProjections(liveSettings, projected);
-}
-
 function hasExaEntry(servers: MCPServerSettingsEntry[] | undefined): boolean {
   return (
     servers?.some((server) => server.name === EXA_MCP_SERVER_NAME) === true
@@ -730,15 +705,6 @@ export interface UnconfiguredConfig {
   settingsDiagnostics?: SettingsLoadDiagnostic[];
 }
 
-// Skip-permissions spellings. `--dangerously-skip-permissions` is the
-// canonical process-only flag; `--yolo` is a kept alias (`/yolo` is the
-// persistent settings writer — a separate surface). One flag, one setting
-// (`dangerouslySkipPermissions`); the parse consults one alias set.
-const SKIP_PERMISSIONS_FLAGS: ReadonlySet<string> = new Set([
-  "--dangerously-skip-permissions",
-  "--yolo",
-]);
-
 /** Printed for `corbits --help` / `-h`. Keep in sync with docs/IMPLEMENTATION.md. */
 export const CLI_HELP_TEXT = `corbits — coding agent CLI
 
@@ -867,9 +833,6 @@ export async function loadConfig(
   let resumeMode: "id" | "pick" | undefined;
   let resumeSessionId: string | undefined;
   const leading = args[0];
-  // One exec verb: `exec` is canonical; `run` is a kept alias (help names
-  // both). One resume verb: `resume` is canonical; `continue` is a kept
-  // alias of the same flow.
   if (leading === "exec" || leading === "run") {
     command = "exec";
     args.shift();
@@ -878,8 +841,6 @@ export async function loadConfig(
     // Bare resume opens the list; a session id is the only direct-resume path.
     // Invalid non-flag positionals error instead of falling through (a
     // free-form token would otherwise become task while skipInitialTask is set).
-    // A following --pick/--list falls through to the loop's single resume
-    // branch, where it is a kept alias of the bare --resume picker flow.
     const next = args.slice()[0];
     if (next !== undefined && !isFlagToken(next)) {
       if (!isSessionId(next)) {
@@ -895,10 +856,9 @@ export async function loadConfig(
     }
   }
 
-  // `-p` is the exec one-shot alias in any position (`-p --provider …`,
+  // `-p` is the exec one-shot path in any position (`-p --provider …`,
   // `--model … -p …`). Detect it before value flags so exec-only options
-  // that appear before `-p` still see exec mode; the loop keeps the token
-  // out of the task positionals.
+  // that appear before `-p` still see exec mode.
   if (args.includes("-p")) {
     if (leading === "resume" || leading === "continue") {
       throw new Error(
@@ -970,13 +930,11 @@ export async function loadConfig(
       continue;
     }
 
-    if (SKIP_PERMISSIONS_FLAGS.has(arg)) {
+    if (arg === "--dangerously-skip-permissions" || arg === "--yolo") {
       dangerouslySkipPermissions = true;
       continue;
     }
     if (arg === "--auto") {
-      // Explicit re-assertion of the on-by-default auto mode. Kept because
-      // sequence order still matters (`--no-auto --auto` re-enables).
       auto = true;
       continue;
     }
@@ -985,15 +943,9 @@ export async function loadConfig(
       continue;
     }
     if (arg === "-p") {
-      // `-p` alias of exec: the pre-scan already fixed command="exec"; the
-      // token must stay out of the task positionals.
       continue;
     }
     if (arg === "--resume" || arg === "--pick" || arg === "--list") {
-      // One resume verb: `--resume` carries the optional session id (exec
-      // requires it; interactive bare `--resume` opens the picker, the alias
-      // for `resume <session-id>`). `--pick`/`--list` are kept aliases of that
-      // bare picker flow and may not follow a session id.
       if (arg === "--resume") {
         if (resumeMode === "id") {
           throw new Error("cannot combine a session id with --resume");
@@ -1020,8 +972,6 @@ export async function loadConfig(
       } else if (resumeMode !== undefined) {
         resumeMode = "pick";
       } else {
-        // Without the resume verb these spellings are not in flow; preserve
-        // the previous fallthrough as an unrecognized flag.
         throw new Error(`unrecognized flag: ${arg}`);
       }
       continue;
@@ -1059,7 +1009,7 @@ export async function loadConfig(
     useOAuthProfiles
       ? await Promise.all([listCodexProfiles(), listXaiProfiles()])
       : [[], []];
-  const projectedOAuthProviders = {
+  let projectedOAuthProviders = {
     ...codexProvidersAsSettings(codexProfiles),
     ...xaiProvidersAsSettings(xaiProfiles),
   };
@@ -1089,11 +1039,16 @@ export async function loadConfig(
     settings?.dangerouslySkipPermissions === true;
   dangerouslySkipPermissions =
     dangerouslySkipPermissions || settings?.dangerouslySkipPermissions === true;
-  // Single ordered projection chain (see applyOAuthProviderProjections).
-  const settingsForResolution: Settings | null = applyOAuthProviderProjections(
+  projectedOAuthProviders = applyPersistedOAuthDefaults(
     settings,
     projectedOAuthProviders,
-    { dropOrphans: useOAuthProfiles },
+  );
+  const liveSettings = useOAuthProfiles
+    ? dropOrphanedOAuthEntries(settings, projectedOAuthProviders)
+    : settings;
+  const settingsForResolution: Settings | null = overlayOAuthProjections(
+    liveSettings,
+    projectedOAuthProviders,
   );
 
   // The per-repo selection file still applies on top of --config: that file

src/config/index.ts

@@ -8,8 +8,9 @@ import {
   codexProvidersAsSettings,
 } from "./codex-providers.js";
 import {
-  applyOAuthProviderProjections,
+  dropOrphanedOAuthEntries,
   mergeOAuthCatalog,
+  overlayOAuthProjections,
   providerCatalogToSettings,
   runtimeSettingsWithCatalog,
 } from "./index.js";
@@ -140,38 +141,34 @@ describe("CL-6728: OAuth projections do not overwrite hand-named provider entrie
   const liveProjected = () => codexProvidersAsSettings([liveMine]);
   const liveCatalog = () => codexProfilesToCatalogEntries([liveMine]);
 
-  test("the projection chain keeps a hand-named codex/<slug> API-key entry", () => {
-    const overlaid = applyOAuthProviderProjections(
+  test("overlayOAuthProjections keeps a hand-named codex/<slug> API-key entry", () => {
+    const overlaid = overlayOAuthProjections(
       settingsWith({ "codex/mine": handNamed() }),
       liveProjected(),
-      { dropOrphans: true },
     );
     expect(overlaid?.providers["codex/mine"]?.apiKey).toBe("hand-named-key");
   });
 
-  test("the projection chain still applies the live token over a credential-less OAuth placeholder", () => {
-    const overlaid = applyOAuthProviderProjections(
+  test("overlayOAuthProjections still applies the live token over a credential-less OAuth placeholder", () => {
+    const overlaid = overlayOAuthProjections(
       settingsWith({ "codex/mine": codexEntry() }),
       liveProjected(),
-      { dropOrphans: true },
     );
     expect(overlaid?.providers["codex/mine"]?.apiKey).toBe("live-token");
   });
 
-  test("the chain never drops a hand-named API-key entry (orphan sweep)", () => {
-    const kept = applyOAuthProviderProjections(
+  test("dropOrphanedOAuthEntries never drops a hand-named API-key entry", () => {
+    const kept = dropOrphanedOAuthEntries(
       settingsWith({ "codex/mine": handNamed() }),
       {},
-      { dropOrphans: true },
     );
     expect(kept?.providers["codex/mine"]?.apiKey).toBe("hand-named-key");
   });
 
-  test("the chain still drops a credential-less orphan", () => {
-    const dropped = applyOAuthProviderProjections(
+  test("dropOrphanedOAuthEntries still drops a credential-less orphan", () => {
+    const dropped = dropOrphanedOAuthEntries(
       settingsWith({ "codex/mine": codexEntry() }),
       {},
-      { dropOrphans: true },
     );
     expect(dropped?.providers["codex/mine"]).toBeUndefined();
   });
@@ -277,8 +274,8 @@ describe("CL-6728: OAuth projections do not overwrite hand-named provider entrie
     expect(runtime.providers["codex/mine"]?.apiKey).toBeUndefined();
   });
 
-  test("the chain treats a whitespace apiKey as hand-named", () => {
-    const overlaid = applyOAuthProviderProjections(
+  test("overlayOAuthProjections treats a whitespace apiKey as hand-named", () => {
+    const overlaid = overlayOAuthProjections(
       settingsWith({
         "codex/mine": {
           baseURL: "https://hand-named.example.com/v1",
@@ -287,13 +284,12 @@ describe("CL-6728: OAuth projections do not overwrite hand-named provider entrie
         },
       }),
       liveProjected(),
-      { dropOrphans: true },
     );
     expect(overlaid?.providers["codex/mine"]?.apiKey).toBe("   ");
   });
 
-  test("the chain treats an empty-string apiKey as a placeholder", () => {
-    const overlaid = applyOAuthProviderProjections(
+  test("overlayOAuthProjections treats an empty-string apiKey as a placeholder", () => {
+    const overlaid = overlayOAuthProjections(
       settingsWith({
         "codex/mine": {
           baseURL: "https://hand-named.example.com/v1",
@@ -302,44 +298,7 @@ describe("CL-6728: OAuth projections do not overwrite hand-named provider entrie
         },
       }),
       liveProjected(),
-      { dropOrphans: true },
     );
     expect(overlaid?.providers["codex/mine"]?.apiKey).toBe("live-token");
   });
 });
-
-describe("applyOAuthProviderProjections: one ordered chain (CL-10672)", () => {
-  const liveMine: CodexProfile = {
-    name: "mine",
-    tokens: { access: "live-token", refresh: "r", expiresAt: 1 },
-    createdAt: 0,
-  };
-  test("persisted defaults feed the projections before the orphan sweep and overlay", () => {
-    // settings carry a persisted defaultModel on the credential-less
-    // placeholder; the projection must inherit it (defaults step) while the
-    // orphan sweep honors the live profile presence (drop guard) and the
-    // overlay delivers the live token.
-    const settings = settingsWith({
-      "codex/mine": { ...codexEntry(), defaultModel: "chosen-model" },
-    });
-    const resolvedChain = applyOAuthProviderProjections(
-      settings,
-      codexProvidersAsSettings([liveMine]),
-      { dropOrphans: true },
-    );
-    const row = resolvedChain?.providers["codex/mine"];
-    expect(row?.apiKey).toBe("live-token");
-    expect(row?.models).toContain("chosen-model");
-    expect(row?.defaultModel).toBe("chosen-model");
-  });
-
-  test("dropOrphans: false skips the orphan sweep verbatim (programmatic settings load)", () => {
-    const placeholder = codexEntry();
-    const kept = applyOAuthProviderProjections(
-      settingsWith({ "codex/mine": placeholder }),
-      {},
-      { dropOrphans: false },
-    );
-    expect(kept?.providers["codex/mine"]).toBe(placeholder);
-  });
-});

src/config/oauth-catalog.test.ts

@@ -42,12 +42,11 @@ export interface ProviderSettings {
   contextWindow?: number;
   // Per-call output cap, overriding SOURCE_MAX_TOKENS when set.
   maxTokens?: number;
-  // Sampling temperature (0..2, OpenAI-compatible); the canonical sampling
-  // knob. Mutually exclusive with topP — never send both on the wire.
+  // Sampling temperature (0..2, OpenAI-compatible); mutually exclusive with
+  // topP — never send both on the wire.
   temperature?: number;
-  // Deprecated non-canonical sampling spelling; honored as-is (loads, flows
-  // to top_p) so existing files keep working. Mutually exclusive with
-  // temperature; new config should use temperature.
+  // Sampling top-p (0..1, OpenAI-compatible); mutually exclusive with
+  // temperature.
   topP?: number;
   // Bifrost virtual key (sk-bf-...): routes through the Bifrost adapter
   // (injects the x-bf-vk header) and enables /v1/models auto-discovery.

src/config/settings.ts

@@ -213,7 +213,6 @@ describe("resolveOtelExportConfig", () => {
       expect(result.config.resourceAttributes["deployment.environment"]).toBe(
         "prod",
       );
-      // Env wins wholesale: settings-only keys do not survive the merge.
       expect(result.config.resourceAttributes.team).toBeUndefined();
       expect(result.config.resourceAttributes["service.name"]).toBe(
         DEFAULT_OTEL_SERVICE_NAME,
@@ -240,9 +239,6 @@ describe("resolveOtelExportConfig", () => {
   });
 
   test("set-but-empty env resource attributes replace with nothing (regression: no settings fallback)", () => {
-    // OTEL_RESOURCE_ATTRIBUTES="" counts as set for the wholesale rule, matching
-    // the same edge on OTEL_EXPORTER_OTLP_HEADERS: the settings map is dropped,
-    // leaving only the service.name sync.
     const result = resolveOtelExportConfig(
       baseSettings({
         endpoint: "https://c.example",

src/perf/otel-config.test.ts

@@ -209,7 +209,7 @@ function validateStringMap(
 }
 
 /**
- * Precedence (env wins wholesale per field):
+ * Precedence (env over settings):
  * - endpoint: env > settings
  * - headers: env list fully replaces settings when set; else settings
  * - serviceName: env > settings.serviceName > attrs["service.name"] > default
@@ -297,9 +297,6 @@ export function resolveOtelExportConfig(
 
   let resourceAttributes: Record<string, string> = {};
   if (envResourceAttrsRaw !== undefined) {
-    // Env wins wholesale: a set OTEL_RESOURCE_ATTRIBUTES replaces settings
-    // attrs entirely, matching the headers rule. An invalid settings map
-    // never blocks a fully-specified env list.
     const parsed = parseOtelKeyValueList(
       envResourceAttrsRaw,
       OTEL_ENV.resourceAttributes,