[summarize-checks] Prepare for tseslint.configs.recommendedTypeChecked (Azure#38872)

- Add/remove missing/unnecessary `async/await`
- Use `zod` to validate input and create strongly-typed objects
- Add type comments
- Add fallbacks for undefined values

Author: mikeharder

.github/workflows/src/summarize-checks/dump-trigger-metadata.js

@@ -1,8 +1,7 @@
 /**
  * @param {import('@actions/github-script').AsyncFunctionArguments} AsyncFunctionArguments
- * @returns {Promise<void>}
  */
-export default async function dumpTriggerMetadata({ context, core }) {
+export default function dumpTriggerMetadata({ context, core }) {
   core.info(`Event name: ${context.eventName}`);
   core.info(`Action: ${context.payload.action}`);
 

.github/workflows/src/summarize-checks/labelling.js

@@ -4,6 +4,7 @@
     2. It calculates what set of labels should be added or removed from the PR.
 */
 
+import * as z from "zod";
 import { includesEvery, includesNone } from "../../../shared/src/array.js";
 import {
   brchTsg,
@@ -45,19 +46,23 @@ import {
  * @property {Set<string>} toRemove - The set of labels to remove
  */
 
+export const ImpactAssessmentSchema = z.object({
+  resourceManagerRequired: z.boolean().describe("Whether a resource manager review is required."),
+  dataPlaneRequired: z.boolean().describe("Whether a data plane review is required."),
+  suppressionReviewRequired: z.boolean().describe("Whether a suppression review is required."),
+  isNewApiVersion: z.boolean().describe("Whether this PR introduces a new API version."),
+  rpaasRpNotInPrivateRepo: z
+    .boolean()
+    .describe("Whether the RPaaS RP is not present in the private repo."),
+  rpaasChange: z.boolean().describe("Whether this PR includes RPaaS changes."),
+  newRP: z.boolean().describe("Whether this PR introduces a new resource provider."),
+  rpaasRPMissing: z.boolean().describe("Whether the RPaaS RP label is missing."),
+  typeSpecChanged: z.boolean().describe("Whether a TypeSpec file has changed."),
+  isDraft: z.boolean().describe("Whether the PR is a draft."),
+  targetBranch: z.string().describe("The name of the target branch for the PR."),
+});
 /**
- * @typedef {Object} ImpactAssessment
- * @property {boolean} resourceManagerRequired - Whether a resource manager review is required.
- * @property {boolean} dataPlaneRequired - Whether a data plane review is required.
- * @property {boolean} suppressionReviewRequired - Whether a suppression review is required.
- * @property {boolean} isNewApiVersion - Whether this PR introduces a new API version.
- * @property {boolean} rpaasRpNotInPrivateRepo - Whether the RPaaS RP is not present in the private repo.
- * @property {boolean} rpaasChange - Whether this PR includes RPaaS changes.
- * @property {boolean} newRP - Whether this PR introduces a new resource provider.
- * @property {boolean} rpaasRPMissing - Whether the RPaaS RP label is missing.
- * @property {boolean} typeSpecChanged - Whether a TypeSpec file has changed.
- * @property {boolean} isDraft - Whether the PR is a draft.
- * @property {string} targetBranch - The name of the target branch for the PR.
+ * @typedef {import("zod").infer<typeof ImpactAssessmentSchema>} ImpactAssessment
  */
 
 /**
@@ -465,9 +470,9 @@ This function does the following, **among other things**:
 /**
  * @param {LabelContext} labelContext
  * @param {ImpactAssessment} impactAssessment
- * @returns {Promise<{armReviewLabelShouldBePresent: boolean}>}
+ * @returns {{armReviewLabelShouldBePresent: boolean}}
  */
-export async function processImpactAssessment(labelContext, impactAssessment) {
+export function processImpactAssessment(labelContext, impactAssessment) {
   console.log("ENTER definition processARMReview");
 
   // By default these two should not be present. We may determine later in this function that they should be present after all.
@@ -535,7 +540,7 @@ export async function processImpactAssessment(labelContext, impactAssessment) {
     }
 
     armReviewLabel.shouldBePresent = impactAssessment.resourceManagerRequired;
-    await processARMReviewWorkflowLabels(
+    processARMReviewWorkflowLabels(
       labelContext,
       armReviewLabel.shouldBePresent,
       impactAssessment.rpaasRPMissing,
@@ -620,9 +625,8 @@ labels are being removed or added to a PR is pipelineBotOnPRLabelEvent.ts.
  * @param {boolean} ciNewRPNamespaceWithoutRpaaSLabelShouldBePresent
  * @param {boolean} rpaasExceptionLabelShouldBePresent
  * @param {boolean} ciRpaasRPNotInPrivateRepoLabelShouldBePresent
- * @returns {Promise<void>}
  */
-async function processARMReviewWorkflowLabels(
+function processARMReviewWorkflowLabels(
   labelContext,
   armReviewLabelShouldBePresent,
   ciNewRPNamespaceWithoutRpaaSLabelShouldBePresent,
@@ -706,7 +710,6 @@ async function processARMReviewWorkflowLabels(
       `blockedOnRpaas: ${blockedOnRpaas}. ` +
       `exactlyOneArmReviewWorkflowLabelShouldBePresent: ${exactlyOneArmReviewWorkflowLabelShouldBePresent}. `,
   );
-  return;
 }
 
 /**
@@ -1164,9 +1167,9 @@ export const requiredLabelsRules = rulesPri0dataPlane
  * @param {string[]} existingLabels
  * @param {string} targetBranch
  *
- * @returns {Promise<{presentBlockingLabels: string[], missingRequiredLabels: string[]}>}
+ * @returns {{presentBlockingLabels: string[], missingRequiredLabels: string[]}}
  */
-export async function getPresentBlockingLabelsAndMissingRequiredLabels(
+export function getPresentBlockingLabelsAndMissingRequiredLabels(
   core,
   repo,
   owner,
@@ -1176,15 +1179,17 @@ export async function getPresentBlockingLabelsAndMissingRequiredLabels(
   // this is a little bit of a strange looking branch format, but not going to touch this for now.
   const repoTargetBranch = `${repo}/${targetBranch}`;
 
-  const violatedReqLabelsRules = await getViolatedRequiredLabelsRules(
+  const violatedReqLabelsRules = getViolatedRequiredLabelsRules(
     core,
     existingLabels,
     repoTargetBranch,
   );
   const presentBlockingLabels = getPresentBlockingLabels(violatedReqLabelsRules);
 
-  const requiredLabelsFromRules = (
-    await getViolatedRequiredLabelsRules(core, existingLabels, repoTargetBranch)
+  const requiredLabelsFromRules = getViolatedRequiredLabelsRules(
+    core,
+    existingLabels,
+    repoTargetBranch,
   )
     .filter((rule) => rule.anyRequiredLabels.length > 0)
     // See comment on RequiredLabelRule.anyRequiredLabels to understand why we pick [0] from rule.anyRequiredLabels here.
@@ -1201,12 +1206,12 @@ export async function getPresentBlockingLabelsAndMissingRequiredLabels(
  * @param {typeof import("@actions/core")} core
  * @param {string[]} labels
  * @param {string} targetBranch This function uses a special format {repo/branch}, e.g. "azure-rest-api-specs/main".
- * @returns {Promise<RequiredLabelRule[]>}
+ * @returns {RequiredLabelRule[]}
  */
-export async function getViolatedRequiredLabelsRules(core, labels, targetBranch) {
+export function getViolatedRequiredLabelsRules(core, labels, targetBranch) {
   const violatedRules = [];
   for (const rule of requiredLabelsRules) {
-    if (await requiredLabelRuleViolated(core, labels, targetBranch, rule)) {
+    if (requiredLabelRuleViolated(core, labels, targetBranch, rule)) {
       violatedRules.push(rule);
     }
   }
@@ -1218,9 +1223,9 @@ export async function getViolatedRequiredLabelsRules(core, labels, targetBranch)
  * @param {string[]} presentLabels
  * @param {string} targetBranch
  * @param {RequiredLabelRule} rule
- * @returns {Promise<boolean>}
+ * @returns {boolean}
  */
-export async function requiredLabelRuleViolated(core, presentLabels, targetBranch, rule) {
+export function requiredLabelRuleViolated(core, presentLabels, targetBranch, rule) {
   const branchIsApplicable = rule.branches === undefined || rule.branches.includes(targetBranch);
 
   const anyPrerequisiteLabelPresent =

.github/workflows/src/summarize-checks/summarize-checks.js

@@ -24,6 +24,7 @@ import { byDate, invert } from "../../../shared/src/sort.js";
 import { commentOrUpdate } from "../comment.js";
 import { extractInputs } from "../context.js";
 import {
+  ImpactAssessmentSchema,
   brChRevApproval,
   getViolatedRequiredLabelsRules,
   processImpactAssessment,
@@ -132,6 +133,11 @@ import path from "path";
  * @property {string} [target_url]
  */
 
+/**
+ * @typedef {import("../github.js").CheckRuns[0]} CheckRun
+ * @typedef {import("../github.js").CommitStatuses[0]} CommitStatus
+ */
+
 // Placing these configuration items here until we decide another way to pull them in.
 const FYI_CHECK_NAMES = [
   "Swagger LintDiff",
@@ -294,7 +300,12 @@ export default async function summarizeChecks({ github, context, core }) {
     return;
   }
 
-  const targetBranch = context.payload.pull_request?.base?.ref;
+  const targetBranch =
+    context.eventName === "pull_request_target"
+      ? /** @type {import("@octokit/webhooks-types").PullRequestEvent} */ (context.payload)
+          .pull_request.base.ref
+      : undefined;
+
   core.info(`PR target branch: ${targetBranch}`);
 
   // Default target is this run itself
@@ -343,7 +354,7 @@ export function outputRunDetails(core, requiredCheckRuns, fyiCheckRuns) {
  * @param {number} issue_number
  * @param {string} head_sha
  * @param {string} event_name
- * @param {string} targetBranch
+ * @param {string|undefined} targetBranch
  * @param {string} target_url
  * @returns {Promise<void>}
  */
@@ -363,7 +374,7 @@ export async function summarizeChecksImpl(
   const prUrl = `https://github.com/${owner}/${repo}/pull/${issue_number}`;
   core.summary.addRaw("PR: ");
   core.summary.addLink(prUrl, prUrl);
-  core.summary.write();
+  await core.summary.write();
   core.setOutput("summary", process.env.GITHUB_STEP_SUMMARY);
 
   let labelNames = await getExistingLabels(github, owner, repo, issue_number);
@@ -391,7 +402,7 @@ export async function summarizeChecksImpl(
     );
   }
 
-  let labelContext = await updateLabels(labelNames, impactAssessment);
+  let labelContext = updateLabels(labelNames, impactAssessment);
 
   core.info(
     `Summarize checks label actions against ${owner}/${repo}#${issue_number}: \n` +
@@ -430,7 +441,7 @@ export async function summarizeChecksImpl(
     }
   }
 
-  const [commentBody, automatedChecksMet] = await createNextStepsComment(
+  const [commentBody, automatedChecksMet] = createNextStepsComment(
     core,
     repo,
     labelNames,
@@ -447,7 +458,7 @@ export async function summarizeChecksImpl(
     `Updating comment '${NEXT_STEPS_COMMENT_ID}' on ${owner}/${repo}#${issue_number} with body: ${commentBody}`,
   );
   core.summary.addRaw(`\n${commentBody}\n\n`);
-  core.summary.write();
+  await core.summary.write();
 
   // this will remain commented until we're comfortable with the change.
   await commentOrUpdate(
@@ -468,7 +479,7 @@ export async function summarizeChecksImpl(
   );
   core.summary.addHeading("Automated Checks Met", 2);
   core.summary.addCodeBlock(JSON.stringify(automatedChecksMet, null, 2));
-  core.summary.write();
+  await core.summary.write();
 }
 
 /**
@@ -646,7 +657,7 @@ export async function getCheckRunTuple(
   // all checks will be considered as "FYI" until we have an impact assessment, so we can
   // determine the target branch, and from there pull branch protect rulesets to ensure we
   // are marking the required checks correctly.
-  /** @type {Array<CheckRunData & {_originalData: any, _source: string}>} */
+  /** @type {Array<CheckRunData & {_originalData: CheckRun|CommitStatus, _source: string}>} */
   const allChecks = [];
 
   allCheckRuns.forEach((checkRun) => {
@@ -694,14 +705,17 @@ export async function getCheckRunTuple(
   });
 
   // Group by name and take the latest for each
+  /** @type {Map<string, Array<CheckRunData & {_originalData: CheckRun|CommitStatus, _source: string}>>} */
   const checksByName = new Map();
 
   allChecks.forEach((check) => {
     const name = check.name;
-    if (!checksByName.has(name)) {
-      checksByName.set(name, []);
+    let checks = checksByName.get(name);
+    if (checks) {
+      checks.push(check);
+    } else {
+      checksByName.set(name, [check]);
     }
-    checksByName.get(name).push(check);
   });
 
   // For each group, sort by date (newest first) and take the first one
@@ -712,15 +726,12 @@ export async function getCheckRunTuple(
       invert(
         byDate((check) => {
           if (check._source === "checkRun") {
-            // Check runs have started_at, completed_at, etc. Use the most recent available date
-            return (
-              check._originalData.completed_at ||
-              check._originalData.started_at ||
-              check._originalData.created_at
-            );
+            const originalData = /** @type {CheckRun} */ (check._originalData);
+            // Use the most recent available date, or "1970" (oldest possible) if the data contains no dates
+            return originalData.completed_at || originalData.started_at || "1970";
           } else {
-            // Commit statuses have created_at and updated_at
-            return check._originalData.updated_at || check._originalData.created_at;
+            const originalData = /** @type {CommitStatus} */ (check._originalData);
+            return originalData.updated_at;
           }
         }),
       ),
@@ -733,18 +744,17 @@ export async function getCheckRunTuple(
       latestCheck.status === "completed" &&
       latestCheck.conclusion === "success"
     ) {
+      const originalData = /** @type {CheckRun} */ (latestCheck._originalData);
       const workflowRuns = await github.paginate(github.rest.actions.listWorkflowRunsForRepo, {
         owner,
         repo,
         head_sha: head_sha,
-        check_suite_id: latestCheck._originalData.check_suite.id,
+        check_suite_id: originalData.check_suite?.id,
         per_page: PER_PAGE_MAX,
       });
 
       if (workflowRuns.length === 0) {
-        core.warning(
-          `No workflow runs found for check suite ID: ${latestCheck._originalData.check_suite.id}`,
-        );
+        core.warning(`No workflow runs found for check suite ID: ${originalData.check_suite?.id}`);
       } else {
         // Sort by updated_at to get the most recent run
         const sortedRuns = workflowRuns.sort(
@@ -754,7 +764,7 @@ export async function getCheckRunTuple(
 
         if (workflowRuns.length > 1) {
           core.info(
-            `Found ${workflowRuns.length} workflow runs for check suite ID: ${latestCheck._originalData.check_suite.id}, using most recent: ${sortedRuns[0].id}`,
+            `Found ${workflowRuns.length} workflow runs for check suite ID: ${originalData.check_suite?.id}, using most recent: ${sortedRuns[0].id}`,
           );
         }
       }
@@ -862,14 +872,14 @@ export function getCheckInfo(checkName) {
  * @param {typeof import("@actions/core")} core
  * @param {string} repo
  * @param {string[]} labels
- * @param {string} targetBranch
+ * @param {string|undefined} targetBranch
  * @param {CheckRunData[]} requiredRuns
  * @param {CheckRunData[]} fyiRuns
  * @param {boolean} assessmentCompleted
  * @param {string} target_url
- * @returns {Promise<[string, CheckRunResult]>}
+ * @returns {[string, CheckRunResult]}
  */
-export async function createNextStepsComment(
+export function createNextStepsComment(
   core,
   repo,
   labels,
@@ -898,7 +908,7 @@ export async function createNextStepsComment(
     .filter((run) => checkRunIsSuccessful(run) === false)
     .map((run) => run.checkInfo);
 
-  const [commentBody, automatedChecksMet] = await buildNextStepsToMergeCommentBody(
+  const [commentBody, automatedChecksMet] = buildNextStepsToMergeCommentBody(
     core,
     labels,
     `${repo}/${targetBranch}`,
@@ -921,9 +931,9 @@ export async function createNextStepsComment(
  * @param {CheckMetadata[]} failingFyiChecksInfo
  * @param {boolean} assessmentCompleted
  * @param {string} target_url
- * @returns {Promise<[string, CheckRunResult]>}
+ * @returns {[string, CheckRunResult]}
  */
-async function buildNextStepsToMergeCommentBody(
+function buildNextStepsToMergeCommentBody(
   core,
   labels,
   targetBranch,
@@ -936,7 +946,7 @@ async function buildNextStepsToMergeCommentBody(
   // Build the comment header
   const commentTitle = `<h2>Next Steps to Merge</h2>`;
 
-  const violatedReqLabelsRules = await getViolatedRequiredLabelsRules(core, labels, targetBranch);
+  const violatedReqLabelsRules = getViolatedRequiredLabelsRules(core, labels, targetBranch);
 
   // we are "blocked" if we have any violated labelling rules OR if we have any failing required checks
   const anyBlockerPresent = failingReqChecksInfo.length > 0 || violatedReqLabelsRules.length > 0;
@@ -1182,9 +1192,6 @@ export async function getImpactAssessment(github, core, owner, repo, runId) {
 
   await fs.unlink(tmpZip);
 
-  /** @type {import("./labelling.js").ImpactAssessment} */
-  // todo: we need to zod this to ensure the structure is correct, however we do not have zod installed at time of run
-  const impact = JSON.parse(jsonContent);
-  return impact;
+  return ImpactAssessmentSchema.parse(JSON.parse(jsonContent));
 }
 // #endregion