From a83314527f680ab026d95457bf01165b6db7d53b Mon Sep 17 00:00:00 2001 From: Michael Daniels Date: Fri, 4 Sep 2026 13:08:58 -0400 Subject: [PATCH] ci/github-script/commits: convert to TypeScript Assisted-by: gpt-5.6-terra (Codex) (cherry picked from commit 7f822c9dc80995c71fd530e0fe43b1aa770f1f01) --- .github/workflows/check.yml | 2 +- .github/workflows/test.yml | 2 +- ci/github-script/{commits.js => commits.ts} | 101 +++++++++++++++++--- ci/github-script/reviews.js | 2 +- ci/github-script/run | 2 +- 5 files changed, 90 insertions(+), 19 deletions(-) rename ci/github-script/{commits.js => commits.ts} (82%) diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 34333b64ec46..faff91d48e6c 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -80,7 +80,7 @@ jobs: github-token: ${{ steps.app-token.outputs.token || github.token }} script: | const targetsStable = JSON.parse(process.env.TARGETS_STABLE) - const { default: commits } = await import('${{ github.workspace }}/trusted/ci/github-script/commits.js') + const { default: commits } = await import('${{ github.workspace }}/trusted/ci/github-script/commits.ts') await commits({ github, context, diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 742e3d8ca6ed..ad1aaed202ff 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -81,7 +81,7 @@ jobs: '.github/workflows/test.yml', 'ci/github-script/bot.js', 'ci/github-script/check-target-branch.ts', - 'ci/github-script/commits.js', + 'ci/github-script/commits.ts', 'ci/github-script/get-pr-commit-details.js', 'ci/github-script/lint-commits.js', 'ci/github-script/manual-file-edits.ts', diff --git a/ci/github-script/commits.js b/ci/github-script/commits.ts similarity index 82% rename from ci/github-script/commits.js rename to ci/github-script/commits.ts index 5a0d46995920..a1fbceda6589 100644 --- a/ci/github-script/commits.js +++ b/ci/github-script/commits.ts @@ -1,18 +1,78 @@ -// @ts-nocheck import { execFileSync } from 'node:child_process' +import type * as actionsCore from '@actions/core' +import type { context as actionsContext } from '@actions/github' +import type { GitHub } from '@actions/github/lib/utils' import { dismissReviews, postReview } from './reviews.js' import { classify } from './supportedBranches.js' import withRateLimit from './withRateLimit.js' const dirname = import.meta.dirname -export default async ({ github, context, core, dry, cherryPicks }) => { +type PullRequestCommit = Awaited< + ReturnType['rest']['pulls']['listCommits']> +>['data'][number] + +type CommitDetails = Pick + +type CherryPick = CommitDetails & { + original_sha: string + severity?: undefined +} + +type CheckResult = CommitDetails & { + diff?: string[] + colored_diff?: string + severity: 'important' | 'warning' | 'error' | 'info' + message: string + type?: 'no-cherry-pick' | 'no-commit-hash' | 'diff' +} + +type ExtractedCommit = CheckResult | CherryPick + +type CheckCommitsProps = { + github: InstanceType + context: typeof actionsContext + core: typeof actionsCore + dry: boolean + cherryPicks: boolean +} + +type CommitBranches = { + branches: { + branch: string + }[] +} + +type RateLimitStats = { + prs: number +} + +function isCherryPick(result: ExtractedCommit): result is CherryPick { + return !result.severity +} + +function isIgnoredBranchError(error: unknown) { + return ( + typeof error === 'object' && + error !== null && + 'status' in error && + (error.status === 404 || error.status === 500) + ) +} + +export default async ({ + github, + context, + core, + dry, + cherryPicks, +}: CheckCommitsProps) => { const reviewKey = 'check-commits' - await withRateLimit({ github, core }, async (stats) => { + await withRateLimit({ github, core }, async (stats: RateLimitStats) => { stats.prs = 1 - const pull_number = context.payload.pull_request.number + const pull_number = context.payload.pull_request!.number const job_url = context.runId && @@ -22,11 +82,14 @@ export default async ({ github, context, core, dry, cherryPicks }) => { run_id: context.runId, per_page: 100, }) - ).find(({ name }) => name.endsWith('Check / commits')).html_url + + ).find(({ name }) => name.endsWith('Check / commits'))!.html_url + '?pr=' + pull_number - async function extract({ sha, commit }) { + async function extract({ + sha, + commit, + }: CommitDetails): Promise { const noCherryPick = Array.from( commit.message.matchAll(/^Not-cherry-picked-because: (.*)$/gm), ).at(0) @@ -57,10 +120,10 @@ export default async ({ github, context, core, dry, cherryPicks }) => { const original_sha = cherry[1] - let branches + let branches: string[] | undefined try { branches = ( - await github.request({ + await github.request({ // This is an undocumented endpoint to fetch the branches a commit is part of. // There is no equivalent in neither the REST nor the GraphQL API. // The endpoint itself is unlikely to go away, because GitHub uses it to display @@ -76,7 +139,7 @@ export default async ({ github, context, core, dry, cherryPicks }) => { } catch (e) { // For some unknown reason a 404 error comes back as 500 without any more details in a GitHub Actions runner. // Ignore these to return a regular error message below. - if (![404, 500].includes(e.status)) throw e + if (!isIgnoredBranchError(e)) throw e } if (!branches?.length) return { @@ -93,7 +156,7 @@ export default async ({ github, context, core, dry, cherryPicks }) => { } } - function diff({ sha, commit, original_sha }) { + function diff({ sha, commit, original_sha }: CherryPick): CheckResult { const diff = execFileSync('git', [ '-C', dirname, @@ -155,7 +218,7 @@ export default async ({ github, context, core, dry, cherryPicks }) => { const extracted = await Promise.all(commits.map(extract)) const fetch = extracted - .filter(({ severity }) => !severity) + .filter(isCherryPick) .flatMap(({ sha, original_sha }) => [sha, original_sha]) if (fetch.length > 0) { @@ -177,8 +240,8 @@ export default async ({ github, context, core, dry, cherryPicks }) => { // Log all results without truncation, with better highlighting and all whitespace changes to the job log. results.forEach(({ sha, commit, severity, message, colored_diff }) => { core.startGroup(`Commit ${sha}`) - core.info(`Author: ${commit.author.name} ${commit.author.email}`) - core.info(`Date: ${new Date(commit.author.date)}`) + core.info(`Author: ${commit.author!.name} ${commit.author!.email}`) + core.info(`Date: ${new Date(commit.author!.date!)}`) switch (severity) { case 'error': core.error(message) @@ -198,7 +261,7 @@ export default async ({ github, context, core, dry, cherryPicks }) => { // An empty results array will always trigger this condition, which is helpful // to clean up reviews created by the prepare step when on the wrong branch. if (results.every(({ severity }) => severity === 'info')) { - await dismissReviews({ github, context, dry, reviewKey }) + await dismissReviews({ github, context, core, dry, reviewKey }) return } @@ -321,6 +384,14 @@ export default async ({ github, context, core, dry, cherryPicks }) => { // Posting a review could fail for very long comments. This can only happen with // multiple commits all hitting the truncation limit for the diff. If you ever hit // this case, consider just splitting up those commits into multiple PRs. - await postReview({ github, context, core, dry, body, reviewKey }) + await postReview({ + github, + context, + core, + dry, + body, + event: 'REQUEST_CHANGES', + reviewKey, + }) }) } diff --git a/ci/github-script/reviews.js b/ci/github-script/reviews.js index 584a035160f1..e47270386f1e 100644 --- a/ci/github-script/reviews.js +++ b/ci/github-script/reviews.js @@ -104,7 +104,7 @@ export async function dismissReviews({ commentResolvedRegex.test(review.body) || (reviewKey && reviewKeyRegex.test(review.body)) || // If we are called by check-commits and the review body is clearly - // from `commits.js`, then we can safely dismiss the review. + // from `commits.ts`, then we can safely dismiss the review. // This helps with pre-existing reviews (before the comments were added). (reviewKey && reviewKey === 'check-commits' && diff --git a/ci/github-script/run b/ci/github-script/run index bc164e017940..001d3e201a2b 100755 --- a/ci/github-script/run +++ b/ci/github-script/run @@ -60,7 +60,7 @@ program .argument('', 'Number of the Pull Request to check') .option('--no-cherry-picks', 'Do not expect cherry-picks.') .action(async (owner, repo, pr, options) => { - const commits = (await import('./commits.js')).default + const commits = (await import('./commits.ts')).default await run(commits, owner, repo, pr, options) })