From 72293f7f4ff617ff80404509aa76917367d7ae73 Mon Sep 17 00:00:00 2001 From: ezra Date: Wed, 13 May 2026 17:52:13 +0800 Subject: [PATCH] fix(server): fall back PR status queries for fine-grained tokens Fixes #926 --- .../src/services/github-service.test.ts | 180 +++++++++++++++--- .../server/src/services/github-service.ts | 52 +++-- 2 files changed, 190 insertions(+), 42 deletions(-) diff --git a/packages/server/src/services/github-service.test.ts b/packages/server/src/services/github-service.test.ts index 15cf5989c..1df3fb231 100644 --- a/packages/server/src/services/github-service.test.ts +++ b/packages/server/src/services/github-service.test.ts @@ -20,6 +20,9 @@ import { CheckoutPrStatusResponseSchema } from "../shared/messages.js"; const EXPECTED_GITHUB_FAST_POLL_MS = 20_000; const EXPECTED_GITHUB_SLOW_POLL_MS = 120_000; const EXPECTED_GITHUB_ERROR_BACKOFF_CAP_MS = 300_000; +const CURRENT_PR_STATUS_BASE_FIELDS = + "number,url,title,state,isDraft,baseRefName,headRefName,mergedAt,reviewDecision,mergeable,headRepositoryOwner"; +const CURRENT_PR_STATUS_FIELDS = `${CURRENT_PR_STATUS_BASE_FIELDS},statusCheckRollup`; interface RunnerCall { args: string[]; @@ -153,6 +156,16 @@ function noPullRequestError(args: string[] = ["pr", "view"]): GitHubCommandError }); } +function statusCheckRollupPermissionError(args: string[]): GitHubCommandError { + return new GitHubCommandError({ + args, + cwd: "/repo", + exitCode: 1, + stderr: + "GraphQL: Resource not accessible by personal access token (repository.pullRequest.statusCheckRollup)", + }); +} + function pullRequestJson(title: string): string { return JSON.stringify([ { @@ -1073,12 +1086,7 @@ describe("GitHubService", () => { headRef: "feature/pr-pane", }); - expect(runner.calls[0]?.args).toEqual([ - "pr", - "view", - "--json", - "number,url,title,state,isDraft,baseRefName,headRefName,mergedAt,statusCheckRollup,reviewDecision,mergeable,headRepositoryOwner", - ]); + expect(runner.calls[0]?.args).toEqual(["pr", "view", "--json", CURRENT_PR_STATUS_FIELDS]); expect(status).toEqual({ number: 42, repoOwner: "acme", @@ -1110,6 +1118,40 @@ describe("GitHubService", () => { }); }); + it("retries current PR view without statusCheckRollup when token permissions are insufficient", async () => { + const runner = createScriptedRunner([ + { + error: statusCheckRollupPermissionError(["pr", "view", "--json", CURRENT_PR_STATUS_FIELDS]), + }, + currentPullRequestJson({ + headRefName: "feature/pr-pane", + reviewDecision: "APPROVED", + }), + ]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + now: () => 100, + }); + + const status = await service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "feature/pr-pane", + }); + + expect(runner.calls.map((call) => call.args)).toEqual([ + ["pr", "view", "--json", CURRENT_PR_STATUS_FIELDS], + ["pr", "view", "--json", CURRENT_PR_STATUS_BASE_FIELDS], + ]); + expect(status).toMatchObject({ + title: "Fork PR", + headRefName: "feature/pr-pane", + checks: [], + checksStatus: "none", + reviewDecision: "approved", + }); + }); + it("defaults unexpected PR mergeability values to unknown", async () => { const runner = createRunner([ currentPullRequestJson({ @@ -1190,12 +1232,7 @@ describe("GitHubService", () => { headRefName: "feature/fork", }); expect(runner.calls.map((call) => call.args)).toEqual([ - [ - "pr", - "view", - "--json", - "number,url,title,state,isDraft,baseRefName,headRefName,mergedAt,statusCheckRollup,reviewDecision,mergeable,headRepositoryOwner", - ], + ["pr", "view", "--json", CURRENT_PR_STATUS_FIELDS], ["repo", "view", "--json", "owner,name,parent"], [ "pr", @@ -1206,10 +1243,107 @@ describe("GitHubService", () => { "all", "--head", "forkOwner:feature/fork", - "--json", - "number,url,title,state,isDraft,baseRefName,headRefName,mergedAt,statusCheckRollup,reviewDecision,mergeable,headRepositoryOwner", "--limit", "10", + "--json", + CURRENT_PR_STATUS_FIELDS, + ], + ]); + }); + + it("retries scoped PR list without statusCheckRollup when token permissions are insufficient", async () => { + const runner = createScriptedRunner([ + currentPullRequestJson({ + number: 7, + url: "https://github.com/parentOwner/parentRepo/pull/7", + title: "Stale tracking PR", + headRefName: "old-branch", + }), + JSON.stringify({ + owner: { login: "forkOwner" }, + name: "parentRepo", + parent: { owner: { login: "parentOwner" }, name: "parentRepo" }, + }), + { + error: statusCheckRollupPermissionError([ + "pr", + "list", + "--repo", + "parentOwner/parentRepo", + "--state", + "all", + "--head", + "forkOwner:feature/fork", + "--json", + CURRENT_PR_STATUS_FIELDS, + "--limit", + "10", + ]), + }, + JSON.stringify([ + { + number: 42, + url: "https://github.com/parentOwner/parentRepo/pull/42", + title: "Real fork PR", + state: "OPEN", + isDraft: false, + baseRefName: "main", + headRefName: "feature/fork", + mergedAt: null, + reviewDecision: "REVIEW_REQUIRED", + headRepositoryOwner: { login: "forkOwner" }, + }, + ]), + ]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + now: () => 100, + }); + + const status = await service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "feature/fork", + }); + + expect(status).toMatchObject({ + number: 42, + repoOwner: "parentOwner", + repoName: "parentRepo", + headRefName: "feature/fork", + checks: [], + checksStatus: "none", + }); + expect(runner.calls.map((call) => call.args)).toEqual([ + ["pr", "view", "--json", CURRENT_PR_STATUS_FIELDS], + ["repo", "view", "--json", "owner,name,parent"], + [ + "pr", + "list", + "--repo", + "parentOwner/parentRepo", + "--state", + "all", + "--head", + "forkOwner:feature/fork", + "--limit", + "10", + "--json", + CURRENT_PR_STATUS_FIELDS, + ], + [ + "pr", + "list", + "--repo", + "parentOwner/parentRepo", + "--state", + "all", + "--head", + "forkOwner:feature/fork", + "--limit", + "10", + "--json", + CURRENT_PR_STATUS_BASE_FIELDS, ], ]); }); @@ -1263,12 +1397,7 @@ describe("GitHubService", () => { }), ).resolves.toBeNull(); expect(calls.map((call) => call.args)).toEqual([ - [ - "pr", - "view", - "--json", - "number,url,title,state,isDraft,baseRefName,headRefName,mergedAt,statusCheckRollup,reviewDecision,mergeable,headRepositoryOwner", - ], + ["pr", "view", "--json", CURRENT_PR_STATUS_FIELDS], ["repo", "view", "--json", "owner,name,parent"], ]); }); @@ -1324,12 +1453,7 @@ describe("GitHubService", () => { headRefName: "main", }); expect(runner.calls.map((call) => call.args)).toEqual([ - [ - "pr", - "view", - "--json", - "number,url,title,state,isDraft,baseRefName,headRefName,mergedAt,statusCheckRollup,reviewDecision,mergeable,headRepositoryOwner", - ], + ["pr", "view", "--json", CURRENT_PR_STATUS_FIELDS], [ "pr", "list", @@ -1337,10 +1461,10 @@ describe("GitHubService", () => { "all", "--head", "main", - "--json", - "number,url,title,state,isDraft,baseRefName,headRefName,mergedAt,statusCheckRollup,reviewDecision,mergeable,headRepositoryOwner", "--limit", "10", + "--json", + CURRENT_PR_STATUS_FIELDS, ], ]); }); diff --git a/packages/server/src/services/github-service.ts b/packages/server/src/services/github-service.ts index 4ab98dd0f..f8f8e86bb 100644 --- a/packages/server/src/services/github-service.ts +++ b/packages/server/src/services/github-service.ts @@ -236,8 +236,9 @@ query PullRequestCheckoutTarget($owner: String!, $name: String!, $number: Int!) } }`; -const CURRENT_PR_STATUS_FIELDS = - "number,url,title,state,isDraft,baseRefName,headRefName,mergedAt,statusCheckRollup,reviewDecision,mergeable,headRepositoryOwner"; +const CURRENT_PR_STATUS_BASE_FIELDS = + "number,url,title,state,isDraft,baseRefName,headRefName,mergedAt,reviewDecision,mergeable,headRepositoryOwner"; +const CURRENT_PR_STATUS_FIELDS = `${CURRENT_PR_STATUS_BASE_FIELDS},statusCheckRollup`; const PULL_REQUEST_TIMELINE_QUERY = ` query PullRequestTimeline($owner: String!, $name: String!, $number: Int!) { @@ -1314,6 +1315,13 @@ function isNoPullRequestFoundError(error: unknown): boolean { return text.includes("no pull requests found"); } +function isStatusCheckRollupPermissionError(error: unknown): boolean { + if (!(error instanceof GitHubCommandError)) { + return false; + } + return error.stderr.toLowerCase().includes("statuscheckrollup"); +} + async function resolveCurrentPullRequestView(options: { cwd: string; headRef: string; @@ -1370,8 +1378,10 @@ async function tryCurrentPullRequestView(options: { run: (args: string[], options: GitHubCommandRunnerOptions) => Promise; }): Promise { try { - const stdout = await options.run(["pr", "view", "--json", CURRENT_PR_STATUS_FIELDS], { + const stdout = await runCurrentPullRequestStatusCommand({ cwd: options.cwd, + run: options.run, + args: ["pr", "view"], }); return parseCurrentPullRequestCandidate(stdout, options.headRef); } catch (error) { @@ -1392,18 +1402,13 @@ async function listCurrentPullRequestCandidates(options: { if (options.repo) { args.push("--repo", options.repo); } - args.push( - "--state", - "all", - "--head", - options.headRef, - "--json", - CURRENT_PR_STATUS_FIELDS, - "--limit", - "10", - ); + args.push("--state", "all", "--head", options.headRef, "--limit", "10"); try { - const stdout = await options.run(args, { cwd: options.cwd }); + const stdout = await runCurrentPullRequestStatusCommand({ + cwd: options.cwd, + run: options.run, + args, + }); return parseCurrentPullRequestCandidateList(stdout, options.headRef); } catch (error) { if (isNoPullRequestFoundError(error)) { @@ -1413,6 +1418,25 @@ async function listCurrentPullRequestCandidates(options: { } } +async function runCurrentPullRequestStatusCommand(options: { + cwd: string; + run: (args: string[], options: GitHubCommandRunnerOptions) => Promise; + args: string[]; +}): Promise { + try { + return await options.run([...options.args, "--json", CURRENT_PR_STATUS_FIELDS], { + cwd: options.cwd, + }); + } catch (error) { + if (!isStatusCheckRollupPermissionError(error)) { + throw error; + } + return options.run([...options.args, "--json", CURRENT_PR_STATUS_BASE_FIELDS], { + cwd: options.cwd, + }); + } +} + async function getGitHubRepoView(options: { cwd: string; run: (args: string[], options: GitHubCommandRunnerOptions) => Promise;