fix(FN-1966): use public skills search fallback for catalog fetches

- Route unauthenticated catalog requests to skills.sh /api/search with wildcard query support and normalized response mapping - Keep authenticated v1 catalog fetch, but fall back to public search on 401/403 while preserving auth metadata - Expand skills route tests for unauthenticated payload shape, query passthrough, and no-query wildcard behavior - Stabilize onboarding reopen tests by awaiting Authentication section render before interaction - Document dashboard dist build prerequisite for CLI test troubleshooting
2026-04-16 15:55:00 -07:00
parent c18c431c48
commit 84bd5e7c9d
4 changed files with 270 additions and 80 deletions
--- a/.fusion/memory.md
+++ b/.fusion/memory.md
@@ -306,6 +306,7 @@ Dashboard SSE (`/api/events`) streams plugin lifecycle events as normalized `plu
 - SQLite `ORDER BY timestamp DESC` alone can be nondeterministic when multiple rows share the same millisecond timestamp; add a stable tiebreaker (for example `rowid DESC`) when selecting a "latest" event.
 - In `TaskCard.tsx`, `isInteractiveTarget` must check `target instanceof Element` (not `HTMLElement`) so SVG elements from lucide-react icons are correctly detected as interactive when inside buttons.
 - If workspace tests fail resolving `@fusion/core` package exports from `packages/core/dist/index.js` (for example `No matching export ...` in CLI/TUI/package-level tests after adding a new core export), run `pnpm --filter @fusion/core build` before rerunning the suite so ignored `dist/` exports are refreshed.
+- If CLI tests fail resolving `@fusion/dashboard` (for example `Could not resolve "@fusion/dashboard"` in `build-exe` or command tests), build dashboard first with `pnpm --filter @fusion/dashboard build` so `packages/dashboard/dist/index.js` exists.
 - QuickEntryBox control test IDs are reused in `ListView` integration tests; when control layout changes (for example nested menu → inline buttons), update both `QuickEntryBox.test.tsx` and `ListView.test.tsx` together to avoid cascading failures.
 - When `InlineCreateCard` layout changes, also check `Column.test.tsx` and `board-mobile.test.tsx` for references to moved/removed test IDs like `inline-create-description-actions`.
 - When adding portal-based dropdown menus to QuickEntryBox, tests may fail in isolation but pass when run together (test isolation issues). This is because tests share DOM state across describe blocks. Always verify new dropdown tests pass both in isolation (`--testNamePattern`) and when run together.
--- a/packages/dashboard/app/components/tests/App.test.tsx
+++ b/packages/dashboard/app/components/tests/App.test.tsx
@@ -2192,7 +2192,8 @@ describe("App onboarding reopen", () => {
    });

    // Navigate to Authentication section (it should be default or click to ensure)
-    const authSection = screen.getAllByText("Authentication")[0];
+    const authSections = await screen.findAllByText("Authentication");
+    const authSection = authSections[0];
    fireEvent.click(authSection);

    await waitFor(() => {
@@ -2254,7 +2255,11 @@ describe("App onboarding reopen", () => {
      expect(screen.getByText("Settings")).toBeTruthy();
    });

-    // Authentication is the default settings section; wait for it to hydrate.
+    // Navigate to Authentication section
+    const authSections = await screen.findAllByText("Authentication");
+    const authSection = authSections[0];
+    fireEvent.click(authSection);
+
    await waitFor(() => {
      expect(fetchAuthStatus).toHaveBeenCalled();
    });
--- a/packages/dashboard/src/tests/routes-skills.test.ts
+++ b/packages/dashboard/src/tests/routes-skills.test.ts
@@ -377,7 +377,56 @@ describe("Skills routes", () => {
      expect(res.body).toMatchObject({ error: "Skills adapter not configured", code: "adapter_not_configured" });
    });

-    it("passes limit and query parameters to catalog fetch", async () => {
+    it("returns unauthenticated public-search style entries", async () => {
+      const mockAdapter = createMockSkillsAdapter({
+        fetchCatalog: vi.fn().mockResolvedValue({
+          entries: [
+            {
+              id: "vercel-labs/agent-skills/vercel-react-best-practices",
+              slug: "vercel-labs/agent-skills/vercel-react-best-practices",
+              name: "vercel-react-best-practices",
+              repo: "vercel-labs/agent-skills",
+              installs: 421,
+              installation: {
+                installed: false,
+                matchingSkillIds: [],
+                matchingPaths: [],
+              },
+            },
+          ],
+          auth: { mode: "unauthenticated", tokenPresent: false, fallbackUsed: false },
+        }),
+      });
+      const store = new MockStore();
+      const app = createServer(store as any, { skillsAdapter: mockAdapter as SkillsAdapter });
+
+      const res = await request(app, "GET", "/api/skills/catalog");
+
+      expect(res.status).toBe(200);
+      expect(res.body).toEqual({
+        entries: [
+          {
+            id: "vercel-labs/agent-skills/vercel-react-best-practices",
+            slug: "vercel-labs/agent-skills/vercel-react-best-practices",
+            name: "vercel-react-best-practices",
+            repo: "vercel-labs/agent-skills",
+            installs: 421,
+            installation: {
+              installed: false,
+              matchingSkillIds: [],
+              matchingPaths: [],
+            },
+          },
+        ],
+        auth: {
+          mode: "unauthenticated",
+          tokenPresent: false,
+          fallbackUsed: false,
+        },
+      });
+    });
+
+    it("passes search query through to catalog adapter", async () => {
      const mockAdapter = createMockSkillsAdapter({
        fetchCatalog: vi.fn().mockResolvedValue({
          entries: [],
@@ -387,10 +436,38 @@ describe("Skills routes", () => {
      const store = new MockStore();
      const app = createServer(store as any, { skillsAdapter: mockAdapter as SkillsAdapter });

-      const res = await request(app, "GET", "/api/skills/catalog?limit=50&q=search-term");
+      const res = await request(app, "GET", "/api/skills/catalog?q=react");

      expect(res.status).toBe(200);
-      expect(mockAdapter.fetchCatalog).toHaveBeenCalledWith({ limit: 50, query: "search-term" });
+      expect(mockAdapter.fetchCatalog).toHaveBeenCalledWith({ limit: 20, query: "react" });
+    });
+
+    it("returns results when query is omitted (adapter handles wildcard fallback)", async () => {
+      const mockAdapter = createMockSkillsAdapter({
+        fetchCatalog: vi.fn().mockResolvedValue({
+          entries: [
+            {
+              id: "default-skill",
+              slug: "default-skill",
+              name: "Default Skill",
+              installation: {
+                installed: false,
+                matchingSkillIds: [],
+                matchingPaths: [],
+              },
+            },
+          ],
+          auth: { mode: "unauthenticated", tokenPresent: false, fallbackUsed: false },
+        }),
+      });
+      const store = new MockStore();
+      const app = createServer(store as any, { skillsAdapter: mockAdapter as SkillsAdapter });
+
+      const res = await request(app, "GET", "/api/skills/catalog");
+
+      expect(res.status).toBe(200);
+      expect(res.body.entries).toHaveLength(1);
+      expect(mockAdapter.fetchCatalog).toHaveBeenCalledWith({ limit: 20, query: undefined });
    });

    it("bounds limit parameter to max 100", async () => {
--- a/packages/dashboard/src/skills-adapter.ts
+++ b/packages/dashboard/src/skills-adapter.ts
@@ -395,92 +395,63 @@ export function createSkillsAdapter(options: {

      // Get skills.sh token if available
      const token = process.env.SKILLS_SH_TOKEN;
+      const catalogUrl = buildCatalogUrl(boundedLimit, query);
+      const searchUrl = buildSearchUrl(boundedLimit, query);

-      const params = new URLSearchParams();
-      params.set("limit", String(boundedLimit));
-      if (query) {
-        params.set("q", query);
+      // No token available - use public search endpoint
+      if (!token) {
+        return fetchPublicCatalog(searchUrl, {
+          mode: "unauthenticated",
+          tokenPresent: false,
+          fallbackUsed: false,
+        });
      }

-      const upstreamUrl = `https://skills.sh/api/v1/skills?${params.toString()}`;
+      // Try authenticated v1 catalog endpoint first
+      try {
+        const authResponse = await fetch(catalogUrl, {
+          headers: {
+            Authorization: `Bearer ${token}`,
+            Accept: "application/json",
+          },
+          signal: AbortSignal.timeout(10_000),
+        });

-      // Try authenticated first if token is available
-      if (token) {
-        try {
-          const authResponse = await fetch(upstreamUrl, {
-            headers: {
-              Authorization: `Bearer ${token}`,
-              Accept: "application/json",
-            },
-            signal: AbortSignal.timeout(10_000),
-          });
-
-          if (authResponse.ok) {
-            const data = await authResponse.json().catch(() => null);
-            if (data) {
-              return normalizeCatalogResponse(data, false);
-            }
+        if (authResponse.ok) {
+          const data = await authResponse.json().catch(() => null);
+          if (data) {
+            return normalizeCatalogResponse(data, false);
          }

-          // 401/403 from authenticated request - fall back to unauthenticated
-          if (authResponse.status === 401 || authResponse.status === 403) {
-            const fallbackResponse = await fetch(upstreamUrl, {
-              headers: { Accept: "application/json" },
-              signal: AbortSignal.timeout(10_000),
-            });
-
-            if (fallbackResponse.ok) {
-              const fallbackData = await fallbackResponse.json().catch(() => null);
-              if (fallbackData) {
-                return normalizeCatalogResponse(fallbackData, true);
-              }
-            }
-          }
-
-          // Upstream error
          return {
-            error: `Upstream returned ${authResponse.status}: ${authResponse.statusText}`,
-            code: "upstream_http_error",
-          };
-        } catch (err) {
-          const error = err as Error;
-          if (error.name === "TimeoutError" || error.message?.includes("timeout")) {
-            return { error: "Upstream request timed out", code: "upstream_timeout" };
-          }
-          return {
-            error: error.message || "Upstream request failed",
-            code: "upstream_http_error",
+            error: "Invalid upstream response format",
+            code: "upstream_invalid_payload",
          };
        }
-      } else {
-        // No token - unauthenticated request
-        try {
-          const response = await fetch(upstreamUrl, {
-            headers: { Accept: "application/json" },
-            signal: AbortSignal.timeout(10_000),
+
+        // 401/403 from authenticated request - fall back to public search endpoint
+        if (authResponse.status === 401 || authResponse.status === 403) {
+          return fetchPublicCatalog(searchUrl, {
+            mode: "fallback-unauthenticated",
+            tokenPresent: true,
+            fallbackUsed: true,
          });
-
-          if (response.ok) {
-            const data = await response.json().catch(() => null);
-            if (data) {
-              return normalizeCatalogResponse(data, false);
-            }
-          }
-
-          return {
-            error: `Upstream returned ${response.status}: ${response.statusText}`,
-            code: "upstream_http_error",
-          };
-        } catch (err) {
-          const error = err as Error;
-          if (error.name === "TimeoutError" || error.message?.includes("timeout")) {
-            return { error: "Upstream request timed out", code: "upstream_timeout" };
-          }
-          return {
-            error: error.message || "Upstream request failed",
-            code: "upstream_http_error",
-          };
        }
+
+        // Upstream error
+        return {
+          error: `Upstream returned ${authResponse.status}: ${authResponse.statusText}`,
+          code: "upstream_http_error",
+        };
+      } catch (err) {
+        const error = err as Error;
+        if (isTimeoutError(error)) {
+          return { error: "Upstream request timed out", code: "upstream_timeout" };
+        }
+        return {
+          error: error.message || "Upstream request failed",
+          code: "upstream_http_error",
+        };
      }
    },
  };
@@ -503,6 +474,142 @@ function extractSkillName(skillPath: string, source: string): string {
  return source;
 }

+/**
+ * Build the authenticated catalog URL.
+ */
+function buildCatalogUrl(limit: number, query?: string): string {
+  const params = new URLSearchParams();
+  params.set("limit", String(limit));
+
+  const normalizedQuery = query?.trim();
+  if (normalizedQuery) {
+    params.set("q", normalizedQuery);
+  }
+
+  return `https://skills.sh/api/v1/skills?${params.toString()}`;
+}
+
+/**
+ * Build the public search URL.
+ *
+ * The search API requires a non-empty query. For initial catalog browsing,
+ * use "*" as a wildcard query.
+ */
+function buildSearchUrl(limit: number, query?: string): string {
+  const params = new URLSearchParams();
+  const normalizedQuery = query?.trim();
+
+  params.set("q", normalizedQuery && normalizedQuery.length > 0 ? normalizedQuery : "*");
+  params.set("limit", String(limit));
+
+  return `https://skills.sh/api/search?${params.toString()}`;
+}
+
+/**
+ * Fetch and normalize catalog data from the public /api/search endpoint.
+ */
+async function fetchPublicCatalog(
+  url: string,
+  auth: CatalogFetchResult["auth"],
+): Promise<CatalogFetchResult | UpstreamError> {
+  try {
+    const response = await fetch(url, {
+      headers: { Accept: "application/json" },
+      signal: AbortSignal.timeout(10_000),
+    });
+
+    if (!response.ok) {
+      return {
+        error: `Upstream returned ${response.status}: ${response.statusText}`,
+        code: "upstream_http_error",
+      };
+    }
+
+    const data = await response.json().catch(() => null);
+    if (!data) {
+      return {
+        error: "Invalid upstream response format",
+        code: "upstream_invalid_payload",
+      };
+    }
+
+    return normalizeSearchResponse(data, auth);
+  } catch (err) {
+    const error = err as Error;
+    if (isTimeoutError(error)) {
+      return { error: "Upstream request timed out", code: "upstream_timeout" };
+    }
+    return {
+      error: error.message || "Upstream request failed",
+      code: "upstream_http_error",
+    };
+  }
+}
+
+/**
+ * Normalize public search endpoint response shape to CatalogFetchResult.
+ */
+function normalizeSearchResponse(
+  data: unknown,
+  auth: CatalogFetchResult["auth"],
+): CatalogFetchResult | UpstreamError {
+  if (!data || typeof data !== "object") {
+    return {
+      error: "Invalid upstream response format",
+      code: "upstream_invalid_payload",
+    };
+  }
+
+  const record = data as Record<string, unknown>;
+  if (!Array.isArray(record.skills)) {
+    return {
+      error: "Invalid upstream response format: expected { skills: [...] }",
+      code: "upstream_invalid_payload",
+    };
+  }
+
+  return {
+    entries: record.skills.map(normalizeSearchEntry),
+    auth,
+  };
+}
+
+/**
+ * Normalize a single /api/search skill entry to CatalogEntry.
+ */
+function normalizeSearchEntry(entry: unknown): CatalogEntry {
+  if (!entry || typeof entry !== "object") {
+    return {
+      id: "",
+      slug: "",
+      name: "Unknown",
+      installation: { installed: false, matchingSkillIds: [], matchingPaths: [] },
+    };
+  }
+
+  const record = entry as Record<string, unknown>;
+  const id = String(record.id ?? record.skillId ?? "");
+  const nameCandidate = String(record.name ?? record.skillId ?? id);
+
+  return {
+    id,
+    slug: id,
+    name: nameCandidate || "Unknown",
+    repo: record.source ? String(record.source) : undefined,
+    installs: typeof record.installs === "number" ? record.installs : undefined,
+    installation: {
+      installed: false,
+      matchingSkillIds: [],
+      matchingPaths: [],
+    },
+  };
+}
+
+function isTimeoutError(error: Error): boolean {
+  const message = error.message?.toLowerCase() ?? "";
+  return error.name === "TimeoutError" || message.includes("timeout");
+}
+
 /**
 * Normalize catalog response to handle both array and wrapped formats.
 */