Fix model picker loading state during initial discovery

apps/web/src/components/ChatView.tsx

@@ -67,6 +67,7 @@ import {
   providerAgentsQueryOptions,
   providerComposerCapabilitiesQueryOptions,
   providerCommandsQueryOptions,
+  isInitialModelDiscoveryPending,
   providerModelsQueryOptions,
   providerPluginsQueryOptions,
   providerSkillsQueryOptions,
@@ -1477,22 +1478,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.
+  // Gate the skeleton on the initial discovery fetch only; once discovery has
+  // settled, background refetches must not re-blank the shared model picker.
   const cursorModelDiscoveryPending =
     cursorModelDiscoveryEnabled &&
     !hasResolvedCursorModelDiscovery &&
-    cursorDynamicModelsQuery.isLoading;
+    isInitialModelDiscoveryPending(cursorDynamicModelsQuery);
   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;
+    kiloModelDiscoveryEnabled &&
+    !hasResolvedKiloModelDiscovery &&
+    isInitialModelDiscoveryPending(kiloDynamicModelsQuery);
   const modelOptionsByProvider = useMemo(() => {
     const staticOptions: Record<ProviderKind, ReturnType<typeof getAppModelOptions>> = {
       codex: getAppModelOptions(

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

@@ -1,12 +1,10 @@
-// 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 { QueryClient, QueryObserver } from "@tanstack/react-query";
 import { afterEach, describe, expect, it, vi } from "vitest";
-import { providerModelsQueryOptions } from "./providerDiscoveryReactQuery";
+import {
+  isInitialModelDiscoveryPending,
+  providerModelsQueryOptions,
+} from "./providerDiscoveryReactQuery";
 import * as nativeApi from "../nativeApi";
 
 function mockListModels(impl: (input: ProviderListModelsInput) => Promise<unknown>) {
@@ -103,3 +101,51 @@ describe("providerModelsQueryOptions", () => {
     expect(cursorKey).not.toEqual(codexKey);
   });
 });
+
+describe("isInitialModelDiscoveryPending", () => {
+  it("gates the picker during the initial fetch but not on background refetches", async () => {
+    let resolveListModels: (value: unknown) => void = () => {};
+    mockListModels(
+      () =>
+        new Promise((resolve) => {
+          resolveListModels = resolve;
+        }),
+    );
+    const discovered = {
+      models: [{ slug: "auto", name: "Auto" }],
+      source: "cursor.cli",
+      cached: false,
+    };
+
+    const queryClient = new QueryClient();
+    const observer = new QueryObserver(
+      queryClient,
+      providerModelsQueryOptions({ provider: "cursor" }),
+    );
+    const unsubscribe = observer.subscribe(() => {});
+
+    // placeholderData keeps the query in "success" status during the first
+    // fetch, so isLoading stays false — only isFetching && isPlaceholderData
+    // marks the initial discovery as pending.
+    expect(observer.getCurrentResult().isLoading).toBe(false);
+    expect(isInitialModelDiscoveryPending(observer.getCurrentResult())).toBe(true);
+
+    resolveListModels(discovered);
+    await vi.waitFor(() => {
+      expect(observer.getCurrentResult().isFetching).toBe(false);
+    });
+    expect(isInitialModelDiscoveryPending(observer.getCurrentResult())).toBe(false);
+
+    // A background refetch over already-settled data must not re-gate the picker.
+    const refetch = observer.refetch();
+    await vi.waitFor(() => {
+      expect(observer.getCurrentResult().isFetching).toBe(true);
+    });
+    expect(observer.getCurrentResult().isPlaceholderData).toBe(false);
+    expect(isInitialModelDiscoveryPending(observer.getCurrentResult())).toBe(false);
+
+    resolveListModels(discovered);
+    await refetch;
+    unsubscribe();
+  });
+});

apps/web/src/lib/providerDiscoveryReactQuery.ts

@@ -29,10 +29,8 @@ 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.
+// Resolved (not rejected) when a provider's model discovery fails, so the
+// failure stays isolated to that provider and the shared picker stays usable.
 const MODEL_DISCOVERY_ERROR_RESULT: ProviderListModelsResult = {
   models: [],
   source: "error",
@@ -176,25 +174,32 @@ export function providerModelsQueryOptions(input: {
           ...(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.
+        // Resolve instead of rejecting so one provider's failure can't blank
+        // the shared picker; the UI falls back to that provider's static models.
         console.warn(`Model discovery failed for provider "${input.provider}".`, error);
         return MODEL_DISCOVERY_ERROR_RESULT;
       }
     },
     enabled: input.enabled ?? true,
-    // Discovery failures are caught inside queryFn and surfaced as a resolved
-    // "error" result, so the query itself never rejects and never retries.
+    // queryFn already catches failures into a resolved result, so a retry would
+    // only re-run a query that never rejects.
     retry: false,
     staleTime: 60_000,
     placeholderData: (previous) => previous ?? EMPTY_MODELS_RESULT,
   });
 }
 
+// True only on a query's first fetch. placeholderData keeps the query in
+// "success" status during that fetch (isLoading never flips true), so
+// isFetching && isPlaceholderData is what separates it from later refetches.
+export function isInitialModelDiscoveryPending(query: {
+  isLoading: boolean;
+  isFetching: boolean;
+  isPlaceholderData: boolean;
+}): boolean {
+  return query.isLoading || (query.isFetching && query.isPlaceholderData);
+}
+
 export function providerAgentsQueryOptions(input: { provider: ProviderKind; enabled?: boolean }) {
   return queryOptions({
     queryKey: providerDiscoveryQueryKeys.agents(input.provider),