[ARM Auto Signoff] Use statuses for LintDiff and Avocado (#36472)

Author: mikeharder

.github/workflows/arm-auto-signoff.yaml

@@ -1,20 +1,18 @@
 name: ARM Auto SignOff
 
 on:
-  issue_comment:
-    types:
-      - edited
   # Must run on pull_request_target instead of pull_request, since the latter cannot trigger on
   # labels from bot accounts in fork PRs. pull_request_target is also more similar to the other
-  # triggers "issue_comment" and "workflow_run" -- they are all privileged# and run in the target
-  # branch and repo -- which simplifies implementation.
+  # trigger "workflow_run" -- they are both privileged and run in the target branch and repo --
+  # which simplifies implementation.
   pull_request_target:
     types:
       # Depends on labels, so must re-evaluate whenever a relevant label is manually added or removed.
       - labeled
       - unlabeled
   workflow_run:
-    workflows: ["ARM Incremental TypeSpec"]
+    workflows:
+      ["ARM Incremental TypeSpec", "Swagger Avocado - Set Status", "Swagger LintDiff - Set Status"]
     types: [completed]
 
 permissions:
@@ -28,9 +26,8 @@ jobs:
   arm-auto-signoff:
     name: ARM Auto SignOff
 
+    # workflow_run - already filtered by triggers above
     # pull_request_target:labeled - filter to only the input and output labels
-    # issue_comment:edited - filter to only PR comments containing "next steps to merge",
-    #   a signal that checks like "Swagger LintDiff" or "Swagger Avocado" status may have changed
     if: |
       github.event_name == 'workflow_run' ||
       (github.event_name == 'pull_request_target' &&
@@ -41,10 +38,7 @@ jobs:
         github.event.label.name == 'ARMReview' ||
         github.event.label.name == 'ARMSignedOff' ||
         github.event.label.name == 'NotReadyForARMReview' ||
-        github.event.label.name == 'SuppressionReviewRequired')) ||
-      (github.event_name == 'issue_comment' &&
-       github.event.issue.pull_request &&
-       contains(github.event.comment.body, 'next steps to merge'))
+        github.event.label.name == 'SuppressionReviewRequired'))
 
     runs-on: ubuntu-24.04
 

.github/workflows/src/arm-auto-signoff.js

@@ -1,7 +1,8 @@
 // @ts-check
 
 import { setEquals } from "../../shared/src/equality.js";
-import { PER_PAGE_MAX } from "../../shared/src/github.js";
+import { CommitStatusState, PER_PAGE_MAX } from "../../shared/src/github.js";
+import { byDate, invert } from "../../shared/src/sort.js";
 import { extractInputs } from "./context.js";
 import { LabelAction } from "./label.js";
 
@@ -142,52 +143,63 @@ export async function getLabelActionImpl({ owner, repo, issue_number, head_sha,
     return removeAction;
   }
 
-  const checkRuns = await github.paginate(github.rest.checks.listForRef, {
+  const statuses = await github.paginate(github.rest.repos.listCommitStatusesForRef, {
     owner: owner,
     repo: repo,
     ref: head_sha,
     per_page: PER_PAGE_MAX,
   });
 
-  const requiredCheckNames = ["Swagger LintDiff", "Swagger Avocado"];
+  core.info("Statuses:");
+  statuses.forEach((status) => {
+    core.info(`- ${status.context}: ${status.state}`);
+  });
+
+  const requiredStatusNames = ["Swagger LintDiff", "Swagger Avocado"];
 
   /**
-   * @type {typeof checkRuns.check_runs}
+   * @type {typeof statuses}
    */
-  let requiredCheckRuns = [];
-
-  for (const checkName of requiredCheckNames) {
-    const matchingRuns = checkRuns.filter((run) => run.name === checkName);
-
-    if (matchingRuns.length > 1) {
-      throw new Error(`Unexpected number of checks named '${checkName}': ${matchingRuns.length}`);
-    }
-
-    const matchingRun = matchingRuns.length === 1 ? matchingRuns[0] : undefined;
-
-    core.info(
-      `${checkName}: Status='${matchingRun?.status}', Conclusion='${matchingRun?.conclusion}'`,
-    );
-
-    if (matchingRun && matchingRun.status === "completed" && matchingRun.conclusion !== "success") {
-      core.info(`Check '${checkName}' did not succeed`);
+  let requiredStatuses = [];
+
+  for (const statusName of requiredStatusNames) {
+    // The "statuses" array may contain multiple statuses with the same "context" (aka "name"),
+    // but different states and update times.  We only care about the latest.
+    const matchingStatuses = statuses
+      .filter((status) => status.context.toLowerCase() === statusName.toLowerCase())
+      .sort(invert(byDate((status) => status.updated_at)));
+
+    // undefined if matchingStatuses.length === 0 (which is OK)
+    const matchingStatus = matchingStatuses[0];
+
+    core.info(`${statusName}: State='${matchingStatus?.state}'`);
+
+    if (
+      matchingStatus &&
+      (matchingStatus.state === CommitStatusState.ERROR ||
+        matchingStatus.state === CommitStatusState.FAILURE)
+    ) {
+      core.info(`Status '${matchingStatus}' did not succeed`);
       return removeAction;
     }
 
-    if (matchingRun) {
-      requiredCheckRuns.push(matchingRun);
+    if (matchingStatus) {
+      requiredStatuses.push(matchingStatus);
     }
   }
 
   if (
-    setEquals(new Set(requiredCheckRuns.map((run) => run.name)), new Set(requiredCheckNames)) &&
-    requiredCheckRuns.every((run) => run.status === "completed" && run.conclusion === "success")
+    setEquals(
+      new Set(requiredStatuses.map((status) => status.context)),
+      new Set(requiredStatusNames),
+    ) &&
+    requiredStatuses.every((status) => status.state === CommitStatusState.SUCCESS)
   ) {
     core.info("All requirements met for auto-signoff");
     return labelActions[LabelAction.Add];
   }
 
-  // If any checks are missing or not completed, no-op to prevent frequent remove/add label as checks re-run
-  core.info("One or more checks are still in-progress");
+  // If any statuses are missing or pending, no-op to prevent frequent remove/add label as checks re-run
+  core.info("One or more statuses are still pending");
   return labelActions[LabelAction.None];
 }

.github/workflows/test/arm-auto-signoff.test.js

@@ -1,4 +1,5 @@
 import { describe, expect, it } from "vitest";
+import { CommitStatusState } from "../../shared/src/github.js";
 import { getLabelActionImpl } from "../src/arm-auto-signoff.js";
 import { LabelAction } from "../src/label.js";
 import { createMockCore, createMockGithub as createMockGithubBase } from "./mocks.js";
@@ -218,16 +219,13 @@ describe("getLabelActionImpl", () => {
       github.rest.issues.listLabelsOnIssue.mockResolvedValue({
         data: [{ name: "ARMAutoSignedOff" }, { name: "ARMReview" }],
       });
-      github.rest.checks.listForRef.mockResolvedValue({
-        data: {
-          check_runs: [
-            {
-              name: check,
-              status: "completed",
-              conclusion: "failure",
-            },
-          ],
-        },
+      github.rest.repos.listCommitStatusesForRef.mockResolvedValue({
+        data: [
+          {
+            context: check,
+            state: CommitStatusState.FAILURE,
+          },
+        ],
       });
 
       await expect(
@@ -243,29 +241,54 @@ describe("getLabelActionImpl", () => {
     },
   );
 
-  it("thows if multiple runs for same check", async () => {
+  it.each([
+    [CommitStatusState.ERROR, ["ARMReview", "ARMAutoSignedOff"]],
+    [CommitStatusState.FAILURE, ["ARMReview", "ARMAutoSignedOff"]],
+    [CommitStatusState.PENDING, ["ARMReview"]],
+    [CommitStatusState.SUCCESS, ["ARMReview"]],
+  ])("uses latest status if multiple (%o)", async (state, labels) => {
     const github = createMockGithub({ incrementalTypeSpec: true });
 
     github.rest.issues.listLabelsOnIssue.mockResolvedValue({
-      data: [{ name: "ARMReview" }],
+      data: labels.map((l) => ({
+        name: l,
+      })),
     });
 
-    github.rest.checks.listForRef.mockResolvedValue({
-      data: {
-        check_runs: [
-          {
-            name: "Swagger LintDiff",
-            status: "in_progress",
-            conclusion: null,
-          },
-          {
-            name: "Swagger LintDiff",
-            status: "in_progress",
-            conclusion: null,
-          },
-        ],
-      },
+    github.rest.repos.listCommitStatusesForRef.mockResolvedValue({
+      data: [
+        {
+          context: "Swagger Avocado",
+          state: CommitStatusState.SUCCESS,
+          updated_at: "2025-01-01",
+        },
+        {
+          context: "Swagger LintDiff",
+          state: CommitStatusState.PENDING,
+          updated_at: "2025-01-01",
+        },
+        {
+          context: "Swagger LintDiff",
+          state,
+          updated_at: "2025-01-02",
+        },
+      ],
     });
+
+    let expectedAction;
+    switch (state) {
+      case CommitStatusState.ERROR:
+      case CommitStatusState.FAILURE:
+        expectedAction = LabelAction.Remove;
+        break;
+      case CommitStatusState.SUCCESS:
+        expectedAction = LabelAction.Add;
+        break;
+      case CommitStatusState.PENDING:
+        expectedAction = LabelAction.None;
+        break;
+    }
+
     await expect(
       getLabelActionImpl({
         owner: "TestOwner",
@@ -275,7 +298,7 @@ describe("getLabelActionImpl", () => {
         github: github,
         core: core,
       }),
-    ).rejects.toThrow();
+    ).resolves.toEqual({ labelAction: expectedAction, headSha: "abc123", issueNumber: 123 });
   });
 
   it("no-ops if check not found or not completed", async () => {
@@ -285,10 +308,8 @@ describe("getLabelActionImpl", () => {
       data: [{ name: "ARMReview" }],
     });
 
-    github.rest.checks.listForRef.mockResolvedValue({
-      data: {
-        check_runs: [],
-      },
+    github.rest.repos.listCommitStatusesForRef.mockResolvedValue({
+      data: [],
     });
     await expect(
       getLabelActionImpl({
@@ -301,16 +322,8 @@ describe("getLabelActionImpl", () => {
       }),
     ).resolves.toEqual({ labelAction: LabelAction.None, headSha: "abc123", issueNumber: 123 });
 
-    github.rest.checks.listForRef.mockResolvedValue({
-      data: {
-        check_runs: [
-          {
-            name: "Swagger LintDiff",
-            status: "in_progress",
-            conclusion: null,
-          },
-        ],
-      },
+    github.rest.repos.listCommitStatusesForRef.mockResolvedValue({
+      data: [{ context: "Swagger LintDiff", state: CommitStatusState.PENDING }],
     });
     await expect(
       getLabelActionImpl({
@@ -330,21 +343,17 @@ describe("getLabelActionImpl", () => {
     github.rest.issues.listLabelsOnIssue.mockResolvedValue({
       data: [{ name: "ARMReview" }],
     });
-    github.rest.checks.listForRef.mockResolvedValue({
-      data: {
-        check_runs: [
-          {
-            name: "Swagger LintDiff",
-            status: "completed",
-            conclusion: "success",
-          },
-          {
-            name: "Swagger Avocado",
-            status: "completed",
-            conclusion: "success",
-          },
-        ],
-      },
+    github.rest.repos.listCommitStatusesForRef.mockResolvedValue({
+      data: [
+        {
+          context: "Swagger LintDiff",
+          state: CommitStatusState.SUCCESS,
+        },
+        {
+          context: "Swagger Avocado",
+          state: CommitStatusState.SUCCESS,
+        },
+      ],
     });
 
     await expect(

.github/workflows/test/mocks.js

@@ -33,6 +33,7 @@ export function createMockGithub() {
       },
       repos: {
         createCommitStatus: vi.fn(),
+        listCommitStatusesForRef: vi.fn().mockResolvedValue({ data: [] }),
         listPullRequestsAssociatedWithCommit: vi.fn().mockResolvedValue({
           data: [],
         }),