fix(wikis): judge discovery completion on cache growth, not absolute size
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) <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
e76cfa750b
commit
fa05fcc1c6
+16
-7
@@ -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<number>();
|
||||
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);
|
||||
}
|
||||
|
||||
@@ -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<number, { title: string; text: string }> = {
|
||||
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]);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user