Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 13 additions & 22 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@
"ws": "^8.21.0",
"tar": "^7.5.19",
"js-yaml": "^4.3.0",
"adm-zip": "^0.6.0",
"lovable-tagger@1.2.0": {
"esbuild": "^0.28.1"
},
Expand Down
6 changes: 6 additions & 0 deletions src/github/backfill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2442,6 +2442,9 @@ const sleep = (ms: number): Promise<void> => new Promise((resolve) => setTimeout
* Fail-safe by construction: a fetch failure returns `[]` (never throws), so the review degrades to the same
* empty-diff state it has today rather than breaking. The persist is best-effort and only runs when the fetch
* actually returned files (a failed REST+GraphQL fetch must not wipe a row another sync just wrote).
*
* A REST+GraphQL double failure is logged (#7602) even though it stays fail-safe -- that combination used to
* discard its `warnings` entry silently, leaving no trace of how often the double-failure case actually happens.
*/
export async function fetchAndStorePullRequestFilesForReview(
env: Env,
Expand All @@ -2457,6 +2460,9 @@ export async function fetchAndStorePullRequestFilesForReview(
await sleep(REVIEW_FILES_EMPTY_RETRY_DELAY_MS);
files = await fetchOnce();
}
if (warnings.length > 0) {
console.error(JSON.stringify({ level: "warn", event: "review_files_fetch_failed", repoFullName, pullNumber, warnings }));
}
if (files.length === 0) return [];
const records = files.map((file) => toPullRequestFileRecordFromGitHub(repoFullName, pullNumber, file));
// Persist so the AI review, grounding, gate, check-run, and unified-comment reads in THIS run (and any later
Expand Down
32 changes: 32 additions & 0 deletions test/unit/backfill-2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,38 @@ describe("GitHub backfill", () => {
await expect(fetchAndStorePullRequestFilesForReview(env, "JSONbored/gittensory", 56, "public-token")).resolves.toEqual([]);
expect(filesCalls).toBe(2);
});

it("logs a structured warning (#7602) when both REST and GraphQL file fetches fail -- this used to be totally silent", async () => {
const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" });
vi.stubGlobal("fetch", async () => new Response("boom", { status: 500 }));
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
try {
await expect(fetchAndStorePullRequestFilesForReview(env, "JSONbored/gittensory", 100, "public-token")).resolves.toEqual([]);
const traceLine = errorSpy.mock.calls.map((c) => String(c[0])).find((line) => line.includes("review_files_fetch_failed"));
expect(traceLine).toBeDefined();
const parsed = JSON.parse(traceLine as string) as Record<string, unknown>;
expect(parsed).toMatchObject({ level: "warn", event: "review_files_fetch_failed", repoFullName: "JSONbored/gittensory", pullNumber: 100 });
// Both fetchOnce() attempts hit the REST+GraphQL double-failure path, so the warning is recorded twice.
expect(parsed.warnings).toEqual([
"File sync failed for #100: GitHub REST and GraphQL detail fetches failed.",
"File sync failed for #100: GitHub REST and GraphQL detail fetches failed.",
]);
} finally {
errorSpy.mockRestore();
}
});

it("never logs a warning when GitHub returns a real, successful empty files list (not a fetch failure)", async () => {
const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" });
vi.stubGlobal("fetch", async () => Response.json([]));
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
try {
await expect(fetchAndStorePullRequestFilesForReview(env, "JSONbored/gittensory", 101, "public-token")).resolves.toEqual([]);
expect(errorSpy).not.toHaveBeenCalled();
} finally {
errorSpy.mockRestore();
}
});
});

describe("fetchLiveCiAggregate", () => {
Expand Down
Loading