fix(v2): avoid empty catalog during discovery refresh (refs #159)

src/v2/index.ts

@@ -169,16 +169,20 @@ export async function setupV2(ctx: Plugin.Context): Promise<() => void> {
   let activeRefresh: Promise<RefreshResult> | undefined
   let pendingRefreshForce = false
 
-  const refreshFromCurrentConfig = (force?: boolean): Promise<RefreshResult> => {
+  const refreshFromCurrentConfig = (force?: boolean, reloadProviderConfig = false): Promise<RefreshResult> => {
     if (force) pendingRefreshForce = true
     if (activeRefresh) {
-      return activeRefresh.then(() => refreshFromCurrentConfig(pendingRefreshForce))
+      return activeRefresh.then(() => refreshFromCurrentConfig(pendingRefreshForce, reloadProviderConfig))
     }
     const runForce = force || pendingRefreshForce
     pendingRefreshForce = false
     activeRefresh = (async () => {
       try {
-        await ctx.provider.reload()
+        // Configuration events update the host provider registry asynchronously, so
+        // reload it before listing providers. A normal discovery refresh must not do
+        // this first: the host can rebuild its catalog with an empty inventory while
+        // the new models request is still in flight, causing a transient 404.
+        if (reloadProviderConfig) await ctx.provider.reload()
         await syncConfiguredProviders()
         return await refreshInventoryInternal(runForce)
       } finally {
@@ -447,7 +451,7 @@ export async function setupV2(ctx: Plugin.Context): Promise<() => void> {
             // Configuration and credential updates are delivered independently from the provider
             // registry. Reload the registry first so provider.list() observes the
             // new state before rebuilding the discovery inventory.
-            await refreshFromCurrentConfig()
+            await refreshFromCurrentConfig(undefined, true)
           } catch {
             if (!abort.signal.aborted) {
               // should not happen, but if it does, we don't want to crash the plugin

test/v2/plugin.test.ts

@@ -71,7 +71,7 @@ describe("V2 plugin entrypoint", () => {
       expect(ctx.integration.transform).toHaveBeenCalledTimes(1)
       expect(ctx.provider.transform).toHaveBeenCalledTimes(1)
       expect(ctx.integration.reload).toHaveBeenCalledTimes(1)
-      expect(ctx.provider.reload).toHaveBeenCalledTimes(2)
+      expect(ctx.provider.reload).toHaveBeenCalledTimes(1)
       expect(fetcher).toHaveBeenCalledTimes(1)
     } finally {
       fetcher.mockRestore()
@@ -197,7 +197,7 @@ describe("V2 plugin entrypoint", () => {
         transform: vi.fn().mockImplementation(async (callback) => callback({ get: vi.fn().mockReturnValue(undefined), add: vi.fn(), update: vi.fn(), models: { set: vi.fn() } })),
         reload: vi.fn().mockImplementation(async () => {
           providerReloads += 1
-          if (providerReloads >= 3) {
+          if (providerReloads >= 1) {
             currentBaseURL = "http://127.0.0.1:5678/v1"
           }
         }),
@@ -216,7 +216,7 @@ describe("V2 plugin entrypoint", () => {
     try {
       await plugin.setup(ctx as never)
       await new Promise((resolve) => setTimeout(resolve, 0))
-      expect(ctx.provider.reload).toHaveBeenCalledTimes(4)
+      expect(ctx.provider.reload).toHaveBeenCalledTimes(3)
       expect(ctx.integration.transform).toHaveBeenCalledTimes(1)
       expect(ctx.provider.transform).toHaveBeenCalledTimes(1)
       expect(fetcher).toHaveBeenNthCalledWith(1, "http://127.0.0.1:1234/v1/models", expect.any(Object))
@@ -316,6 +316,41 @@ describe("V2 plugin entrypoint", () => {
     }
   })
 
+  it("does not reload the provider catalog until a refresh has discovered models", async () => {
+    let releaseRefresh: (() => void) | undefined
+    const refreshStarted = new Promise<void>((resolve) => { releaseRefresh = resolve })
+    const fetcher = vi.spyOn(globalThis, "fetch")
+      .mockResolvedValueOnce({ ok: true, json: async () => ({ data: [{ id: "initial-model" }] }) } as Response)
+      .mockImplementationOnce(async () => {
+        await refreshStarted
+        return { ok: true, json: async () => ({ data: [{ id: "refreshed-model" }] }) } as Response
+      })
+    const rpcHandlers: Record<string, (...args: unknown[]) => Promise<unknown>> = {}
+    const ctx = context({
+      rpc: {
+        register: vi.fn().mockImplementation(async (definition, handlers) => {
+          if (definition.id === "opencode.models-discovery") Object.assign(rpcHandlers, handlers)
+        }),
+      },
+    })
+
+    try {
+      await plugin.setup(ctx as never)
+      expect(ctx.provider.reload).toHaveBeenCalledTimes(1)
+
+      const refresh = rpcHandlers.refresh as (input?: unknown) => Promise<unknown>
+      const pending = refresh({ force: true })
+      await new Promise((resolve) => setTimeout(resolve, 0))
+      expect(ctx.provider.reload).toHaveBeenCalledTimes(1)
+
+      releaseRefresh?.()
+      await pending
+      expect(ctx.provider.reload).toHaveBeenCalledTimes(2)
+    } finally {
+      fetcher.mockRestore()
+    }
+  })
+
   it("keeps discovered defaults separate from overridden values after a failed refresh", async () => {
     const fetcher = vi.spyOn(globalThis, "fetch")
       .mockResolvedValueOnce({ ok: true, json: async () => ({ data: [{ id: "stable-model" }] }) } as Response)