fix(review): forced re-run steals the AI-review lock; re-run checkbox ignores closed/merged PRs

src/github/app.ts

@@ -827,6 +827,12 @@ export async function createOrUpdateSkippedGateCheckRun(
         summary: reason,
         text: "LoopOver does not post late first comments on closed or merged pull requests.",
       },
+      // #9020: without this, the no-checkRunId lookup path's default (effectively "any") lets this call PATCH
+      // an already-`completed` Gate run (e.g. a fully-evaluated, green run on a since-merged head) down to
+      // `skipped` -- falsifying the historical verdict. This call's own purpose is narrower: finalize an
+      // in-progress "evaluating" run when the PR closed before evaluation finished, which "in_progress_only"
+      // already covers correctly; it must never touch a run that already reached a real terminal conclusion.
+      updateExisting: "in_progress_only",
       supersedeLegacyNames: [GITTENSORY_LEGACY_GATE_CHECK_NAME, GITTENSORY_LEGACY_ORB_GATE_CHECK_NAME],
       mode,
     },

src/queue/ai-review-orchestration.ts

@@ -116,11 +116,13 @@ export async function claimAiReviewLock(
   prNumber: number,
   headSha: string,
   mode: string,
+  options?: { steal?: boolean },
 ): Promise<TransientLockClaim> {
   return claimTransientLock(
     env,
     aiReviewLockKey(repoFullName, prNumber, headSha, mode),
     AI_REVIEW_LOCK_TTL_SECONDS,
+    options,
   );
 }
 
@@ -185,10 +187,16 @@ export async function shouldStartAiReviewForAdvisory(
     // no second REPUTATION_WINDOW_ROW_CAP-bounded review_targets scan when the caller already ran one this pass.
     // Absent (every existing/direct caller) ⇒ computed here exactly as before.
     preComputedReputationSkip?: boolean | undefined;
+    // #9008: true for a maintainer's explicit forced re-run (the PR-panel checkbox / `@loopover review`).
+    // Bypasses ONLY the reputation anti-abuse skip below -- a transient, heuristic gate about spend, not a
+    // configuration decision. It deliberately does NOT bypass shouldRequirePublicAiReviewForAdvisory's hard
+    // gates (aiReviewMode off, an ineligible author under aiReviewConfirmedContributorsOnly, no AI binding,
+    // ...): those are maintainer-configured policy a click cannot override. Absent/false ⇒ byte-identical.
+    forceAiReview?: boolean | undefined;
   },
 ): Promise<boolean> {
   if (!shouldRequirePublicAiReviewForAdvisory(env, args)) return false;
-  if (args.settings.aiReviewAllAuthors) return true;
+  if (args.settings.aiReviewAllAuthors || args.forceAiReview) return true;
   if (!(isReputationEnabled(env) && isConvergenceRepoAllowed(env, args.repoFullName))) return true;
   const reputationSkip =
     args.preComputedReputationSkip ??

src/queue/processors.ts

@@ -9998,6 +9998,7 @@ async function maybePublishPrPublicSurface(
         confirmedContributor,
         skipAiReview: webhook.skipAiReview,
         preComputedReputationSkip,
+        forceAiReview: webhook.forceAiReview,
       }));
     aiReviewExpected = aiReviewWillRun;
     if (isFrozenForManualReview) {
@@ -10185,8 +10186,31 @@ async function maybePublishPrPublicSurface(
         pr.number,
         aiReviewHeadSha,
         settings.aiReviewMode,
+        // #9008: a maintainer's explicit forced re-run (the panel checkbox / `@loopover review`) steals this
+        // lock instead of deferring to whatever currently holds it. Without this, an orphaned lock left behind
+        // by a self-host process that died mid-review made the re-run button permanently inert for the full
+        // TTL (confirmed live: 15+ minutes) -- a duplicate LLM call is the explicitly accepted cost.
+        { steal: webhook.forceAiReview === true },
       );
       if (!aiReviewLock.acquired) {
+        // #9008: this branch used to be completely silent -- no audit event, so a killed process's orphaned
+        // lock made every subsequent pass (forced or not) vanish with zero trace. Named unconditionally; a
+        // FORCED pass landing here at all is itself notable (steal should make that unreachable barring a
+        // cache error, so `forced: true` here is a signal worth alerting on, not just informational).
+        await recordAuditEvent(env, {
+          eventType: "github_app.ai_review_lock_contended",
+          actor: author,
+          targetKey: `${repoFullName}#${pr.number}`,
+          outcome: "completed",
+          detail: "Another pass already holds the AI-review lock for this exact PR head; deferring rather than duplicating the LLM call.",
+          metadata: {
+            deliveryId: webhook.deliveryId,
+            repoFullName,
+            headSha: aiReviewHeadSha,
+            aiReviewMode: settings.aiReviewMode,
+            forced: webhook.forceAiReview === true,
+          },
+        }).catch(() => undefined);
         aiReview = aiReviewLockContendedResult(advisory);
       } else {
         try {
@@ -10577,7 +10601,14 @@ async function maybePublishPrPublicSurface(
         },
       );
     }
-    if (aiReviewExpected && !hasPublicReviewAssessment(aiReview?.notes)) {
+    // #9008: `aiReview?.persistable === false` is produced ONLY by aiReviewLockContendedResult
+    // (ai-review-orchestration.ts) — its plain-prose "AI review is already running..." notes have no
+    // Blockers/Nits section, so hasPublicReviewAssessment previously read them as a legitimate summary and
+    // silently suppressed this exact alarm — a lock-contended pass (forced or not) masqueraded as a real,
+    // completed review with a public assessment, defeating the one safety net meant to catch "no fresh review
+    // happened." Checking persistable directly (rather than special-casing the notes string) is precise: no
+    // other producer ever sets it false.
+    if (aiReviewExpected && (aiReview?.persistable === false || !hasPublicReviewAssessment(aiReview?.notes))) {
       const message =
         "AI review did not produce a public summary; publishing deterministic PR surface without AI notes";
       await recordAuditEvent(env, {
@@ -13056,6 +13087,24 @@ async function maybeProcessPrPanelRetrigger(
     );
     return true;
   }
+  // #9020: the panel comment is deliberately preserved after merge/close (#preserve-review-on-close) and its
+  // re-run checkbox stays interactive on GitHub forever, but every downstream step below assumes an OPEN PR --
+  // prReadyForReview returns true unconditionally for a non-open PR (it has no CI/mergeable state left to wait
+  // on), so a post-merge click used to sail straight through readiness, spend a real gate evaluation + live CI
+  // reads, and only die deep in the pipeline at the freshness guard (which records a contradictory "denied" on
+  // top of this call's own "completed" outcome) -- real spend, no user-visible feedback, checkbox left ticked.
+  // Mirrors reReviewStoredPullRequest's identical `pr.state !== "open"` guard (the sibling entry point).
+  if (pr.state !== "open") {
+    await recordPrPanelRetriggerSkip(
+      env,
+      deliveryId,
+      repoFullName,
+      targetKey,
+      actor,
+      "pr_not_open",
+    );
+    return true;
+  }
 
   const { authorization } = await authorizePrActionActor({
     env,

src/queue/submission-lock.ts

@@ -14,6 +14,9 @@ type LockRecord = {
 type ClaimBody = {
   ownerToken?: unknown;
   ttlSeconds?: unknown;
+  // #9008: a forced re-run intentionally takes ownership even from a still-live holder — mirrors the
+  // cache-path steal in claimTransientLock (transient-locks.ts), kept consistent across both lock backends.
+  steal?: unknown;
 };
 
 type ReleaseBody = {
@@ -44,9 +47,10 @@ export class SubmissionLock extends DurableObject<Env> {
       return Response.json({ error: "invalid_claim" }, { status: 400 });
     }
 
+    const steal = body?.steal === true;
     const now = Date.now();
     const existing = await this.ctx.storage.get<LockRecord>(STORAGE_KEY);
-    if (existing && existing.expiresAt > now && existing.ownerToken !== ownerToken) {
+    if (!steal && existing && existing.expiresAt > now && existing.ownerToken !== ownerToken) {
       return Response.json({ acquired: false });
     }
 

src/queue/transient-locks.ts

@@ -44,8 +44,9 @@ export async function claimTransientLock(
   env: Env,
   key: string,
   ttlSeconds: number,
+  options?: { steal?: boolean },
 ): Promise<TransientLockClaim> {
-  const viaDo = await claimSubmissionLockIfBound(env, key, ttlSeconds);
+  const viaDo = await claimSubmissionLockIfBound(env, key, ttlSeconds, options);
   if (viaDo !== null) return viaDo;
 
   const cache = env.SELFHOST_TRANSIENT_CACHE;
@@ -55,6 +56,19 @@ export async function claimTransientLock(
   // calling claim() so misconfigured test/custom adapters never acquire an unreleasable lock (#2129/#3153).
   if (!cache.releaseIfValue) return { acquired: true, ownerToken: null };
   const ownerToken = randomUUID();
+  // #9008: a `steal` caller (a maintainer's explicit forced re-run) intentionally takes ownership even from a
+  // still-live holder — a duplicate LLM call is the accepted cost, and a self-host process that died mid-review
+  // otherwise orphans this key for the FULL TTL with no recovery path (confirmed live: a lock outlived the
+  // process that claimed it by 15+ minutes). `set` unconditionally overwrites (unlike `claim`'s SET-NX), so a
+  // steal can never itself contend — that is the entire point.
+  if (options?.steal) {
+    try {
+      await cache.set(key, ownerToken, ttlSeconds);
+      return { acquired: true, ownerToken };
+    } catch {
+      return { acquired: true, ownerToken: null }; // fail open — same posture as every other claim failure
+    }
+  }
   try {
     const acquired = await cache.claim(key, ownerToken, ttlSeconds);
     return { acquired, ownerToken: acquired ? ownerToken : null };
@@ -85,6 +99,7 @@ async function claimSubmissionLockIfBound(
   env: Env,
   key: string,
   ttlSeconds: number,
+  options?: { steal?: boolean },
 ): Promise<TransientLockClaim | null> {
   const ns = env.SUBMISSION_LOCK;
   if (!ns) return null;
@@ -94,7 +109,7 @@ async function claimSubmissionLockIfBound(
     const response = await ns.get(id).fetch("https://submission-lock/claim", {
       method: "POST",
       headers: { "content-type": "application/json" },
-      body: JSON.stringify({ ownerToken, ttlSeconds }),
+      body: JSON.stringify({ ownerToken, ttlSeconds, steal: options?.steal === true }),
     });
     const body = (await response.json().catch(() => null)) as { acquired?: unknown } | null;
     if (typeof body?.acquired !== "boolean") return { acquired: true, ownerToken: null }; // fail open

test/unit/ai-review-advisory.test.ts

@@ -191,6 +191,24 @@ describe("shouldStartAiReviewForAdvisory", () => {
       }),
     ).resolves.toBe(true);
   });
+
+  // #9008: a maintainer clicking the panel's re-run checkbox wants a fresh opinion regardless of the
+  // reputation heuristic -- before this, forceAiReview was silently ignored here, so a forced re-run for a
+  // low-reputation author would end with NO review and no explanation, the exact "silent skip" the issue
+  // reported.
+  it("#9008: forceAiReview bypasses ONLY the reputation skip, not the hard entry gates", async () => {
+    const env = createTestEnv({ AI: { run: vi.fn() } as unknown as Ai, AI_SUMMARIES_ENABLED: "true", AI_PUBLIC_COMMENTS_ENABLED: "true", LOOPOVER_REVIEW_REPUTATION: "true", LOOPOVER_REVIEW_REPOS: "acme/widgets" });
+    await env.DB.prepare("INSERT INTO submitter_stats (project, submitter, submissions, merged, closed, manual, last_seen) VALUES (?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP)").bind("acme/widgets", "alice", 8, 0, 8, 0).run();
+    // Without force, reputation still skips (pinned above) -- WITH force, the same low-reputation author is
+    // reviewed.
+    await expect(shouldStartAiReviewForAdvisory(env, { ...base, forceAiReview: true })).resolves.toBe(true);
+    // But force never overrides a HARD, maintainer-configured gate: aiReviewMode: "off" still returns false,
+    // and an explicit skipAiReview (a different caller-level suppression) still wins too.
+    await expect(
+      shouldStartAiReviewForAdvisory(env, { ...base, forceAiReview: true, settings: { aiReviewMode: "off" } as RepositorySettings }),
+    ).resolves.toBe(false);
+    await expect(shouldStartAiReviewForAdvisory(env, { ...base, forceAiReview: true, skipAiReview: true })).resolves.toBe(false);
+  });
 });
 
 describe("runAiReviewForAdvisory", () => {

test/unit/github-app.test.ts

@@ -1864,6 +1864,63 @@ describe("GitHub check runs", () => {
     );
   });
 
+  // #9020: a late-arriving panel re-run click on an ALREADY-MERGED PR used to reach this call with the head's
+  // Gate check run already `completed` (a real, fully-evaluated verdict) -- and demote it to `skipped`,
+  // falsifying the historical result. `updateExisting: "in_progress_only"` means this call may only finalize a
+  // still-`in_progress` run; an already-completed one is left alone (a second, separate run is posted instead,
+  // so the call still does its job without touching the terminal one).
+  it("#9020: never demotes an already-completed Gate check run to skipped -- posts a separate run instead", async () => {
+    const privateKey = await generatePrivateKeyPem();
+    let patchedExisting = false;
+    let newRunPosted = false;
+    vi.stubGlobal(
+      "fetch",
+      async (input: RequestInfo | URL, init?: RequestInit) => {
+        const url = input.toString();
+        const method = (init?.method ?? "GET").toUpperCase();
+        if (url.includes("/access_tokens"))
+          return Response.json({ token: "installation-token" });
+        if (url.includes("/commits/merged123/check-runs")) {
+          return Response.json({
+            total_count: 1,
+            check_runs: [
+              {
+                id: 999,
+                name: "LoopOver Orb Review Agent",
+                status: "completed",
+                conclusion: "success",
+              },
+            ],
+          });
+        }
+        if (method === "PATCH" && url.includes("/check-runs/999")) {
+          patchedExisting = true;
+          return Response.json({ id: 999, html_url: "https://github.com/checks/999" });
+        }
+        if (method === "POST" && url.includes("/check-runs")) {
+          newRunPosted = true;
+          return Response.json(
+            { id: 1000, html_url: "https://github.com/checks/1000" },
+            { status: 201 },
+          );
+        }
+        return new Response("not found", { status: 404 });
+      },
+    );
+
+    const result = await createOrUpdateSkippedGateCheckRun(
+      createTestEnv({ GITHUB_APP_PRIVATE_KEY: privateKey }),
+      123,
+      "JSONbored/gittensory",
+      gateAdvisory("merged123"),
+      "Merged before LoopOver finished.",
+    );
+
+    expect(patchedExisting).toBe(false); // the completed run was never touched
+    expect(newRunPosted).toBe(true); // the call still completes its own job
+    expect(result).toMatchObject({ kind: "published", id: 1000 });
+  });
+
   it("reposts a known check-run id when the old run belongs to a prior App", async () => {
     const privateKey = await generatePrivateKeyPem();
     const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});

test/unit/queue-4.test.ts

@@ -2527,6 +2527,63 @@ describe("queue processors", () => {
     });
   });
 
+  // #9020: the panel comment is deliberately preserved after merge/close (#preserve-review-on-close) and its
+  // re-run checkbox stays interactive on a closed/merged PR forever. Before this guard, a post-merge click
+  // sailed past every other check (bot_author/missing_repo_pr_or_installation/cached_pr_missing/authorization),
+  // ran deep into the pipeline, and only died later at the freshness guard -- real spend, contradictory
+  // telemetry, and no user-visible feedback. Mirrors reReviewStoredPullRequest's own `pr.state !== "open"` guard.
+  it("#9020: a re-run click on an already-merged PR is skipped immediately with pr_not_open, no live GitHub calls", async () => {
+    const env = createTestEnv();
+    await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 123);
+    await upsertRepoFocusManifest(env, "JSONbored/gittensory", {});
+    await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", {
+      number: 49,
+      title: "Already merged",
+      state: "closed",
+      merged_at: "2026-07-26T00:00:00.000Z",
+      user: { login: "contributor" },
+      author_association: "CONTRIBUTOR",
+      head: { sha: "merged-sha" },
+      labels: [],
+      body: "Validation: npm test",
+    });
+    const checkedPanel = [
+      "<!-- gittensory-pr-panel:v1 -->",
+      "",
+      "- [x] <!-- gittensory-rerun-review:v1 --> Re-run LoopOver review",
+    ].join("\n");
+    let fetchCalls = 0;
+    vi.stubGlobal("fetch", async () => {
+      fetchCalls += 1;
+      return new Response("unexpected fetch", { status: 500 });
+    });
+
+    await processJob(env, {
+      type: "github-webhook",
+      deliveryId: "panel-retrigger-merged-pr",
+      eventName: "issue_comment",
+      payload: {
+        action: "edited",
+        installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } },
+        repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } },
+        issue: { number: 49, title: "Already merged", state: "closed", user: { login: "contributor" }, pull_request: {} },
+        comment: { id: 781, body: checkedPanel, user: { login: "loopover-orb[bot]", type: "Bot" } },
+        sender: { login: "maintainer", type: "User" },
+      },
+    });
+
+    expect(fetchCalls).toBe(0); // never reaches authorization/readiness/gate at all
+    const audit = await env.DB.prepare("select actor, target_key, detail from audit_events where event_type = ?")
+      .bind("github_app.pr_panel_retrigger_skipped")
+      .first<{ actor: string | null; target_key: string; detail: string }>();
+    expect(audit).toMatchObject({ target_key: "JSONbored/gittensory#49", detail: "pr_not_open" });
+    // No contradictory "completed" retrigger outcome either -- this is the ONLY audit row for the delivery.
+    const retriggered = await env.DB.prepare("select count(*) as n from audit_events where event_type = ?")
+      .bind("github_app.pr_panel_retriggered")
+      .first<{ n: number }>();
+    expect(retriggered?.n).toBe(0);
+  });
+
   it("ignores invalid rerun task edits and audits skipped rerun requests", async () => {
     const env = createTestEnv();
     const checkedPanel = [