diff --git a/CHANGELOG.md b/CHANGELOG.md index b3e5981a3..465b8e221 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ Docs: https://docs.clawd.bot - Discovery: shorten Bonjour DNS-SD service type to `_clawdbot-gw._tcp` and update discovery clients/docs. - 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. +- Model catalog: avoid caching import failures, log transient discovery errors, and keep partial results. (#1326) — thanks @dougvk. - Doctor: clarify plugin auto-enable hint text in the startup banner. - Gateway: clarify unauthorized handshake responses with token/password mismatch guidance. - Web search: infer Perplexity base URL from API key source (direct vs OpenRouter). diff --git a/src/agents/model-catalog.test.ts b/src/agents/model-catalog.test.ts new file mode 100644 index 000000000..2a6d97934 --- /dev/null +++ b/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); + }); +}); diff --git a/src/agents/model-catalog.ts b/src/agents/model-catalog.ts index 8043eb9b6..3a3db7a28 100644 --- a/src/agents/model-catalog.ts +++ b/src/agents/model-catalog.ts @@ -18,10 +18,22 @@ type DiscoveredModel = { reasoning?: boolean; }; +type PiSdkModule = typeof import("@mariozechner/pi-coding-agent"); + let modelCatalogPromise: Promise | 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) { + 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 models: ModelCatalogEntry[] = []; - const cfg = params?.config ?? loadConfig(); - await ensureClawdbotModelsJson(cfg); + const piSdk = await importPiSdk(); 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) => { - const p = a.provider.localeCompare(b.provider); - if (p !== 0) return p; - return a.name.localeCompare(b.name); - }); - } catch { + return sortModels(models); + } catch (error) { + if (!hasLoggedModelCatalogError) { + hasLoggedModelCatalogError = true; + console.warn(`[model-catalog] Failed to load model catalog: ${String(error)}`); + } // Don't poison the cache on transient dependency/filesystem issues. modelCatalogPromise = null; + if (models.length > 0) { + return sortModels(models); + } return []; } })(); diff --git a/src/tui/components/custom-editor.ts b/src/tui/components/custom-editor.ts index b66452e61..80feb4145 100644 --- a/src/tui/components/custom-editor.ts +++ b/src/tui/components/custom-editor.ts @@ -1,4 +1,4 @@ -import { Editor, type EditorTheme, Key, matchesKey } from "@mariozechner/pi-tui"; +import { Editor, type EditorTheme, Key, matchesKey, type TUI } from "@mariozechner/pi-tui"; export class CustomEditor extends Editor { onEscape?: () => void; @@ -12,8 +12,8 @@ export class CustomEditor extends Editor { onShiftTab?: () => void; onAltEnter?: () => void; - constructor(theme: EditorTheme) { - super(theme); + constructor(tui: TUI, theme: EditorTheme) { + super(tui, theme); } handleInput(data: string): void { if (matchesKey(data, Key.alt("enter")) && this.onAltEnter) { diff --git a/src/tui/tui.ts b/src/tui/tui.ts index 753f5511f..a5e6e34d7 100644 --- a/src/tui/tui.ts +++ b/src/tui/tui.ts @@ -193,7 +193,7 @@ export async function runTui(opts: TuiOptions) { const statusContainer = new Container(); const footer = new Text("", 1, 0); const chatLog = new ChatLog(); - const editor = new CustomEditor(editorTheme); + const editor = new CustomEditor(tui, editorTheme); const root = new Container(); root.addChild(header); root.addChild(chatLog);