Skip to content

Commit 1cd4429

Browse files
committed
Fix model picker breaking when Cursor CLI is unavailable
Model discovery runs per provider, but a failing CLI rejected the whole query and left the shared model picker stuck in a loading/error state instead of degrading only that provider. - Catch discovery failures in providerModelsQueryOptions and resolve to an empty result, so one provider's failure no longer blanks the picker. Every other provider keeps its own models. - Gate the Cursor/Kilo discovery skeleton on isLoading so background refetches don't re-blank the picker once discovery has settled. - Add coverage proving a single provider failure doesn't affect the others, and fix a missing kilo key in an existing test fixture. Fixes #103
1 parent 29fc3cf commit 1cd4429

4 files changed

Lines changed: 142 additions & 11 deletions

File tree

apps/web/src/components/ChatView.tsx

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1477,20 +1477,22 @@ export default function ChatView({
14771477
const hasResolvedCursorModelDiscovery =
14781478
cursorDynamicModelsQuery.data?.source === "cursor.cli" &&
14791479
(cursorDynamicModelsQuery.data.models.length ?? 0) > 0;
1480+
// Only the very first discovery attempt (no settled result yet) should gate
1481+
// the picker. Once discovery resolves — including a fault-isolated "error"
1482+
// result when the Cursor CLI is unavailable — background refetches must not
1483+
// re-blank the shared model picker.
14801484
const cursorModelDiscoveryPending =
14811485
cursorModelDiscoveryEnabled &&
14821486
!hasResolvedCursorModelDiscovery &&
1483-
(cursorDynamicModelsQuery.isLoading || cursorDynamicModelsQuery.isFetching);
1487+
cursorDynamicModelsQuery.isLoading;
14841488
const kiloModelDiscoveryEnabled =
14851489
selectedProvider === "kilo" || lockedProvider === "kilo" || isModelPickerOpen;
14861490
const hasResolvedKiloModelDiscovery =
14871491
(kiloDynamicModelsQuery.data?.source === "kilo-cli" ||
14881492
kiloDynamicModelsQuery.data?.source === "kilo") &&
14891493
(kiloDynamicModelsQuery.data.models.length ?? 0) > 0;
14901494
const kiloModelDiscoveryPending =
1491-
kiloModelDiscoveryEnabled &&
1492-
!hasResolvedKiloModelDiscovery &&
1493-
(kiloDynamicModelsQuery.isLoading || kiloDynamicModelsQuery.isFetching);
1495+
kiloModelDiscoveryEnabled && !hasResolvedKiloModelDiscovery && kiloDynamicModelsQuery.isLoading;
14941496
const modelOptionsByProvider = useMemo(() => {
14951497
const staticOptions: Record<ProviderKind, ReturnType<typeof getAppModelOptions>> = {
14961498
codex: getAppModelOptions(

apps/web/src/composerDraftStore.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1204,6 +1204,7 @@ describe("composerDraftStore modelSelection", () => {
12041204
claudeAgent: [],
12051205
cursor: [],
12061206
gemini: [],
1207+
kilo: [],
12071208
opencode: [],
12081209
pi: [],
12091210
},
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
// FILE: providerDiscoveryReactQuery.test.ts
2+
// Purpose: Verifies per-provider model discovery stays fault-isolated.
3+
// Layer: Web data fetching tests
4+
// Depends on: Vitest, React Query, and the native API bridge mock.
5+
6+
import type { NativeApi, ProviderKind, ProviderListModelsInput } from "@t3tools/contracts";
7+
import { QueryClient } from "@tanstack/react-query";
8+
import { afterEach, describe, expect, it, vi } from "vitest";
9+
import { providerModelsQueryOptions } from "./providerDiscoveryReactQuery";
10+
import * as nativeApi from "../nativeApi";
11+
12+
function mockListModels(impl: (input: ProviderListModelsInput) => Promise<unknown>) {
13+
const listModels = vi.fn(impl);
14+
vi.spyOn(nativeApi, "ensureNativeApi").mockReturnValue({
15+
provider: { listModels },
16+
} as unknown as NativeApi);
17+
return listModels;
18+
}
19+
20+
afterEach(() => {
21+
vi.restoreAllMocks();
22+
});
23+
24+
describe("providerModelsQueryOptions", () => {
25+
it("degrades to an empty 'error' result when a provider's discovery fails", async () => {
26+
vi.spyOn(console, "warn").mockImplementation(() => {});
27+
mockListModels(async () => {
28+
throw new Error("Cursor CLI is not installed.");
29+
});
30+
31+
const queryClient = new QueryClient();
32+
const result = await queryClient.fetchQuery(providerModelsQueryOptions({ provider: "cursor" }));
33+
34+
expect(result).toEqual({ models: [], source: "error", cached: false });
35+
});
36+
37+
it("keeps other providers' models when one provider's discovery fails", async () => {
38+
vi.spyOn(console, "warn").mockImplementation(() => {});
39+
const codexModels = [{ slug: "gpt-5-codex", name: "GPT-5 Codex" }];
40+
mockListModels(async ({ provider }) => {
41+
if (provider === "cursor") {
42+
throw new Error("Cursor CLI is not authenticated.");
43+
}
44+
return { models: codexModels, source: "codex-app-server", cached: false };
45+
});
46+
47+
const queryClient = new QueryClient();
48+
const [cursorResult, codexResult] = await Promise.all([
49+
queryClient.fetchQuery(providerModelsQueryOptions({ provider: "cursor" })),
50+
queryClient.fetchQuery(providerModelsQueryOptions({ provider: "codex" })),
51+
]);
52+
53+
// The failing provider degrades on its own...
54+
expect(cursorResult.models).toEqual([]);
55+
expect(cursorResult.source).toBe("error");
56+
// ...while every other provider keeps its discovered models.
57+
expect(codexResult.models).toEqual(codexModels);
58+
expect(codexResult.source).toBe("codex-app-server");
59+
});
60+
61+
it("never rejects, so a failing CLI cannot blank the shared model picker", async () => {
62+
vi.spyOn(console, "warn").mockImplementation(() => {});
63+
mockListModels(async () => {
64+
throw new Error("Timed out while discovering Cursor models via CLI.");
65+
});
66+
67+
const queryClient = new QueryClient();
68+
await expect(
69+
queryClient.fetchQuery(providerModelsQueryOptions({ provider: "cursor" })),
70+
).resolves.toMatchObject({ source: "error" });
71+
72+
expect(providerModelsQueryOptions({ provider: "cursor" }).retry).toBe(false);
73+
});
74+
75+
it("forwards optional discovery inputs and returns discovered models on success", async () => {
76+
const listModels = mockListModels(async () => ({
77+
models: [{ slug: "auto", name: "Auto" }],
78+
source: "cursor.cli",
79+
cached: false,
80+
}));
81+
82+
const queryClient = new QueryClient();
83+
const result = await queryClient.fetchQuery(
84+
providerModelsQueryOptions({
85+
provider: "cursor",
86+
binaryPath: "/usr/bin/cursor-agent",
87+
apiEndpoint: "https://example.test",
88+
}),
89+
);
90+
91+
expect(listModels).toHaveBeenCalledWith({
92+
provider: "cursor",
93+
binaryPath: "/usr/bin/cursor-agent",
94+
apiEndpoint: "https://example.test",
95+
});
96+
expect(result.source).toBe("cursor.cli");
97+
expect(result.models).toEqual([{ slug: "auto", name: "Auto" }]);
98+
});
99+
100+
it("scopes query keys per provider so discovery results never collide", () => {
101+
const cursorKey = providerModelsQueryOptions({ provider: "cursor" as ProviderKind }).queryKey;
102+
const codexKey = providerModelsQueryOptions({ provider: "codex" as ProviderKind }).queryKey;
103+
expect(cursorKey).not.toEqual(codexKey);
104+
});
105+
});

apps/web/src/lib/providerDiscoveryReactQuery.ts

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,16 @@ const EMPTY_MODELS_RESULT: ProviderListModelsResult = {
2929
cached: false,
3030
};
3131

32+
// Returned when a single provider's model discovery fails (e.g. the Cursor CLI
33+
// is not installed or not authenticated). Resolving to this instead of
34+
// rejecting keeps the failure isolated to that provider so the shared model
35+
// picker — and every other provider's models — stays usable.
36+
const MODEL_DISCOVERY_ERROR_RESULT: ProviderListModelsResult = {
37+
models: [],
38+
source: "error",
39+
cached: false,
40+
};
41+
3242
const EMPTY_AGENTS_RESULT: ProviderListAgentsResult = {
3343
agents: [],
3444
source: "empty",
@@ -158,15 +168,28 @@ export function providerModelsQueryOptions(input: {
158168
),
159169
queryFn: async () => {
160170
const api = ensureNativeApi();
161-
return api.provider.listModels({
162-
provider: input.provider,
163-
...(input.binaryPath ? { binaryPath: input.binaryPath } : {}),
164-
...(input.apiEndpoint ? { apiEndpoint: input.apiEndpoint } : {}),
165-
...(input.agentDir ? { agentDir: input.agentDir } : {}),
166-
});
171+
try {
172+
return await api.provider.listModels({
173+
provider: input.provider,
174+
...(input.binaryPath ? { binaryPath: input.binaryPath } : {}),
175+
...(input.apiEndpoint ? { apiEndpoint: input.apiEndpoint } : {}),
176+
...(input.agentDir ? { agentDir: input.agentDir } : {}),
177+
});
178+
} catch (error) {
179+
// Fault isolation: model discovery runs per provider, so one failing
180+
// CLI must degrade only that provider. Rejecting here would put the
181+
// query in an error state and blank the shared model picker; instead
182+
// we resolve to an empty "error" result and let the UI fall back to
183+
// that provider's static models while every other provider is
184+
// unaffected.
185+
console.warn(`Model discovery failed for provider "${input.provider}".`, error);
186+
return MODEL_DISCOVERY_ERROR_RESULT;
187+
}
167188
},
168189
enabled: input.enabled ?? true,
169-
retry: input.provider === "cursor" ? 1 : 3,
190+
// Discovery failures are caught inside queryFn and surfaced as a resolved
191+
// "error" result, so the query itself never rejects and never retries.
192+
retry: false,
170193
staleTime: 60_000,
171194
placeholderData: (previous) => previous ?? EMPTY_MODELS_RESULT,
172195
});

0 commit comments

Comments
 (0)