JSONbored · loopover-orb · Jul 7, 2026 · Jul 7, 2026
@@ -16,7 +16,7 @@ import { isAuthorBlacklisted } from "../settings/contributor-blacklist";
 import { classifyMergeFailure, MERGE_RETRY_CAP } from "./merge-failure";
 import { notifyActionToDiscord, notifyActionToSlack, type NotifyOutcome } from "./notify-discord";
 import { cancelInFlightWorkflowRunsForHeadSha, createInstallationToken, githubErrorStatus, isGitHubRateLimitedError } from "../github/app";
-import { fetchLiveCiAggregate, fetchLivePullRequestMergeState, mergeRequiredCiContexts, refreshInstallationHealthForInstallation } from "../github/backfill";
+import { fetchLiveCiAggregate, fetchLivePullRequestMergeState, fetchLiveReviewThreadBlockers, mergeRequiredCiContexts, refreshInstallationHealthForInstallation } from "../github/backfill";
 import { githubRateLimitAdmissionKeyForToken } from "../github/client";
 import { ensurePullRequestAssignee } from "../github/assignees";
 import { ensurePullRequestLabel, removePullRequestLabel } from "../github/labels";
@@ -363,9 +363,11 @@ export async function executeAgentMaintenanceActions(env: Env, ctx: AgentActionE
     //    guard above only re-checks head SHA/state, not CI. GitHub's own merge endpoint enforces
     //    branch-protection REQUIRED checks server-side, but only as a backstop when a repo actually configures
     //    them; a red-CI close has no server-side check at all. Re-read live CI right before the mutation so a
-    //    check that flipped in this narrow window is never acted on from stale information. Non-CI closes
-    //    (gate verdict, duplicate/slop, linked-issue hard-rule, blacklist) are exempt — their adverse signal
-    //    does not depend on CI still being red.
+    //    check that flipped in this narrow window is never acted on from stale information. Non-CI closes whose
+    //    justification has no cheap live re-derivation (gate verdict, duplicate/slop, linked-issue hard-rule,
+    //    blacklist) are exempt from THIS specific CI recheck — their adverse signal does not depend on CI still
+    //    being red. A base conflict and an unresolved review thread DO have cheap live signals and get their own
+    //    dedicated rechecks below (requiresLiveMergeableRecheck / requiresLiveThreadRecheck) instead.
     //    A heuristic close staged BEFORE #2478 has no closeRequiresCiState at all -- that field didn't exist yet
     //    -- so `undefined` here is genuinely ambiguous (a legacy CI-driven close and a legacy non-CI close are
     //    byte-identical in storage). The planner now ALWAYS sets the field going forward (never omits it), so
@@ -381,17 +383,23 @@ export async function executeAgentMaintenanceActions(env: Env, ctx: AgentActionE
     // live re-check for a STAGED close (agent-approval-queue.ts); this is the immediate, same-pass execution
     // path, which had no equivalent.
     const requiresLiveMergeableRecheck = action.actionClass === "close" && action.closeKind === "heuristic" && action.closeRequiresMergeableState === true;
-    if (requiresLiveCiRecheck || requiresLiveMergeableRecheck) {
+    // #review-thread-staleness: mirrors requiresLiveMergeableRecheck's exact shape (#3863) -- a review-thread-
+    // justified heuristic close is read from the SAME planning-pass snapshot, and a contributor clicking
+    // "Resolve conversation" on GitHub during a slow review pass clears it before this mutation runs, same as
+    // an unrelated PR clearing a base conflict. Same immediate, same-pass execution path gap as #3863 had.
+    const requiresLiveThreadRecheck = action.actionClass === "close" && action.closeKind === "heuristic" && action.closeRequiresThreadResolved === true;
+    if (requiresLiveCiRecheck || requiresLiveMergeableRecheck || requiresLiveThreadRecheck) {
       const ciToken = await createInstallationToken(env, ctx.installationId).catch(() => undefined);
       const admissionKey = githubRateLimitAdmissionKeyForToken(env, ciToken, ctx.installationId);
       // mergeRequiredCiContexts(null, ...) -- no live branch-protection re-fetch here, just the maintainer's own
       // configured expectedCiContexts (or null/fold-all when unset), matching the "no branch protection" arm of
       // the planning pass's own merge (mergeRequiredCiContexts is pure and already exported for that call site).
-      const [liveCi, liveMergeableState] = await Promise.all([
+      const [liveCi, liveMergeableState, liveThreadBlockers] = await Promise.all([
         requiresLiveCiRecheck
           ? fetchLiveCiAggregate(env, ctx.repoFullName, expectedHeadSha, ciToken, mergeRequiredCiContexts(null, ctx.expectedCiContexts), admissionKey)
           : Promise.resolve(undefined),
         requiresLiveMergeableRecheck ? fetchLivePullRequestMergeState(env, ctx.repoFullName, ctx.pullNumber, ciToken, admissionKey) : Promise.resolve(undefined),
+        requiresLiveThreadRecheck ? fetchLiveReviewThreadBlockers(env, ctx.repoFullName, ctx.pullNumber, ciToken, admissionKey) : Promise.resolve(undefined),
       ]);
       // The planner itself only ever stages a merge when ciState === "passed" exactly (reviewGood in
       // agent-actions.ts; "pending" short-circuits to no actions at all upstream) -- the live re-check must
@@ -414,7 +422,15 @@ export async function executeAgentMaintenanceActions(env: Env, ctx: AgentActionE
       // resolved, matching the approval-queue's own fail-safe-toward-keeping-the-close precedent (#3863).
       const mergeableStaleReason =
         requiresLiveMergeableRecheck && liveMergeableState === "clean" ? "the base-branch conflict that justified this close has since cleared" : null;
-      const staleReason = ciStaleReason ?? mergeableStaleReason;
+      // Only a CONFIRMED empty result clears a thread-justified close -- fetchLiveReviewThreadBlockers already
+      // fails open to [] on its own internal GraphQL error, so `undefined` here means the Promise.resolve(undefined)
+      // no-op arm (requiresLiveThreadRecheck was false) rather than a genuine "no threads left" signal, matching
+      // the mergeable-state recheck's own fail-safe-toward-keeping-the-close precedent above.
+      const threadStaleReason =
+        requiresLiveThreadRecheck && liveThreadBlockers !== undefined && liveThreadBlockers.length === 0
+          ? "the review thread(s) that justified this close are now all resolved"
+          : null;
+      const staleReason = ciStaleReason ?? mergeableStaleReason ?? threadStaleReason;
       if (staleReason) {
         await audit("denied", `${staleReason} — action not executed`);
         continue;
@@ -862,6 +878,9 @@ export function actionParams(action: PlannedAgentAction): AgentPendingActionPara
     // Round-trip the mergeable-state dependency likewise: only a conflict-justified close needs the approval
     // queue's accept-time mergeable-state recheck (see the field's doc comment on AgentPendingActionParams).
     ...(action.closeRequiresMergeableState !== undefined ? { closeRequiresMergeableState: action.closeRequiresMergeableState } : {}),
+    // Round-trip the review-thread dependency likewise: only a thread-justified close needs the accept-time /
+    // pre-mutation live thread-blocker recheck (see the field's doc comment on AgentPendingActionParams).
+    ...(action.closeRequiresThreadResolved !== undefined ? { closeRequiresThreadResolved: action.closeRequiresThreadResolved } : {}),
     // Round-trip the concrete-evidence tag so the breaker's exemption still applies when a staged close accepts.
     ...(action.closeConcreteEvidence !== undefined ? { closeConcreteEvidence: action.closeConcreteEvidence } : {}),
   };

@@ -6,7 +6,7 @@ import { executeAgentMaintenanceActions, pendingActionToPlanned } from "./agent-
 import { downgradeCloseToHold, downgradeMergeToHold, isProtectedAutomationAuthor, type PlannedAgentAction } from "../settings/agent-actions";
 import { findBlacklistEntry } from "../settings/contributor-blacklist";
 import { isCloseHoldOnly, isHoldOnly } from "../review/outcomes-wire";
-import { fetchLiveCiAggregate, fetchLivePullRequestMergeState, fetchLivePullRequestReviewDecision, mergeRequiredCiContexts } from "../github/backfill";
+import { fetchLiveCiAggregate, fetchLivePullRequestMergeState, fetchLivePullRequestReviewDecision, fetchLiveReviewThreadBlockers, mergeRequiredCiContexts } from "../github/backfill";
 import { githubRateLimitAdmissionKeyForToken } from "../github/client";
 import type { AgentPendingActionParams, AgentPendingActionRecord } from "../types";
 
@@ -200,24 +200,30 @@ export async function decidePendingAgentAction(env: Env, input: { id: string; de
   // silently skip the live recheck for any pre-existing auto_with_approval close row staged before this
   // field was introduced, even one that WAS originally conflict-justified -- exactly the safety gap this
   // recheck exists to close. Fail toward "revalidate" for the unknown case, not "skip" (gate review finding).
-  const shouldRecheckLiveDisposition =
-    pr?.headSha &&
-    (pending.actionClass === "merge" ||
-      (pending.actionClass === "close" && pending.params.closeKind === "heuristic" && pending.params.closeRequiresMergeableState !== false));
+  const isMergeableRecheck = pending.actionClass === "close" && pending.params.closeKind === "heuristic" && pending.params.closeRequiresMergeableState !== false;
+  // Mirrors isMergeableRecheck's LIVE-SIGNAL shape (#review-thread-staleness) but deliberately scoped to
+  // `=== true`, not `!== false`: unlike closeRequiresMergeableState, closeRequiresThreadResolved has NO
+  // pre-existing legacy rows anywhere -- it is introduced in the same change as its only producer, so a
+  // freshly planned heuristic close ALWAYS sets it explicitly (mirroring closeRequiresMergeableState's own
+  // "never omitted" discipline). `undefined` here can therefore only mean "not thread-justified", never an
+  // ambiguous legacy row, so there is no equivalent "fail toward revalidate" case to guard against.
+  const isThreadRecheck = pending.actionClass === "close" && pending.params.closeKind === "heuristic" && pending.params.closeRequiresThreadResolved === true;
+  const shouldRecheckLiveDisposition = pr?.headSha && (pending.actionClass === "merge" || isMergeableRecheck || isThreadRecheck);
   if (shouldRecheckLiveDisposition) {
     const token = await createInstallationToken(env, pending.installationId).catch(() => undefined);
     const admissionKey = githubRateLimitAdmissionKeyForToken(env, token, pending.installationId);
     // Promise.allSettled, not Promise.all: each live re-check is independently best-effort (per the comment
     // above), so ONE transient rejection must fail open on that specific check, not throw the whole accept
     // out of decidePendingAgentAction. A settled-rejected check is treated the same as "nothing concerning
     // found" -- exactly what each function's own internal fail-safe catch already resolves to on success.
-    const [ciResult, mergeableResult, reviewResult] = await Promise.allSettled([
+    const [ciResult, mergeableResult, reviewResult, threadResult] = await Promise.allSettled([
       // mergeRequiredCiContexts(null, ...) -- no live branch-protection re-fetch here, just the maintainer's own
       // configured expectedCiContexts (or null/fold-all when unset), so this accept-time re-check honors the
       // same required-contexts view the original plan was evaluated against (#selfhost-ci-verification).
       fetchLiveCiAggregate(env, pending.repoFullName, pr.headSha, token, mergeRequiredCiContexts(null, settings.expectedCiContexts), admissionKey),
       fetchLivePullRequestMergeState(env, pending.repoFullName, pending.pullNumber, token, admissionKey),
       fetchLivePullRequestReviewDecision(env, pending.repoFullName, pending.pullNumber, token, admissionKey),
+      isThreadRecheck ? fetchLiveReviewThreadBlockers(env, pending.repoFullName, pending.pullNumber, token, admissionKey) : Promise.resolve(undefined),
     ]);
     // A REJECTED promise stays undefined (fail-open — the read itself failed, not a genuine CI signal); a
     // FULFILLED promise reporting anything other than "passed" (failed, pending, or unverified) is a real,
@@ -230,6 +236,19 @@ export async function decidePendingAgentAction(env: Env, input: { id: string; de
     // below instead of failing open on it (gate review finding).
     const reviewFetchSucceeded = reviewResult.status === "fulfilled";
     const reviewDecision = reviewFetchSucceeded ? reviewResult.value : undefined;
+    // Tracked separately from the VALUE for the same reason as reviewFetchSucceeded above: a REJECTED promise
+    // also resolves to undefined, which must not read as "confirmed no threads remain" -- fetchLiveReviewThreadBlockers
+    // itself already fails open to [] on a GraphQL error, so a genuinely FULFILLED empty array is the only
+    // signal that legitimately means "no live blockers left".
+    const threadFetchSucceeded = threadResult.status === "fulfilled";
+    const liveThreadBlockers = threadFetchSucceeded ? threadResult.value : undefined;
+    const threadsNowResolved = isThreadRecheck && threadFetchSucceeded && (liveThreadBlockers?.length ?? 0) === 0;
+    // Gated on isMergeableRecheck explicitly (not just "reached the close branch"): a thread-only close
+    // (isThreadRecheck true, isMergeableRecheck false) also reaches this branch now, and mergeableState reads
+    // "clean" for most never-conflicted PRs by default -- without this gate, a thread-only close would be
+    // wrongly superseded as if it were conflict-justified merely because mergeability happens to read clean
+    // (the SAME over-broad-predicate class the #2478 gate review already caught once for closeRequiresMergeableState).
+    const mergeableNowCleared = isMergeableRecheck && reviewFetchSucceeded && mergeableState === "clean" && reviewDecision !== "CHANGES_REQUESTED";
     const staleReason =
       pending.actionClass === "merge"
         ? ciState !== undefined && ciState !== "passed"
@@ -239,14 +258,18 @@ export async function decidePendingAgentAction(env: Env, input: { id: string; de
             : reviewDecision === "CHANGES_REQUESTED"
               ? "a reviewer has since requested changes"
               : null
-        : // Only reached when closeRequiresMergeableState !== false (see shouldRecheckLiveDisposition above), so
-          // CI state is irrelevant to this specific close's justification and the only live signal that matters
-          // is whether the conflict has cleared. reviewFetchSucceeded is required alongside the value check --
-          // see its own comment above -- so a failed live-review read fails open instead of masquerading as
-          // "confirmed no changes requested".
-          reviewFetchSucceeded && mergeableState === "clean" && reviewDecision !== "CHANGES_REQUESTED"
+        : // Only reached when closeRequiresMergeableState !== false or closeRequiresThreadResolved === true (see
+          // shouldRecheckLiveDisposition above), so CI state is irrelevant to this specific close's justification
+          // and the only live signals that matter are whether the conflict has cleared or the thread(s) resolved --
+          // each gated individually below (mergeableNowCleared / threadsNowResolved) so a close justified by only
+          // ONE of the two axes is never wrongly cleared by the other axis's unrelated live state.
+          // reviewFetchSucceeded is required alongside the value check -- see its own comment above -- so a failed
+          // live-review read fails open instead of masquerading as "confirmed no changes requested".
+          mergeableNowCleared
           ? "the conflict that justified this close has since cleared"
-          : null;
+          : threadsNowResolved
+            ? "the review thread(s) that justified this close are now all resolved"
+            : null;
     if (staleReason) {
       await setPendingAgentActionStatus(env, pending.id, { status: "rejected", decidedBy: input.decidedBy });
       await recordAuditEvent(env, {
@@ -255,7 +278,13 @@ export async function decidePendingAgentAction(env: Env, input: { id: string; de
         targetKey,
         outcome: "denied",
         detail: `superseded ${pending.actionClass}: ${staleReason} since staging`,
-        metadata: { ...baseMetadata, ciState: ciState ?? null, mergeableState: mergeableState ?? null, reviewDecision: reviewDecision ?? null },
+        metadata: {
+          ...baseMetadata,
+          ciState: ciState ?? null,
+          mergeableState: mergeableState ?? null,
+          reviewDecision: reviewDecision ?? null,
+          liveThreadBlockerCount: liveThreadBlockers?.length ?? null,
+        },
       });
       return { status: "rejected", action: { ...pending, status: "rejected", decidedBy: input.decidedBy }, executionOutcome: "stale_disposition" };
     }

@@ -3,6 +3,7 @@ import { AI_JUDGMENT_BLOCKER_CODES, type GateCheckConclusion } from "../rules/ad
 import { DEFAULT_AUTO_MAINTAIN_POLICY, autonomyRequiresApproval, isActingAutonomyLevel, resolveAutonomy } from "./autonomy";
 import { changedPathsHittingGuardrail, isGuardrailHit } from "../signals/change-guardrail";
 import { AGENT_LABEL_PENDING_CLOSURE } from "../review/linked-issue-hard-rules";
+import { REVIEW_THREAD_BLOCKER_CODE } from "../review/review-thread-findings";
 import { sanitizePublicComment } from "../github/commands";
 
 // High-slop threshold default when a repo hasn't set slopGateMinScore (mirrors the gate's `high` band).
@@ -111,6 +112,10 @@ export type PlannedAgentAction = {
   // AgentPendingActionParams in types.ts for why the approval queue's accept-time recheck is scoped to this
   // specific case rather than every non-CI heuristic close. ALWAYS set for a heuristic close (never omitted).
   closeRequiresMergeableState?: boolean;
+  // True when an unresolved GitHub review thread (REVIEW_THREAD_BLOCKER_CODE) was part of this close's
+  // justification -- see the doc comment on AgentPendingActionParams in types.ts. Mirrors
+  // closeRequiresMergeableState's own discipline: ALWAYS set for a heuristic close (never omitted).
+  closeRequiresThreadResolved?: boolean;
   // For a "heuristic" close: true when the close is backed by CONCRETE, non-judgment evidence — a committed
   // secret, a failing/red CI run, a base conflict, a deterministic linked-issue-overlap duplicate, or a
   // rule-based lane/manifest/pre-merge rejection — rather than any AI/model-derived verdict or a fuzzy score.
@@ -687,6 +692,11 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne
   // or review-thread blocker into success once the gate has classified it as blocking.
   const conclusion: GateCheckConclusion = input.conclusion;
   const isConflict = input.pr.mergeableState === "dirty"; // conflicts with base — can't merge as-is
+  // True when an unresolved GitHub review thread is (at least one of) this close's justifications -- the SAME
+  // staleness class as isConflict above (#3863), just triggered by a contributor clicking "Resolve conversation"
+  // on GitHub instead of the base branch becoming mergeable again. A mixed blocker set (thread + something else)
+  // still counts: the thread recheck only re-verifies ITS OWN signal, so it's harmless to also gate on it here.
+  const isReviewThreadJustified = (input.gateBlockerCodes ?? []).includes(REVIEW_THREAD_BLOCKER_CODE);
   const isContributor = !input.authorIsOwner && !input.authorIsAdmin && !input.authorIsAutomationBot;
   // The owner-close exemption is PER-REPO CONFIGURABLE (#configurable-owner-close): by default the repo owner's
   // own PRs are exempt from auto-close (closeOwnerAuthors !== true ⇒ merge or manual-hold only), but a maintainer
@@ -1154,6 +1164,8 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne
       closeRequiresCiState: ciFailed ? "failed" : "not_required",
       // Always explicit (never omitted), mirroring closeRequiresCiState's own discipline above.
       closeRequiresMergeableState: isConflict,
+      // Always explicit (never omitted), mirroring closeRequiresCiState's own discipline above.
+      closeRequiresThreadResolved: isReviewThreadJustified,
     });
   }
   // else: guarded → manual; not-good OWNER/automation → manual; action-required/unverified → manual;