Skip to content

Commit a750a78

Browse files
josephperrottjosephperrott
committed
fix(github-actions): properly update mergeability status in early failures (#2706)
Previously during early failure cases, i.e. when the API request for the pull request validation tooling failed, the action would fail, but never update the mergeability status. This refactor attempts to ensure that we properly set a failing status in those situations. PR Close #2706
1 parent 492cc1f commit a750a78

File tree

5 files changed

+36918
-36801
lines changed

5 files changed

+36918
-36801
lines changed

‎.github/local-actions/branch-manager/action.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ inputs:
1414
owner:
1515
description: 'The owner of the repo for the pull request'
1616
required: true
17+
sha:
18+
description: 'The latest sha for the pull request'
19+
required: false
1720
runs:
1821
using: 'node20'
1922
main: 'main.js'

‎.github/local-actions/branch-manager/lib/git.ts‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import {
2+
assertValidPullRequestConfig,
3+
PullRequestConfig,
4+
} from '../../../../ng-dev/pr/config/index.js';
5+
6+
import {AuthenticatedGitClient} from '../../../../ng-dev/utils/git/authenticated-git-client.js';
7+
import {
8+
assertValidCaretakerConfig,
9+
assertValidGithubConfig,
10+
CaretakerConfig,
11+
getConfig,
12+
GithubConfig,
13+
setConfig,
14+
} from '../../../../ng-dev/utils/config.js';
15+
16+
/** The branch used as the primary branch for the temporary repo. */
17+
const mainBranchName = 'main';
18+
19+
export async function setupConfigAndGitClient(token: string, repo: {owner: string; repo: string}) {
20+
// Manually define the configuration for the pull request and github to prevent having to
21+
// checkout the repository before defining the config.
22+
// TODO(josephperrott): Load this from the actual repository.
23+
setConfig(<{pullRequest: PullRequestConfig; github: GithubConfig; caretaker: CaretakerConfig}>{
24+
github: {
25+
mainBranchName,
26+
owner: repo.owner,
27+
name: repo.repo,
28+
},
29+
pullRequest: {
30+
githubApiMerge: false,
31+
},
32+
caretaker: {},
33+
});
34+
/** The configuration used for the ng-dev tooling. */
35+
const config = await getConfig([
36+
assertValidGithubConfig,
37+
assertValidPullRequestConfig,
38+
assertValidCaretakerConfig,
39+
]);
40+
41+
AuthenticatedGitClient.configure(token);
42+
/** The git client used to perform actions. */
43+
const git = await AuthenticatedGitClient.get();
44+
45+
// Needed for testing the merge-ability via `git cherry-pick` in the merge strategy.
46+
git.run(['config', 'user.email', '[email protected]']);
47+
git.run(['config', 'user.name', 'Angular Robot']);
48+
49+
return {
50+
config,
51+
git,
52+
};
53+
}

‎.github/local-actions/branch-manager/lib/main.ts‎

Lines changed: 144 additions & 154 deletions
Original file line numberDiff line numberDiff line change
@@ -1,174 +1,25 @@
11
import * as core from '@actions/core';
2-
import {
3-
assertValidPullRequestConfig,
4-
PullRequestConfig,
5-
} from '../../../../ng-dev/pr/config/index.js';
2+
import {context as actionContext} from '@actions/github';
63
import {loadAndValidatePullRequest} from '../../../../ng-dev/pr/merge/pull-request.js';
74
import {AutosquashMergeStrategy} from '../../../../ng-dev/pr/merge/strategies/autosquash-merge.js';
8-
import {
9-
assertValidCaretakerConfig,
10-
assertValidGithubConfig,
11-
CaretakerConfig,
12-
getConfig,
13-
GithubConfig,
14-
setConfig,
15-
} from '../../../../ng-dev/utils/config.js';
16-
import {AuthenticatedGitClient} from '../../../../ng-dev/utils/git/authenticated-git-client.js';
5+
import {setupConfigAndGitClient} from './git.js';
6+
import {cloneRepoIntoTmpLocation} from './tmp.js';
177
import {
188
ANGULAR_ROBOT,
199
getAuthTokenFor,
2010
revokeActiveInstallationToken,
2111
} from '../../../../github-actions/utils.js';
2212
import {MergeConflictsFatalError} from '../../../../ng-dev/pr/merge/failures.js';
23-
import {chdir} from 'process';
24-
import {spawnSync} from 'child_process';
2513
import {createPullRequestValidationConfig} from '../../../../ng-dev/pr/common/validation/validation-config.js';
2614

2715
interface CommmitStatus {
2816
state: 'pending' | 'error' | 'failure' | 'success';
2917
description: string;
18+
targetUrl?: string;
3019
}
3120

32-
/** The directory name for the temporary repo used for validation. */
33-
const tempRepo = 'branch-mananger-repo';
3421
/** The context name used for the commmit status applied. */
3522
const statusContextName = 'mergeability';
36-
/** The branch used as the primary branch for the temporary repo. */
37-
const mainBranchName = 'main';
38-
39-
async function main(repo: {owner: string; repo: string}, token: string, pr: number) {
40-
// Because we want to perform this check in the targetted repository, we first need to check out the repo
41-
// and then move to the directory it is cloned into.
42-
chdir('/tmp');
43-
console.log(
44-
spawnSync('git', [
45-
'clone',
46-
'--depth=1',
47-
`https://github.com/${repo.owner}/${repo.repo}.git`,
48-
`./${tempRepo}`,
49-
]).output.toString(),
50-
);
51-
chdir(`/tmp/${tempRepo}`);
52-
53-
// Manually define the configuration for the pull request and github to prevent having to
54-
// checkout the repository before defining the config.
55-
// TODO(josephperrott): Load this from the actual repository.
56-
setConfig(<{pullRequest: PullRequestConfig; github: GithubConfig; caretaker: CaretakerConfig}>{
57-
github: {
58-
mainBranchName,
59-
owner: repo.owner,
60-
name: repo.repo,
61-
},
62-
pullRequest: {
63-
githubApiMerge: false,
64-
},
65-
caretaker: {},
66-
});
67-
/** The configuration used for the ng-dev tooling. */
68-
const config = await getConfig([
69-
assertValidGithubConfig,
70-
assertValidPullRequestConfig,
71-
assertValidCaretakerConfig,
72-
]);
73-
74-
AuthenticatedGitClient.configure(token);
75-
/** The git client used to perform actions. */
76-
const git = await AuthenticatedGitClient.get();
77-
78-
// Needed for testing the merge-ability via `git cherry-pick` in the merge strategy.
79-
git.run(['config', 'user.email', '[email protected]']);
80-
git.run(['config', 'user.name', 'Angular Robot']);
81-
82-
/** The pull request after being retrieved and validated. */
83-
const pullRequest = await loadAndValidatePullRequest(
84-
{git, config},
85-
pr,
86-
createPullRequestValidationConfig({
87-
assertSignedCla: true,
88-
assertMergeReady: true,
89-
90-
assertPending: false,
91-
assertChangesAllowForTargetLabel: false,
92-
assertPassingCi: false,
93-
assertCompletedReviews: false,
94-
assertEnforcedStatuses: false,
95-
assertMinimumReviews: false,
96-
}),
97-
);
98-
core.info('Validated PR information:');
99-
core.info(JSON.stringify(pullRequest));
100-
/** Whether any fatal validation failures were discovered. */
101-
let hasFatalFailures = false;
102-
/** The status information to be pushed as a status to the pull request. */
103-
let statusInfo: CommmitStatus = await (async () => {
104-
// Log validation failures and check for any fatal failures.
105-
if (pullRequest.validationFailures.length !== 0) {
106-
core.info(`Found ${pullRequest.validationFailures.length} failing validation(s)`);
107-
await core.group('Validation failures', async () => {
108-
for (const failure of pullRequest.validationFailures) {
109-
hasFatalFailures = !failure.canBeForceIgnored || hasFatalFailures;
110-
core.info(failure.message);
111-
}
112-
});
113-
}
114-
115-
// With any fatal failure the check is not necessary to do.
116-
if (hasFatalFailures) {
117-
core.info('One of the validations was fatal, setting the status as pending for the pr');
118-
return {
119-
description: 'Waiting to check until the pull request is ready',
120-
state: 'pending',
121-
};
122-
}
123-
124-
try {
125-
git.run(['checkout', mainBranchName]);
126-
/**
127-
* A merge strategy used to perform the merge check.
128-
* Any concrete class implementing MergeStrategy is sufficient as all of our usage is
129-
* defined in the abstract base class.
130-
* */
131-
const strategy = new AutosquashMergeStrategy(git);
132-
await strategy.prepare(pullRequest);
133-
await strategy.check(pullRequest);
134-
core.info('Merge check passes, setting a passing status on the pr');
135-
return {
136-
description: `Merges cleanly to ${pullRequest.targetBranches.join(', ')}`,
137-
state: 'success',
138-
};
139-
} catch (e) {
140-
// As the merge strategy class will express the failures during checks, any thrown error is a
141-
// failure for our merge check.
142-
let description: string;
143-
if (e instanceof MergeConflictsFatalError) {
144-
core.info('Merge conflict found');
145-
const passingBranches = pullRequest.targetBranches.filter(
146-
(branch) => !e.failedBranches.includes(branch),
147-
);
148-
description = `Unable to merge into: ${e.failedBranches.join(', ')} | Can merge into: ${passingBranches.join(',')}`;
149-
} else {
150-
core.info('Unknown error found when checking merge:');
151-
core.error(e as Error);
152-
description =
153-
'Cannot cleanly merge to all target branches, please update changes or PR target';
154-
}
155-
return {
156-
description,
157-
state: 'failure',
158-
};
159-
}
160-
})();
161-
162-
await git.github.repos.createCommitStatus({
163-
...repo,
164-
state: statusInfo.state,
165-
// Status descriptions are limited to 140 characters.
166-
description: statusInfo.description.substring(0, 139),
167-
sha: pullRequest.headSha,
168-
context: statusContextName,
169-
});
170-
}
171-
17223
/** The repository name for the pull request. */
17324
const repo = core.getInput('repo', {required: true, trimWhitespace: true});
17425
/** The owner of the repository for the pull request. */
@@ -182,9 +33,148 @@ if (isNaN(pr)) {
18233
}
18334
/** The token for the angular robot to perform actions in the requested repo. */
18435
const token = await getAuthTokenFor(ANGULAR_ROBOT, {repo, owner});
36+
const {
37+
/** The ng-dev configuration used for the environment */
38+
config,
39+
/** The Authenticated Git Client instance. */
40+
git,
41+
} = await setupConfigAndGitClient(token, {owner, repo});
42+
/** The sha of the latest commit on the pull request, which when provided is what triggered the check. */
43+
const sha = await (async () => {
44+
let sha = core.getInput('sha', {required: false, trimWhitespace: true}) || undefined;
45+
if (sha === undefined) {
46+
sha = (await git.github.pulls.get({owner, repo, pull_number: pr})).data.head.sha as string;
47+
}
48+
return sha;
49+
})();
50+
51+
/** Set the mergability status on the pull request provided in the environment. */
52+
async function setMergeabilityStatusOnPullRequest({state, description, targetUrl}: CommmitStatus) {
53+
await git.github.repos.createCommitStatus({
54+
owner,
55+
repo,
56+
sha,
57+
context: statusContextName,
58+
state,
59+
// Status descriptions are limited to 140 characters.
60+
description: description.substring(0, 139),
61+
target_url: targetUrl,
62+
});
63+
}
64+
65+
async function main() {
66+
try {
67+
// This is intentionally not awaited because we are just setting the status to pending, and wanting
68+
// to continue working.
69+
let _unawaitedPromise = setMergeabilityStatusOnPullRequest({
70+
state: 'pending',
71+
description: 'Mergability check in progress',
72+
});
73+
74+
// Create a tmp directory to perform checks in and change working to directory to it.
75+
await cloneRepoIntoTmpLocation({owner, repo});
76+
77+
/** The pull request after being retrieved and validated. */
78+
const pullRequest = await loadAndValidatePullRequest(
79+
{git, config},
80+
pr,
81+
createPullRequestValidationConfig({
82+
assertSignedCla: true,
83+
assertMergeReady: true,
84+
assertPending: false,
85+
assertChangesAllowForTargetLabel: false,
86+
assertPassingCi: false,
87+
assertCompletedReviews: false,
88+
assertEnforcedStatuses: false,
89+
assertMinimumReviews: false,
90+
}),
91+
);
92+
core.info('Validated PR information:');
93+
core.info(JSON.stringify(pullRequest, null, 2));
94+
/** Whether any fatal validation failures were discovered. */
95+
let hasFatalFailures = false;
96+
/** The status information to be pushed as a status to the pull request. */
97+
let statusInfo: CommmitStatus = await (async () => {
98+
// Log validation failures and check for any fatal failures.
99+
if (pullRequest.validationFailures.length !== 0) {
100+
core.info(`Found ${pullRequest.validationFailures.length} failing validation(s)`);
101+
await core.group('Validation failures', async () => {
102+
for (const failure of pullRequest.validationFailures) {
103+
hasFatalFailures = !failure.canBeForceIgnored || hasFatalFailures;
104+
core.info(failure.message);
105+
}
106+
});
107+
}
108+
109+
// With any fatal failure the check is not necessary to do.
110+
if (hasFatalFailures) {
111+
core.info('One of the validations was fatal, setting the status as pending for the pr');
112+
return {
113+
description: 'Waiting to check until the pull request is ready',
114+
state: 'pending',
115+
};
116+
}
117+
118+
try {
119+
git.run(['checkout', config.github.mainBranchName]);
120+
/**
121+
* A merge strategy used to perform the merge check.
122+
* Any concrete class implementing MergeStrategy is sufficient as all of our usage is
123+
* defined in the abstract base class.
124+
* */
125+
const strategy = new AutosquashMergeStrategy(git);
126+
await strategy.prepare(pullRequest);
127+
await strategy.check(pullRequest);
128+
core.info('Merge check passes, setting a passing status on the pr');
129+
return {
130+
description: `Merges cleanly to ${pullRequest.targetBranches.join(', ')}`,
131+
state: 'success',
132+
};
133+
} catch (e) {
134+
// As the merge strategy class will express the failures during checks, any thrown error is a
135+
// failure for our merge check.
136+
let description: string;
137+
if (e instanceof MergeConflictsFatalError) {
138+
core.info('Merge conflict found');
139+
const passingBranches = pullRequest.targetBranches.filter(
140+
(branch) => !e.failedBranches.includes(branch),
141+
);
142+
description = `Unable to merge into: ${e.failedBranches.join(', ')} | Can merge into: ${passingBranches.join(',')}`;
143+
} else {
144+
core.info('Unknown error found when checking merge:');
145+
core.error(e as Error);
146+
description =
147+
'Cannot cleanly merge to all target branches, please update changes or PR target';
148+
}
149+
return {
150+
description,
151+
state: 'failure',
152+
};
153+
}
154+
})();
155+
156+
await setMergeabilityStatusOnPullRequest(statusInfo);
157+
} catch (e: Error | unknown) {
158+
let description: string;
159+
const {runId, repo, serverUrl} = actionContext;
160+
const targetUrl = `${serverUrl}/${repo.owner}/${repo.repo}/actions/runs/${runId}`;
161+
if (e instanceof Error) {
162+
description = e.message;
163+
} else {
164+
description = 'Internal Error, see link for action log';
165+
}
166+
await setMergeabilityStatusOnPullRequest({
167+
state: 'error',
168+
description,
169+
targetUrl,
170+
});
171+
// Re-throw the error so that the action run is set as failing.
172+
throw e;
173+
}
174+
}
185175

186176
try {
187-
await main({repo, owner}, token, pr).catch((e: Error) => {
177+
await main().catch((e: Error) => {
188178
core.error(e);
189179
core.setFailed(e.message);
190180
});

0 commit comments

Comments
 (0)