fix(agent-actions): add ctx.headSha fallback + rationale to update_branch

src/services/agent-action-executor.ts

@@ -265,9 +265,23 @@ async function performAction(env: Env, ctx: AgentActionExecutionContext, action:
       if (action.closeComment) await createIssueComment(env, ctx.installationId, ctx.repoFullName, ctx.pullNumber, action.closeComment);
       await closePullRequest(env, ctx.installationId, ctx.repoFullName, ctx.pullNumber);
       return;
-    case "update_branch":
-      await updatePullRequestBranch(env, ctx.installationId, ctx.repoFullName, ctx.pullNumber, action.expectedHeadSha);
+    case "update_branch": {
+      // update_branch does NOT need the accept-flow-level "unpinned → deny" gate that #2377/#2422 added for
+      // approve/merge: it only merges the current BASE into the head (never contributor-controlled content), so
+      // it cannot itself ratify unreviewed code the way an approval or a merge does -- the worst case is a
+      // premature rebase that fires a fresh synchronize and gets re-reviewed on the next pass (#2424). It's also
+      // already covered by the generic guards that run before ANY action class reaches this switch: step 5's
+      // freshness check (`expectedHeadSha ?? ctx.headSha`) denies on a moved head, and the approval-queue
+      // accept-flow's supersede check (agent-approval-queue.ts) is actionClass-agnostic. The `?? ctx.headSha`
+      // fallback below is pure parity/defense-in-depth for the tiny window between that freshness read and this
+      // call, matching the same pattern used by approve/merge immediately above.
+      const updateSha = action.expectedHeadSha ?? ctx.headSha;
+      /* v8 ignore next -- the step-5 freshness guard above already denies the action when
+       * action.expectedHeadSha ?? ctx.headSha is falsy, so updateSha (the same expression) is always a
+       * truthy string here; the ?? undefined only satisfies updatePullRequestBranch's string|undefined type. */
+      await updatePullRequestBranch(env, ctx.installationId, ctx.repoFullName, ctx.pullNumber, updateSha ?? undefined);
       return;
+    }
   }
 }
 

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

@@ -103,6 +103,18 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => {
     expect((await auditFor(env, "merge"))?.outcome).toBe("completed");
   });
 
+  it("REGRESSION (#2424): LIVE update_branch falls back to ctx.headSha when the action carries no expectedHeadSha of its own", async () => {
+    // The `updateBranch` fixture above is pre-pinned (expectedHeadSha: "sha7"), so the big LIVE test never
+    // exercises the `?? ctx.headSha` fallback -- it's parity with approve/merge for the tiny window between
+    // step 5's freshness read and this call, matching a live sweep's construction (processors.ts:2196-2202
+    // always sets expectedHeadSha, but the fallback exists for any future/legacy caller that omits it).
+    const env = createTestEnv({});
+    const unpinnedUpdateBranch: PlannedAgentAction = { actionClass: "update_branch", requiresApproval: false, reason: "behind base" };
+    const outcomes = await executeAgentMaintenanceActions(env, ctx({ headSha: "sha7" }), [unpinnedUpdateBranch]);
+    expect(outcomes[0]?.outcome).toBe("completed");
+    expect(updatePullRequestBranch).toHaveBeenCalledWith(env, 123, "owner/repo", 7, "sha7");
+  });
+
   it("LIVE approve with dismissStaleApproval retracts the stale review instead of posting a new one (#2254)", async () => {
     const env = createTestEnv({});
     const dismiss: PlannedAgentAction = { actionClass: "approve", requiresApproval: false, reason: "stale approval retracted", dismissStaleApproval: true };