From ead6fdd358985fc3a301a03cf3cbabd7733de6de Mon Sep 17 00:00:00 2001 From: shin-core <153108882+shin-core@users.noreply.github.com> Date: Sat, 18 Jul 2026 06:51:55 +1000 Subject: [PATCH] fix(review): bound fetchShotContentBlock's screenshot fetch with a timeout fetchShotContentBlock fetched a screenshot URL with a bare fetch(url) and no time bound, unlike every other external fetch in this file and its siblings -- including resolveShotUrl in this same file, which already uses AbortSignal.timeout(EXTERNAL_SCREENSHOT_FETCH_TIMEOUT_MS). A slow or hanging shot host could stall the review pipeline for that PR indefinitely. Pass the same AbortSignal.timeout to the fetch. A TimeoutError rejection is already handled by the surrounding catch (returns undefined), so no other change is needed. Closes #7070 --- src/review/visual/capture.ts | 4 +++- test/unit/visual-capture.test.ts | 11 +++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/review/visual/capture.ts b/src/review/visual/capture.ts index c110562066..56fbe4e57b 100644 --- a/src/review/visual/capture.ts +++ b/src/review/visual/capture.ts @@ -127,7 +127,9 @@ export function hasSuccessfulBotCapture(routes: readonly CaptureRoute[]): boolea */ export async function fetchShotContentBlock(url: string): Promise { try { - const response = await fetch(url); + // Bound the fetch like every other external screenshot call in this file (#7070) -- resolveShotUrl below + // already uses this same timeout; a TimeoutError rejection is caught by the surrounding catch. + const response = await fetch(url, { signal: AbortSignal.timeout(EXTERNAL_SCREENSHOT_FETCH_TIMEOUT_MS) }); if (!response.ok) return undefined; const bytes = new Uint8Array(await response.arrayBuffer()); return { type: "image", data: base64Encode(await downscaleForVision(bytes)), mimeType: "image/png" }; diff --git a/test/unit/visual-capture.test.ts b/test/unit/visual-capture.test.ts index 3eed20326d..95ce18eaf4 100644 --- a/test/unit/visual-capture.test.ts +++ b/test/unit/visual-capture.test.ts @@ -2171,6 +2171,17 @@ describe("fetchShotContentBlock (#4111)", () => { vi.stubGlobal("fetch", vi.fn(async () => { throw new Error("network down"); })); await expect(fetchShotContentBlock("https://x/loopover/shot?key=broken")).resolves.toBeUndefined(); }); + + it("bounds the fetch with an AbortSignal timeout (#7070)", async () => { + const fetchMock = vi.fn( + async (_url: string, _init?: RequestInit) => new Response(new Uint8Array([137, 80, 78, 71]), { status: 200 }), + ); + vi.stubGlobal("fetch", fetchMock); + await fetchShotContentBlock("https://x/loopover/shot?key=timed"); + // Mirrors resolveShotUrl's own bounded fetch in this file -- an unresponsive shot host can't hang the call. + const init = fetchMock.mock.calls[0]?.[1]; + expect(init?.signal).toBeInstanceOf(AbortSignal); + }); });