feat(external-pr): re-review on change, skip unchanged (head-SHA dedup)

The reviewer was one-shot: external_review_task_exists deduped on
(project, pr_number) only, so an external PR was reviewed exactly once ever —
a contributor pushing a fix never triggered a re-review (the review went
stale). Drive re-review off the PR's head commit instead:

- list_open_prs now returns head_sha (the change signal).
- ingest records the reviewed SHA as an external_pr_head=<sha> marker in the
  review task's quick_context.
- external_review_task_exists is head-SHA aware: same SHA -> skip (unchanged);
  new SHA -> open a fresh review (changed); no task yet -> first review;
  legacy/markerless task or unknown SHA -> skip (never re-review on a guess,
  so existing reviews don't re-fire after deploy).

No migration — reuses quick_context, like the supersede markers.

roboco/services/git.py

@@ -1414,7 +1414,8 @@ class GitService(BaseService):
         The inbound counterpart to the org's outbound PR calls: lists ALL open
         PRs (no ``head=`` filter), so it sees external/fork contributions the org
         did not create. Each record carries ``number``, ``url``, ``title``,
-        ``head_ref``, ``is_fork`` (head repo differs from base repo),
+        ``head_ref``, ``head_sha`` (the head commit — the change signal for
+        re-review), ``is_fork`` (head repo differs from base repo),
         ``user_login`` and ``author_association`` so the caller can classify
         trust. Returns ``[]`` on a missing token, unparseable remote, or any
         GitHub error — it never raises into the poll loop.
@@ -1467,6 +1468,7 @@ class GitService(BaseService):
                     "url": pr.get("html_url") or "",
                     "title": pr.get("title") or "",
                     "head_ref": head.get("ref"),
+                    "head_sha": head.get("sha"),
                     "is_fork": bool(head_full and head_full != base_full),
                     "user_login": (pr.get("user") or {}).get("login"),
                     "author_association": pr.get("author_association"),

roboco/services/task.py

@@ -628,22 +628,39 @@ class TaskService(BaseService):
         return task
 
     async def external_review_task_exists(
-        self, project_id: UUID, pr_number: int
+        self, project_id: UUID, pr_number: int, head_sha: str | None = None
     ) -> bool:
-        """True if a review task already exists for this (project, external PR).
+        """True if this (project, external PR) at this head commit is already reviewed.
 
-        The de-dupe key for inbound external-PR ingestion: one review task per
-        ``(project_id, source='external_pr', pr_number)`` so re-polling an open
-        PR never creates a duplicate.
+        De-dupe key for inbound external-PR ingestion. Re-review is driven by the
+        PR's head commit: a review task records the SHA it covered as an
+        ``external_pr_head=<sha>`` marker in ``quick_context``. So:
+
+        - no review task for this PR yet -> False (ingest the first review);
+        - a task already covers THIS ``head_sha`` -> True (skip — nothing changed);
+        - a legacy/markerless task exists, or ``head_sha`` is unknown -> True
+          (can't prove it changed, so don't re-review / don't spam);
+        - tasks exist but all cover OTHER SHAs -> False (the PR got new commits —
+          open a fresh review for the change).
         """
         result = await self.session.execute(
-            select(TaskTable.id).where(
+            select(TaskTable.quick_context).where(
                 TaskTable.project_id == project_id,
                 TaskTable.source == "external_pr",
                 TaskTable.pr_number == pr_number,
             )
         )
-        return result.first() is not None
+        contexts = result.scalars().all()
+        if not contexts:
+            return False
+        if not head_sha:
+            return True
+        marker = f"external_pr_head={head_sha}"
+        for qc in contexts:
+            text = qc or ""
+            if marker in text or "external_pr_head=" not in text:
+                return True
+        return False
 
     async def ingest_external_pr(
         self,
@@ -656,7 +673,9 @@ class TaskService(BaseService):
         """Create one review task for a newly-seen external PR; ``None`` if it exists.
 
         ``pr`` is a normalized record from ``GitService.list_open_prs`` (number,
-        url, title). De-duped on ``(project_id, source='external_pr', pr_number)``.
+        url, title, head_sha). De-duped per ``(project_id, pr_number, head_sha)``
+        — re-polling an unchanged PR is skipped, but new commits (a new head SHA)
+        open a fresh review (see ``external_review_task_exists``).
         The task is CODE-typed with ``source='external_pr'`` and
         ``confirmed_by_human=False`` — a deliberate gate: no agent fetches, checks
         out, or runs the contributor's code until a human confirms the PR. Caller
@@ -665,7 +684,8 @@ class TaskService(BaseService):
         pr_number = int(pr["number"])
         pr_url = str(pr.get("url") or "")
         pr_title = str(pr.get("title") or "")
-        if await self.external_review_task_exists(project_id, pr_number):
+        head_sha = str(pr.get("head_sha") or "")
+        if await self.external_review_task_exists(project_id, pr_number, head_sha):
             return None
         title = f"Review external PR #{pr_number}: {pr_title}".strip()
         req = TaskCreateRequest(
@@ -693,6 +713,10 @@ class TaskService(BaseService):
         task = await self.create(req)
         task.pr_number = pr_number
         task.pr_url = pr_url
+        # Record the reviewed head commit so a later push (new SHA) re-reviews,
+        # while an unchanged PR is skipped (see external_review_task_exists).
+        if head_sha:
+            task.quick_context = f"external_pr_head={head_sha}"
         await self.session.flush()
         return task
 

tests/unit/services/test_external_pr_ingest.py

@@ -0,0 +1,63 @@
+"""External-PR review dedup — review once per (project, PR, head commit).
+
+``external_review_task_exists`` drives re-review off the PR's head SHA: an
+unchanged PR (same head) is skipped, new commits (a new head SHA) open a fresh
+review, and legacy/unknown-SHA tasks are never re-reviewed (no spam).
+"""
+
+from __future__ import annotations
+
+from unittest.mock import AsyncMock, MagicMock
+from uuid import uuid4
+
+import pytest
+from roboco.services.task import TaskService
+
+
+def _service(quick_contexts: list[str | None]) -> TaskService:
+    """A TaskService whose review-task query returns these quick_context values."""
+    res = MagicMock()
+    res.scalars.return_value.all.return_value = quick_contexts
+    session = MagicMock()
+    session.execute = AsyncMock(return_value=res)
+    return TaskService(session)
+
+
+@pytest.mark.asyncio
+async def test_no_task_yet_ingests() -> None:
+    svc = _service([])
+    assert await svc.external_review_task_exists(uuid4(), 170, "abc") is False
+
+
+@pytest.mark.asyncio
+async def test_same_head_sha_skips() -> None:
+    svc = _service(["external_pr_head=abc"])
+    assert await svc.external_review_task_exists(uuid4(), 170, "abc") is True
+
+
+@pytest.mark.asyncio
+async def test_new_head_sha_rereviews() -> None:
+    # PR got new commits since the last review → open a fresh review.
+    svc = _service(["external_pr_head=abc"])
+    assert await svc.external_review_task_exists(uuid4(), 170, "def") is False
+
+
+@pytest.mark.asyncio
+async def test_legacy_markerless_task_not_rereviewed() -> None:
+    # A task ingested before head-SHA tracking existed → don't re-review it.
+    svc = _service([None])
+    assert await svc.external_review_task_exists(uuid4(), 170, "def") is True
+
+
+@pytest.mark.asyncio
+async def test_unknown_head_sha_does_not_spam() -> None:
+    # Can't detect change (no SHA from GitHub) → treat as reviewed.
+    svc = _service(["external_pr_head=abc"])
+    assert await svc.external_review_task_exists(uuid4(), 170, None) is True
+
+
+@pytest.mark.asyncio
+async def test_multiple_old_shas_still_rereviews_new() -> None:
+    svc = _service(["external_pr_head=abc", "external_pr_head=def"])
+    assert await svc.external_review_task_exists(uuid4(), 170, "ghi") is False
+    assert await svc.external_review_task_exists(uuid4(), 170, "def") is True