fix(webhook): catch a base retarget with no new commit, tighten event health, and stop label/issue churn from going stale

src/github/backfill.ts

@@ -885,9 +885,46 @@ export const OPTIONAL_CONTENTS_WRITE_PERMISSION: Record<string, string> = {
   contents: "write",
 };
 
-export const REQUIRED_INSTALLATION_EVENTS = ["issues", "issue_comment", "pull_request", "repository"] as const;
+/**
+ * Events without which LoopOver is functionally broken, so a missing one keeps the installation at
+ * `needs_attention` with named remediation.
+ *
+ * #9058 promoted three. `check_run` and `check_suite` are how CI settlement reaches ORB at all —
+ * maybeReReviewOnCiCompletion is documented in its own header as THE auto-merge / close-on-red trigger — so an
+ * installation missing them reported "healthy" while silently degrading from event-driven to sweep-only, which
+ * is exactly the "the gate hung" shape an operator cannot diagnose from a green health page. `pull_request_review`
+ * is the same story for the human-approval signal the merge gate reads.
+ */
+export const REQUIRED_INSTALLATION_EVENTS = [
+  "issues",
+  "issue_comment",
+  "pull_request",
+  "pull_request_review",
+  "repository",
+  "check_run",
+  "check_suite",
+] as const;
+
+/**
+ * Events LoopOver uses but can do without — a missing one degrades a specific path rather than the product, so
+ * these are DIAGNOSED (surfaced in health) without holding the installation at needs_attention (#9058).
+ * `status` in particular matters more than its optional standing suggests: codecov/patch is a commit status,
+ * and it is the gate's hardest required check.
+ */
+export const DIAGNOSED_INSTALLATION_EVENTS = ["pull_request_review_thread", "status", "workflow_run", "deployment_status", "push"] as const;
+
 export const OPTIONAL_VISIBLE_INSTALLATION_EVENTS = ["installation_target", "installation_repositories"] as const;
 
+/**
+ * The complete event set a new App should subscribe to: everything required, plus everything diagnosed.
+ *
+ * #9058 — the setup wizard used to hand-maintain its own `default_events` list, and it had drifted: it omitted
+ * `issue_comment` and `repository`, both of which the health check calls REQUIRED. A wizard-created self-host
+ * therefore started with every `@loopover …` command dead and no rename handling, from a manifest the product's
+ * own health page would immediately mark unhealthy. Deriving one from the other makes that drift impossible.
+ */
+export const RECOMMENDED_APP_EVENTS: readonly string[] = [...REQUIRED_INSTALLATION_EVENTS, ...DIAGNOSED_INSTALLATION_EVENTS];
+
 type InstallationModeImpact = {
   mode: "comment" | "label" | "check_run" | "gate_check" | "agent_pr_action" | "agent_merge";
   enabled: boolean;

src/github/pr-freshness.ts

@@ -10,6 +10,9 @@ type PullRequestFreshnessOptions = {
   requireDraft?: boolean;
   unavailableSource?: PullRequestUnavailableSource;
   unavailableDetail?: string;
+  // #9055: the base the caller last computed the diff/review/CI against. Present only when the caller actually
+  // tracked one (older stored PRs predate the column); absent preserves every existing caller's behavior exactly.
+  expectedBaseRef?: string | null | undefined;
 };
 
 export type PullRequestFreshness =
@@ -24,7 +27,13 @@ export type PullRequestFreshness =
     }
   | {
       status: "stale";
-      reason: "unavailable" | "closed" | "head_unresolved" | "head_changed" | "no_longer_draft";
+      // #9055: `base_changed` — a contributor can retarget a PR's base AFTER CI is green with no new commit,
+      // so head/state/draft alone see nothing wrong. Everything downstream (diff, review, CI, guardrail path
+      // matching, migration-collision detection) was computed against the ABANDONED base, and the divergence
+      // was permanent for that head: nothing re-syncs on a base change alone. This is checked at the last
+      // possible moment, immediately before a merge/approve mutation, using the SAME live fetch that already
+      // proves the head — no extra GitHub call.
+      reason: "unavailable" | "closed" | "head_unresolved" | "head_changed" | "no_longer_draft" | "base_changed";
       expectedHeadSha: string | null;
       liveHeadSha: string | null;
       liveState: string | null;
@@ -45,7 +54,7 @@ export function reviewedPullRequestHeadSha(
 }
 
 export function classifyPullRequestFreshness(
-  live: Pick<GitHubPullRequestPayload, "state" | "head" | "draft" | "labels"> | null | undefined,
+  live: Pick<GitHubPullRequestPayload, "state" | "head" | "base" | "draft" | "labels"> | null | undefined,
   expectedHeadSha: string | null | undefined,
   options?: PullRequestFreshnessOptions,
 ): PullRequestFreshness {
@@ -83,6 +92,12 @@ export function classifyPullRequestFreshness(
   if (expected && liveHeadSha !== expected) {
     return { status: "stale", reason: "head_changed", expectedHeadSha: expected, liveHeadSha, liveState };
   }
+  // #9055: the base can change with the head UNCHANGED, which is exactly the case the check above cannot see.
+  // A repo whose per-repo settings pin a specific expected base (options?.expectedBaseRef) denies the mutation
+  // rather than merging into a base the diff/review/CI were never computed against.
+  if (options?.expectedBaseRef && live.base?.ref && live.base.ref !== options.expectedBaseRef) {
+    return { status: "stale", reason: "base_changed", expectedHeadSha: expected, liveHeadSha, liveState };
+  }
   // The draft-dodge close is only justified while the PR is STILL a draft -- a same-head, still-open PR
   // that was converted back to ready_for_review before the close fires has cleared its own justification
   // (#2130 follow-up: head/state alone can't see this transition).
@@ -103,10 +118,14 @@ export async function fetchPullRequestFreshness(
     // Require the LIVE PR to still be a draft (the draft-dodge close's own justification). Absent/false
     // preserves every other caller's existing head/state-only behavior exactly.
     requireDraft?: boolean;
+    // #9055: see PullRequestFreshnessOptions' own doc comment.
+    expectedBaseRef?: string | null | undefined;
   },
 ): Promise<PullRequestFreshness> {
-  const options: PullRequestFreshnessOptions =
-    args.requireDraft !== undefined ? { requireDraft: args.requireDraft } : {};
+  const options: PullRequestFreshnessOptions = {
+    ...(args.requireDraft !== undefined ? { requireDraft: args.requireDraft } : {}),
+    ...(args.expectedBaseRef ? { expectedBaseRef: args.expectedBaseRef } : {}),
+  };
   let tokenError: unknown;
   const installationToken = await createInstallationToken(env, args.installationId).catch((error) => {
     tokenError = error;
@@ -148,5 +167,6 @@ export function pullRequestFreshnessDetail(result: PullRequestFreshness): string
   if (result.reason === "closed") return `PR is no longer open (live state: ${result.liveState ?? "unknown"})`;
   if (result.reason === "head_unresolved") return "live PR head SHA could not be verified";
   if (result.reason === "no_longer_draft") return "PR is no longer a draft";
+  if (result.reason === "base_changed") return "PR base branch changed since the diff/review/CI it will merge with were computed";
   return `PR head changed from ${result.expectedHeadSha ?? "unknown"} to ${result.liveHeadSha ?? "unknown"}`;
 }

src/queue/processors.ts

@@ -698,6 +698,13 @@ const PR_PUBLIC_SURFACE_ACTIONS = new Set([
   "synchronize",
   "ready_for_review",
   "edited",
+  // #9059: a maintainer adding or removing a disposition label IS a disposition input -- the manual-review hold
+  // is read straight off the PR's labels. Without these, adding the label did a row re-sync and nothing else,
+  // so the hold only took effect on the next ~2-minute sweep, and REMOVING it to unblock a PR had the same lag
+  // in the other direction. A sweep that is itself skipped under REST-budget backpressure makes that lag
+  // unbounded, which is how manually unblocking a PR ends up looking like the gate ignoring you.
+  "labeled",
+  "unlabeled",
 ]);
 const PR_GATE_CLOSED_ACTIONS = new Set(["closed"]);
 // #4818 follow-up: the three review-family event names `shouldProcessPullRequestPublicSurface` (below) also
@@ -2883,6 +2890,9 @@ async function maybeCloseForContributorCapOnOpen(
         repoFullName,
         pullNumber: pr.number,
         headSha: pr.headSha,
+        // #9055: threaded so the executor's live pre-merge check denies a merge/approve into a base the diff,
+        // review, and CI it is acting on were never computed against.
+        expectedBaseRef: pr.baseRef,
         autonomy: settings.autonomy,
         agentPaused: settings.agentPaused,
         agentDryRun: settings.agentDryRun,
@@ -3604,6 +3614,7 @@ async function runAgentMaintenancePlanAndExecute(
       repoFullName,
       pullNumber: pr.number,
       headSha: pr.headSha,
+      expectedBaseRef: pr.baseRef,
       autonomy: settings.autonomy,
       agentPaused: settings.agentPaused,
       agentDryRun: settings.agentDryRun,
@@ -4984,6 +4995,19 @@ async function maybeReReviewOnLinkedIssueChange(
   const installationId = getInstallationId(payload);
   const issueNumber = payload.issue?.number;
   if (!repoFullName || !installationId || !issueNumber) return false;
+  // #9059: persist the issue row BEFORE the wake fan-out. This function returns `true` unconditionally once
+  // repo + installation + issue are present, and the caller short-circuits on that -- so handleIssueWebhookEvent
+  // never ran, and `issues.labels_json` plus assignees only advanced on opened/edited/closed/reopened or when
+  // the (up to 6-hourly) backfill reached the repo. Relabelling is the single most common issue mutation, so
+  // the row most likely to be read was the row most likely to be stale.
+  //
+  // Blast radius was bounded -- the linked-issue HARD rules fetch live and uncached -- but
+  // resolveLinkedIssueAuthorLogins is cache-first and only falls back live on a MISS, not on staleness, and the
+  // issue-side advisories, slop triage, enrichment and the MCP/API issue surfaces all read these rows directly.
+  //
+  // Best-effort: this is a bookkeeping write, and failing it must not cost the PRs their wake, which is the
+  // part that actually changes a disposition.
+  if (payload.issue) await upsertIssueFromGitHub(env, repoFullName, payload.issue).catch(() => undefined);
   // #5385: mirrors sweepRepoRegate's own gate exactly -- a repo with acting autonomy configured but NOT in the
   // LOOPOVER_REVIEW_REPOS allowlist (e.g. removed during a rollback, or a self-hoster who configured autonomy
   // without also updating the env allowlist) used to silently never wake affected PRs here, leaving a stale
@@ -6443,6 +6467,20 @@ async function handlePullRequestWebhookEvent(
       /* v8 ignore next -- best-effort: invalidatePrStateCache never rejects against a healthy D1, and a cache-invalidation failure here must never block the webhook. */
       await invalidatePrStateCache(env, repoFullName, pr.number).catch(() => undefined);
     }
+    // #9055 — a base retarget with the HEAD unchanged. A contributor can move a green PR onto a new base after
+    // CI passed against the old one; GitHub does not re-run `pull_request` workflows or emit a new head SHA for
+    // this, so nothing else in this handler notices. Left alone, the stored diff/patches, the AI-review cache
+    // (its fingerprint deliberately excludes baseSha), and the CI aggregate all keep describing the ABANDONED
+    // base — and the divergence is permanent for that head, since the sweep re-syncs only on head/label drift.
+    // `changes.base` is GitHub's own signal that this specific mutation happened; treated as review-invalidating
+    // exactly like a new commit or a fresh review event, and files are FORCED (bypassing the head-SHA-keyed
+    // `filesUpToDate` check that would otherwise skip the refetch entirely since the head has not moved).
+    if (eventName === "pull_request" && payload.action === "edited" && payload.changes?.base?.from?.ref) {
+      await invalidatePrStateCache(env, repoFullName, pr.number).catch(() => undefined);
+      await invalidateCiStateCache(env, repoFullName, pr.number).catch(() => undefined);
+      await markPullRequestReviewsInvalidated(env, repoFullName, pr.number).catch(() => undefined);
+      await refreshPullRequestDetails(env, repoFullName, pr.number, { force: true }).catch(() => undefined);
+    }
     // Review-evasion protection (#review-evasion-protection): a head change (synchronize) invalidates any
     // active-review tracking for the OLD head immediately -- a fresh pass starts its own tracking later in
     // this same handler. Best-effort; the guarded CAS update is a safe no-op when nothing is active. The

src/selfhost/setup-wizard.ts

@@ -5,6 +5,7 @@
 // disabled once an App is configured (server.ts gates on GITHUB_APP_ID), so this can't rebind a live install.
 import { createHmac, timingSafeEqual } from "node:crypto";
 import { timeoutFetch } from "../github/client";
+import { RECOMMENDED_APP_EVENTS } from "../github/backfill";
 
 export const SETUP_TOKEN_FORM_MAX_BYTES = 4096;
 
@@ -59,7 +60,11 @@ export function buildManifest(origin: string, state: string): Record<string, unk
       // itself is off by default and degrades gracefully (skipped + logged, never blocks the close) until then.
       actions: "write",
     },
-    default_events: ["pull_request", "pull_request_review", "push", "issues", "check_suite", "check_run", "status"],
+    // #9058: derived, never hand-listed. The previous literal had drifted from REQUIRED_INSTALLATION_EVENTS --
+    // it omitted `issue_comment` and `repository` -- so a wizard-created App started with every `@loopover …`
+    // command dead and no rename handling, and the product's own health page would have called it unhealthy the
+    // moment it booted. One list means the manifest and the health check can never disagree again.
+    default_events: [...RECOMMENDED_APP_EVENTS],
   };
 }
 

src/services/agent-action-executor.ts

@@ -158,6 +158,11 @@ export type AgentActionExecutionContext = {
   repoFullName: string;
   pullNumber: number;
   headSha?: string | null | undefined;
+  // #9055: the base ORB last computed the diff/review/CI against. A contributor can retarget a PR's base with
+  // the HEAD unchanged — the freshness check that already gates every merge/approve mutation sees nothing wrong
+  // in that case, since it only compares head SHAs. Threaded through so the SAME live fetch that proves the
+  // head also proves the base, denying a merge into an abandoned base rather than silently completing it.
+  expectedBaseRef?: string | null | undefined;
   autonomy: AutonomyPolicy | null | undefined;
   agentPaused?: boolean | undefined;
   agentDryRun?: boolean | undefined;
@@ -374,6 +379,7 @@ export async function executeAgentMaintenanceActions(env: Env, ctx: AgentActionE
         repoFullName: ctx.repoFullName,
         pullNumber: ctx.pullNumber,
         expectedHeadSha,
+        expectedBaseRef: ctx.expectedBaseRef,
       });
       if (freshness.status !== "current") {
         await audit("denied", `${pullRequestFreshnessDetail(freshness)} — action not executed`);

src/types.ts

@@ -366,6 +366,16 @@ export type GitHubWebhookPayload = {
         from?: string;
       };
     };
+    /** #9055: present on a `pull_request.edited` webhook when the CONTRIBUTOR retargeted the PR's base branch.
+     *  `from.ref` is the PREVIOUS base; the current one is `pull_request.base.ref` on this same payload. The
+     *  head SHA does not change on a retarget, so nothing else about this payload signals that everything
+     *  computed so far (diff, review, CI, guardrail path matching, migration-collision detection) was
+     *  computed against a base that no longer applies. */
+    base?: {
+      from?: {
+        ref?: string;
+      };
+    };
     /** #9056: `repository.transferred` carries the PREVIOUS owner here (the repo name itself is unchanged),
      *  so the old full name is `changes.owner.from.{organization,user}.login` + the current repo name. */
     owner?: {

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

@@ -410,6 +410,25 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => {
     expect(fetchPullRequestFreshness).toHaveBeenCalledWith(env, expect.objectContaining({ expectedHeadSha: "reviewed-sha" }));
   });
 
+  // #9055: a contributor can retarget a PR's base with the head unchanged, so nothing else in the freshness
+  // check sees anything wrong. The executor's live pre-mutation check is the last chance to catch it, and it
+  // must be given something to check against.
+  it("threads the context's expectedBaseRef into the live freshness check before a merge (#9055)", async () => {
+    const env = createTestEnv({});
+    await executeAgentMaintenanceActions(env, ctx({ expectedBaseRef: "main" }), [merge]);
+    expect(fetchPullRequestFreshness).toHaveBeenCalledWith(env, expect.objectContaining({ expectedBaseRef: "main" }));
+  });
+
+  it("denies a merge when the live base has moved even though the head is current (#9055)", async () => {
+    const env = createTestEnv({});
+    vi.mocked(fetchPullRequestFreshness).mockResolvedValueOnce({ status: "stale", reason: "base_changed", expectedHeadSha: "sha7", liveHeadSha: "sha7", liveState: "open" });
+
+    const outcomes = await executeAgentMaintenanceActions(env, ctx({ expectedBaseRef: "main" }), [merge]);
+
+    expect(outcomes[0]).toMatchObject({ actionClass: "merge", outcome: "denied", detail: expect.stringContaining("base branch changed") });
+    expect(mergePullRequest).not.toHaveBeenCalled();
+  });
+
   it("LIVE approve pins the review to the action's reviewed head (expectedHeadSha) over the context head, falling back to an empty body (#2262)", async () => {
     const env = createTestEnv({});
     // A staged approve replayed on accept carries the REVIEWED head — same pin as merge already has — and this