Merge pull request #117 from Zeus-Deus/feature/103-bug-retry-models-when-cursor-cli-is-not-available

Fix model picker breaking when Cursor CLI is unavailable

Author: Emanuele-web04

apps/web/src/components/ChatView.tsx

@@ -1477,20 +1477,22 @@ export default function ChatView({
   const hasResolvedCursorModelDiscovery =
     cursorDynamicModelsQuery.data?.source === "cursor.cli" &&
     (cursorDynamicModelsQuery.data.models.length ?? 0) > 0;
+  // Only the very first discovery attempt (no settled result yet) should gate
+  // the picker. Once discovery resolves — including a fault-isolated "error"
+  // result when the Cursor CLI is unavailable — background refetches must not
+  // re-blank the shared model picker.
   const cursorModelDiscoveryPending =
     cursorModelDiscoveryEnabled &&
     !hasResolvedCursorModelDiscovery &&
-    (cursorDynamicModelsQuery.isLoading || cursorDynamicModelsQuery.isFetching);
+    cursorDynamicModelsQuery.isLoading;
   const kiloModelDiscoveryEnabled =
     selectedProvider === "kilo" || lockedProvider === "kilo" || isModelPickerOpen;
   const hasResolvedKiloModelDiscovery =
     (kiloDynamicModelsQuery.data?.source === "kilo-cli" ||
       kiloDynamicModelsQuery.data?.source === "kilo") &&
     (kiloDynamicModelsQuery.data.models.length ?? 0) > 0;
   const kiloModelDiscoveryPending =
-    kiloModelDiscoveryEnabled &&
-    !hasResolvedKiloModelDiscovery &&
-    (kiloDynamicModelsQuery.isLoading || kiloDynamicModelsQuery.isFetching);
+    kiloModelDiscoveryEnabled && !hasResolvedKiloModelDiscovery && kiloDynamicModelsQuery.isLoading;
   const modelOptionsByProvider = useMemo(() => {
     const staticOptions: Record<ProviderKind, ReturnType<typeof getAppModelOptions>> = {
       codex: getAppModelOptions(

apps/web/src/composerDraftStore.test.ts

@@ -1204,6 +1204,7 @@ describe("composerDraftStore modelSelection", () => {
         claudeAgent: [],
         cursor: [],
         gemini: [],
+        kilo: [],
         opencode: [],
         pi: [],
       },

apps/web/src/lib/providerDiscoveryReactQuery.test.ts

@@ -0,0 +1,105 @@
+// FILE: providerDiscoveryReactQuery.test.ts
+// Purpose: Verifies per-provider model discovery stays fault-isolated.
+// Layer: Web data fetching tests
+// Depends on: Vitest, React Query, and the native API bridge mock.
+
+import type { NativeApi, ProviderKind, ProviderListModelsInput } from "@t3tools/contracts";
+import { QueryClient } from "@tanstack/react-query";
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { providerModelsQueryOptions } from "./providerDiscoveryReactQuery";
+import * as nativeApi from "../nativeApi";
+
+function mockListModels(impl: (input: ProviderListModelsInput) => Promise<unknown>) {
+  const listModels = vi.fn(impl);
+  vi.spyOn(nativeApi, "ensureNativeApi").mockReturnValue({
+    provider: { listModels },
+  } as unknown as NativeApi);
+  return listModels;
+}
+
+afterEach(() => {
+  vi.restoreAllMocks();
+});
+
+describe("providerModelsQueryOptions", () => {
+  it("degrades to an empty 'error' result when a provider's discovery fails", async () => {
+    vi.spyOn(console, "warn").mockImplementation(() => {});
+    mockListModels(async () => {
+      throw new Error("Cursor CLI is not installed.");
+    });
+
+    const queryClient = new QueryClient();
+    const result = await queryClient.fetchQuery(providerModelsQueryOptions({ provider: "cursor" }));
+
+    expect(result).toEqual({ models: [], source: "error", cached: false });
+  });
+
+  it("keeps other providers' models when one provider's discovery fails", async () => {
+    vi.spyOn(console, "warn").mockImplementation(() => {});
+    const codexModels = [{ slug: "gpt-5-codex", name: "GPT-5 Codex" }];
+    mockListModels(async ({ provider }) => {
+      if (provider === "cursor") {
+        throw new Error("Cursor CLI is not authenticated.");
+      }
+      return { models: codexModels, source: "codex-app-server", cached: false };
+    });
+
+    const queryClient = new QueryClient();
+    const [cursorResult, codexResult] = await Promise.all([
+      queryClient.fetchQuery(providerModelsQueryOptions({ provider: "cursor" })),
+      queryClient.fetchQuery(providerModelsQueryOptions({ provider: "codex" })),
+    ]);
+
+    // The failing provider degrades on its own...
+    expect(cursorResult.models).toEqual([]);
+    expect(cursorResult.source).toBe("error");
+    // ...while every other provider keeps its discovered models.
+    expect(codexResult.models).toEqual(codexModels);
+    expect(codexResult.source).toBe("codex-app-server");
+  });
+
+  it("never rejects, so a failing CLI cannot blank the shared model picker", async () => {
+    vi.spyOn(console, "warn").mockImplementation(() => {});
+    mockListModels(async () => {
+      throw new Error("Timed out while discovering Cursor models via CLI.");
+    });
+
+    const queryClient = new QueryClient();
+    await expect(
+      queryClient.fetchQuery(providerModelsQueryOptions({ provider: "cursor" })),
+    ).resolves.toMatchObject({ source: "error" });
+
+    expect(providerModelsQueryOptions({ provider: "cursor" }).retry).toBe(false);
+  });
+
+  it("forwards optional discovery inputs and returns discovered models on success", async () => {
+    const listModels = mockListModels(async () => ({
+      models: [{ slug: "auto", name: "Auto" }],
+      source: "cursor.cli",
+      cached: false,
+    }));
+
+    const queryClient = new QueryClient();
+    const result = await queryClient.fetchQuery(
+      providerModelsQueryOptions({
+        provider: "cursor",
+        binaryPath: "/usr/bin/cursor-agent",
+        apiEndpoint: "https://example.test",
+      }),
+    );
+
+    expect(listModels).toHaveBeenCalledWith({
+      provider: "cursor",
+      binaryPath: "/usr/bin/cursor-agent",
+      apiEndpoint: "https://example.test",
+    });
+    expect(result.source).toBe("cursor.cli");
+    expect(result.models).toEqual([{ slug: "auto", name: "Auto" }]);
+  });
+
+  it("scopes query keys per provider so discovery results never collide", () => {
+    const cursorKey = providerModelsQueryOptions({ provider: "cursor" as ProviderKind }).queryKey;
+    const codexKey = providerModelsQueryOptions({ provider: "codex" as ProviderKind }).queryKey;
+    expect(cursorKey).not.toEqual(codexKey);
+  });
+});

apps/web/src/lib/providerDiscoveryReactQuery.ts

@@ -29,6 +29,16 @@ const EMPTY_MODELS_RESULT: ProviderListModelsResult = {
   cached: false,
 };
 
+// Returned when a single provider's model discovery fails (e.g. the Cursor CLI
+// is not installed or not authenticated). Resolving to this instead of
+// rejecting keeps the failure isolated to that provider so the shared model
+// picker — and every other provider's models — stays usable.
+const MODEL_DISCOVERY_ERROR_RESULT: ProviderListModelsResult = {
+  models: [],
+  source: "error",
+  cached: false,
+};
+
 const EMPTY_AGENTS_RESULT: ProviderListAgentsResult = {
   agents: [],
   source: "empty",
@@ -158,15 +168,28 @@ export function providerModelsQueryOptions(input: {
     ),
     queryFn: async () => {
       const api = ensureNativeApi();
-      return api.provider.listModels({
-        provider: input.provider,
-        ...(input.binaryPath ? { binaryPath: input.binaryPath } : {}),
-        ...(input.apiEndpoint ? { apiEndpoint: input.apiEndpoint } : {}),
-        ...(input.agentDir ? { agentDir: input.agentDir } : {}),
-      });
+      try {
+        return await api.provider.listModels({
+          provider: input.provider,
+          ...(input.binaryPath ? { binaryPath: input.binaryPath } : {}),
+          ...(input.apiEndpoint ? { apiEndpoint: input.apiEndpoint } : {}),
+          ...(input.agentDir ? { agentDir: input.agentDir } : {}),
+        });
+      } catch (error) {
+        // Fault isolation: model discovery runs per provider, so one failing
+        // CLI must degrade only that provider. Rejecting here would put the
+        // query in an error state and blank the shared model picker; instead
+        // we resolve to an empty "error" result and let the UI fall back to
+        // that provider's static models while every other provider is
+        // unaffected.
+        console.warn(`Model discovery failed for provider "${input.provider}".`, error);
+        return MODEL_DISCOVERY_ERROR_RESULT;
+      }
     },
     enabled: input.enabled ?? true,
-    retry: input.provider === "cursor" ? 1 : 3,
+    // Discovery failures are caught inside queryFn and surfaced as a resolved
+    // "error" result, so the query itself never rejects and never retries.
+    retry: false,
     staleTime: 60_000,
     placeholderData: (previous) => previous ?? EMPTY_MODELS_RESULT,
   });