fix(review): stop paging Sentry on an expected update_branch merge conflict

src/services/agent-action-executor.ts

@@ -15,7 +15,7 @@ import {
   upsertGlobalContributorBlacklist,
 } from "../db/repositories";
 import { isAuthorBlacklisted } from "../settings/contributor-blacklist";
-import { classifyMergeFailure, MERGE_RETRY_CAP } from "./merge-failure";
+import { classifyMergeFailure, isMergeConflictMessage, MERGE_RETRY_CAP } from "./merge-failure";
 import { notifyActionToDiscord, notifyActionToSlack, type NotifyOutcome } from "./notify-discord";
 import { resolveDispositionReason } from "../review/outcomes-wire";
 import { cancelInFlightWorkflowRunsForHeadSha, createInstallationToken, githubErrorStatus, isGitHubRateLimitedError } from "../github/app";
@@ -618,6 +618,14 @@ export async function executeAgentMaintenanceActions(env: Env, ctx: AgentActionE
       // after the gate publishes. A possibly-transient failure is retried up to MERGE_RETRY_CAP, then held.
       if (action.actionClass === "merge" && ctx.headSha) {
         await handleMergeFailure(env, ctx, error);
+      } else if (action.actionClass === "update_branch" && isMergeConflictMessage(errorMessage(error))) {
+        // LOOPOVER-24: update_branch performs a real merge internally, so it fails with the same "merge
+        // conflict" shape a MERGE action does -- but unlike a merge's terminal hold (a PR permanently blocked
+        // until a human intervenes), this is NOT a stuck state: forceUpdateBranch's caller (prReadyForReview)
+        // already falls through to reviewing the PR on its current, non-rebased head when this returns false
+        // (see forceUpdateBranch's own doc comment), exactly like every other "couldn't rebase, review anyway"
+        // path. The branch owner, not the bot, needs to resolve the conflict -- paging on every naturally-
+        // diverged PR this happens to hit isn't warranted. Still recorded by the audit() call above.
       } else {
         // Non-merge action classes have no retry loop -- a single failure here is already this pass's terminal
         // outcome (the planner may re-attempt on the next sweep if the underlying condition clears itself), so

src/services/merge-failure.ts

@@ -22,8 +22,11 @@ import { errorMessage } from "../utils/json";
 // MERGE_RETRY_CAP before escalating to the same terminal hold.
 export const MERGE_RETRY_CAP = 5;
 
-/** True when the merge error TEXT describes a real content conflict (vs a behind-but-clean branch). */
-function isMergeConflictMessage(message: string): boolean {
+/** True when the merge error TEXT describes a real content conflict (vs a behind-but-clean branch). Exported
+ *  for reuse by the update_branch action class (LOOPOVER-24): update-branch performs a real merge internally,
+ *  so it fails with this SAME message shape, and the classification is identical -- the branch owner, not the
+ *  bot, must resolve it. */
+export function isMergeConflictMessage(message: string): boolean {
   return /merge conflict|not mergeable|cannot be merged|has conflicts|conflicts? with the base/i.test(message);
 }
 

test/unit/agent-action-executor.test.ts

@@ -1456,6 +1456,33 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => {
     expect(refreshInstallationHealthForInstallation).not.toHaveBeenCalled();
   });
 
+  it("REGRESSION (LOOPOVER-24): a merge-conflict update_branch failure does not page Sentry", async () => {
+    const env = createTestEnv({});
+    vi.mocked(updatePullRequestBranch).mockRejectedValueOnce(new Error("merge conflict between base and head"));
+    const captureSpy = vi.spyOn(sentryModule, "captureError");
+
+    const outcomes = await executeAgentMaintenanceActions(env, ctx(), [updateBranch]);
+
+    expect(outcomes[0]).toMatchObject({ actionClass: "update_branch", outcome: "error" });
+    expect((await auditFor(env, "update_branch"))?.outcome).toBe("error");
+    // The caller (prReadyForReview/forceUpdateBranch) already falls through to reviewing the PR on its
+    // current head when update_branch fails -- this is not a stuck state the way a merge's terminal hold
+    // is, so it must stay out of Sentry entirely rather than paging on every naturally-diverged PR.
+    expect(captureSpy).not.toHaveBeenCalled();
+    captureSpy.mockRestore();
+  });
+
+  it("a non-conflict update_branch failure still pages Sentry (#agent_action_execution_failed unchanged)", async () => {
+    const env = createTestEnv({});
+    vi.mocked(updatePullRequestBranch).mockRejectedValueOnce(new Error("network timeout"));
+    const captureSpy = vi.spyOn(sentryModule, "captureError");
+
+    await executeAgentMaintenanceActions(env, ctx(), [updateBranch]);
+
+    expect(captureSpy).toHaveBeenCalledWith(expect.any(Error), expect.objectContaining({ kind: "agent_action_execution_failed", actionClass: "update_branch" }), "agent_action_execution_failed");
+    captureSpy.mockRestore();
+  });
+
   it("debounces permission-looking installation health refreshes per installation (#2265)", async () => {
     const env = createTestEnv({});
     vi.useFakeTimers();