From 54a5f6f5c2a9d93b36a9a5aeb4eaf57d02ab5e6b Mon Sep 17 00:00:00 2001
From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Tue, 11 Aug 2026 18:00:27 -0400
Subject: [PATCH] Key the prgate comment marker on the reason alone

The hidden marker the gate leaves on a pull request was
"<!-- prgate:<reason>:<sha> -->", carrying the head sha, so
_already_commented() only suppressed a repeat when the reason *and* the
commit both matched.  The gate runs on opened/reopened, which means a
contributor who is closed, pushes a commit and reopens gets a second
copy of the same rejection.

The sha isn't part of the decision: the gate reads the pull request
title and body and the state of the issue they name, never the code, so
a new head commit cannot change the outcome without also changing the
reason, which is in the marker already.  Drop it.  The pull request
number doesn't need to be in the marker either, since the comments are
fetched per pull request to begin with.

Matching is done against the unterminated "<!-- prgate:<reason>"
prefix so that comments already posted in the old format are still
recognized and don't draw a duplicate on their next reopen.
---
 publishthing/apps/prgate/github.py   | 16 +++++-------
 publishthing/apps/prgate/messages.py | 25 ++++++++++++++----
 tests/test_prgate_hook.py            | 38 ++++++++++++++++++++--------
 3 files changed, 54 insertions(+), 25 deletions(-)

diff --git a/publishthing/apps/prgate/github.py b/publishthing/apps/prgate/github.py
index 6c3210b..f41301b 100644
--- a/publishthing/apps/prgate/github.py
+++ b/publishthing/apps/prgate/github.py
@@ -73,7 +73,6 @@ def gate_pull_request(
         pull_request = event.json_data["pull_request"]
         number = str(event.json_data["number"])
         sender = event.json_data["sender"]["login"]
-        sha = pull_request["head"]["sha"]
 
         gh_repo = thing.github_repo(event.repo_name)
 
@@ -132,23 +131,22 @@ def holds_claim(issue_number: int) -> bool:
         message = messages.close_message(
             result,
             label,
-            sha,
             review_label=review_label,
             deny_label=deny_label,
             policy_url=entry.get("policy_url"),
         )
 
         # github redelivers webhooks, and a reopen re-runs the gate; only
-        # skip the comment when we've already said this exact thing about
-        # this exact commit.  the close below still runs either way, and
+        # skip the comment when we've already said this exact thing on
+        # this pull request.  the close below still runs either way, and
         # closing an already-closed pull request is a no-op.
-        if _already_commented(gh_repo, number, result.reason, sha):
+        if _already_commented(gh_repo, number, result.reason):
             thing.debug(
                 "prgate",
-                "already commented on %s #%s for sha %s, not repeating",
+                "already commented on %s #%s for %s, not repeating",
                 event.repo_name,
                 number,
-                sha,
+                result.reason,
             )
         else:
             # comment before closing: if the close fails we'd rather have
@@ -162,9 +160,9 @@ def holds_claim(issue_number: int) -> bool:
 
 
 def _already_commented(
-    gh_repo: github.GithubRepo, number: str, reason: str, sha: str
+    gh_repo: github.GithubRepo, number: str, reason: str
 ) -> bool:
-    marker = messages.marker(reason, sha)
+    marker = messages.marker_match(reason)
     for comment in gh_repo.get_issue_comments(number):
         if marker in (comment.get("body") or ""):
             return True
diff --git a/publishthing/apps/prgate/messages.py b/publishthing/apps/prgate/messages.py
index 7ad54e7..a7e94c7 100644
--- a/publishthing/apps/prgate/messages.py
+++ b/publishthing/apps/prgate/messages.py
@@ -17,7 +17,11 @@
 from . import util
 
 # hidden marker so the gate can recognize its own previous comment on a
-# pull request and not repeat itself if github redelivers a webhook.
+# pull request and not repeat itself if github redelivers a webhook or
+# the pull request is closed and reopened.  keyed on the reason alone:
+# the gate's decision is a function of the pull request's description and
+# the issue it names, never of the code, so a new head commit can't
+# change the outcome without also changing the reason.
 MARKER_PREFIX = "<!-- prgate"
 
 # hidden marker recording that this pull request is the one that claimed
@@ -27,8 +31,20 @@
 CLAIM_MARKER_PREFIX = "<!-- prgate-claim"
 
 
-def marker(reason: str, sha: str) -> str:
-    return "%s:%s:%s -->" % (MARKER_PREFIX, reason, sha)
+def marker(reason: str) -> str:
+    return "%s:%s -->" % (MARKER_PREFIX, reason)
+
+
+def marker_match(reason: str) -> str:
+    """The substring that recognizes any past comment for ``reason``.
+
+    Deliberately unterminated: the marker used to carry the head sha as
+    well, and matching on the open prefix means the comments already
+    posted with the old format are still recognized.
+
+    """
+
+    return "%s:%s" % (MARKER_PREFIX, reason)
 
 
 def claim_marker(issue: int) -> str:
@@ -132,7 +148,6 @@ def accepted_message(issue: int, label: str, review_label: str) -> str:
 def close_message(
     result: util.GateResult,
     label: str,
-    sha: str,
     review_label: str = util.DEFAULT_REVIEW_LABEL,
     deny_label: str = util.DEFAULT_DENY_LABEL,
     policy_url: Optional[str] = None,
@@ -162,7 +177,7 @@ def close_message(
         steps = _STEPS_NO_ISSUE
 
     paragraphs = [
-        marker(result.reason, sha),
+        marker(result.reason),
         _INTRO,
         _POLICY % subs,
         (steps % subs).strip(),
diff --git a/tests/test_prgate_hook.py b/tests/test_prgate_hook.py
index d58ed25..666218e 100644
--- a/tests/test_prgate_hook.py
+++ b/tests/test_prgate_hook.py
@@ -304,13 +304,27 @@ def test_redelivery_does_not_repeat_the_comment():
     assert gh_repo.closed == ["7", "7"]
 
 
-def test_new_commit_gets_a_fresh_comment():
+def test_new_commit_does_not_get_a_fresh_comment():
+    """A reopen after a push is the same rejection, not a new one.
+
+    The marker used to carry the head sha, so pushing a commit and
+    reopening produced a second identical comment.
+
+    """
+
+    gh_repo = FakeRepo(comments=[messages.marker(util.CLOSE_NO_ISSUE)])
+    run(gh_repo, make_event(body="no reference"))
+
+    assert len(gh_repo.comments) == 1
+
+
+def test_comments_from_the_old_sha_bearing_marker_are_recognized():
     gh_repo = FakeRepo(
-        comments=[messages.marker(util.CLOSE_NO_ISSUE, "different-sha")]
+        comments=["<!-- prgate:%s:some-sha -->" % util.CLOSE_NO_ISSUE]
     )
     run(gh_repo, make_event(body="no reference"))
 
-    assert len(gh_repo.comments) == 2
+    assert len(gh_repo.comments) == 1
 
 
 def test_close_can_be_disabled():
@@ -335,19 +349,21 @@ def test_maintainer_exemption_is_per_repo():
     assert gh_repo.closed == ["7"]
 
 
-def test_marker_distinguishes_reason_and_sha():
-    assert messages.marker("no_issue", "aaa") != messages.marker(
-        "no_issue", "bbb"
-    )
-    assert messages.marker("no_issue", "aaa") != messages.marker(
-        "issue_closed", "aaa"
-    )
+def test_marker_distinguishes_reason():
+    assert messages.marker("no_issue") != messages.marker("issue_closed")
 
 
 def test_already_commented_matches_only_its_own_marker():
     gh_repo = FakeRepo(comments=["a normal human comment mentioning #5"])
     assert not prgate_github._already_commented(
-        gh_repo, "7", util.CLOSE_NO_ISSUE, SHA
+        gh_repo, "7", util.CLOSE_NO_ISSUE
+    )
+
+
+def test_already_commented_does_not_match_a_different_reason():
+    gh_repo = FakeRepo(comments=[messages.marker(util.CLOSE_ISSUE_CLOSED)])
+    assert not prgate_github._already_commented(
+        gh_repo, "7", util.CLOSE_NO_ISSUE
     )