From fa05fcc1c6c244e83231f7e6b348c28f1765e29e Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Thu, 24 Sep 2026 09:38:30 +0200 Subject: [PATCH] fix(wikis): judge discovery completion on cache growth, not absolute size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The completion guard added in a8d3c23 read `cacheMap.size > 0`, but `cacheMap` is the tenant-wide map that `getWikiPage` also writes to on every single-page fetch. One prior `openproject_get_wiki_page` call on the same cache key therefore leaves the map non-empty before the probe loop issues its first request, and a scan in which every probe fails with a 500 inherits that entry and marks itself discovery-complete anyway — permanently, for the process lifetime. "Get a wiki page, then search" is an ordinary LLM tool sequence, so the contaminated path is likely rather than exotic, and it lands back in exactly the failure this branch exists to eliminate: wiki search returns [] forever and the model tells the user their pages do not exist. Completion is now judged on growth during the scan (`cacheMap.size > cachedBefore`), captured before the probe loop. The cold-cache behaviour is unchanged, since `cachedBefore` is then 0 and the condition is equivalent — both existing F5b tests stay green. A permanently-failing instance now re-probes on every search. That is the accepted cost of retrying rather than caching a lie. Co-Authored-By: Claude Opus 5 (1M context) --- src/services/wikis.ts | 23 +++++++---- tests/wikis.test.ts | 91 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 7 deletions(-) diff --git a/src/services/wikis.ts b/src/services/wikis.ts index 8cd5a3e..c1edf1e 100644 --- a/src/services/wikis.ts +++ b/src/services/wikis.ts @@ -295,6 +295,15 @@ export async function searchWikiPages( } if (needsDiscovery) { + // `cacheMap` is the tenant-wide map that getWikiPage ALSO writes to on + // every single-page fetch, so it is not empty just because discovery has + // not run: one prior openproject_get_wiki_page call on this cache key + // leaves an entry behind. Completion must therefore be judged on growth + // during this scan, not on absolute size — otherwise "get a page, then + // search" (an ordinary LLM tool sequence) lets an all-errors scan inherit + // that entry and mark itself complete. + const cachedBefore = cacheMap.size; + // 1. Check wiki_page_links to collect referenced wiki page IDs const discoveredIds = new Set(); try { @@ -382,13 +391,13 @@ 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; + // this scan caches nothing — 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 page cached BY + // THIS SCAN 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 > cachedBefore || reachedMissCutoff; if (discoveryCompleted) { discoveredCacheKeys.add(cacheKey); } diff --git a/tests/wikis.test.ts b/tests/wikis.test.ts index 3ba367f..61eb204 100644 --- a/tests/wikis.test.ts +++ b/tests/wikis.test.ts @@ -844,3 +844,94 @@ describe("Wiki Error Propagation", () => { expect(probes).toBe(afterFirst); }); }); + +describe("Wiki Discovery Completion With A Primed Cache", () => { + beforeEach(() => { + clearWikiCache(); + }); + + const CACHE_KEY = "primed-tenant"; + + const pages: Record = { + 1: { title: "Onboarding", text: "How to get started" }, + 2: { title: "Deployment Runbook", text: "How to deploy the service" }, + }; + + /** Serves the fixture wiki. Shares a cache key with the failing client. */ + function healthyClient(probedIds: number[] = []) { + return { + baseUrl: "http://localhost:8080", + getCacheKey: () => CACHE_KEY, + get: async (path: string) => { + if (path === "/api/v3/wiki_page_links") { + return { _type: "Collection", total: 0, _embedded: { elements: [] } }; + } + const id = Number(path.match(/wiki_pages\/(\d+)/)?.[1] ?? NaN); + if (path.endsWith("/attachments")) { + return { _type: "Collection", total: 0, _embedded: { elements: [] } }; + } + probedIds.push(id); + const page = pages[id]; + if (!page) { + throw new OpenProjectError("not found", { statusCode: 404 }); + } + return { + _type: "WikiPage", + id, + title: page.title, + text: { raw: page.text }, + _links: { project: { href: "/api/v3/projects/1", title: "Demo Project" } }, + }; + }, + } as unknown as OpenProjectClient; + } + + /** Same tenant, but every wiki page probe fails non-fatally. */ + function outageClient(probedIds: number[] = []) { + return { + baseUrl: "http://localhost:8080", + getCacheKey: () => CACHE_KEY, + get: async (path: string) => { + if (path === "/api/v3/wiki_page_links") { + return { _type: "Collection", total: 0, _embedded: { elements: [] } }; + } + const id = Number(path.match(/wiki_pages\/(\d+)/)?.[1] ?? NaN); + probedIds.push(id); + throw new OpenProjectError("upstream exploded", { statusCode: 500 }); + }, + } as unknown as OpenProjectClient; + } + + /** + * `getWikiPage` writes into the same tenant-wide cache map that discovery + * populates, so "get a wiki page, then search" — an entirely ordinary LLM + * tool sequence — leaves the map non-empty before the probe loop has issued + * a single request. Judging completion on absolute cache size therefore + * lets an all-errors scan inherit that entry and mark itself complete, and + * wiki search then returns [] for the life of the process. + */ + test("a primed cache does not let an all-errors scan mark discovery complete", async () => { + // 1. Prime the tenant cache with one successful single-page fetch. + const primed = await getWikiPage(1, healthyClient()); + expect(primed.title).toBe("Onboarding"); + + // 2. A discovery scan in which every probe fails non-fatally. It caches + // nothing new and the 404 cutoff never trips, so discovery did NOT + // complete — even though the map is non-empty thanks to step 1. + const outageProbes: number[] = []; + const duringOutage = await searchWikiPages({ query: "deployment" }, outageClient(outageProbes)); + expect(outageProbes.length).toBeGreaterThan(0); + expect(duringOutage).toEqual([]); + + // 3. The instance recovers. Discovery must run again rather than + // short-circuiting on a cache that was wrongly marked complete. + const recoveryProbes: number[] = []; + const afterRecovery = await searchWikiPages( + { query: "deployment" }, + healthyClient(recoveryProbes) + ); + + expect(recoveryProbes.length).toBeGreaterThan(0); + expect(afterRecovery.map((page) => page.id)).toEqual([2]); + }); +});