Fix proposal approval status automation

.github/workflows/lib/approval.js

@@ -20,6 +20,33 @@ class ApprovalManager {
     return users.length ? users.map((u) => `[@${u}](https://github.com/${u})`).join(", ") : "-";
   }
 
+  // Determine the status of a pipeline proposal from durable issue state and
+  // votes, rather than from the event that happened to trigger the workflow.
+  determinePipelineStatus(issue) {
+    const labels = (issue.labels || []).map((label) => (typeof label === "string" ? label : label.name));
+    const approvalThresholdReached =
+      this.coreApprovals.size >= 2 || (this.coreApprovals.size >= 1 && this.maintainerApprovals.size >= 1);
+    const rejectionThresholdReached =
+      (this.coreRejections.size >= 2 || (this.coreRejections.size >= 1 && this.maintainerRejections.size >= 1)) &&
+      this.coreApprovals.size === 0 &&
+      this.maintainerApprovals.size === 0;
+    const closedAsRejected =
+      issue.state === "closed" &&
+      issue.state_reason === "not_planned" &&
+      (this.coreRejections.size > 0 || this.maintainerRejections.size > 0);
+
+    if (labels.includes("timed-out")) {
+      return "⏰ Timed Out";
+    }
+    if (closedAsRejected || rejectionThresholdReached) {
+      return "❌ Rejected";
+    }
+    if (approvalThresholdReached) {
+      return "✅ Approved";
+    }
+    return "🕐 Pending";
+  }
+
   // Helper to fetch team members
   async getTeamMembers(teamSlug) {
     try {
@@ -91,6 +118,16 @@ class ApprovalManager {
     // Combine existing non-status labels with new status label
     const updatedLabels = [...existingLabels, newStatusLabel];
 
+    // Avoid triggering another workflow run when the labels are already right.
+    const currentLabels = issue.data.labels.map((label) => (typeof label === "string" ? label : label.name));
+    if (
+      currentLabels.length === updatedLabels.length &&
+      currentLabels.every((label) => updatedLabels.includes(label))
+    ) {
+      console.log("Issue labels already up to date - no update required.");
+      return;
+    }
+
     // Update labels
     await this.github.rest.issues.update({
       owner: this.org,
@@ -102,7 +139,21 @@ class ApprovalManager {
 
   // Helper to find and update status comment
   async updateStatusComment(statusBody) {
-    let statusComment = this.comments.find((c) => c.body.startsWith("## Approval status:"));
+    const statusComments = this.comments.filter((c) => c.body && c.body.startsWith("## Approval status:"));
+    const statusComment = statusComments[0];
+
+    // Opening an issue also applies its template labels. Older workflow runs
+    // could race and create more than one status comment; clean those up when
+    // the issue is next processed.
+    for (const duplicate of statusComments.slice(1)) {
+      console.log(`Deleting duplicate status comment ${duplicate.id}.`);
+      await this.github.rest.issues.deleteComment({
+        owner: this.org,
+        repo: this.repo,
+        comment_id: duplicate.id,
+      });
+    }
+
     if (statusComment) {
       if (statusComment.body.trim() === statusBody.trim()) {
         console.log("Status comment already up to date - no update required.");

.github/workflows/lib/approval.test.js

@@ -11,6 +11,7 @@ const mockGithub = {
       update: jest.fn(),
       updateComment: jest.fn(),
       createComment: jest.fn(),
+      deleteComment: jest.fn(),
     },
   },
 };
@@ -60,6 +61,56 @@ describe("ApprovalManager", () => {
     });
   });
 
+  describe("determinePipelineStatus", () => {
+    const issue = (overrides = {}) => ({ state: "open", state_reason: null, labels: [], ...overrides });
+
+    it("should approve with 2 core approvals", () => {
+      approvalManager.coreApprovals = new Set(["core1", "core2"]);
+
+      expect(approvalManager.determinePipelineStatus(issue())).toBe("✅ Approved");
+    });
+
+    it("should reject with 2 core rejections and no approvals", () => {
+      approvalManager.coreRejections = new Set(["core1", "core2"]);
+
+      expect(approvalManager.determinePipelineStatus(issue())).toBe("❌ Rejected");
+    });
+
+    it("should reject with 1 core and 1 maintainer rejection", () => {
+      approvalManager.coreRejections = new Set(["core1"]);
+      approvalManager.maintainerRejections = new Set(["maintainer1"]);
+
+      expect(approvalManager.determinePipelineStatus(issue())).toBe("❌ Rejected");
+    });
+
+    it("should stay pending when rejection votes are opposed by an approval", () => {
+      approvalManager.coreRejections = new Set(["core1", "core2"]);
+      approvalManager.coreApprovals = new Set(["core3"]);
+
+      expect(approvalManager.determinePipelineStatus(issue())).toBe("🕐 Pending");
+    });
+
+    it("should preserve a manual rejected closure on later events", () => {
+      approvalManager.coreRejections = new Set(["core1"]);
+
+      expect(approvalManager.determinePipelineStatus(issue({ state: "closed", state_reason: "not_planned" }))).toBe(
+        "❌ Rejected",
+      );
+    });
+
+    it("should handle the Dragen case regardless of the incorrect close reason", () => {
+      approvalManager.coreRejections = new Set(["core1", "core2", "core3", "core4"]);
+
+      expect(approvalManager.determinePipelineStatus(issue({ state: "closed", state_reason: "completed" }))).toBe(
+        "❌ Rejected",
+      );
+    });
+
+    it("should preserve the timed-out label", () => {
+      expect(approvalManager.determinePipelineStatus(issue({ labels: [{ name: "timed-out" }] }))).toBe("⏰ Timed Out");
+    });
+  });
+
   describe("getTeamMembers", () => {
     it("should fetch team members successfully", async () => {
       const mockMembers = [{ login: "user1" }, { login: "user2" }, { login: "user3" }];
@@ -174,6 +225,12 @@ describe("ApprovalManager", () => {
     });
 
     it("should add proposed label for pending status", async () => {
+      mockGithub.rest.issues.get.mockResolvedValueOnce({
+        data: {
+          labels: [{ name: "bug" }, { name: "accepted" }, { name: "enhancement" }],
+        },
+      });
+
       await approvalManager.updateIssueStatus("🕐 Pending");
 
       expect(mockGithub.rest.issues.get).toHaveBeenCalledWith({
@@ -211,6 +268,16 @@ describe("ApprovalManager", () => {
         labels: ["documentation", "priority-high", "turned-down"],
       });
     });
+
+    it("should not update labels when they are already correct", async () => {
+      mockGithub.rest.issues.get.mockResolvedValueOnce({
+        data: { labels: [{ name: "bug" }, { name: "accepted" }] },
+      });
+
+      await approvalManager.updateIssueStatus("✅ Approved");
+
+      expect(mockGithub.rest.issues.update).not.toHaveBeenCalled();
+    });
   });
 
   describe("updateStatusComment", () => {
@@ -252,6 +319,42 @@ describe("ApprovalManager", () => {
         body: statusBody,
       });
     });
+
+    it("should delete duplicate status comments and update the first", async () => {
+      approvalManager.comments = [
+        { id: 1, body: "## Approval status: Old status" },
+        { id: 2, body: "## Approval status: Old status" },
+      ];
+      const newStatusBody = "## Approval status: New status";
+
+      await approvalManager.updateStatusComment(newStatusBody);
+
+      expect(mockGithub.rest.issues.deleteComment).toHaveBeenCalledWith({
+        owner: mockOrg,
+        repo: mockRepo,
+        comment_id: 2,
+      });
+      expect(mockGithub.rest.issues.updateComment).toHaveBeenCalledWith({
+        owner: mockOrg,
+        repo: mockRepo,
+        comment_id: 1,
+        body: newStatusBody,
+      });
+    });
+
+    it("should ignore comments with no body", async () => {
+      approvalManager.comments = [{ id: 1, body: null }];
+      const statusBody = "## Approval status: New status";
+
+      await approvalManager.updateStatusComment(statusBody);
+
+      expect(mockGithub.rest.issues.createComment).toHaveBeenCalledWith({
+        owner: mockOrg,
+        repo: mockRepo,
+        issue_number: mockIssueNumber,
+        body: statusBody,
+      });
+    });
   });
 
   describe("processComments", () => {

.github/workflows/pipeline_proposals.yml

@@ -6,6 +6,12 @@ on:
   issue_comment:
     types: [created, edited]
 
+# Issue templates apply labels at the same time as the issue is opened. Process
+# those events in order so they cannot race to create or overwrite status state.
+concurrency:
+  group: ${{ github.workflow }}-${{ github.event.issue.number }}
+  cancel-in-progress: false
+
 jobs:
   pipeline_approval:
     # Only run for pipeline proposal issues
@@ -69,37 +75,9 @@ jobs:
               return body;
             }
 
-            // Handle label changes
-            if (context.eventName === 'issues' && (context.payload.action === 'labeled' || context.payload.action === 'unlabeled')) {
-              const label = context.payload.label.name;
-              if (label === 'timed-out') {
-                console.log('Timed-out label detected, updating status');
-                const statusBody = generateStatusBody('⏰ Timed Out');
-                await approvalManager.updateStatusComment(statusBody);
-                return;
-              }
-            }
-
-            // Handle new issue creation
-            if (context.eventName === 'issues' && context.payload.action === 'opened') {
-              const body = generateStatusBody('🕐 Pending');
-              console.log('Creating initial comment for review status');
-              await approvalManager.updateStatusComment(body);
-              await approvalManager.updateIssueStatus('🕐 Pending');
-              return;
-            }
-
-
-
-
-            // Determine status
-            let status = '🕐 Pending';
-
-            if (context.eventName === 'issues' && context.payload.action === 'closed' && context.payload.issue.state_reason === 'not_planned' && (approvalManager.coreRejections.size > 0 || approvalManager.maintainerRejections.size > 0)) {
-              status = '❌ Rejected';
-            } else if ((approvalManager.coreApprovals.size >= 2) || (approvalManager.coreApprovals.size >= 1 && approvalManager.maintainerApprovals.size >= 1)) {
-              status = '✅ Approved';
-            }
+            // Derive status from the issue and all votes on every event. This
+            // keeps later label events from reverting a rejected issue to pending.
+            const status = approvalManager.determinePipelineStatus(context.payload.issue);
 
             const statusBody = generateStatusBody(status);
             console.log('New status body to post:\n', statusBody);

.github/workflows/rfc_approval.yml

@@ -6,6 +6,10 @@ on:
   issue_comment:
     types: [created, edited]
 
+concurrency:
+  group: ${{ github.workflow }}-${{ github.event.issue.number }}
+  cancel-in-progress: false
+
 jobs:
   rfc_approval:
     # Only run for RFC proposal issues

.github/workflows/sig_approval.yml

@@ -6,6 +6,10 @@ on:
   issue_comment:
     types: [created, edited]
 
+concurrency:
+  group: ${{ github.workflow }}-${{ github.event.issue.number }}
+  cancel-in-progress: false
+
 jobs:
   sig_approval:
     # Only run for SIG proposal issues

.github/workflows/test-approval-automation.yml

@@ -7,13 +7,15 @@ on:
       - ".github/workflows/lib/**"
       - ".github/workflows/pipeline_proposals.yml"
       - ".github/workflows/rfc_approval.yml"
+      - ".github/workflows/sig_approval.yml"
       - ".github/workflows/test-approval-automation.yml"
   push:
     branches: [main, master]
     paths:
       - ".github/workflows/lib/**"
       - ".github/workflows/pipeline_proposals.yml"
       - ".github/workflows/rfc_approval.yml"
+      - ".github/workflows/sig_approval.yml"
       - ".github/workflows/test-approval-automation.yml"
 
 jobs: