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
71 changes: 71 additions & 0 deletions src/github/githubRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { mergeQuerySchemaWithShared, OctokitCommon } from './common';
import { CredentialStore, GitHub } from './credentials';
import {
AssignableUsersResponse,
CheckSuiteForRollup,
CreatePullRequestResponse,
FileContentResponse,
ForkDetailsResponse,
Expand Down Expand Up @@ -1962,6 +1963,22 @@ export class GitHubRepository extends Disposable {
};
}

// Workflows triggered from forks (e.g. by first-time contributors) can sit in an
// "awaiting approval" state where GitHub has created the workflow run but no check
// runs yet. These don't appear in the statusCheckRollup, which would otherwise leave
// the PR with no reported checks (surfaced as "Unknown"). Surface them as pending so
// the status reflects that something is waiting.
const checkSuites = result.data.repository.pullRequest.commits.nodes[0].commit.checkSuites?.nodes;
const awaitingApprovalStatuses = this.computeAwaitingApprovalStatuses(
checkSuites,
checks.statuses,
result.data.repository.pullRequest.url,
);
if (awaitingApprovalStatuses.length) {
checks.statuses = checks.statuses.concat(awaitingApprovalStatuses);
checks.state = this.computeOverallCheckState(checks.statuses);
}

let reviewRequirement: PullRequestReviewRequirement | null = null;
const rule = result.data.repository.pullRequest.baseRef?.refUpdateRule;
if (rule) {
Expand Down Expand Up @@ -2112,6 +2129,60 @@ export class GitHubRepository extends Disposable {
return CheckState.Success;
}

/**
* Build synthetic pending statuses for workflow runs that are awaiting approval
* (typically for pull requests from forks). Such runs exist as check suites but have
* not produced any check runs yet, so they are absent from the statusCheckRollup.
* Only suites that aren't already represented by an existing status are included, to
* avoid duplicating checks that GitHub has already reported.
*/
private computeAwaitingApprovalStatuses(
checkSuites: CheckSuiteForRollup[] | undefined,
existingStatuses: PullRequestCheckStatus[],
prUrl: string,
): PullRequestCheckStatus[] {
if (!checkSuites?.length) {
return [];
}

const existingWorkflowNames = new Set(
existingStatuses.map(status => status.workflowName).filter((name): name is string => !!name),
);

const awaitingApproval: PullRequestCheckStatus[] = [];
for (const suite of checkSuites) {
// A suite that is waiting or has only been requested and hasn't concluded is
// awaiting approval before it can run.
if (suite.conclusion !== null || (suite.status !== 'WAITING' && suite.status !== 'REQUESTED')) {
continue;
}

const workflowName = suite.workflowRun?.workflow.name;
if (workflowName && existingWorkflowNames.has(workflowName)) {
continue;
}
Comment on lines +2148 to +2163

awaitingApproval.push({
id: `awaiting-approval:${workflowName ?? awaitingApproval.length}`,
databaseId: undefined,
url: suite.app?.url,
avatarUrl:
suite.app?.logoUrl &&
getAvatarWithEnterpriseFallback(suite.app.logoUrl, undefined, this.remote.isEnterprise),
state: CheckState.Pending,
description: vscode.l10n.t('Waiting for approval'),
context: workflowName ?? vscode.l10n.t('Workflow awaiting approval'),
workflowName,
event: suite.workflowRun?.event,
targetUrl: prUrl,
isRequired: false,
isCheckRun: true,
});
}

return awaitingApproval;
}

/**
* Upload a file to GitHub via the mobile upload policy API. Returns a markdown
* snippet appropriate for embedding in an issue/PR comment.
Expand Down
27 changes: 27 additions & 0 deletions src/github/graphql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1039,6 +1039,30 @@ export function isCheckRun(x: CheckRun | StatusContext): x is CheckRun {
return x.__typename === 'CheckRun';
}

export interface CheckSuiteForRollup {
status: 'COMPLETED' | 'IN_PROGRESS' | 'PENDING' | 'QUEUED' | 'REQUESTED' | 'WAITING' | null;
conclusion:
| 'ACTION_REQUIRED'
| 'CANCELLED'
| 'FAILURE'
| 'NEUTRAL'
| 'SKIPPED'
| 'STALE'
| 'SUCCESS'
| 'TIMED_OUT'
| null;
workflowRun?: {
event: string;
workflow: {
name: string;
};
} | null;
app?: {
logoUrl: string;
url: string;
} | null;
}

export interface ChecksReviewNode {
authorAssociation: 'MEMBER' | 'OWNER' | 'MANNEQUIN' | 'COLLABORATOR' | 'CONTRIBUTOR' | 'FIRST_TIME_CONTRIBUTOR' | 'FIRST_TIMER' | 'NONE';
authorCanPushToRepository: boolean
Expand Down Expand Up @@ -1075,6 +1099,9 @@ export interface GetChecksResponse {
nodes: (StatusContext | CheckRun)[];
};
};
checkSuites?: {
nodes: CheckSuiteForRollup[];
} | null;
};
}[] | undefined;
};
Expand Down
28 changes: 28 additions & 0 deletions src/github/queriesShared.gql
Original file line number Diff line number Diff line change
Expand Up @@ -1108,6 +1108,22 @@ query GetChecks($owner: String!, $name: String!, $number: Int!) {
}
}
}
checkSuites(first: 100) {
nodes {
status
conclusion
workflowRun {
event
workflow {
name
}
}
app {
logoUrl
url
}
}
}
}
}
}
Expand Down Expand Up @@ -1180,6 +1196,18 @@ query GetChecksWithoutSuite($owner: String!, $name: String!, $number: Int!) {
}
}
}
checkSuites(first: 100) {
nodes {
status
conclusion
workflowRun {
event
workflow {
name
}
}
}
}
Comment on lines +1199 to +1210
}
}
}
Expand Down
89 changes: 89 additions & 0 deletions src/test/github/githubRepository.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,4 +131,93 @@ describe('GitHubRepository', function () {
assert.strictEqual(result.length, 2);
});
});

describe('computeAwaitingApprovalStatuses', function () {
function callComputeAwaitingApprovalStatuses(
repo: GitHubRepository,
checkSuites: any[] | undefined,
existingStatuses: PullRequestCheckStatus[],
prUrl: string,
): PullRequestCheckStatus[] {
return (repo as any).computeAwaitingApprovalStatuses(checkSuites, existingStatuses, prUrl);
}

function createSuite(overrides: Partial<{ status: string; conclusion: string | null; workflowName: string; event: string }>) {
const { status = 'WAITING', conclusion = null, workflowName, event } = overrides;
return {
status,
conclusion,
workflowRun: workflowName ? { event: event ?? 'pull_request', workflow: { name: workflowName } } : null,
app: null,
};
}

let repo: GitHubRepository;

beforeEach(function () {
const url = 'https://github.com/some/repo';
const remote = new GitHubRemote('origin', url, new Protocol(url), GitHubServerType.GitHubDotCom);
const rootUri = Uri.file('C:\\users\\test\\repo');
repo = new GitHubRepository(1, remote, rootUri, credentialStore, telemetry);
});

it('returns nothing when there are no check suites', function () {
assert.strictEqual(callComputeAwaitingApprovalStatuses(repo, undefined, [], 'url').length, 0);
assert.strictEqual(callComputeAwaitingApprovalStatuses(repo, [], [], 'url').length, 0);
});

it('surfaces a pending status for a waiting workflow', function () {
const suites = [createSuite({ status: 'WAITING', workflowName: 'CI' })];
const result = callComputeAwaitingApprovalStatuses(repo, suites, [], 'https://github.com/some/repo/pull/1');
assert.strictEqual(result.length, 1);
assert.strictEqual(result[0].state, CheckState.Pending);
assert.strictEqual(result[0].context, 'CI');
assert.strictEqual(result[0].workflowName, 'CI');
assert.strictEqual(result[0].targetUrl, 'https://github.com/some/repo/pull/1');
});

it('surfaces a pending status for a requested workflow', function () {
const suites = [createSuite({ status: 'REQUESTED', workflowName: 'CI' })];
const result = callComputeAwaitingApprovalStatuses(repo, suites, [], 'url');
assert.strictEqual(result.length, 1);
assert.strictEqual(result[0].state, CheckState.Pending);
});

it('ignores suites that have already concluded', function () {
const suites = [createSuite({ status: 'COMPLETED', conclusion: 'SUCCESS', workflowName: 'CI' })];
assert.strictEqual(callComputeAwaitingApprovalStatuses(repo, suites, [], 'url').length, 0);
});

it('ignores suites that are in progress', function () {
const suites = [createSuite({ status: 'IN_PROGRESS', workflowName: 'CI' })];
assert.strictEqual(callComputeAwaitingApprovalStatuses(repo, suites, [], 'url').length, 0);
});

it('does not duplicate a workflow already represented by an existing status', function () {
const suites = [createSuite({ status: 'WAITING', workflowName: 'CI' })];
const existing = [{
id: '1',
databaseId: undefined,
url: undefined,
avatarUrl: undefined,
state: CheckState.Success,
description: null,
targetUrl: null,
context: 'CI / build',
workflowName: 'CI',
event: 'pull_request',
isRequired: false,
isCheckRun: true,
} as PullRequestCheckStatus];
assert.strictEqual(callComputeAwaitingApprovalStatuses(repo, suites, existing, 'url').length, 0);
});

it('falls back to a generic context when the workflow name is unknown', function () {
const suites = [createSuite({ status: 'WAITING' })];
const result = callComputeAwaitingApprovalStatuses(repo, suites, [], 'url');
assert.strictEqual(result.length, 1);
assert.strictEqual(result[0].workflowName, undefined);
assert.ok(result[0].context.length > 0);
});
});
});