Fix Incorrectly removing label ARMSignedOff for trivial PRs

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

@@ -1,3 +1,3 @@
 import { inspect } from "util";
 import { CommitStatusState, PER_PAGE_MAX } from "../../../shared/src/github.js";
 import { equals } from "../../../shared/src/set.js";
@@ -105,8 +105,15 @@
     labelNames.includes(ArmAutoSignoffLabel.ArmAutoSignedOff) ||
     labelNames.includes(ArmAutoSignoffLabel.ArmAutoSignedOffIncrementalTSP) ||
     labelNames.includes(ArmAutoSignoffLabel.ArmAutoSignedOffTrivialTest);
+
+  // Track if ARMSignedOff was auto-added (IncrementalTSP present) vs manually added
+  const hasAutoAddedArmSignedOff =
+    // need to consider the legacy label
+    labelNames.includes(ArmAutoSignoffLabel.ArmAutoSignedOff) ||
+    labelNames.includes(ArmAutoSignoffLabel.ArmAutoSignedOffIncrementalTSP);
   core.info(`Labels: ${inspect(labelNames)}`);
   core.info(`Has auto signed-off labels: ${hasAutoSignedOffLabels}`);
+  core.info(`Has auto-added ARMSignedOff: ${hasAutoAddedArmSignedOff}`);
 
   // permissions: { actions: read }
   /** @type {WorkflowRun[]} */
@@ -136,12 +143,18 @@
       return noneResult;
     }
 
+    // Only remove ARMSignedOff if it was auto-added (IncrementalTSP present)
+    // Preserve manually-added ARMSignedOff labels
     return {
       ...noneResult,
       labelActions: {
         ...noneLabelActions,
-        [ArmAutoSignoffLabel.ArmSignedOff]: LabelAction.Remove,
-        [ArmAutoSignoffLabel.ArmAutoSignedOffIncrementalTSP]: LabelAction.Remove,
+        [ArmAutoSignoffLabel.ArmSignedOff]: hasAutoAddedArmSignedOff
+          ? LabelAction.Remove
+          : LabelAction.None,
+        [ArmAutoSignoffLabel.ArmAutoSignedOffIncrementalTSP]: hasAutoAddedArmSignedOff
+          ? LabelAction.Remove
+          : LabelAction.None,
         [ArmAutoSignoffLabel.ArmAutoSignedOffTrivialTest]: LabelAction.Remove,
       },
     };
@@ -221,9 +234,11 @@
 
     // Add ARMSignOff label only when the PR is identified as an incremental typespec
     // As the trivial changes sign-off is being released in test mode
+    // Only remove ARMSignedOff if it was auto-added (IncrementalTSP present)
+    // Preserve manually-added ARMSignedOff labels
     const armSignOffAction = autoIncrementalTSP
       ? LabelAction.Add
-      : hasAutoSignedOffLabels
+      : hasAutoAddedArmSignedOff
         ? LabelAction.Remove
         : LabelAction.None;
     const autoIncrementalTSPAction = autoIncrementalTSP ? LabelAction.Add : LabelAction.Remove;

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

@@ -1,3 +1,3 @@
 import { describe, expect, it } from "vitest";
 import { CommitStatusState } from "../../../shared/src/github.js";
 import { getLabelActionImpl } from "../../src/arm-auto-signoff/arm-auto-signoff-status.js";
@@ -175,24 +175,6 @@
     ).resolves.toEqual(createRemoveManagedLabelsResult("abc123", 123));
   });
 
-  it("removes ARMAutoSignedOff-Trivial-Test if analysis no longer trivial", async () => {
-    const github = createMockGithub({ incrementalTypeSpec: false, isTrivial: false });
-    github.rest.issues.listLabelsOnIssue.mockResolvedValue({
-      data: [{ name: managedLabels.autoSignedOffTrivialTest }],
-    });
-
-    await expect(
-      getLabelActionImpl({
-        owner: "TestOwner",
-        repo: "TestRepo",
-        issue_number: 123,
-        head_sha: "abc123",
-        github: github,
-        core: core,
-      }),
-    ).resolves.toEqual(createRemoveManagedLabelsResult("abc123", 123));
-  });
-
   it("removes label if analysis artifacts missing (fail closed)", async () => {
     const github = createMockGithub({ incrementalTypeSpec: true });
     github.rest.issues.listLabelsOnIssue.mockResolvedValue({
@@ -560,4 +542,223 @@
       }),
     ).resolves.toEqual(createRemoveManagedLabelsResult("abc123", 123));
   });
+
+  it("preserves existing ARMSignedOff and adds trivial test label when PR qualifies by trivial changes only", async () => {
+    const github = createMockGithub({ incrementalTypeSpec: false, isTrivial: true });
+
+    // PR already has ARMSignedOff (manually added) and ARMReview, but no auto-signoff labels
+    github.rest.issues.listLabelsOnIssue.mockResolvedValue({
+      data: [{ name: "ARMSignedOff" }, { name: "ARMReview" }],
+    });
+
+    // All required checks pass
+    github.rest.repos.listCommitStatusesForRef.mockResolvedValue({
+      data: [
+        {
+          context: "Swagger LintDiff",
+          state: CommitStatusState.SUCCESS,
+        },
+        {
+          context: "Swagger Avocado",
+          state: CommitStatusState.SUCCESS,
+        },
+      ],
+    });
+
+    const result = await getLabelActionImpl({
+      owner: "TestOwner",
+      repo: "TestRepo",
+      issue_number: 123,
+      head_sha: "abc123",
+      github: github,
+      core: core,
+    });
+
+    // ARMSignedOff should remain unchanged (LabelAction.None) since it already exists
+    // and is not managed by auto-signoff when hasAutoSignedOffLabels is false
+    expect(result.labelActions[managedLabels.armSignedOff]).toBe(LabelAction.None);
+    // Trivial test label should be added
+    expect(result.labelActions[managedLabels.autoSignedOffTrivialTest]).toBe(LabelAction.Add);
+    // Incremental TSP label should be removed (not applicable)
+    expect(result.labelActions[managedLabels.autoSignedOffIncrementalTsp]).toBe(LabelAction.Remove);
+  });
+
+  it("preserves manual ARMSignedOff when trivial test label present and no longer qualifies", async () => {
+    const github = createMockGithub({ incrementalTypeSpec: false, isTrivial: false });
+
+    // PR has manually added ARMSignedOff + trivial test label (from previous run)
+    github.rest.issues.listLabelsOnIssue.mockResolvedValue({
+      data: [
+        { name: "ARMSignedOff" },
+        { name: managedLabels.autoSignedOffTrivialTest },
+        { name: "ARMReview" },
+      ],
+    });
+
+    const result = await getLabelActionImpl({
+      owner: "TestOwner",
+      repo: "TestRepo",
+      issue_number: 123,
+      head_sha: "abc123",
+      github: github,
+      core: core,
+    });
+
+    // ARMSignedOff was manually added (no IncrementalTSP), so preserve it
+    expect(result.labelActions[managedLabels.armSignedOff]).toBe(LabelAction.None);
+    // Trivial test label should be removed (no longer qualifies)
+    expect(result.labelActions[managedLabels.autoSignedOffTrivialTest]).toBe(LabelAction.Remove);
+    // Incremental TSP label was never present, so no-op
+    expect(result.labelActions[managedLabels.autoSignedOffIncrementalTsp]).toBe(LabelAction.None);
+  });
+
+  it("removes all auto-signoff labels when IncrementalTSP was present and no longer qualifies", async () => {
+    const github = createMockGithub({ incrementalTypeSpec: false, isTrivial: false });
+
+    // PR had auto-added ARMSignedOff via IncrementalTSP
+    github.rest.issues.listLabelsOnIssue.mockResolvedValue({
+      data: [
+        { name: "ARMSignedOff" },
+        { name: managedLabels.autoSignedOffIncrementalTsp },
+        { name: "ARMReview" },
+      ],
+    });
+
+    const result = await getLabelActionImpl({
+      owner: "TestOwner",
+      repo: "TestRepo",
+      issue_number: 123,
+      head_sha: "abc123",
+      github: github,
+      core: core,
+    });
+
+    // ARMSignedOff was auto-added (IncrementalTSP present), so remove it
+    expect(result.labelActions[managedLabels.armSignedOff]).toBe(LabelAction.Remove);
+    // Both auto-signoff labels should be removed
+    expect(result.labelActions[managedLabels.autoSignedOffTrivialTest]).toBe(LabelAction.Remove);
+    expect(result.labelActions[managedLabels.autoSignedOffIncrementalTsp]).toBe(LabelAction.Remove);
+  });
+
+  it("transitions from trivial to incremental typespec correctly", async () => {
+    const github = createMockGithub({ incrementalTypeSpec: true, isTrivial: false });
+
+    // PR previously had trivial test label, now qualifies via incremental typespec
+    github.rest.issues.listLabelsOnIssue.mockResolvedValue({
+      data: [{ name: managedLabels.autoSignedOffTrivialTest }, { name: "ARMReview" }],
+    });
+
+    github.rest.repos.listCommitStatusesForRef.mockResolvedValue({
+      data: [
+        { context: "Swagger LintDiff", state: CommitStatusState.SUCCESS },
+        { context: "Swagger Avocado", state: CommitStatusState.SUCCESS },
+      ],
+    });
+
+    const result = await getLabelActionImpl({
+      owner: "TestOwner",
+      repo: "TestRepo",
+      issue_number: 123,
+      head_sha: "abc123",
+      github: github,
+      core: core,
+    });
+
+    // Now qualifies via incremental typespec, so add ARMSignedOff
+    expect(result.labelActions[managedLabels.armSignedOff]).toBe(LabelAction.Add);
+    expect(result.labelActions[managedLabels.autoSignedOffIncrementalTsp]).toBe(LabelAction.Add);
+    // Trivial test label should be removed (not applicable)
+    expect(result.labelActions[managedLabels.autoSignedOffTrivialTest]).toBe(LabelAction.Remove);
+  });
+
+  it("transitions from incremental typespec to trivial correctly", async () => {
+    const github = createMockGithub({ incrementalTypeSpec: false, isTrivial: true });
+
+    // PR previously had incremental typespec labels, now qualifies only via trivial
+    github.rest.issues.listLabelsOnIssue.mockResolvedValue({
+      data: [
+        { name: "ARMSignedOff" },
+        { name: managedLabels.autoSignedOffIncrementalTsp },
+        { name: "ARMReview" },
+      ],
+    });
+
+    github.rest.repos.listCommitStatusesForRef.mockResolvedValue({
+      data: [
+        { context: "Swagger LintDiff", state: CommitStatusState.SUCCESS },
+        { context: "Swagger Avocado", state: CommitStatusState.SUCCESS },
+      ],
+    });
+
+    const result = await getLabelActionImpl({
+      owner: "TestOwner",
+      repo: "TestRepo",
+      issue_number: 123,
+      head_sha: "abc123",
+      github: github,
+      core: core,
+    });
+
+    // IncrementalTSP was present, so ARMSignedOff should be removed (it was auto-added)
+    expect(result.labelActions[managedLabels.armSignedOff]).toBe(LabelAction.Remove);
+    // Incremental TSP label should be removed
+    expect(result.labelActions[managedLabels.autoSignedOffIncrementalTsp]).toBe(LabelAction.Remove);
+    // Trivial test label should be added
+    expect(result.labelActions[managedLabels.autoSignedOffTrivialTest]).toBe(LabelAction.Add);
+  });
+
+  it("adds both labels when PR qualifies for both incremental typespec and trivial", async () => {
+    const github = createMockGithub({ incrementalTypeSpec: true, isTrivial: true });
+
+    github.rest.issues.listLabelsOnIssue.mockResolvedValue({
+      data: [{ name: "ARMReview" }],
+    });
+
+    github.rest.repos.listCommitStatusesForRef.mockResolvedValue({
+      data: [
+        { context: "Swagger LintDiff", state: CommitStatusState.SUCCESS },
+        { context: "Swagger Avocado", state: CommitStatusState.SUCCESS },
+      ],
+    });
+
+    const result = await getLabelActionImpl({
+      owner: "TestOwner",
+      repo: "TestRepo",
+      issue_number: 123,
+      head_sha: "abc123",
+      github: github,
+      core: core,
+    });
+
+    // Incremental typespec takes precedence for ARMSignedOff
+    expect(result.labelActions[managedLabels.armSignedOff]).toBe(LabelAction.Add);
+    expect(result.labelActions[managedLabels.autoSignedOffIncrementalTsp]).toBe(LabelAction.Add);
+    // Trivial test label should also be added
+    expect(result.labelActions[managedLabels.autoSignedOffTrivialTest]).toBe(LabelAction.Add);
+  });
+
+  it("removes only trivial test label when only TrivialTest present and no longer qualifies", async () => {
+    const github = createMockGithub({ incrementalTypeSpec: false, isTrivial: false });
+
+    // PR only has trivial test label (no ARMSignedOff, no IncrementalTSP)
+    github.rest.issues.listLabelsOnIssue.mockResolvedValue({
+      data: [{ name: managedLabels.autoSignedOffTrivialTest }, { name: "ARMReview" }],
+    });
+
+    const result = await getLabelActionImpl({
+      owner: "TestOwner",
+      repo: "TestRepo",
+      issue_number: 123,
+      head_sha: "abc123",
+      github: github,
+      core: core,
+    });
+
+    // No IncrementalTSP, so ARMSignedOff action is None
+    expect(result.labelActions[managedLabels.armSignedOff]).toBe(LabelAction.None);
+    // Trivial test label should be removed
+    expect(result.labelActions[managedLabels.autoSignedOffTrivialTest]).toBe(LabelAction.Remove);
+    // Incremental TSP was never present, so no-op
+    expect(result.labelActions[managedLabels.autoSignedOffIncrementalTsp]).toBe(LabelAction.None);
+  });
 });