fix(ci): accept unbound /test e2e comments and post replies (#7267)

Signed-off-by: Zujian Zhang <zhangzujian.7@gmail.com>

Author: zhangzujian

.github/workflows/x86-e2e-dispatcher.yaml

@@ -28,7 +28,7 @@ permissions:
   contents: read
   actions: write
   checks: read
-  pull-requests: read
+  pull-requests: write
   issues: write
 
 jobs:
@@ -393,6 +393,11 @@ jobs:
       github.event.issue.pull_request &&
       (startsWith(github.event.comment.body, '/test e2e') ||
        startsWith(github.event.comment.body, '/retest e2e-failed'))
+    permissions:
+      actions: write
+      contents: read
+      issues: write
+      pull-requests: write
     outputs:
       accepted: ${{ steps.decision.outputs.accepted }}
       action: ${{ steps.decision.outputs.action }}

hack/e2e_control.py

@@ -58,14 +58,34 @@ def parseCommand(body):
         rf"/test e2e ({groupPattern}(?:,{groupPattern})*){binding}",
         body,
     )
+    if match:
+        return {
+            "action": "dispatch",
+            "requestedGroups": sorted(set(match.group(1).split(","))),
+            "full": False,
+            "headSHA": match.group(2),
+            "nonce": match.group(3),
+        }
+    unbound = {
+        "/test e2e": {"action": "dispatch", "requestedGroups": [], "full": False},
+        "/test e2e-all": {"action": "dispatch", "requestedGroups": [], "full": True},
+        "/retest e2e-failed": {
+            "action": "rerun-failed",
+            "requestedGroups": [],
+            "full": False,
+        },
+    }
+    if body in unbound:
+        return {**unbound[body], "headSHA": "", "nonce": ""}
+    match = re.fullmatch(rf"/test e2e ({groupPattern}(?:,{groupPattern})*)", body)
     if not match:
         raise ValueError("invalid E2E command")
     return {
         "action": "dispatch",
         "requestedGroups": sorted(set(match.group(1).split(","))),
         "full": False,
-        "headSHA": match.group(2),
-        "nonce": match.group(3),
+        "headSHA": "",
+        "nonce": "",
     }
 
 
@@ -151,11 +171,13 @@ def decideDispatch(
     if not re.fullmatch(r"[0-9a-f]{40}", baseSHA or ""):
         return rejectedDecision(command, "pull request base revision is invalid")
     prNumber = pullRequest.get("number")
-    if command["headSHA"] != headSHA:
-        return rejectedDecision(command, "comment is bound to another pull request HEAD")
     expectedNonce = requestNonce(prNumber, headSHA, baseSHA, catalogRevision)
-    if command["nonce"] != expectedNonce:
-        return rejectedDecision(command, "comment binding nonce is stale or invalid")
+    if command.get("headSHA") or command.get("nonce"):
+        if command["headSHA"] != headSHA:
+            return rejectedDecision(command, "comment is bound to another pull request HEAD")
+        if command["nonce"] != expectedNonce:
+            return rejectedDecision(command, "comment binding nonce is stale or invalid")
+    command = {**command, "headSHA": headSHA, "nonce": expectedNonce}
     approvalGeneration = event.get("comment", {}).get("id")
     if not isinstance(approvalGeneration, int) or approvalGeneration <= 0:
         return rejectedDecision(command, "comment identifier is invalid")

hack/test_e2e_control.py

@@ -44,9 +44,25 @@ def testParsesSupportedCommentCommands(self):
                 self.assertEqual(command["headSHA"], headSHA)
                 self.assertEqual(command["nonce"], nonce)
 
+    def testParsesUnboundCommentCommands(self):
+        cases = {
+            "/test e2e": ("dispatch", [], False),
+            "/test e2e core": ("dispatch", ["core"], False),
+            "/test e2e-all": ("dispatch", [], True),
+            "/retest e2e-failed": ("rerun-failed", [], False),
+        }
+        for body, expected in cases.items():
+            with self.subTest(body=body):
+                command = e2eControl.parseCommand(body)
+                self.assertEqual(
+                    (command["action"], command["requestedGroups"], command["full"]),
+                    expected,
+                )
+                self.assertEqual(command["headSHA"], "")
+                self.assertEqual(command["nonce"], "")
+
     def testRejectsMalformedOrInjectedCommands(self):
         for body in [
-            "/test e2e",
             "/test e2e policy bogus",
             "/test e2e policy;echo-owned",
             "/test e2e policy\n/test e2e-all",
@@ -69,14 +85,15 @@ def dispatchDecision(
         state="open",
         baseRef="master",
         controlledLabels=(),
+        bindHead=True,
     ):
         catalog = json.loads((repoRoot / ".github/e2e-selection.json").read_text())
         catalogRevision = e2eSelector.catalogRevision(catalog)
         baseSHA = "b" * 40
         if body is None:
             nonce = e2eControl.requestNonce(7231, observedHeadSHA, baseSHA, catalogRevision)
             body = f"/test e2e policy --head {observedHeadSHA} --nonce {nonce}"
-        elif " --head " not in body:
+        elif bindHead and " --head " not in body:
             nonce = e2eControl.requestNonce(7231, observedHeadSHA, baseSHA, catalogRevision)
             body = f"{body} --head {observedHeadSHA} --nonce {nonce}"
         event = {
@@ -102,6 +119,16 @@ def dispatchDecision(
             controlledLabels=controlledLabels,
         )
 
+    def testUnboundCommandBindsToLiveHead(self):
+        decision = self.dispatchDecision(body="/test e2e core", bindHead=False)
+
+        self.assertTrue(decision["accepted"])
+        self.assertEqual(decision["action"], "dispatch")
+        self.assertEqual(decision["requestedGroups"], ["core"])
+        self.assertEqual(decision["headSHA"], "a" * 40)
+        self.assertEqual(decision["baseSHA"], "b" * 40)
+        self.assertRegex(decision["nonce"], r"^[0-9a-f]{16}$")
+
     def testAuthorizedCommandIsBoundToCurrentHead(self):
         decision = self.dispatchDecision()
 
@@ -842,6 +869,7 @@ def testDispatcherWorkflowUsesOnlyTrustedWritePermissions(self):
         self.assertIn("checks: read", workflow)
         self.assertNotIn("checks: write", workflow)
         self.assertIn("pull-requests: read", workflow)
+        self.assertIn("pull-requests: write", workflow)
         self.assertIn("issues: write", workflow)
         self.assertIn("--catalog trusted-catalog.json", workflow)
         self.assertIn("--live-comment-file live-comment.json", workflow)