fix(wikis): stop swallowing fatal errors and marking failed scans complete

`getWikiPage`'s attachments fetch had a bare `catch` that swallowed 401/403/429 one function away from discovery code that correctly rethrows them. During a 50-page discovery scan a rate limit was eaten once per page and every page cached with `attachments: []`, then reported to the model as fact. The discovery probe loop incremented `consecutiveMisses` only on 404, so a run where every probe failed with a 500 or a network error never tripped the cutoff, never rethrew, and cached nothing — yet still ran `discoveredCacheKeys.add(cacheKey)` unconditionally, marking discovery complete. Every later wiki search then short-circuited on an empty corpus and returned `[]` for the life of the process. Discovery is now marked complete only when at least one page was cached or the loop ended via the 404 cutoff. An all-errors scan satisfies neither, so it retries; a genuinely empty wiki trips the cutoff and is still marked complete rather than re-probed on every search. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
2026-09-24 09:27:33 +02:00
parent 216b460c56
commit a8d3c23096
2 changed files with 141 additions and 3 deletions
@@ -11,6 +11,7 @@ import { extractIdFromHref, extractRawText } from "../client/hal-parser.ts";
 import type { HalCollection, HalLink, HalResource } from "../client/types.ts";
 import { resolveClient, resolveProjectId } from "./helper.ts";
 import { rankRecords, type FieldSpec, type MatchMode } from "../search/rank.ts";
+import { isFatalSearchError } from "../search/pipeline.ts";

 export interface WikiPageAttachment {
  id: number;
@@ -216,8 +217,15 @@ export async function getWikiPage(
    if (attachmentsResponse._embedded?.elements) {
      attachments = attachmentsResponse._embedded.elements.map(normalizeWikiAttachment);
    }
-  } catch {
-    // If attachments collection cannot be retrieved, default to empty list
+  } catch (error: unknown) {
+    // A 401/403/429 here is not "this page has no attachments" — it means we
+    // could not read. Swallowing it caches (and reports) an empty attachment
+    // list as fact, and during a discovery scan eats the same rate limit once
+    // per probed page.
+    if (isFatalSearchError(error)) {
+      throw error;
+    }
+    // Otherwise (404, 500, network) degrade to an empty list.
  }

  const detail = normalizeWikiPageDetail(pageResource, attachments);
@@ -324,6 +332,9 @@ export async function searchWikiPages(
    const MAX_PROBE_ID = 50;
    const CONSECUTIVE_404_CUTOFF = 5;
    let consecutiveMisses = 0;
+    // True only when the scan stopped because the 404 cutoff said "the wiki
+    // ends here", as opposed to erroring its way through every probe.
+    let reachedMissCutoff = false;

    for (let id = 1; id <= MAX_PROBE_ID; id++) {
      try {
@@ -343,6 +354,7 @@ export async function searchWikiPages(
          consecutiveMisses++;
        }
        if (consecutiveMisses >= CONSECUTIVE_404_CUTOFF) {
+          reachedMissCutoff = true;
          break;
        }
      }
@@ -368,8 +380,19 @@ export async function searchWikiPages(
      }
    }

+    // Mark discovery complete only when it genuinely completed. If every
+    // probe failed with a 500 or a network error the cutoff never trips and
+    // nothing is cached — marking that scan "done" would short-circuit every
+    // later search for the life of the process and return [] forever.
+    // Either condition on its own is proof of a real scan: a cached page
+    // means we read the wiki, and the 404 cutoff means we read to its end
+    // (a genuinely empty wiki is 404s all the way down, and must still be
+    // marked complete or every search re-probes it).
+    const discoveryCompleted = cacheMap.size > 0 || reachedMissCutoff;
+    if (discoveryCompleted) {
      discoveredCacheKeys.add(cacheKey);
    }
+  }

  // 3. Resolve target project ID if provided
  let targetProjectId: number | undefined;
@@ -729,3 +729,118 @@ describe("Wiki Fuzzy Search", () => {
    expect(results).toHaveLength(0);
  });
 });
+
+describe("Wiki Error Propagation", () => {
+  beforeEach(() => {
+    clearWikiCache();
+  });
+
+  function pageWithFailingAttachments(error: unknown) {
+    return {
+      baseUrl: "http://localhost:8080",
+      getCacheKey: () => "attachments-test",
+      get: async (path: string) => {
+        if (path.endsWith("/attachments")) {
+          throw error;
+        }
+        if (path === "/api/v3/wiki_pages/1") {
+          return {
+            _type: "WikiPage",
+            id: 1,
+            title: "Project Wiki",
+            _links: { project: { href: "/api/v3/projects/1", title: "Demo Project" } },
+          };
+        }
+        throw new OpenProjectError("not found", { statusCode: 404 });
+      },
+    } as unknown as OpenProjectClient;
+  }
+
+  test("getWikiPage propagates a rate limit from the attachments fetch", async () => {
+    await expect(
+      getWikiPage(
+        1,
+        pageWithFailingAttachments(new OpenProjectError("rate limited", { statusCode: 429 }))
+      )
+    ).rejects.toThrow("rate limited");
+  });
+
+  test("getWikiPage propagates an authentication failure from the attachments fetch", async () => {
+    await expect(
+      getWikiPage(1, pageWithFailingAttachments(new OpenProjectAuthenticationError()))
+    ).rejects.toThrow(OpenProjectAuthenticationError);
+  });
+
+  test("getWikiPage still degrades to an empty attachment list on a non-fatal failure", async () => {
+    const page = await getWikiPage(
+      1,
+      pageWithFailingAttachments(new OpenProjectError("boom", { statusCode: 500 }))
+    );
+    expect(page.id).toBe(1);
+    expect(page.attachments).toEqual([]);
+  });
+
+  test("a discovery scan that errors through every probe is not marked complete", async () => {
+    // Phase 1: every wiki page probe fails with a 500. Nothing is cached, and
+    // the 404 cutoff never trips, so discovery did not actually complete.
+    let failEverything = true;
+    const client = {
+      baseUrl: "http://localhost:8080",
+      getCacheKey: () => "flaky-tenant",
+      get: async (path: string) => {
+        if (path === "/api/v3/wiki_page_links") {
+          return { _type: "Collection", total: 0, _embedded: { elements: [] } };
+        }
+        if (failEverything) {
+          throw new OpenProjectError("upstream exploded", { statusCode: 500 });
+        }
+        if (path === "/api/v3/wiki_pages/1") {
+          return {
+            _type: "WikiPage",
+            id: 1,
+            title: "Deployment Runbook",
+            text: { raw: "How to deploy the service" },
+            _links: { project: { href: "/api/v3/projects/1", title: "Demo Project" } },
+          };
+        }
+        if (path === "/api/v3/wiki_pages/1/attachments") {
+          return { _type: "Collection", total: 0, _embedded: { elements: [] } };
+        }
+        throw new OpenProjectError("not found", { statusCode: 404 });
+      },
+    } as unknown as OpenProjectClient;
+
+    const first = await searchWikiPages({ query: "deployment" }, client);
+    expect(first).toEqual([]);
+
+    // Phase 2: the instance recovers. Discovery must run again rather than
+    // short-circuiting on a cache that was wrongly marked complete and
+    // returning [] for the life of the process.
+    failEverything = false;
+    const second = await searchWikiPages({ query: "deployment" }, client);
+    expect(second.map((page) => page.id)).toEqual([1]);
+  });
+
+  test("a genuinely empty wiki is marked complete and is not re-probed", async () => {
+    let probes = 0;
+    const client = {
+      baseUrl: "http://localhost:8080",
+      getCacheKey: () => "empty-tenant",
+      get: async (path: string) => {
+        if (path === "/api/v3/wiki_page_links") {
+          return { _type: "Collection", total: 0, _embedded: { elements: [] } };
+        }
+        probes++;
+        throw new OpenProjectError("not found", { statusCode: 404 });
+      },
+    } as unknown as OpenProjectClient;
+
+    expect(await searchWikiPages({ query: "anything" }, client)).toEqual([]);
+    const afterFirst = probes;
+    // The 404 cutoff is a real completion signal: five consecutive misses.
+    expect(afterFirst).toBe(5);
+
+    expect(await searchWikiPages({ query: "anything" }, client)).toEqual([]);
+    expect(probes).toBe(afterFirst);
+  });
+});