fix(ci): post new Discord notice for each PR build (#2314)

Post a fresh signed test-build notice for each new PR commit, retire the
previous notice, and include the latest commit subject.

tools/release/pr_discord.py

@@ -101,23 +101,31 @@ def description_excerpt(body):
     return ""
 
 
-def build_content(repo, number, title, sha, apk, release, description=None):
+def commit_subject(message):
+    subject = (message or "").splitlines()[0].strip() if message else ""
+    return plain_title(subject[:300])
+
+
+def build_content(repo, number, title, sha, apk, release, description=None, commit_message=None):
     excerpt = description_excerpt(description)
+    subject = commit_subject(commit_message)
     return (f"**Signed test build** `pr-{number}` · `{sha[:7]}`\n"
             f"[#{number}](https://github.com/{repo}/pull/{number}): {plain_title(title)}\n"
+            + (f"Latest commit: {subject}\n" if subject else "")
             + (f"\n{excerpt}\n\n" if excerpt else "") +
             "_Not merged — for testing._\n"
             f"In the app: **Updates → Test builds → #{number}**\n"
             f"Download: [standard APK](<{apk}>) · [all variants](<{release}>)")
 
 
-def retired_content(content, merged):
-    label = "Merged" if merged else "Closed"
-    if content.startswith(("**Merged** —", "**Closed** —")):
+def retired_content(content, merged=False, superseded_by=None):
+    label = f"Superseded by `{superseded_by[:7]}`" if superseded_by else ("Merged" if merged else "Closed")
+    if content.startswith(("**Merged** —", "**Closed** —", "**Superseded by `")):
         content = "\n".join(line[2:-2] if line.startswith("~~") and line.endswith("~~") else line
                             for line in content.splitlines()[2:])
     struck = "\n".join(f"~~{line}~~" if line else "" for line in content.splitlines())
-    return f"**{label}** — this test build is no longer available.\n\n{struck}"
+    note = "This test build has been replaced." if superseded_by else "This test build is no longer available."
+    return f"**{label}** — {note}\n\n{struck}"
 
 
 def sync(mode, repo, number, discord, sha="", apk="", release=""):
@@ -136,6 +144,11 @@ def sync(mode, repo, number, discord, sha="", apk="", release=""):
             return
         raise RuntimeError("Signed build report comment is missing")
     message_id = saved_message(comment["body"], discord.id)
+    state_match = STATE.search(comment["body"])
+    try:
+        old_state = json.loads(state_match.group(1)) if state_match else {}
+    except ValueError:
+        old_state = {}
     if mode == "retire":
         if message_id:
             message = discord.request("GET", f"/messages/{message_id}")
@@ -144,16 +157,41 @@ def sync(mode, repo, number, discord, sha="", apk="", release=""):
                                 {"content": retired_content(message["content"], pull.get("merged", False)),
                                  "allowed_mentions": {"parse": []}, "flags": 4})
         return
-    content = build_content(repo, number, pull["title"], sha, apk, release, pull.get("body"))
+    try:
+        commit = github(f"repos/{repo}/commits/{sha}") or {}
+    except RuntimeError:
+        commit = {}
+    commit_message = (commit.get("commit") or {}).get("message", "")
+    content = build_content(repo, number, pull["title"], sha, apk, release, pull.get("body"), commit_message)
     if len(content) > 1800:
         raise RuntimeError("Discord announcement is too long")
     payload = {"content": content, "allowed_mentions": {"parse": []}, "flags": 4}
-    message = discord.request("PATCH", f"/messages/{message_id}", payload) if message_id else None
+    old_message = None
+    previous_sha = old_state.get("sha")
+    if message_id and not previous_sha:
+        old_message = discord.request("GET", f"/messages/{message_id}")
+        if old_message:
+            match = re.search(r"`([0-9a-f]{7})`", old_message.get("content", ""))
+            previous_sha = match.group(1) if match else None
+    if message_id and (previous_sha == sha or previous_sha == sha[:7]):
+        message = discord.request("PATCH", f"/messages/{message_id}", payload)
+    else:
+        if message_id and old_state.get("sha"):
+            previous = discord.request("GET", f"/messages/{message_id}")
+            if previous:
+                discord.request("PATCH", f"/messages/{message_id}",
+                                {"content": retired_content(previous["content"], superseded_by=sha),
+                                 "allowed_mentions": {"parse": []}, "flags": 4})
+        elif message_id and old_message:
+            discord.request("PATCH", f"/messages/{message_id}",
+                            {"content": retired_content(old_message["content"], superseded_by=sha),
+                             "allowed_mentions": {"parse": []}, "flags": 4})
+        message = None
     if message is None:
         message = discord.request("POST", "?wait=true", payload)
         if message is None:
             raise RuntimeError("Discord webhook no longer exists")
-    state = {"webhook_id": discord.id, "message_id": message["id"]}
+    state = {"webhook_id": discord.id, "message_id": message["id"], "sha": sha}
     latest = github(f"repos/{repo}/issues/comments/{comment['id']}")
     body = STATE.sub("", latest["body"]).rstrip() + f"\n<!-- droiddeck-discord: {json.dumps(state)} -->"
     github(f"repos/{repo}/issues/comments/{comment['id']}", "PATCH", {"body": body})

tools/tests/test_pr_discord.py

@@ -14,17 +14,30 @@ class PrDiscordTest(unittest.TestCase):
         self.discord.request.return_value = {"id": "1557547293375332465"}
         self.github = patch.object(module, "github").start()
         self.addCleanup(patch.stopall)
-        self.github.side_effect = lambda path, method="GET", body=None: self.pull if "/pulls/" in path else self.comment
+        def github_request(path, method="GET", body=None):
+            if "/pulls/" in path:
+                return self.pull
+            if "/commits/" in path:
+                return {"commit": {"message": "Fix reconnect behavior\n\nMore details"}}
+            if path.endswith("/issues/comments/10"):
+                if method == "PATCH":
+                    self.comment["body"] = body["body"]
+                return self.comment
+            return self.comment
+        self.github.side_effect = github_request
         patch.object(module, "report_comment", return_value=self.comment).start()
 
-    def publish(self):
-        module.sync("publish", "Droid-Deck/DroidDeck", "410", self.discord, "a" * 40,
+    def publish(self, sha=None):
+        if sha is not None:
+            self.pull["head"]["sha"] = sha
+        sha = sha or "a" * 40
+        module.sync("publish", "Droid-Deck/DroidDeck", "410", self.discord, sha,
                     "https://github.com/Droid-Deck/DroidDeck-CI/releases/download/pr-410/build.apk",
                     "https://github.com/Droid-Deck/DroidDeck-CI/releases/tag/pr-410")
 
     def save_state(self):
         self.comment["body"] += '\n<!-- droiddeck-discord: ' + json.dumps({
-            "webhook_id": self.discord.id, "message_id": "1557547293375332465"}) + ' -->'
+            "webhook_id": self.discord.id, "message_id": "1557547293375332465", "sha": "a" * 40}) + ' -->'
 
     def test_create_confirmed_message_and_persist_reference(self):
         self.publish()
@@ -36,12 +49,76 @@ class PrDiscordTest(unittest.TestCase):
         self.assertEqual("1557547293375332465", module.saved_message(saved, self.discord.id))
         self.assertIn("Build download", saved)
 
-    def test_new_build_updates_existing_message(self):
+    def test_same_build_updates_existing_message_without_duplicate(self):
         self.save_state()
         self.publish()
         self.assertEqual("PATCH", self.discord.request.call_args.args[0])
         saved = self.github.call_args.args[2]["body"]
         self.assertEqual(1, len(module.STATE.findall(saved)))
+        self.assertIn('"sha": "' + "a" * 40 + '"', saved)
+
+    def test_new_commit_retires_old_message_and_posts_replacement(self):
+        self.save_state()
+        old_content = module.build_content("Droid-Deck/DroidDeck", "410", self.pull["title"], "a" * 40,
+                                           "apk", "release")
+        self.discord.request.side_effect = [
+            {"content": old_content}, {"id": "1557547293375332465"}, {"id": "1557547293375332466"}]
+        self.publish("b" * 40)
+        calls = self.discord.request.call_args_list
+        self.assertEqual(["GET", "PATCH", "POST"], [call.args[0] for call in calls])
+        self.assertIn("**Superseded by `bbbbbbb`**", calls[1].args[2]["content"])
+        self.assertIn("~~Download:", calls[1].args[2]["content"])
+        self.assertIn("Latest commit: Fix reconnect behavior", calls[2].args[2]["content"])
+        saved = self.github.call_args.args[2]["body"]
+        self.assertEqual("1557547293375332466", module.saved_message(saved, self.discord.id))
+        self.assertIn('"sha": "' + "b" * 40 + '"', saved)
+
+    def test_publish_supersede_rerun_and_close_lifecycle(self):
+        messages = {}
+        created = []
+        next_id = 1
+
+        def discord_request(method, suffix="", body=None):
+            nonlocal next_id
+            if method == "POST":
+                message_id = str(1557547293375332465 + next_id)
+                next_id += 1
+                messages[message_id] = {"id": message_id, "content": body["content"]}
+                created.append(message_id)
+                return messages[message_id]
+            message_id = suffix.split("/")[-1]
+            if method == "GET":
+                return messages.get(message_id)
+            if method == "PATCH":
+                if message_id not in messages:
+                    return None
+                messages[message_id].update(body)
+                return messages[message_id]
+            self.fail(f"Unexpected Discord method: {method}")
+
+        self.discord.request.side_effect = discord_request
+        self.publish("a" * 40)
+        first_id = created[-1]
+        self.publish("b" * 40)
+        second_id = created[-1]
+        self.publish("b" * 40)
+        self.assertEqual(2, len(created))
+        self.assertIn("**Superseded by `bbbbbbb`**", messages[first_id]["content"])
+        self.assertIn("`bbbbbbb`", messages[second_id]["content"])
+
+        self.pull.update(state="closed", merged=True)
+        module.sync("retire", "Droid-Deck/DroidDeck", "410", self.discord)
+        self.assertTrue(messages[second_id]["content"].startswith("**Merged**"))
+        self.assertIn("~~Latest commit:", messages[second_id]["content"])
+
+    def test_legacy_state_uses_embedded_short_sha_to_supersede(self):
+        self.save_state()
+        self.comment["body"] = self.comment["body"].replace(', "sha": "' + "a" * 40 + '"', '')
+        old_content = "**Signed test build** `pr-410` · `aaaaaaa`\nold build"
+        self.discord.request.side_effect = [
+            {"content": old_content}, {"id": "1557547293375332465"}, {"id": "1557547293375332466"}]
+        self.publish("b" * 40)
+        self.assertEqual(["GET", "PATCH", "POST"], [call.args[0] for call in self.discord.request.call_args_list])
 
     def test_deleted_message_is_recreated(self):
         self.save_state()
@@ -78,6 +155,10 @@ class PrDiscordTest(unittest.TestCase):
         module.sync("retire", "Droid-Deck/DroidDeck", "410", self.discord)
         self.discord.request.assert_not_called()
 
+    def test_superseded_content_is_idempotent(self):
+        once = module.retired_content("first\n\nlast", superseded_by="a" * 40)
+        self.assertEqual(once, module.retired_content(once, superseded_by="a" * 40))
+
     def test_old_build_without_saved_id_is_not_guessed(self):
         self.pull["state"] = "closed"
         module.sync("retire", "Droid-Deck/DroidDeck", "410", self.discord)
@@ -153,6 +234,29 @@ class PrDiscordTest(unittest.TestCase):
         self.assertLessEqual(len(content), 1800)
         self.assertLessEqual(len(module.retired_content(content, True)), 2000)
 
+    def test_commit_subject_is_sanitized_and_uses_only_first_line(self):
+        content = module.build_content("Droid-Deck/DroidDeck", "410", "title", "a" * 40,
+                                       "apk", "release", commit_message="Fix *input* @everyone\nDetails")
+        self.assertIn(r"Latest commit: Fix \*input\* @everyone", content)
+        self.assertNotIn("Details", content)
+
+    def test_commit_lookup_failure_does_not_block_announcement(self):
+        def unavailable_commit(path, method="GET", body=None):
+            if "/commits/" in path:
+                raise RuntimeError("GitHub request failed")
+            return self.pull if "/pulls/" in path else self.comment
+        self.github.side_effect = unavailable_commit
+        self.publish()
+        self.assertEqual("POST", self.discord.request.call_args.args[0])
+        self.assertNotIn("Latest commit:", self.discord.request.call_args.args[2]["content"])
+
+    def test_empty_commit_response_does_not_block_announcement(self):
+        self.github.side_effect = lambda path, method="GET", body=None: (
+            self.pull if "/pulls/" in path else None if "/commits/" in path else self.comment)
+        self.publish()
+        self.assertEqual("POST", self.discord.request.call_args.args[0])
+        self.assertNotIn("Latest commit:", self.discord.request.call_args.args[2]["content"])
+
 
 original_report = module.report_comment