Skip to content

chore(prlint): add need-xxx-reviews when building with github action #35326

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 26, 2025
Merged

chore(prlint): add need-xxx-reviews when building with github action #35326

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2dc20fa
chore(prlint): add need-xxx-reviews when building with github action
Aug 25, 2025
46ec7a3
Run on workflow end
Aug 26, 2025
26685de
Merge branch 'main' into labels
mergify[bot] Aug 26, 2025
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
5 changes: 1 addition & 4 deletions .github/workflows/pr-linter.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,10 @@ on:

# Triggered from a separate job when a review is added
workflow_run:
workflows: [PR Linter Trigger]
workflows: ["PR Linter Trigger", "Codebuild PR Build"]
Copy link
Contributor

@kumvprat kumvprat Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should trigger the linter workflow again and add proper label based on Codebuild Action's result

types:
- completed

# Trigger when a status is updated (CodeBuild leads to statuses)
status: {}

# Trigger when a check suite is completed (GitHub actions and CodeCov create checks)
check_suite:
types: [completed]
Expand Down
2 changes: 1 addition & 1 deletion tools/@aws-cdk/prlint/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@

export const DEFAULT_LINTER_LOGIN = 'aws-cdk-automation';

export const CODE_BUILD_CONTEXT = 'AWS CodeBuild us-east-1 (AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv)';
export const CODE_BUILD_WORKFLOW_FILE = 'codebuild-pr-build.yml';

/**
* Types of exemption labels in aws-cdk project.
Expand Down
19 changes: 1 addition & 18 deletions tools/@aws-cdk/prlint/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import * as github from '@actions/github';
import { Octokit } from '@octokit/rest';
import { StatusEvent, PullRequestEvent, CheckSuiteEvent } from '@octokit/webhooks-definitions/schema';
import { PullRequestLinter } from './lint';
import { LinterActions } from './linter-base';
import { DEFAULT_LINTER_LOGIN } from './constants';

/**
Expand Down Expand Up @@ -42,23 +41,7 @@ async function run() {
linterLogin: process.env.LINTER_LOGIN || DEFAULT_LINTER_LOGIN,
});

let actions: LinterActions | undefined;

switch (github.context.eventName) {
case 'status':
// Triggering on a 'status' event is solely used to see if the CodeBuild
// job just turned green, and adding certain 'ready for review' labels
// if it does.
const statusPayload = github.context.payload as StatusEvent;
console.log('validating status event');
actions = await prLinter.validateStatusEvent(statusPayload);
break;

default:
// This is the main PR validator action.
actions = await prLinter.validatePullRequestTarget();
break;
}
const actions = await prLinter.validatePullRequestTarget();

if (actions) {
console.log(actions);
Expand Down
21 changes: 7 additions & 14 deletions tools/@aws-cdk/prlint/lint.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ import { findModulePath, moduleStability } from './module';
import { breakingModules } from './parser';
import { CheckRun, GitHubFile, GitHubPr, Review, sumChanges, summarizeRunConclusions } from "./github";
import { TestResult, ValidationCollector } from './results';
import { CODE_BUILD_CONTEXT, CODECOV_CHECKS, Exemption } from './constants';
import { StatusEvent } from '@octokit/webhooks-definitions/schema';
import { CODE_BUILD_WORKFLOW_FILE, CODECOV_CHECKS, Exemption } from './constants';
import { LinterActions, mergeLinterActions, PR_FROM_MAIN_ERROR, PullRequestLinterBase } from './linter-base';

/**
Expand All @@ -20,21 +19,15 @@ export class PullRequestLinter extends PullRequestLinterBase {
* @param sha the commit sha to evaluate
*/
private async codeBuildJobSucceeded(sha: string): Promise<boolean> {
const statuses = await this.client.repos.listCommitStatusesForRef({
const statuses = await this.client.actions.listWorkflowRuns({
owner: this.prParams.owner,
repo: this.prParams.repo,
ref: sha,
head_sha: sha,
workflow_id: CODE_BUILD_WORKFLOW_FILE,
});
let status = statuses.data.filter(status => status.context === CODE_BUILD_CONTEXT).map(status => status.state);
console.log("CodeBuild Commit Statuses: ", status);
return statuses.data.some(status => status.context === CODE_BUILD_CONTEXT && status.state === 'success');
}

public async validateStatusEvent(status: StatusEvent): Promise<LinterActions> {
if (status.context === CODE_BUILD_CONTEXT && status.state === 'success') {
return this.assessNeedsReview();
}
return {};
let conclusions = statuses.data.workflow_runs.filter(run => run.status === 'completed').map(run => run.conclusion);
console.log("CodeBuild Commit Conclusions: ", conclusions);
return conclusions.some(conclusion => conclusion === 'success');
}

/**
Expand Down
155 changes: 52 additions & 103 deletions tools/@aws-cdk/prlint/test/lint.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import * as path from 'path';
import { GitHubFile, GitHubLabel, GitHubPr } from '../github';
import { CODE_BUILD_CONTEXT, CODECOV_CHECKS } from '../constants';
import { CODECOV_CHECKS } from '../constants';
import { PullRequestLinter } from '../lint';
import { StatusEvent } from '@octokit/webhooks-definitions/schema';
import { createOctomock, OctoMock } from './octomock';

type GitHubFileName = Omit<GitHubFile, 'deletions' | 'additions'>;
Expand Down Expand Up @@ -589,13 +588,22 @@ describe('integration tests required on features', () => {
});
});

describe('assess needs review from status event', () => {
describe('assess needs review', () => {
const pr = {
title: 'chore(s3): something',
draft: false,
mergeable_state: 'behind',
number: 1234,
labels: [{ name: 'p2' }],
};
const files = [
{
filename: 'some-test.test.ts',
},
{
filename: 'readme.md',
},
];
beforeEach(() => {
mockListReviews.mockImplementation(() => {
return {
Expand All @@ -606,12 +614,8 @@ describe('integration tests required on features', () => {

test('needs a review', async () => {
// WHEN
const prLinter = configureMock(pr);
await legacyValidateStatusEvent(prLinter, {
sha: SHA,
context: CODE_BUILD_CONTEXT,
state: 'success',
} as any);
const prLinter = configureMock(pr, files);
await legacyValidatePullRequestTarget(prLinter);

// THEN
expect(mockAddLabel.mock.calls[0][0]).toEqual({
Expand All @@ -626,12 +630,8 @@ describe('integration tests required on features', () => {
test('needs a review and is p1', async () => {
// WHEN
pr.labels = [{ name: 'p1' }];
const prLinter = configureMock(pr);
await legacyValidateStatusEvent(prLinter, {
sha: SHA,
context: CODE_BUILD_CONTEXT,
state: 'success',
} as any);
const prLinter = configureMock(pr, files);
await legacyValidatePullRequestTarget(prLinter);

// THEN
expect(mockAddLabel.mock.calls[0][0]).toEqual({
Expand All @@ -657,12 +657,8 @@ describe('integration tests required on features', () => {
];

// WHEN
const prLinter = configureMock(pr);
await legacyValidateStatusEvent(prLinter, {
sha: SHA,
context: CODE_BUILD_CONTEXT,
state: 'success',
} as any);
const prLinter = configureMock(pr, files);
await legacyValidatePullRequestTarget(prLinter);

// THEN
expect(mockRemoveLabel.mock.calls[0][0]).toEqual({
Expand All @@ -688,12 +684,8 @@ describe('integration tests required on features', () => {
];

// WHEN
const prLinter = configureMock(pr);
await legacyValidateStatusEvent(prLinter, {
sha: SHA,
context: CODE_BUILD_CONTEXT,
state: 'success',
} as any);
const prLinter = configureMock(pr, files);
await legacyValidatePullRequestTarget(prLinter);

// THEN
expect(mockAddLabel.mock.calls[0][0]).toEqual({
Expand Down Expand Up @@ -725,12 +717,8 @@ describe('integration tests required on features', () => {
];

// WHEN
const prLinter = configureMock(pr);
await legacyValidateStatusEvent(prLinter, {
sha: SHA,
context: CODE_BUILD_CONTEXT,
state: 'success',
} as any);
const prLinter = configureMock(pr, files);
await legacyValidatePullRequestTarget(prLinter);

// THEN
expect(mockRemoveLabel.mock.calls[0][0]).toEqual({
Expand Down Expand Up @@ -758,12 +746,8 @@ describe('integration tests required on features', () => {
];

// WHEN
const prLinter = configureMock(pr);
await legacyValidateStatusEvent(prLinter, {
sha: SHA,
context: CODE_BUILD_CONTEXT,
state: 'success',
} as any);
const prLinter = configureMock(pr, files);
await legacyValidatePullRequestTarget(prLinter);

// THEN
expect(mockRemoveLabel.mock.calls[0][0]).toEqual({
Expand Down Expand Up @@ -791,12 +775,8 @@ describe('integration tests required on features', () => {
];

// WHEN
const prLinter = configureMock(pr);
await legacyValidateStatusEvent(prLinter, {
sha: SHA,
context: CODE_BUILD_CONTEXT,
state: 'success',
} as any);
const prLinter = configureMock(pr, files);
await legacyValidatePullRequestTarget(prLinter);

// THEN
expect(mockRemoveLabel.mock.calls[0][0]).toEqual({
Expand Down Expand Up @@ -831,12 +811,8 @@ describe('integration tests required on features', () => {
];

// WHEN
const prLinter = configureMock(pr);
await legacyValidateStatusEvent(prLinter, {
sha: SHA,
context: CODE_BUILD_CONTEXT,
state: 'success',
} as any);
const prLinter = configureMock(pr, files);
await legacyValidatePullRequestTarget(prLinter);

// THEN
expect(mockRemoveLabel.mock.calls[0][0]).toEqual({
Expand Down Expand Up @@ -869,12 +845,8 @@ describe('integration tests required on features', () => {
];

// WHEN
const prLinter = configureMock(pr);
await legacyValidateStatusEvent(prLinter, {
sha: SHA,
context: CODE_BUILD_CONTEXT,
state: 'success',
} as any);
const prLinter = configureMock(pr, files);
await legacyValidatePullRequestTarget(prLinter);

// THEN
expect(mockRemoveLabel.mock.calls[0][0]).toEqual({
Expand All @@ -899,12 +871,8 @@ describe('integration tests required on features', () => {
(pr as any).labels = [];

// WHEN
const prLinter = configureMock(pr);
await legacyValidateStatusEvent(prLinter, {
sha: SHA,
context: CODE_BUILD_CONTEXT,
state: 'success',
} as any);
const prLinter = configureMock(pr, files);
await legacyValidatePullRequestTarget(prLinter);

// THEN
expect(mockRemoveLabel.mock.calls).toEqual([]);
Expand All @@ -927,12 +895,8 @@ describe('integration tests required on features', () => {
];

// WHEN
const prLinter = configureMock(pr);
await legacyValidateStatusEvent(prLinter, {
sha: SHA,
context: CODE_BUILD_CONTEXT,
state: 'success',
} as any);
const prLinter = configureMock(pr, files);
await legacyValidatePullRequestTarget(prLinter);

// THEN
expect(mockRemoveLabel.mock.calls).toEqual([]);
Expand All @@ -956,12 +920,8 @@ describe('integration tests required on features', () => {
];

// WHEN
const prLinter = configureMock(pr);
await legacyValidateStatusEvent(prLinter, {
sha: SHA,
context: CODE_BUILD_CONTEXT,
state: 'success',
} as any);
const prLinter = configureMock(pr, files);
await legacyValidatePullRequestTarget(prLinter);

// THEN
expect(mockRemoveLabel.mock.calls[0][0]).toEqual({
Expand All @@ -986,12 +946,8 @@ describe('integration tests required on features', () => {
(pr as any).labels = [];

// WHEN
const prLinter = configureMock(pr);
await legacyValidateStatusEvent(prLinter, {
sha: SHA,
context: CODE_BUILD_CONTEXT,
state: 'success',
} as any);
const prLinter = configureMock(pr, files);
await legacyValidatePullRequestTarget(prLinter);

// THEN
expect(mockRemoveLabel.mock.calls).toEqual([]);
Expand Down Expand Up @@ -1019,12 +975,8 @@ describe('integration tests required on features', () => {
];

// WHEN
const prLinter = configureMock(pr);
await legacyValidateStatusEvent(prLinter, {
sha: SHA,
context: CODE_BUILD_CONTEXT,
state: 'success',
} as any);
const prLinter = configureMock(pr, files);
await legacyValidatePullRequestTarget(prLinter);

// THEN
expect(mockRemoveLabel.mock.calls).toEqual([]);
Expand Down Expand Up @@ -1367,12 +1319,6 @@ function configureMock(pr: Subset<GitHubPr>, prFiles?: GitHubFileName[], existin
});
octomock.issues.addLabels = mockAddLabel;
octomock.issues.removeLabel = mockRemoveLabel;
octomock.repos.listCommitStatusesForRef.mockImplementation(() => ({
data: [{
context: CODE_BUILD_CONTEXT,
state: 'success',
}],
}));

// We need to pretend that all CodeCov checks are passing by default, otherwise
// the linter will complain about these even in tests that aren't testing for this.
Expand All @@ -1384,6 +1330,19 @@ function configureMock(pr: Subset<GitHubPr>, prFiles?: GitHubFileName[], existin
})),
}));

// Mock workflow runs to simulate successful CodeBuild jobs
octomock.actions.listWorkflowRuns.mockImplementation(() => ({
data: {
workflow_runs: [
{
head_sha: SHA,
status: 'completed',
conclusion: 'success',
},
],
},
}));

const linter = new PullRequestLinter({
owner: 'aws',
repo: 'aws-cdk',
Expand Down Expand Up @@ -1420,13 +1379,3 @@ async function legacyValidatePullRequestTarget(prLinter: PullRequestLinter) {
await prLinter.executeActions(actions);
prLinter.actionsToException(actions);
}

/**
* Same as for validatePullRequesTarget
*
* @deprecated Assert on the contents of `LinterActions` instead.
*/
async function legacyValidateStatusEvent(prLinter: PullRequestLinter, statusEvent: StatusEvent) {
const actions = await prLinter.validateStatusEvent(statusEvent);
await prLinter.executeActions(actions);
}
2 changes: 1 addition & 1 deletion tools/@aws-cdk/prlint/test/linter-base.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ beforeEach(() => {
},
});
octomock.pulls.listReviews.mockReturnValue({ data: [] });
octomock.repos.listCommitStatusesForRef.mockReturnValue({ data: [] });
octomock.actions.listWorkflowRuns.mockReturnValue({ data: [] });
});

test('ignore if dismissing reviews throws a specific "already dismissed" error', async () => {
Expand Down
Loading
Loading