fix(server): fall back PR status queries for fine-grained tokens
Fixes #926
This commit is contained in:
@@ -20,6 +20,9 @@ import { CheckoutPrStatusResponseSchema } from "../shared/messages.js";
|
|||||||
const EXPECTED_GITHUB_FAST_POLL_MS = 20_000;
|
const EXPECTED_GITHUB_FAST_POLL_MS = 20_000;
|
||||||
const EXPECTED_GITHUB_SLOW_POLL_MS = 120_000;
|
const EXPECTED_GITHUB_SLOW_POLL_MS = 120_000;
|
||||||
const EXPECTED_GITHUB_ERROR_BACKOFF_CAP_MS = 300_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 {
|
interface RunnerCall {
|
||||||
args: string[];
|
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 {
|
function pullRequestJson(title: string): string {
|
||||||
return JSON.stringify([
|
return JSON.stringify([
|
||||||
{
|
{
|
||||||
@@ -1073,12 +1086,7 @@ describe("GitHubService", () => {
|
|||||||
headRef: "feature/pr-pane",
|
headRef: "feature/pr-pane",
|
||||||
});
|
});
|
||||||
|
|
||||||
expect(runner.calls[0]?.args).toEqual([
|
expect(runner.calls[0]?.args).toEqual(["pr", "view", "--json", CURRENT_PR_STATUS_FIELDS]);
|
||||||
"pr",
|
|
||||||
"view",
|
|
||||||
"--json",
|
|
||||||
"number,url,title,state,isDraft,baseRefName,headRefName,mergedAt,statusCheckRollup,reviewDecision,mergeable,headRepositoryOwner",
|
|
||||||
]);
|
|
||||||
expect(status).toEqual({
|
expect(status).toEqual({
|
||||||
number: 42,
|
number: 42,
|
||||||
repoOwner: "acme",
|
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 () => {
|
it("defaults unexpected PR mergeability values to unknown", async () => {
|
||||||
const runner = createRunner([
|
const runner = createRunner([
|
||||||
currentPullRequestJson({
|
currentPullRequestJson({
|
||||||
@@ -1190,12 +1232,7 @@ describe("GitHubService", () => {
|
|||||||
headRefName: "feature/fork",
|
headRefName: "feature/fork",
|
||||||
});
|
});
|
||||||
expect(runner.calls.map((call) => call.args)).toEqual([
|
expect(runner.calls.map((call) => call.args)).toEqual([
|
||||||
[
|
["pr", "view", "--json", CURRENT_PR_STATUS_FIELDS],
|
||||||
"pr",
|
|
||||||
"view",
|
|
||||||
"--json",
|
|
||||||
"number,url,title,state,isDraft,baseRefName,headRefName,mergedAt,statusCheckRollup,reviewDecision,mergeable,headRepositoryOwner",
|
|
||||||
],
|
|
||||||
["repo", "view", "--json", "owner,name,parent"],
|
["repo", "view", "--json", "owner,name,parent"],
|
||||||
[
|
[
|
||||||
"pr",
|
"pr",
|
||||||
@@ -1206,10 +1243,107 @@ describe("GitHubService", () => {
|
|||||||
"all",
|
"all",
|
||||||
"--head",
|
"--head",
|
||||||
"forkOwner:feature/fork",
|
"forkOwner:feature/fork",
|
||||||
"--json",
|
|
||||||
"number,url,title,state,isDraft,baseRefName,headRefName,mergedAt,statusCheckRollup,reviewDecision,mergeable,headRepositoryOwner",
|
|
||||||
"--limit",
|
"--limit",
|
||||||
"10",
|
"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();
|
).resolves.toBeNull();
|
||||||
expect(calls.map((call) => call.args)).toEqual([
|
expect(calls.map((call) => call.args)).toEqual([
|
||||||
[
|
["pr", "view", "--json", CURRENT_PR_STATUS_FIELDS],
|
||||||
"pr",
|
|
||||||
"view",
|
|
||||||
"--json",
|
|
||||||
"number,url,title,state,isDraft,baseRefName,headRefName,mergedAt,statusCheckRollup,reviewDecision,mergeable,headRepositoryOwner",
|
|
||||||
],
|
|
||||||
["repo", "view", "--json", "owner,name,parent"],
|
["repo", "view", "--json", "owner,name,parent"],
|
||||||
]);
|
]);
|
||||||
});
|
});
|
||||||
@@ -1324,12 +1453,7 @@ describe("GitHubService", () => {
|
|||||||
headRefName: "main",
|
headRefName: "main",
|
||||||
});
|
});
|
||||||
expect(runner.calls.map((call) => call.args)).toEqual([
|
expect(runner.calls.map((call) => call.args)).toEqual([
|
||||||
[
|
["pr", "view", "--json", CURRENT_PR_STATUS_FIELDS],
|
||||||
"pr",
|
|
||||||
"view",
|
|
||||||
"--json",
|
|
||||||
"number,url,title,state,isDraft,baseRefName,headRefName,mergedAt,statusCheckRollup,reviewDecision,mergeable,headRepositoryOwner",
|
|
||||||
],
|
|
||||||
[
|
[
|
||||||
"pr",
|
"pr",
|
||||||
"list",
|
"list",
|
||||||
@@ -1337,10 +1461,10 @@ describe("GitHubService", () => {
|
|||||||
"all",
|
"all",
|
||||||
"--head",
|
"--head",
|
||||||
"main",
|
"main",
|
||||||
"--json",
|
|
||||||
"number,url,title,state,isDraft,baseRefName,headRefName,mergedAt,statusCheckRollup,reviewDecision,mergeable,headRepositoryOwner",
|
|
||||||
"--limit",
|
"--limit",
|
||||||
"10",
|
"10",
|
||||||
|
"--json",
|
||||||
|
CURRENT_PR_STATUS_FIELDS,
|
||||||
],
|
],
|
||||||
]);
|
]);
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -236,8 +236,9 @@ query PullRequestCheckoutTarget($owner: String!, $name: String!, $number: Int!)
|
|||||||
}
|
}
|
||||||
}`;
|
}`;
|
||||||
|
|
||||||
const CURRENT_PR_STATUS_FIELDS =
|
const CURRENT_PR_STATUS_BASE_FIELDS =
|
||||||
"number,url,title,state,isDraft,baseRefName,headRefName,mergedAt,statusCheckRollup,reviewDecision,mergeable,headRepositoryOwner";
|
"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 = `
|
const PULL_REQUEST_TIMELINE_QUERY = `
|
||||||
query PullRequestTimeline($owner: String!, $name: String!, $number: Int!) {
|
query PullRequestTimeline($owner: String!, $name: String!, $number: Int!) {
|
||||||
@@ -1314,6 +1315,13 @@ function isNoPullRequestFoundError(error: unknown): boolean {
|
|||||||
return text.includes("no pull requests found");
|
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: {
|
async function resolveCurrentPullRequestView(options: {
|
||||||
cwd: string;
|
cwd: string;
|
||||||
headRef: string;
|
headRef: string;
|
||||||
@@ -1370,8 +1378,10 @@ async function tryCurrentPullRequestView(options: {
|
|||||||
run: (args: string[], options: GitHubCommandRunnerOptions) => Promise<string>;
|
run: (args: string[], options: GitHubCommandRunnerOptions) => Promise<string>;
|
||||||
}): Promise<ResolvedPullRequestCandidate | null> {
|
}): Promise<ResolvedPullRequestCandidate | null> {
|
||||||
try {
|
try {
|
||||||
const stdout = await options.run(["pr", "view", "--json", CURRENT_PR_STATUS_FIELDS], {
|
const stdout = await runCurrentPullRequestStatusCommand({
|
||||||
cwd: options.cwd,
|
cwd: options.cwd,
|
||||||
|
run: options.run,
|
||||||
|
args: ["pr", "view"],
|
||||||
});
|
});
|
||||||
return parseCurrentPullRequestCandidate(stdout, options.headRef);
|
return parseCurrentPullRequestCandidate(stdout, options.headRef);
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
@@ -1392,18 +1402,13 @@ async function listCurrentPullRequestCandidates(options: {
|
|||||||
if (options.repo) {
|
if (options.repo) {
|
||||||
args.push("--repo", options.repo);
|
args.push("--repo", options.repo);
|
||||||
}
|
}
|
||||||
args.push(
|
args.push("--state", "all", "--head", options.headRef, "--limit", "10");
|
||||||
"--state",
|
|
||||||
"all",
|
|
||||||
"--head",
|
|
||||||
options.headRef,
|
|
||||||
"--json",
|
|
||||||
CURRENT_PR_STATUS_FIELDS,
|
|
||||||
"--limit",
|
|
||||||
"10",
|
|
||||||
);
|
|
||||||
try {
|
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);
|
return parseCurrentPullRequestCandidateList(stdout, options.headRef);
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
if (isNoPullRequestFoundError(error)) {
|
if (isNoPullRequestFoundError(error)) {
|
||||||
@@ -1413,6 +1418,25 @@ async function listCurrentPullRequestCandidates(options: {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async function runCurrentPullRequestStatusCommand(options: {
|
||||||
|
cwd: string;
|
||||||
|
run: (args: string[], options: GitHubCommandRunnerOptions) => Promise<string>;
|
||||||
|
args: string[];
|
||||||
|
}): Promise<string> {
|
||||||
|
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: {
|
async function getGitHubRepoView(options: {
|
||||||
cwd: string;
|
cwd: string;
|
||||||
run: (args: string[], options: GitHubCommandRunnerOptions) => Promise<string>;
|
run: (args: string[], options: GitHubCommandRunnerOptions) => Promise<string>;
|
||||||
|
|||||||
Reference in New Issue
Block a user