diff --git a/src/v2/index.ts b/src/v2/index.ts index 9ab581d..a775cd5 100644 --- a/src/v2/index.ts +++ b/src/v2/index.ts @@ -169,16 +169,20 @@ export async function setupV2(ctx: Plugin.Context): Promise<() => void> { let activeRefresh: Promise | undefined let pendingRefreshForce = false - const refreshFromCurrentConfig = (force?: boolean): Promise => { + const refreshFromCurrentConfig = (force?: boolean, reloadProviderConfig = false): Promise => { 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 diff --git a/test/v2/plugin.test.ts b/test/v2/plugin.test.ts index 6fd073b..2095407 100644 --- a/test/v2/plugin.test.ts +++ b/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((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 Promise> = {} + 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 + 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)