fix: honor fresh provider catalog discovery (#130412)

* fix: honor fresh provider catalog discovery

* test: use compatible deferred catalog fixture

extensions/llama-cpp/src/external-server/discovery.test.ts

@@ -1,3 +1,5 @@
+import { once } from "node:events";
+import { createServer } from "node:http";
 import { clearLiveCatalogCacheForTests } from "openclaw/plugin-sdk/provider-catalog-shared";
 import { beforeEach, describe, expect, it, vi } from "vitest";
 import { discoverLlamaServer } from "./discovery.js";
@@ -88,21 +90,56 @@ describe("llama-server discovery projection", () => {
     });
   });
 
-  it("bypasses the shared cache for credential-scoped discovery", async () => {
-    discoverRowsMock.mockResolvedValue({
-      kind: "success",
-      health: "ready",
-      fetchedAt: 123,
-      rows: [],
+  it.each([
+    { name: "API key", access: { apiKey: "endpoint-key" } },
+    { name: "authorization header", access: { headers: { Authorization: "Bearer endpoint-key" } } },
+    { name: "explicit refresh", access: { cacheTtlMs: 0 } },
+  ])("fetches $name discovery after an anonymous catalog was cached", async ({ access }) => {
+    const actual = await vi.importActual<typeof import("openclaw/plugin-sdk/provider-setup")>(
+      "openclaw/plugin-sdk/provider-setup",
+    );
+    discoverRowsMock.mockImplementation(actual.discoverOpenAICompatibleLocalModels);
+    let modelId = "anonymous-model";
+    const modelRequests: Array<string | undefined> = [];
+    const server = createServer((request, response) => {
+      response.setHeader("Content-Type", "application/json");
+      if (request.url === "/models") {
+        modelRequests.push(request.headers.authorization);
+        response.end(JSON.stringify({ data: [{ id: modelId, status: { value: "unloaded" } }] }));
+      } else {
+        response.end("{}");
+      }
     });
-
-    for (let index = 0; index < 2; index += 1) {
-      await discoverLlamaServer({
-        baseUrl: "http://localhost:8080",
-        headers: { Authorization: "Bearer endpoint-key" },
+    server.listen(0, "127.0.0.1");
+    await once(server, "listening");
+    try {
+      const address = server.address();
+      if (!address || typeof address === "string") {
+        throw new Error("expected a listening TCP server");
+      }
+      const baseUrl = `http://127.0.0.1:${address.port}`;
+      await expect(discoverLlamaServer({ baseUrl })).resolves.toMatchObject({
+        kind: "success",
+        models: [{ config: { id: "anonymous-model" } }],
+      });
+      modelId = "fresh-model";
+      await expect(discoverLlamaServer({ baseUrl, ...access })).resolves.toMatchObject({
+        kind: "success",
+        models: [{ config: { id: "fresh-model" } }],
+      });
+      await expect(discoverLlamaServer({ baseUrl })).resolves.toMatchObject({
+        kind: "success",
+        models: [{ config: { id: "anonymous-model" } }],
+      });
+      expect(modelRequests).toEqual([
+        undefined,
+        "cacheTtlMs" in access ? undefined : "Bearer endpoint-key",
+      ]);
+    } finally {
+      server.closeAllConnections();
+      await new Promise<void>((resolve, reject) => {
+        server.close((error) => (error ? reject(error) : resolve()));
       });
     }
-
-    expect(discoverRowsMock).toHaveBeenCalledTimes(2);
   });
 });

src/plugin-sdk/provider-catalog-shared.test.ts

@@ -1,6 +1,7 @@
 // Provider catalog shared tests cover catalog hashing, normalization, and model visibility.
 import type { ModelCatalogProvider } from "@openclaw/model-catalog-core/model-catalog-types";
 import { beforeEach, describe, expect, it, vi } from "vitest";
+import { createDeferred } from "../../test/helpers/promise.js";
 import {
   applyProviderNativeStreamingUsageCompat,
   buildManifestModelDefinition,
@@ -94,6 +95,77 @@ describe("provider-catalog-shared live catalog cache", () => {
     expect(load).toHaveBeenCalledTimes(2);
   });
 
+  it.each(["resolve", "reject", "throw"] as const)(
+    "bypasses a warm cache without modifying it when the uncached loader will %s",
+    async (outcome) => {
+      const keyParts = ["provider", "models"];
+      await getCachedLiveCatalogValue({ keyParts, load: async () => "cached" });
+      const error = new Error("uncached failure");
+      const load = vi.fn(() => {
+        if (outcome === "throw") {
+          throw error;
+        }
+        return outcome === "reject" ? Promise.reject(error) : Promise.resolve("fresh");
+      });
+      const shouldCache = vi.fn(() => false);
+      const fresh = getCachedLiveCatalogValue({ keyParts, load, shouldCache, ttlMs: 0 });
+      if (outcome === "resolve") {
+        await expect(fresh).resolves.toBe("fresh");
+      } else {
+        await expect(fresh).rejects.toBe(error);
+      }
+      expect(shouldCache).not.toHaveBeenCalled();
+      await expect(getCachedLiveCatalogValue({ keyParts, load })).resolves.toBe("cached");
+      expect(load).toHaveBeenCalledTimes(1);
+    },
+  );
+
+  it.each(["reject", "predicate-false", "predicate-throw", "same-promise"] as const)(
+    "preserves a replacement cache entry after expired work finishes with %s",
+    async (outcome) => {
+      let now = 1_000;
+      const keyParts = ["provider", "models"];
+      const pending = createDeferred<string>();
+      const error = new Error("expired failure");
+      const first = getCachedLiveCatalogValue({
+        keyParts,
+        load: () => pending.promise,
+        ttlMs: 100,
+        now: () => now,
+        shouldCache: () => {
+          if (outcome === "predicate-throw") {
+            throw error;
+          }
+          return false;
+        },
+      });
+      now = 1_101;
+      const replacement = getCachedLiveCatalogValue({
+        keyParts,
+        load: () => (outcome === "same-promise" ? pending.promise : Promise.resolve("replacement")),
+        ttlMs: 100,
+        now: () => now,
+      });
+      if (outcome === "reject") {
+        pending.reject(error);
+      } else {
+        pending.resolve("expired");
+      }
+      if (outcome === "reject" || outcome === "predicate-throw") {
+        await expect(first).rejects.toBe(error);
+      } else {
+        await expect(first).resolves.toBe("expired");
+      }
+      const expected = outcome === "same-promise" ? "expired" : "replacement";
+      await expect(replacement).resolves.toBe(expected);
+      const load = vi.fn(async () => "unnecessary reload");
+      await expect(getCachedLiveCatalogValue({ keyParts, load, now: () => now })).resolves.toBe(
+        expected,
+      );
+      expect(load).not.toHaveBeenCalled();
+    },
+  );
+
   it("does not retain resolved live catalog values rejected by the cache predicate", async () => {
     const load = vi
       .fn<() => Promise<string>>()

src/plugin-sdk/provider-catalog-shared.ts

@@ -80,7 +80,11 @@ export async function getCachedLiveCatalogValue<T>(params: {
   now?: () => number;
 }): Promise<T> {
   const rawNow = params.now?.() ?? Date.now();
-  const ttlMs = params.ttlMs ?? 30_000;
+  const expiresAt = resolveExpiresAtMsFromDurationMs(params.ttlMs ?? 30_000, { nowMs: rawNow });
+  // Uncached callers must neither reuse nor disturb an existing entry.
+  if (expiresAt === undefined) {
+    return await params.load();
+  }
   const key = buildLiveCatalogCacheKey(params.keyParts);
   const existing = liveCatalogCache.get(key) as LiveCatalogCacheEntry<T> | undefined;
   if (existing) {
@@ -89,27 +93,22 @@ export async function getCachedLiveCatalogValue<T>(params: {
     }
     liveCatalogCache.delete(key);
   }
-  const value = params.load();
-  const expiresAt = resolveExpiresAtMsFromDurationMs(ttlMs, { nowMs: rawNow });
-  if (expiresAt !== undefined) {
-    // Auth-scoped live provider catalogs can vary by token; keep this
-    // process-local cache bounded so discovery cannot grow without limit.
-    pruneMapToMaxSize(liveCatalogCache, LIVE_CATALOG_CACHE_MAX_ENTRIES - 1);
-    liveCatalogCache.set(key, {
-      expiresAt,
-      value,
-    });
-  }
+  const entry = { expiresAt, value: params.load() };
+  // Auth-scoped live provider catalogs can vary by token; keep this
+  // process-local cache bounded so discovery cannot grow without limit.
+  pruneMapToMaxSize(liveCatalogCache, LIVE_CATALOG_CACHE_MAX_ENTRIES - 1);
+  liveCatalogCache.set(key, entry);
+  let retain = false;
   try {
-    const resolved = await value;
-    if (params.shouldCache && !params.shouldCache(resolved)) {
+    const resolved = await entry.value;
+    retain = params.shouldCache?.(resolved) ?? true;
+    return resolved;
+  } finally {
+    // Expired work may finish after a replacement load. Only its own entry
+    // can be removed when loading or the cache predicate fails.
+    if (!retain && liveCatalogCache.get(key) === entry) {
       liveCatalogCache.delete(key);
     }
-    return resolved;
-  } catch (err) {
-    // Failed live discovery should not poison later retries for the same provider/config.
-    liveCatalogCache.delete(key);
-    throw err;
   }
 }