fix(review): resync bot re-entry before skipping

src/queue/processors.ts

@@ -3053,17 +3053,11 @@ export async function reReviewStoredPullRequest(
   ]);
   let pr = await getPullRequest(env, repoFullName, prNumber);
   if (!pr || pr.state !== "open") return;
-  // Waste elimination for known automation authors (settings/automation-bot-skip.ts): every re-entry path
-  // that can reach a PR without a fresh webhook (a scheduled sweep, CI-completion, or linked-issue-change
-  // re-review -- all funnel through this function via regatePullRequest) funnels through here, so this one
-  // check closes all of them. Uses the STORED author (isTrustedAutomationBotAuthor), not a live webhook
-  // sender -- see that function's own doc comment for why that's still safe: authorLogin is immutable,
-  // GitHub-attested metadata already verified against the actor at the original `opened` webhook.
-  if (
-    resolveSkipAutomationBotPullRequests(isSkipAutomationBotPullRequestsEnabledGlobally(env), settings.skipAutomationBotAuthors) &&
-    isTrustedAutomationBotAuthor(pr.authorLogin)
-  )
-    return;
+  const automationBotSkipEnabled = resolveSkipAutomationBotPullRequests(
+    isSkipAutomationBotPullRequestsEnabledGlobally(env),
+    settings.skipAutomationBotAuthors,
+  );
+  const storedHeadShaBeforeResync = pr.headSha;
   const autoreviewPaused = await hasAutoreviewPausedMarker(env, repoFullName, prNumber);
   const liveFacts = createLiveGithubFacts();
   // #sweep-resync: RESYNC the stored PR to its LIVE head before reviewing. The self-host relay can drop the
@@ -3110,6 +3104,15 @@ export async function reReviewStoredPullRequest(
     /* v8 ignore next -- the row was just upserted above, so the re-read always returns it; `?? pr` is belt-and-suspenders fail-open. */
     pr = (await getPullRequest(env, repoFullName, prNumber)) ?? pr;
   }
+  // Actorless re-entry paths may only skip a known bot-authored PR after the live-head resync proves the stored
+  // row was already current. If the live fetch fails, or if the live head drifted (the dropped-synchronize case
+  // this resync exists to recover), fail open into the full review/gate instead of trusting the immutable author.
+  if (
+    automationBotSkipEnabled &&
+    isTrustedAutomationBotAuthor(pr.authorLogin) &&
+    live?.head?.sha === storedHeadShaBeforeResync
+  )
+    return;
   // Operator review flow: rebase-if-behind → wait for ALL CI to finish → only THEN review. Defers (returns) when
   // a rebase fired a synchronize, or CI is still running — the synchronize / CI-completion webhook re-triggers
   // once the head is current and CI has settled (the sweep backstops a missed event). REST-budget dedup

src/settings/automation-bot-skip.ts

@@ -62,12 +62,10 @@ export function isTrustedAutomationBotWebhookActor(
 }
 
 /**
- * Re-entry paths (a scheduled sweep, CI-completion, or linked-issue-change re-review -- all funnel through
- * `reReviewStoredPullRequest`) have no live webhook `sender` to check; they're re-evaluating an ALREADY
- * PERSISTED PR record, not processing a fresh, potentially actor-ambiguous event. The PR's stored author was
- * already verified against `isTrustedAutomationBotWebhookActor` (both `sender` AND author) at the ORIGINAL
- * `opened` webhook that created the row, and `authorLogin` is immutable GitHub-attested metadata (never
- * user-editable) -- re-checking just the stored author here is safe and consistent with that original check.
+ * Re-entry paths have no live webhook `sender`, so stored authorship is only a necessary precondition for the
+ * automation skip. Callers must first perform their own freshness/provenance check (for example, confirming the
+ * live PR head still matches the stored head) before using this result to bypass review. Original PR authorship
+ * alone does not prove that later commits on the branch were still produced by the trusted bot.
  */
 export function isTrustedAutomationBotAuthor(prAuthorLogin: string | null | undefined): boolean {
   return isProtectedAutomationAuthor(prAuthorLogin);

test/unit/automation-bot-skip.test.ts

@@ -50,7 +50,7 @@ describe("resolveSkipAutomationBotPullRequests", () => {
   });
 });
 
-describe("isTrustedAutomationBotAuthor (re-entry paths: stored author only)", () => {
+describe("isTrustedAutomationBotAuthor (re-entry paths: stored author precondition)", () => {
   it("true for every known automation login, case-insensitively", () => {
     expect(isTrustedAutomationBotAuthor("github-actions[bot]")).toBe(true);
     expect(isTrustedAutomationBotAuthor("Renovate[Bot]")).toBe(true);

test/unit/queue-lifecycle-guards.test.ts

@@ -4739,10 +4739,26 @@ describe("automation-bot-skip: end-to-end webhook + re-entry wiring (#automation
   // on EVERY webhook, not the "waste" this feature eliminates. The real signal is that NOTHING beyond that
   // touches the actual GitHub REST API (api.github.com) -- no installation-token fetch, no PR/files read, no
   // comment/check-run publish, no AI provider call.
-  async function fetchCallTracker() {
+  async function fetchCallTracker(livePulls: Record<number, { sha: string; state?: string }> = {}) {
     const state = { urls: [] as string[] };
     vi.stubGlobal("fetch", async (input: RequestInfo | URL) => {
-      state.urls.push(input.toString());
+      const url = input.toString();
+      state.urls.push(url);
+      const match = /\/repos\/owner\/bot-skip-repo\/pulls\/(\d+)$/.exec(url);
+      if (match) {
+        const live = livePulls[Number(match[1])];
+        if (live) {
+          return Response.json({
+            number: Number(match[1]),
+            title: "chore(deps): bump baz",
+            state: live.state ?? "open",
+            user: { login: "renovate[bot]", type: "Bot" },
+            head: { sha: live.sha },
+            labels: [],
+            body: "",
+          });
+        }
+      }
       return new Response("not found", { status: 404 });
     });
     return state;
@@ -4835,20 +4851,34 @@ describe("automation-bot-skip: end-to-end webhook + re-entry wiring (#automation
     expect(skipAudit?.n).toBe(1);
   });
 
-  it("the re-entry sweep path (agent-regate-pr) also respects the skip for a stored bot author, without even the live resync fetch", async () => {
+  it("the re-entry sweep path (agent-regate-pr) only skips a stored bot author after the live resync proves the head is unchanged", async () => {
     const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() });
     await upsertInstallation(env, { action: "created", installation: { id: 9101, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: {}, events: [] } });
     await upsertRepositoryFromGitHub(env, { name: "bot-skip-repo", full_name: "owner/bot-skip-repo", private: false, owner: { login: "owner" } }, 9101);
     await upsertPullRequestFromGitHub(env, "owner/bot-skip-repo", { number: 405, title: "chore(deps): bump baz", state: "open", user: { login: "renovate[bot]", type: "Bot" }, head: { sha: "sha405" }, labels: [], body: "" });
-    const calls = await fetchCallTracker();
+    const calls = await fetchCallTracker({ 405: { sha: "sha405" } });
 
     await processJob(env, { type: "agent-regate-pr", deliveryId: "bot-skip-sweep", repoFullName: "owner/bot-skip-repo", prNumber: 405, installationId: 9101 });
 
-    // The re-entry check runs BEFORE even the live-head resync GET, so a genuine bot author skips without any
-    // GitHub REST API call at all -- not merely without a comment/check-run publish.
-    expect(calls.urls.some((url) => url.includes("api.github.com"))).toBe(false);
+    // The actorless re-entry path must pay for the live-head resync before trusting a stored bot author; once the
+    // live head matches the stored head, it can still skip the later review/comment/check-run work.
+    expect(calls.urls.some((url) => url.endsWith("/repos/owner/bot-skip-repo/pulls/405"))).toBe(true);
     const stored = await getPullRequest(env, "owner/bot-skip-repo", 405);
     expect(stored?.headSha).toBe("sha405");
   });
+
+  it("SECURITY: actorless re-entry does not skip a stored bot author when the live head drifted", async () => {
+    const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() });
+    await upsertInstallation(env, { action: "created", installation: { id: 9101, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: {}, events: [] } });
+    await upsertRepositoryFromGitHub(env, { name: "bot-skip-repo", full_name: "owner/bot-skip-repo", private: false, owner: { login: "owner" } }, 9101);
+    await upsertPullRequestFromGitHub(env, "owner/bot-skip-repo", { number: 406, title: "chore(deps): bump baz", state: "open", user: { login: "renovate[bot]", type: "Bot" }, head: { sha: "bot-recorded-sha" }, labels: [], body: "" });
+    const calls = await fetchCallTracker({ 406: { sha: "human-pushed-sha" } });
+
+    await processJob(env, { type: "agent-regate-pr", deliveryId: "bot-skip-sweep-drift", repoFullName: "owner/bot-skip-repo", prNumber: 406, installationId: 9101 });
+
+    expect(calls.urls.some((url) => url.endsWith("/repos/owner/bot-skip-repo/pulls/406"))).toBe(true);
+    const stored = await getPullRequest(env, "owner/bot-skip-repo", 406);
+    expect(stored?.headSha).toBe("human-pushed-sha");
+  });
 });