Merge pull request #4 from pemamian/pr-rules-engine

feat: updating with guardrails on label removal

Author: pemamian

.github/workflows/scripts/routing/README.md

@@ -59,7 +59,9 @@ Triage logic is organized into specialized rule subclasses inheriting from `Base
 1. **`FileRoutingRule`**: Compiles modified files against the configuration and dynamically applies the corresponding `needs_review_label` and removes the `approved_label`.
 2. **`ReviewerApprovalRule`**: Tracks active `pygithub` approvals against the resolved organization team members list:
    * **Superpower Override**: If a designated superpower user (like Amit `amithanda`) approves, all TC and GC rules are satisfied instantly, transitioning the PR to `gov:approved` / `status:ready-to-merge`.
-   * **Label Security Guardrail**: Restricts `gov:tc-approved` and `status:tc-majority-approved` application. If applied by an unauthorized user outside Tech Council or DevOps, the script revokes the label with an automated warning comment.
+   * **Label Application & Removal Guardrails**: 
+     * Restricts `gov:tc-approved` and `status:tc-majority-approved` application. If applied by an unauthorized user outside Tech Council or DevOps, the script revokes the label.
+     * If an active `needs_review_label` is manually **unlabeled (removed)** by an unauthorized user while approvals are still pending, the engine **automatically re-applies the label** dynamically and posts a warning comment on the PR.
    * **SDK Relaxed Mode**: Repositories matching `sdk` or `meeting-minutes` automatically default team thresholds to `1` to expedite SDK review cycles.
 3. **`LabelLifecycleRule`**: Resolves blocked feedback loops. Clears `Label.LABEL_BLOCKED` and restores `Label.LABEL_UNDER_REVIEW` when the author pushes a new commit or comments on the PR.
 4. **`StalePRRule`**: Scans active timestamps:

.github/workflows/scripts/routing/test_routing.py

@@ -265,6 +265,38 @@ def test_amit_superpower_override_approval(self):
         self.assertTrue(Label.LABEL_READY_TO_MERGE in result.labels_to_add)
         self.assertTrue(result.satisfied)
 
+    def test_unauthorized_needs_review_label_removal_guardrail(self):
+        """Verifies ReviewerApprovalRule blocks unauthorized removal of active needs-review labels."""
+        # Scenario: Changed schemas file (Core Spec), user 'unauthorized_tester' manually removed needs_review label
+        # Target org/team handles: @Universal-Commerce-Protocol/tech-council
+        self.mock_client.check_team_membership.return_value = False # User is not a member of TC or DevOps
+
+        context_unlabel = PRContext(
+            pr_number=111,
+            repo_name="Universal-Commerce-Protocol/ucp",
+            title="feat: corespec modifications",
+            author="developer2",
+            is_draft=False,
+            labels=set(), # Set is empty because the user removed the label
+            modified_files=["schemas/v1/transaction.json"],
+            reviews=[],
+            event_name="pull_request",
+            event_payload={
+                "action": "unlabeled",
+                "sender": {"login": "unauthorized_tester"},
+                "label": {"name": "gov:needs-tc-review"} # Removed label name
+            }
+        )
+
+        rule = ReviewerApprovalRule(self.mock_config)
+        result = rule.evaluate(context_unlabel, self.mock_client)
+
+        # Assert that the needs_review label is dynamically re-applied
+        self.assertTrue("gov:needs-tc-review" in result.labels_to_add)
+        # Assert warning comment is prepared
+        self.assertEqual(len(result.comments_to_create), 1)
+        self.assertTrue("Warning: @unauthorized_tester, you do not have permission to remove `gov:needs-tc-review`." in result.comments_to_create[0])
+
     def test_label_lifecycle_blocked_resume(self):
         """Verifies LabelLifecycleRule handles blocked and resumed triggers."""
         rule = LabelLifecycleRule()

.github/workflows/scripts/routing/triage/rules.py

@@ -185,14 +185,16 @@ def evaluate(self, context: PRContext, client: GitHubAPIClient) -> RuleResult:
                     if approved_label:
                         labels_to_add.add(approved_label)
 
-        # Enforce guardrail logic for governance labels
+        # Enforce guardrail logic for governance labels application & removal security
+        event_user = context.event_payload.get("sender", {}).get("login")
+        event_action = context.event_payload.get("action")
+        org_name = context.repo_name.split("/")[0]
+
+        # 1. Guard against unauthorized addition of TC Majority Label
         if Label.LABEL_TC_MAJORITY_APPROVED in context.labels:
-            # Ensure applier was actually TC or DevOps
-            event_user = context.event_payload.get("sender", {}).get("login")
-            if event_user and context.event_name == "pull_request" and context.event_payload.get("action") == "labeled":
+            if event_user and context.event_name == "pull_request" and event_action == "labeled":
                 label_added = context.event_payload.get("label", {}).get("name")
                 if label_added == Label.LABEL_TC_MAJORITY_APPROVED:
-                    org_name = context.repo_name.split("/")[0]
                     is_tc = client.check_team_membership(org_name, "tech-council", event_user)
                     is_devops = client.check_team_membership(org_name, "devops-maintainers", event_user)
                     if not is_tc and not is_devops:
@@ -203,6 +205,33 @@ def evaluate(self, context: PRContext, client: GitHubAPIClient) -> RuleResult:
                             f"`{Label.LABEL_TC_MAJORITY_APPROVED}`. This action has been automatically reverted."
                         )
 
+        # 2. Guard against unauthorized removal of active needs-review labels
+        if context.event_name == "pull_request" and event_action == "unlabeled":
+            label_removed = context.event_payload.get("label", {}).get("name")
+            
+            for rule in self.config:
+                for team_handle, req_details in rule.get("review_requirements", {}).items():
+                    needs_label = req_details.get("needs_review_label")
+                    
+                    if needs_label and label_removed == needs_label:
+                        # Verify if the user who removed it is a member of the team or DevOps
+                        clean_handle = team_handle.lstrip("@")
+                        team_org, team_slug = clean_handle.split("/", 1)
+                        
+                        is_team_member = client.check_team_membership(team_org, team_slug, event_user)
+                        is_devops = client.check_team_membership(org_name, "devops-maintainers", event_user)
+                        
+                        # If the requirements are not satisfied and the user is unauthorized, re-apply!
+                        satisfied, _ = verify_team_approvals(context, team_handle, req_details.get("threshold", 1), client)
+                        if not satisfied and not is_team_member and not is_devops:
+                            print(f"[GUARDRAIL] Unauthorized user {event_user} removed required label {needs_label}. Re-applying.")
+                            labels_to_add.add(needs_label)
+                            comments.append(
+                                f"Warning: @{event_user}, you do not have permission to remove "
+                                f"`{needs_label}`. Reviews from `{team_handle}` are still pending. "
+                                f"This action has been automatically reverted."
+                            )
+
         # If all rules passed, transition to ready-to-merge
         if all_rules_satisfied and rules_evaluated > 0:
             labels_to_add.add(Label.LABEL_APPROVED)