discovery-index(query): stop caching a GitHub-failure-truncated candidate set for the full TTL and serving it as complete

Fixes #10031

Author: phamngocquy

packages/discovery-index/src/discovery-query.ts

@@ -147,9 +147,17 @@ function surfaceGithubWarnings(source: string, warnings: string[]): void {
   }
 }
 
-async function computeCandidates(query: DiscoveryIndexQuery, deps: DiscoveryQueryDeps): Promise<DiscoveryIndexCandidate[]> {
+interface ComputeCandidatesResult {
+  candidates: DiscoveryIndexCandidate[];
+  /** False when any fetchRepoIssues/searchIssues call returned warnings — an incomplete live snapshot is
+   *  inconclusive, not evidence, and must not be promoted into the shared result cache. */
+  complete: boolean;
+}
+
+async function computeCandidates(query: DiscoveryIndexQuery, deps: DiscoveryQueryDeps): Promise<ComputeCandidatesResult> {
   const seen = new Set<string>();
   const candidates: DiscoveryIndexCandidate[] = [];
+  let complete = true;
 
   const addCandidate = (repoFullName: string, issue: GitHubIssue, verdict: AiPolicyVerdict): void => {
     const candidate = buildCandidate(repoFullName, issue, verdict);
@@ -174,28 +182,31 @@ async function computeCandidates(query: DiscoveryIndexQuery, deps: DiscoveryQuer
     const verdict = await resolveRepoAiPolicy(repoFullName, deps);
     if (!verdict.allowed) continue;
     const { issues, warnings } = await deps.github.fetchRepoIssues(repoFullName);
+    if (warnings.length > 0) complete = false;
     surfaceGithubWarnings(repoFullName, warnings);
     for (const issue of issues) addCandidate(repoFullName, issue, verdict);
   }
 
   for (const org of query.orgs) {
     const searchQuery = `org:${org} state:open type:issue`;
     const { issues, warnings } = await deps.github.searchIssues(searchQuery);
+    if (warnings.length > 0) complete = false;
     surfaceGithubWarnings(searchQuery, warnings);
     await addFromSearch(issues);
   }
 
   for (const term of query.searchTerms) {
     const searchQuery = `${term} state:open type:issue`;
     const { issues, warnings } = await deps.github.searchIssues(searchQuery);
+    if (warnings.length > 0) complete = false;
     surfaceGithubWarnings(searchQuery, warnings);
     await addFromSearch(issues);
   }
 
   // Deterministic ordering so pagination offsets are stable across a cache lifetime (and identical for two
-  // requests that happen to race a cache miss — see computeCandidates' getOrCompute caller).
+  // requests that happen to race a cache miss — see runDiscoveryQuery's result-cache caller).
   candidates.sort((a, b) => (a.repoFullName === b.repoFullName ? a.issueNumber - b.issueNumber : a.repoFullName.localeCompare(b.repoFullName)));
-  return candidates;
+  return { candidates, complete };
 }
 
 /**
@@ -210,10 +221,17 @@ async function computeCandidates(query: DiscoveryIndexQuery, deps: DiscoveryQuer
 export async function runDiscoveryQuery(query: DiscoveryIndexQuery, deps: DiscoveryQueryDeps): Promise<DiscoveryIndexResponse> {
   const scopeKey = scopeCacheKey(query);
   let missed = false;
-  const allCandidates = await deps.resultCache.getOrCompute(scopeKey, deps.cacheTtlMs, () => {
+  let allCandidates = deps.resultCache.get(scopeKey);
+  if (allCandidates === undefined) {
     missed = true;
-    return computeCandidates(query, deps);
-  });
+    const computed = await computeCandidates(query, deps);
+    allCandidates = computed.candidates;
+    // An incomplete live snapshot is inconclusive, not evidence — return it to this caller but never
+    // promote it into the shared result cache (mirrors listMigrationFilenamesAtRef's truncated-tree posture).
+    if (computed.complete) {
+      deps.resultCache.set(scopeKey, allCandidates, deps.cacheTtlMs);
+    }
+  }
   incr("discovery_index_cache_lookups_total", { cache: "result", outcome: missed ? "miss" : "hit" });
   const offset = decodeCursor(query.cursor);
   const page = allCandidates.slice(offset, offset + query.limit);

test/unit/discovery-index/discovery-query.test.ts

@@ -359,4 +359,65 @@ describe("discovery-index runDiscoveryQuery (#7164)", () => {
     await runDiscoveryQuery(query({ repos: ["acme/one"] }), makeDeps(github));
     expect(errorSpy).not.toHaveBeenCalled();
   });
+
+  it("caches a complete pass and serves the second identical query from the result cache", async () => {
+    const { github, calls } = makeStubGitHub({
+      issuesByRepo: { "owner/repo": [{ number: 42, title: "Cached issue" }] },
+      filesByRepo: { "owner/repo": { "AI-USAGE.md": ALLOWED_AI_USAGE } },
+    });
+    const deps = makeDeps(github);
+    const first = await runDiscoveryQuery(query({ repos: ["owner/repo"] }), deps);
+    const second = await runDiscoveryQuery(query({ repos: ["owner/repo"] }), deps);
+    expect(calls.filter((c) => c.method === "fetchRepoIssues")).toHaveLength(1);
+    expect(second).toEqual(first);
+    expect(first.candidates).toHaveLength(1);
+    expect(first.candidates[0]?.issueNumber).toBe(42);
+  });
+
+  it("REGRESSION: a GitHub-failure-truncated candidate set is not cached for the TTL", async () => {
+    const oneIssue: GitHubIssue = { number: 1, title: "Partial page" };
+    const { github, calls } = makeStubGitHub({
+      issuesByRepo: { "owner/repo": [oneIssue] },
+      warningsByRepo: { "owner/repo": ["GitHub returned 500 for owner/repo issues"] },
+      filesByRepo: { "owner/repo": { "AI-USAGE.md": ALLOWED_AI_USAGE } },
+    });
+    const deps = makeDeps(github);
+    const first = await runDiscoveryQuery(query({ repos: ["owner/repo"] }), deps);
+    const second = await runDiscoveryQuery(query({ repos: ["owner/repo"] }), deps);
+    expect(calls.filter((c) => c.method === "fetchRepoIssues")).toHaveLength(2);
+    expect(first.candidates).toHaveLength(1);
+    expect(second.candidates).toEqual(first.candidates);
+    expect(counterValue("discovery_index_cache_lookups_total", { cache: "result", outcome: "miss" })).toBe(2);
+    expect(counterValue("discovery_index_cache_lookups_total", { cache: "result", outcome: "hit" })).toBe(0);
+  });
+
+  it("does not cache org-search results when searchIssues returns warnings", async () => {
+    const { github, calls } = makeStubGitHub({
+      searchResults: {
+        "org:acme state:open type:issue": [{ number: 7, title: "Org hit", repository_url: "https://api.github.com/repos/acme/one" }],
+      },
+      warningsBySearch: { "org:acme state:open type:issue": ["GitHub returned 500 for search"] },
+      filesByRepo: { "acme/one": { "AI-USAGE.md": ALLOWED_AI_USAGE } },
+    });
+    const deps = makeDeps(github);
+    const q = query({ orgs: ["acme"] });
+    await runDiscoveryQuery(q, deps);
+    await runDiscoveryQuery(q, deps);
+    expect(calls.filter((c) => c.method === "searchIssues")).toHaveLength(2);
+  });
+
+  it("does not cache search-term results when searchIssues returns warnings", async () => {
+    const { github, calls } = makeStubGitHub({
+      searchResults: {
+        "flaky test state:open type:issue": [{ number: 9, title: "Term hit", repository_url: "https://api.github.com/repos/acme/one" }],
+      },
+      warningsBySearch: { "flaky test state:open type:issue": ["GitHub returned 503 for search"] },
+      filesByRepo: { "acme/one": { "AI-USAGE.md": ALLOWED_AI_USAGE } },
+    });
+    const deps = makeDeps(github);
+    const q = query({ searchTerms: ["flaky test"] });
+    await runDiscoveryQuery(q, deps);
+    await runDiscoveryQuery(q, deps);
+    expect(calls.filter((c) => c.method === "searchIssues")).toHaveLength(2);
+  });
 });