Skip to content
Open
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
40 changes: 26 additions & 14 deletions .github/local-actions/branch-manager/main.js

Large diffs are not rendered by default.

9 changes: 7 additions & 2 deletions ng-dev/pr/common/fetch-pull-request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ export const PR_SCHEMA = {
commit: {
oid: graphqlTypes.string,
authoredDate: graphqlTypes.string,
committedDate: graphqlTypes.string,
pushedDate: graphqlTypes.custom<string | null>(),
statusCheckRollup: optional({
state: graphqlTypes.custom<StatusState>(),
contexts: params(
Expand Down Expand Up @@ -168,6 +170,7 @@ export const PR_COMMENTS_SCHEMA = params(
},
authorAssociation: graphqlTypes.custom<CommentAuthorAssociation>(),
bodyText: graphqlTypes.string,
createdAt: graphqlTypes.string,
},
);

Expand Down Expand Up @@ -246,17 +249,19 @@ export function getStatusesForPullRequest(
.forEach((context) => {
switch (context.__typename) {
case 'CheckRun':
statusMap.set(context.name, {
statusMap.set(`check:${context.name}`, {
type: 'check' as const,
name: context.name,
status: normalizeGithubCheckState(context.conclusion, context.status),
});
break;
case 'StatusContext':
statusMap.set(context.context!, {
statusMap.set(`status:${context.context!}`, {
type: 'status' as const,
name: context.context!,
status: normalizeGithubStatusState(context.state!),
});
break;
}
});

Expand Down
233 changes: 232 additions & 1 deletion ng-dev/pr/common/test/common.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,12 @@ import {GoogleSyncConfig} from '../../../utils/g3-sync-config.js';
import {targetLabels} from '../labels/target.js';
import {Log} from '../../../utils/logging.js';

import {PullRequestFromGithub} from '../fetch-pull-request.js';
import {
getStatusesForPullRequest,
PullRequestFromGithub,
PullRequestStatus,
} from '../fetch-pull-request.js';
import {mergeLabels} from '../labels/merge.js';
import {requiresLabels} from '../labels/requires.js';

import {assertValidPullRequest} from '../validation/validate-pull-request.js';
Expand Down Expand Up @@ -88,6 +93,173 @@ describe('pull request validation', () => {
return configFileName;
}

describe('assert-signed-cla', () => {
it('should pass when cla/google commit status is passing', async () => {
const config = createIsolatedValidationConfig({assertSignedCla: true});
const pr = createTestPullRequest();
pr.commits.nodes[1].commit.statusCheckRollup = {
state: 'SUCCESS',
contexts: {
nodes: [
{
__typename: 'StatusContext',
context: 'cla/google',
state: 'SUCCESS',
createdAt: '2026-07-13T10:00:00Z',
} as any,
],
},
};

const results = await assertValidPullRequest(pr, config, ngDevConfig, null, prTarget, git);
expect(results.length).toBe(0);
});

it('should fail when only a CheckRun named cla/google is passing', async () => {
const config = createIsolatedValidationConfig({assertSignedCla: true});
const pr = createTestPullRequest();
pr.commits.nodes[1].commit.statusCheckRollup = {
state: 'SUCCESS',
contexts: {
nodes: [
{
__typename: 'CheckRun',
name: 'cla/google',
status: 'COMPLETED',
conclusion: 'SUCCESS',
completedAt: '2026-07-13T10:05:00Z',
} as any,
],
},
};

const results = await assertValidPullRequest(pr, config, ngDevConfig, null, prTarget, git);
expect(results.length).toBe(1);
expect(results[0].message).toBe('CLA is not signed by the contributor.');
});

it('should not allow a passing CheckRun named cla/google to overwrite a failing StatusContext', async () => {
const config = createIsolatedValidationConfig({assertSignedCla: true});
const pr = createTestPullRequest();
pr.commits.nodes[1].commit.statusCheckRollup = {
state: 'FAILURE',
contexts: {
nodes: [
{
__typename: 'StatusContext',
context: 'cla/google',
state: 'FAILURE',
createdAt: '2026-07-13T10:00:00Z',
} as any,
{
__typename: 'CheckRun',
name: 'cla/google',
status: 'COMPLETED',
conclusion: 'SUCCESS',
completedAt: '2026-07-13T10:05:00Z',
} as any,
],
},
};

const {statuses} = getStatusesForPullRequest(pr);
expect(statuses).toEqual([
{type: 'status', name: 'cla/google', status: PullRequestStatus.FAILING},
{type: 'check', name: 'cla/google', status: PullRequestStatus.PASSING},
]);

const results = await assertValidPullRequest(pr, config, ngDevConfig, null, prTarget, git);
expect(results.length).toBe(1);
expect(results[0].message).toBe('CLA is not signed by the contributor.');
});
});

describe('assert-allowed-target-label', () => {
const fakeReleaseTrains = {isFeatureFreeze: () => false} as any;

it('should reject target: automation when author is not an automation bot', async () => {
const config = createIsolatedValidationConfig({assertChangesAllowForTargetLabel: true});
const pr = createTestPullRequest();
pr.author = {login: 'external-user'};
const automationTarget = {branches: ['main'], label: targetLabels['TARGET_AUTOMATION']};

const results = await assertValidPullRequest(
pr,
config,
ngDevConfig,
fakeReleaseTrains,
automationTarget,
git,
);
expect(results.length).toBe(1);
expect(results[0].message).toContain(
'Cannot merge into branch for "target: automation" as the pull request is authored by "external-user"',
);
});

it('should reject target: automation for non-bot author even when merge: fix commit message is applied', async () => {
const config = createIsolatedValidationConfig({assertChangesAllowForTargetLabel: true});
const pr = createTestPullRequest();
pr.author = {login: 'external-user'};
pr.labels.nodes.push({name: mergeLabels['MERGE_FIX_COMMIT_MESSAGE'].name});
const automationTarget = {branches: ['main'], label: targetLabels['TARGET_AUTOMATION']};

const results = await assertValidPullRequest(
pr,
config,
ngDevConfig,
fakeReleaseTrains,
automationTarget,
git,
);
expect(results.length).toBe(1);
expect(results[0].message).toContain(
'Cannot merge into branch for "target: automation" as the pull request is authored by "external-user"',
);
});

it('should allow target: automation when author is angular-robot', async () => {
const config = createIsolatedValidationConfig({assertChangesAllowForTargetLabel: true});
const pr = createTestPullRequest();
pr.author = {login: 'angular-robot'};
const automationTarget = {branches: ['main'], label: targetLabels['TARGET_AUTOMATION']};

const results = await assertValidPullRequest(
pr,
config,
ngDevConfig,
fakeReleaseTrains,
automationTarget,
git,
);
expect(results.length).toBe(0);
});

it('should still skip commit message validation for non-automation labels when merge: fix commit message is applied', async () => {
const config = createIsolatedValidationConfig({assertChangesAllowForTargetLabel: true});
const pr = createTestPullRequest();
pr.commits.nodes = [
{
commit: {
oid: '1234',
message: 'feat(ng-dev): add new feature',
} as any,
},
];
pr.labels.nodes.push({name: mergeLabels['MERGE_FIX_COMMIT_MESSAGE'].name});

const results = await assertValidPullRequest(
pr,
config,
ngDevConfig,
fakeReleaseTrains,
prTarget,
git,
);
expect(results.length).toBe(0);
});
});

describe('assert-enforce-tested', () => {
it('should require a TGP when label is present', async () => {
const config = createIsolatedValidationConfig({assertEnforceTested: true});
Expand All @@ -113,6 +285,7 @@ describe('pull request validation', () => {
login: 'fakelogin',
},
bodyText: 'TESTED="blah"',
createdAt: '2026-07-13T12:00:00Z',
},
];
const commentHelper = PullRequestComments.create(git, pr.number);
Expand All @@ -135,6 +308,7 @@ describe('pull request validation', () => {
login: 'fakelogin',
},
bodyText: 'TESTED="blah"',
createdAt: '2026-07-13T12:00:00Z',
},
];
const commentHelper = PullRequestComments.create(git, pr.number);
Expand All @@ -154,6 +328,7 @@ describe('pull request validation', () => {
authorAssociation: 'MEMBER' as CommentAuthorAssociation,
author: null as any,
bodyText: 'TESTED="blah"',
createdAt: '2026-07-13T12:00:00Z',
},
];
const commentHelper = PullRequestComments.create(git, pr.number);
Expand All @@ -166,6 +341,62 @@ describe('pull request validation', () => {
expect(results.length).toBe(1);
expect(results[0].message).toContain('Pull Request requires a TGP');
});

it('should reject a stale TESTED comment created before the latest commit', async () => {
const config = createIsolatedValidationConfig({assertEnforceTested: true});
let pr = createTestPullRequest();
pr.commits.nodes[1].commit.committedDate = '2026-07-13T12:05:00Z';
const comments = [
{
authorAssociation: 'MEMBER' as CommentAuthorAssociation,
author: {
login: 'fakelogin',
},
bodyText: 'TESTED="stale tgp"',
createdAt: '2026-07-13T12:00:00Z',
},
];
const commentHelper = PullRequestComments.create(git, pr.number);
spyOn(PullRequestComments, 'create').and.returnValue(commentHelper);
spyOn(commentHelper, 'loadPullRequestComments').and.returnValue(Promise.resolve(comments));

pr.labels.nodes.push({name: requiresLabels['REQUIRES_TGP'].name});
const results = await assertValidPullRequest(pr, config, ngDevConfig, null, prTarget, git);
expect(results.length).toBe(1);
expect(results[0].message).toContain('Pull Request requires a TGP');
});

it('should pass when a fresh TESTED comment is created at or after the latest commit', async () => {
const config = createIsolatedValidationConfig({assertEnforceTested: true});
let pr = createTestPullRequest();
pr.commits.nodes[1].commit.committedDate = '2026-07-13T12:05:00Z';
const comments = [
{
authorAssociation: 'MEMBER' as CommentAuthorAssociation,
author: {
login: 'fakelogin',
},
bodyText: 'TESTED="stale tgp"',
createdAt: '2026-07-13T12:00:00Z',
},
{
authorAssociation: 'MEMBER' as CommentAuthorAssociation,
author: {
login: 'fakelogin',
},
bodyText: 'TESTED="fresh tgp"',
createdAt: '2026-07-13T12:10:00Z',
},
];
const commentHelper = PullRequestComments.create(git, pr.number);
spyOn(PullRequestComments, 'create').and.returnValue(commentHelper);
spyOn(commentHelper, 'loadPullRequestComments').and.returnValue(Promise.resolve(comments));

pr.labels.nodes.push({name: requiresLabels['REQUIRES_TGP'].name});
interceptOrgsMembershipRequest('fakelogin', true);
const results = await assertValidPullRequest(pr, config, ngDevConfig, null, prTarget, git);
expect(results.length).toBe(0);
});
});

describe('assert-isolate-primitives', () => {
Expand Down
18 changes: 10 additions & 8 deletions ng-dev/pr/common/validation/assert-allowed-target-label.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,16 @@ class Validation extends PullRequestValidation {
labelsOnPullRequest: string[],
pullRequest: PullRequestFromGithub,
) {
if (targetLabel === targetLabels['TARGET_AUTOMATION']) {
if (!pullRequest.author || !automationBots.includes(pullRequest.author.login)) {
throw this._createUserUsingAutomationLabelError(
targetLabel,
pullRequest.author?.login ?? 'unknown',
);
}
return;
}

if (labelsOnPullRequest.includes(mergeLabels['MERGE_FIX_COMMIT_MESSAGE'].name)) {
Log.debug(
'Skipping commit message target label validation because the commit message fixup label is ' +
Expand Down Expand Up @@ -74,14 +84,6 @@ class Validation extends PullRequestValidation {
throw this._createHasDeprecationsError(targetLabel);
}
break;
case targetLabels['TARGET_AUTOMATION']:
if (!pullRequest.author || !automationBots.includes(pullRequest.author.login)) {
throw this._createUserUsingAutomationLabelError(
targetLabel,
pullRequest.author?.login ?? 'unknown',
);
}
break;
default:
Log.warn(red('WARNING: Unable to confirm all commits in the pull request are'));
Log.warn(red(`eligible to be merged into the target branches for: ${targetLabel.name}`));
Expand Down
16 changes: 14 additions & 2 deletions ng-dev/pr/common/validation/assert-enforce-tested.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,11 @@ class Validation extends PullRequestValidation {
pullRequest.number,
).loadPullRequestComments();

if (await pullRequestHasValidTestedComment(comments, gitClient)) {
const latestCommit = pullRequest.commits.nodes[pullRequest.commits.nodes.length - 1]?.commit;
const latestCommitDate =
latestCommit?.pushedDate ?? latestCommit?.committedDate ?? latestCommit?.authoredDate;

if (await pullRequestHasValidTestedComment(comments, gitClient, latestCommitDate)) {
return;
}

Expand Down Expand Up @@ -76,8 +80,16 @@ export class PullRequestComments {
export async function pullRequestHasValidTestedComment(
comments: PullRequestCommentsFromGithub[],
gitClient: AuthenticatedGitClient,
latestCommitDate?: string,
): Promise<boolean> {
for (const {bodyText, author} of comments) {
for (const comment of comments) {
const {bodyText, author} = comment;
if (
latestCommitDate &&
(!comment.createdAt || !(Date.parse(comment.createdAt) >= Date.parse(latestCommitDate)))
) {
continue;
}
Comment thread
josephperrott marked this conversation as resolved.
if (
bodyText.startsWith(`TESTED=`) &&
author &&
Expand Down
4 changes: 2 additions & 2 deletions ng-dev/pr/common/validation/assert-signed-cla.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ export const signedClaValidation = createPullRequestValidation(

class Validation extends PullRequestValidation {
assert(pullRequest: PullRequestFromGithub) {
const passing = getStatusesForPullRequest(pullRequest).statuses.some(({name, status}) => {
return name === 'cla/google' && status === PullRequestStatus.PASSING;
const passing = getStatusesForPullRequest(pullRequest).statuses.some(({type, name, status}) => {
return type === 'status' && name === 'cla/google' && status === PullRequestStatus.PASSING;
});

if (!passing) {
Expand Down
1 change: 1 addition & 0 deletions ng-dev/pr/merge/messages.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ describe('caretaker note prompt messages', () => {
bodyText,
author: {login: 'testuser'},
authorAssociation: authorAssociation as any,
createdAt: '2020-01-01T00:00:00Z',
};
}

Expand Down
Loading
Loading