Merge pull request #1297 from corbitsdev/feat/remove-provider-1294

feat(providers): remove connected provider from model picker

Author: TheGreatAxios

docs/TUI.md

@@ -568,7 +568,12 @@ pair as the default (global `defaultProvider` + that provider's `defaultModel`
 
 - project-local selection) without switching the live session or closing the
   picker. Alt+F on a model row
-  still toggles favorite when a favorite hook is wired. While type-to-filter is
+  still toggles favorite when a favorite hook is wired. **Alt+R** on a model
+  row removes that whole provider after a two-press armed confirm: the first
+  press names what will be forgotten (catalog entry, stored key or auth
+  profile, and default repair), the second press on the same row deletes it;
+  Esc, moving focus, or the arm timeout cancels. The live session's own
+  provider is refused rather than orphaned. While type-to-filter is
   active, bare `j`/`k` type into the filter rather than moving the highlight —
   use arrow keys (or the filtered list's navigation) to move.
 

src/auth/remove-provider.test.ts

@@ -0,0 +1,138 @@
+import { mkdtemp, rm } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { afterEach, describe, expect, test } from "bun:test";
+
+import {
+  listCodexProfiles,
+  loadCodexProfile,
+  saveCodexProfile,
+} from "../config/oauth-stores.js";
+import {
+  listXaiProfiles,
+  loadXaiProfile,
+  saveXaiProfile,
+} from "../config/oauth-stores.js";
+import {
+  clearSourceCredentials,
+  registerSourceCredentialRecord,
+} from "../config/source-credentials.js";
+import { oauthStoreForProvider } from "./remove-provider.js";
+
+function registerOAuthProvider(
+  providerName: string,
+  provider: "codex" | "xai",
+  profile: string,
+): void {
+  registerSourceCredentialRecord(providerName, {
+    provenance: { kind: "oauth", provider, profile },
+    material: { secret: "token" },
+  });
+}
+
+async function withHome<T>(fn: (home: string) => Promise<T>): Promise<T> {
+  const home = await mkdtemp(join(tmpdir(), "remove-provider-"));
+  try {
+    return await fn(home);
+  } finally {
+    await rm(home, { recursive: true, force: true });
+  }
+}
+
+describe("oauthStoreForProvider", () => {
+  afterEach(() => {
+    clearSourceCredentials();
+  });
+
+  test("maps xai/ and codex/ provider names to their store and profile", () => {
+    clearSourceCredentials();
+    registerOAuthProvider("xai/work", "xai", "work");
+    registerOAuthProvider("codex/personal", "codex", "personal");
+    expect(oauthStoreForProvider("xai/work")?.profile).toBe("work");
+    expect(oauthStoreForProvider("codex/personal")?.profile).toBe("personal");
+  });
+
+  test("rejects namespaced custom providers without OAuth ownership", () => {
+    clearSourceCredentials();
+    registerSourceCredentialRecord("codex/manual", {
+      provenance: { kind: "api-key" },
+      material: { secret: "manual-key" },
+    });
+    registerSourceCredentialRecord("xai/manual", {
+      provenance: { kind: "api-key" },
+      material: { secret: "manual-key" },
+    });
+
+    expect(oauthStoreForProvider("codex/manual")).toBeNull();
+    expect(oauthStoreForProvider("xai/manual")).toBeNull();
+  });
+
+  test("returns null for API-key, keyless, and degenerate names", () => {
+    expect(oauthStoreForProvider("openai")).toBeNull();
+    expect(oauthStoreForProvider("ollama")).toBeNull();
+    expect(oauthStoreForProvider("my-custom")).toBeNull();
+    expect(oauthStoreForProvider("xai/")).toBeNull();
+    expect(oauthStoreForProvider("codex/")).toBeNull();
+  });
+
+  test("removing an xai profile leaves sibling profiles untouched", async () => {
+    await withHome(async (home) => {
+      clearSourceCredentials();
+      registerOAuthProvider("xai/work", "xai", "work");
+      const tokens = { access: "a", refresh: "r", expiresAt: 10_000_000 };
+      await saveXaiProfile({ name: "work", createdAt: 0, tokens }, home);
+      await saveXaiProfile({ name: "personal", createdAt: 0, tokens }, home);
+
+      const target = oauthStoreForProvider("xai/work");
+      expect(target?.profile).toBe("work");
+      expect(await target?.removeProfile(target.profile, home)).toEqual([
+        "work",
+      ]);
+      expect(await loadXaiProfile("work", home)).toBeUndefined();
+      expect(
+        (await listXaiProfiles(home)).map((profile) => profile.name),
+      ).toEqual(["personal"]);
+    });
+  });
+
+  test("removing a codex profile leaves sibling profiles untouched", async () => {
+    await withHome(async (home) => {
+      clearSourceCredentials();
+      registerOAuthProvider("codex/night", "codex", "night");
+      const tokens = { access: "a", refresh: "r", expiresAt: 10_000_000 };
+      await saveCodexProfile({ name: "day", createdAt: 0, tokens }, home);
+      await saveCodexProfile({ name: "night", createdAt: 0, tokens }, home);
+
+      const target = oauthStoreForProvider("codex/night");
+      expect(await target?.removeProfile(target.profile, home)).toEqual([
+        "night",
+      ]);
+      expect(await loadCodexProfile("night", home)).toBeUndefined();
+      expect(
+        (await listCodexProfiles(home)).map((profile) => profile.name),
+      ).toEqual(["day"]);
+    });
+  });
+
+  test("removing an already-absent profile is a no-op", async () => {
+    await withHome(async (home) => {
+      clearSourceCredentials();
+      registerOAuthProvider("xai/ghost", "xai", "ghost");
+      const target = oauthStoreForProvider("xai/ghost");
+      expect(await target?.removeProfile(target.profile, home)).toEqual([]);
+    });
+  });
+
+  test("missing auth files are tolerated", async () => {
+    await withHome(async (home) => {
+      clearSourceCredentials();
+      registerOAuthProvider("xai/work", "xai", "work");
+      registerOAuthProvider("codex/work", "codex", "work");
+      // No profiles ever saved: the store treats a missing file as empty.
+      const xai = oauthStoreForProvider("xai/work");
+      expect(await xai?.removeProfile(xai.profile, home)).toEqual([]);
+      const codex = oauthStoreForProvider("codex/work");
+      expect(await codex?.removeProfile(codex.profile, home)).toEqual([]);
+    });
+  });
+});

src/auth/remove-provider.ts

@@ -0,0 +1,49 @@
+import { codexProfileFromProviderName } from "../config/codex-providers.js";
+import {
+  removeCodexProfile,
+  removeXaiProfile,
+} from "../config/oauth-stores.js";
+import { findSourceCredentialRecord } from "../config/source-credentials.js";
+import { xaiProfileFromProviderName } from "../config/xai-providers.js";
+
+/**
+ * The auth-store side of removing an OAuth-projected provider. Owns no UI
+ * and performs no settings edits — the caller deletes the catalog row and
+ * only then drops the credential through this target.
+ */
+export interface OAuthStoreTarget {
+  /** Profile name embedded in the "<prefix><profile>" provider name. */
+  readonly profile: string;
+  /** Bound store removal; sibling profiles in the same file are untouched. */
+  readonly removeProfile: (
+    name: string | undefined,
+    home?: string,
+  ) => Promise<string[]>;
+}
+
+// Maps a catalog provider to its OAuth auth store only when the live source
+// credential record proves ownership of the same provider family and profile.
+// Names alone are user-controlled and cannot authorize credential deletion.
+export function oauthStoreForProvider(
+  providerName: string,
+): OAuthStoreTarget | null {
+  const provenance = findSourceCredentialRecord(providerName)?.provenance;
+  if (provenance?.kind !== "oauth") return null;
+  const codexProfile = codexProfileFromProviderName(providerName);
+  if (
+    provenance.provider === "codex" &&
+    codexProfile === provenance.profile &&
+    codexProfile.length > 0
+  ) {
+    return { profile: codexProfile, removeProfile: removeCodexProfile };
+  }
+  const xaiProfile = xaiProfileFromProviderName(providerName);
+  if (
+    provenance.provider === "xai" &&
+    xaiProfile === provenance.profile &&
+    xaiProfile.length > 0
+  ) {
+    return { profile: xaiProfile, removeProfile: removeXaiProfile };
+  }
+  return null;
+}

src/config/settings.ts

@@ -285,6 +285,86 @@ export function listFavoriteModels(settings: Settings): ModelRef[] {
   return settings.favoriteModels ?? [];
 }
 
+// Removal report for removeProviderFromSettings, so the caller can build a
+// truthful notice without re-deriving what changed.
+export interface ProviderRemovalRepair {
+  /** True when defaultProvider pointed at the removed provider. */
+  removedDefault: boolean;
+  /** Repointed default; absent when the default was unset instead. */
+  newDefaultProvider?: string;
+  droppedRecents: number;
+  droppedFavorites: number;
+}
+
+// Delete one provider catalog entry plus every reference to it (recents,
+// favorites, default). Unknown names are a no-op returning the input
+// unchanged. Default repair is deterministic: the live session's provider if
+// still configured, else the newest surviving recent whose provider still
+// exists, else the sole remaining provider, else unset (valid — optional).
+// Pure: no disk I/O, no auth-store knowledge.
+export function removeProviderFromSettings(
+  settings: Settings,
+  providerName: string,
+  liveProvider?: string,
+): {
+  settings: Settings;
+  removed: boolean;
+  repair: ProviderRemovalRepair;
+} {
+  const emptyRepair: ProviderRemovalRepair = {
+    removedDefault: false,
+    droppedRecents: 0,
+    droppedFavorites: 0,
+  };
+  if (settings.providers[providerName] === undefined) {
+    return { settings, removed: false, repair: emptyRepair };
+  }
+  const remaining = Object.fromEntries(
+    Object.entries(settings.providers).filter(
+      ([name]) => name !== providerName,
+    ),
+  );
+  const recents = settings.recentModels ?? [];
+  const keptRecents = recents.filter((r) => r.provider !== providerName);
+  const favorites = settings.favoriteModels ?? [];
+  const keptFavorites = favorites.filter((r) => r.provider !== providerName);
+  let next: Settings = {
+    ...settings,
+    providers: remaining,
+    ...(settings.recentModels !== undefined
+      ? { recentModels: keptRecents }
+      : {}),
+    ...(settings.favoriteModels !== undefined
+      ? { favoriteModels: keptFavorites }
+      : {}),
+  };
+  const repair: ProviderRemovalRepair = {
+    removedDefault: settings.defaultProvider === providerName,
+    droppedRecents: recents.length - keptRecents.length,
+    droppedFavorites: favorites.length - keptFavorites.length,
+  };
+  if (settings.defaultProvider === providerName) {
+    const remainingNames = Object.keys(remaining);
+    const newestSurvivor = keptRecents.find(
+      (r) => remaining[r.provider] !== undefined,
+    )?.provider;
+    const nextDefault =
+      liveProvider !== undefined && remaining[liveProvider] !== undefined
+        ? liveProvider
+        : (newestSurvivor ??
+          (remainingNames.length === 1 ? remainingNames[0] : undefined));
+    if (nextDefault === undefined) {
+      const { defaultProvider: _dropped, ...withoutDefault } = next;
+      void _dropped;
+      next = withoutDefault;
+    } else {
+      next = { ...next, defaultProvider: nextDefault };
+      repair.newDefaultProvider = nextDefault;
+    }
+  }
+  return { settings: next, removed: true, repair };
+}
+
 // Maps the settings shell block to the shape the shell-guard plugin expects.
 // Returns undefined when unset so the plugin applies the 120s foreground
 // default itself. timeoutMs overrides that default; maxTimeoutMs clamps the