fix: model catalog cache + TUI editor ctor (#1326) (thanks @dougvk)

CHANGELOG.md

@@ -22,6 +22,7 @@ Docs: https://docs.clawd.bot
 - Agents: preserve subagent announce thread/topic routing + queued replies across channels. (#1241) — thanks @gnarco.
 - Agents: avoid treating timeout errors with "aborted" messages as user aborts, so model fallback still runs.
 - Diagnostics: export OTLP logs, correct queue depth tracking, and document message-flow telemetry.
+- Model catalog: avoid caching import failures, log transient discovery errors, and keep partial results. (#1332) — thanks @dougvk.
 - Doctor: clarify plugin auto-enable hint text in the startup banner.
 - Gateway: clarify unauthorized handshake responses with token/password mismatch guidance.
 - UI: keep config form enums typed, preserve empty strings, protect sensitive defaults, and deepen config search. (#1315) — thanks @MaudeBot.

src/agents/model-catalog.test.ts

@@ -0,0 +1,82 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+
+import type { ClawdbotConfig } from "../config/config.js";
+import {
+  __setModelCatalogImportForTest,
+  loadModelCatalog,
+  resetModelCatalogCacheForTest,
+} from "./model-catalog.js";
+
+type PiSdkModule = typeof import("@mariozechner/pi-coding-agent");
+
+vi.mock("./models-config.js", () => ({
+  ensureClawdbotModelsJson: vi.fn().mockResolvedValue({ agentDir: "/tmp", wrote: false }),
+}));
+
+vi.mock("./agent-paths.js", () => ({
+  resolveClawdbotAgentDir: () => "/tmp/clawdbot",
+}));
+
+describe("loadModelCatalog", () => {
+  beforeEach(() => {
+    resetModelCatalogCacheForTest();
+  });
+
+  afterEach(() => {
+    __setModelCatalogImportForTest();
+    resetModelCatalogCacheForTest();
+    vi.restoreAllMocks();
+  });
+
+  it("retries after import failure without poisoning the cache", async () => {
+    const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
+    let call = 0;
+
+    __setModelCatalogImportForTest(async () => {
+      call += 1;
+      if (call === 1) {
+        throw new Error("boom");
+      }
+      return {
+        discoverAuthStorage: () => ({}),
+        discoverModels: () => [{ id: "gpt-4.1", name: "GPT-4.1", provider: "openai" }],
+      } as unknown as PiSdkModule;
+    });
+
+    const cfg = {} as ClawdbotConfig;
+    const first = await loadModelCatalog({ config: cfg });
+    expect(first).toEqual([]);
+
+    const second = await loadModelCatalog({ config: cfg });
+    expect(second).toEqual([{ id: "gpt-4.1", name: "GPT-4.1", provider: "openai" }]);
+    expect(call).toBe(2);
+    expect(warnSpy).toHaveBeenCalledTimes(1);
+  });
+
+  it("returns partial results on discovery errors", async () => {
+    const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
+
+    __setModelCatalogImportForTest(
+      async () =>
+        ({
+          discoverAuthStorage: () => ({}),
+          discoverModels: () => ({
+            getAll: () => [
+              { id: "gpt-4.1", name: "GPT-4.1", provider: "openai" },
+              {
+                get id() {
+                  throw new Error("boom");
+                },
+                provider: "openai",
+                name: "bad",
+              },
+            ],
+          }),
+        }) as unknown as PiSdkModule,
+    );
+
+    const result = await loadModelCatalog({ config: {} as ClawdbotConfig });
+    expect(result).toEqual([{ id: "gpt-4.1", name: "GPT-4.1", provider: "openai" }]);
+    expect(warnSpy).toHaveBeenCalledTimes(1);
+  });
+});

src/agents/model-catalog.ts

@@ -18,10 +18,22 @@ type DiscoveredModel = {
   reasoning?: boolean;
 };
 
+type PiSdkModule = typeof import("@mariozechner/pi-coding-agent");
+
 let modelCatalogPromise: Promise<ModelCatalogEntry[]> | null = null;
+let hasLoggedModelCatalogError = false;
+const defaultImportPiSdk = () => import("@mariozechner/pi-coding-agent");
+let importPiSdk = defaultImportPiSdk;
 
 export function resetModelCatalogCacheForTest() {
   modelCatalogPromise = null;
+  hasLoggedModelCatalogError = false;
+  importPiSdk = defaultImportPiSdk;
+}
+
+// Test-only escape hatch: allow mocking the dynamic import to simulate transient failures.
+export function __setModelCatalogImportForTest(loader?: () => Promise<PiSdkModule>) {
+  importPiSdk = loader ?? defaultImportPiSdk;
 }
 
 export async function loadModelCatalog(params?: {
@@ -34,16 +46,21 @@ export async function loadModelCatalog(params?: {
   if (modelCatalogPromise) return modelCatalogPromise;
 
   modelCatalogPromise = (async () => {
+    const models: ModelCatalogEntry[] = [];
+    const sortModels = (entries: ModelCatalogEntry[]) =>
+      entries.sort((a, b) => {
+        const p = a.provider.localeCompare(b.provider);
+        if (p !== 0) return p;
+        return a.name.localeCompare(b.name);
+      });
     try {
+      const cfg = params?.config ?? loadConfig();
+      await ensureClawdbotModelsJson(cfg);
       // IMPORTANT: keep the dynamic import *inside* the try/catch.
       // If this fails once (e.g. during a pnpm install that temporarily swaps node_modules),
       // we must not poison the cache with a rejected promise (otherwise all channel handlers
       // will keep failing until restart).
-      const piSdk = await import("@mariozechner/pi-coding-agent");
+      const piSdk = await importPiSdk();
-
-      const models: ModelCatalogEntry[] = [];
-      const cfg = params?.config ?? loadConfig();
-      await ensureClawdbotModelsJson(cfg);
       const agentDir = resolveClawdbotAgentDir();
       const authStorage = piSdk.discoverAuthStorage(agentDir);
       const registry = piSdk.discoverModels(authStorage, agentDir) as
@@ -71,14 +88,17 @@ export async function loadModelCatalog(params?: {
         modelCatalogPromise = null;
       }
 
-      return models.sort((a, b) => {
+      return sortModels(models);
-        const p = a.provider.localeCompare(b.provider);
+    } catch (error) {
-        if (p !== 0) return p;
+      if (!hasLoggedModelCatalogError) {
-        return a.name.localeCompare(b.name);
+        hasLoggedModelCatalogError = true;
-      });
+        console.warn(`[model-catalog] Failed to load model catalog: ${String(error)}`);
-    } catch {
+      }
       // Don't poison the cache on transient dependency/filesystem issues.
       modelCatalogPromise = null;
+      if (models.length > 0) {
+        return sortModels(models);
+      }
       return [];
     }
   })();