From e4f5e1ddf1ca2879114005a1278d98693f5332d0 Mon Sep 17 00:00:00 2001 From: Donach <39565367+Donach@users.noreply.github.com> Date: Sat, 24 Oct 2026 04:20:13 +0200 Subject: [PATCH] fix(gitea): treat HTTP 404 as no-PR in the hosted-review branch lookup (#10411) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(gitea): treat HTTP 404 as no-PR in throwing hosted-review lookup hostedReview:forBranch always uses the throwing Gitea lookup variant, and requestJsonAtBase threw on any non-OK status. A 404 (PR deleted, repo absent on the forge, or Gitea masking a private repo) rejected the IPC handler and spammed the main-process log on every refresh for every unaffiliated branch. 404 means the resource is absent — an accepted "no PR", not a transport or auth failure. Map it to null even with throwOnFailure; 401/403/5xx/network errors still throw so creation eligibility keeps reporting 'unavailable'. Co-Authored-By: Claude Fable 5 * fix(gitea): scope the 404 no-PR exemption to the linked-PR fetch A blanket status !== 404 exemption in the shared request helper also covered the repo-level /pulls scan, where a 404 means a wrong API base URL, a repo absent from the forge, or a token without scope. Collapsing that to "no PR" would make a misconfigured Gitea host look permanently PR-less instead of surfacing reviewLookupOutcome: 'unavailable'. Gate the exemption on an explicit allowNotFound request option, set only on the single-resource /pulls/{n} fetch. Co-authored-by: Orca * test(gitea): pin the 200-with-empty-list no-PR shape alongside the 404 case Co-authored-by: Orca * fix(gitea): preserve deleted linked pull request absence handling Refresh the original Donach PR and its prior maintainer correction on current main, keeping the HTTP404 exemption limited to the linked pull resource. --------- Co-authored-by: Claude Fable 5 Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com> Co-authored-by: Orca --- src/main/gitea/client.test.ts | 38 +++++++++++++++++++++++++++++++++++ src/main/gitea/client.ts | 7 +++++-- 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/main/gitea/client.test.ts b/src/main/gitea/client.test.ts index 06efc4079e2..b87339067c2 100644 --- a/src/main/gitea/client.test.ts +++ b/src/main/gitea/client.test.ts @@ -261,6 +261,44 @@ describe('Gitea client', () => { ) }) + it('treats a 404 on the linked PR as "no PR" rather than a lookup failure', async () => { + const requested: string[] = [] + const fetchMock = vi.fn(async (url: string | URL) => { + requested.push(new URL(String(url)).pathname) + // The branch scan succeeds and is simply empty; only PR #42 is gone. + return String(url).includes('/pulls/42') + ? Response.json({ message: 'not found' }, { status: 404 }) + : Response.json([]) + }) + vi.stubGlobal('fetch', fetchMock) + + await expect( + getGiteaPullRequestForBranchOrThrow('/repo', 'feature/gitea', 42) + ).resolves.toBeNull() + expect(requested).toEqual(['/api/v1/repos/team/repo/pulls', '/api/v1/repos/team/repo/pulls/42']) + }) + + it('resolves null without throwing when the scan returns an empty list', async () => { + // Gitea has no head-branch filter, so "branch has no PR" arrives as a 200 + // with [] from the repo listing — never a 404. The throwing variant must + // still report that as a plain "no PR". + const fetchMock = vi.fn(async () => Response.json([])) + vi.stubGlobal('fetch', fetchMock) + + await expect(getGiteaPullRequestForBranchOrThrow('/repo', 'feature/gitea')).resolves.toBeNull() + }) + + it('still throws on a 404 from the /pulls scan so a misconfigured host is not read as "no PR"', async () => { + const fetchMock = vi.fn(async () => Response.json({ message: 'not found' }, { status: 404 })) + vi.stubGlobal('fetch', fetchMock) + + // A repo-level 404 means wrong base URL / repo absent / token lacks scope — + // collapsing it to "no PR" would hide the misconfiguration indefinitely. + await expect(getGiteaPullRequestForBranchOrThrow('/repo', 'feature/gitea')).rejects.toThrow( + /Gitea request failed: HTTP 404/ + ) + }) + it('expires successful scans and bounds retained repository listings', async () => { vi.useFakeTimers() try { diff --git a/src/main/gitea/client.ts b/src/main/gitea/client.ts index 90415dc0279..5e5b2da69b1 100644 --- a/src/main/gitea/client.ts +++ b/src/main/gitea/client.ts @@ -37,6 +37,8 @@ export type GiteaAuthStatus = { type RequestOptions = { searchParams?: Record timeoutMs?: number + // Why: only a single-resource fetch may read 404 as "absent". See requestJsonAtBase. + allowNotFound?: boolean } function envValue(name: string): string | null { @@ -93,7 +95,8 @@ async function requestJsonAtBase( }) if (!response.ok) { await cancelUnreadResponseBody(response) - if (throwOnFailure) { + // Why: listing 404s still signal repository or access failures. + if (throwOnFailure && !(options.allowNotFound && response.status === 404)) { throw new Error(`Gitea request failed: HTTP ${response.status}`) } return null @@ -299,7 +302,7 @@ export async function getGiteaPullRequestForBranch( const raw = await requestJson( repo, `/repos/${encodedRepoPath(repo)}/pulls/${encodeURIComponent(String(linkedPRNumber))}`, - {}, + { allowNotFound: true }, throwOnFailure ) return raw ? normalizePullRequest(repo, raw) : null