fix(orb): stop two wrong-close paths — superseded cancelled checks, permanent linked-issue violations

migrations/0184_linked_issue_violation_snapshot.sql

@@ -0,0 +1,14 @@
+-- #9029: the linked-issue hard-rule violation marker (linked_issue_hard_rule_violated_at) is written the first
+-- time ANY pass observes a violation and is then NEVER cleared, so an ephemeral, benign issue state condemns a
+-- PR for its entire lifetime. Concrete case from the audit: a maintainer momentarily self-assigns the linked
+-- issue to triage it, the next pass stamps "issue assigned to someone else", and the PR is permanently marked
+-- for a one-shot close -- removing the assignment does not exonerate it. That punishes a contributor for the
+-- maintainer's own action, with no recourse short of disabling every hard rule repo-wide.
+--
+-- The persistence itself is deliberate and must stay: it exists so a contributor cannot stamp-then-dodge by
+-- editing the PR body to unlink the ineligible issue after the violation was observed. The missing piece is the
+-- ability to tell those two situations apart. Recording WHICH linked issues were observed at violation time
+-- makes that a decidable question: if the PR still links the SAME issue set and the live rules now pass, the
+-- change happened on the ISSUE side (assignee removed, label added, reopened) and the PR is exonerated; if the
+-- linked set CHANGED, the author edited the link and the remembered violation stands.
+ALTER TABLE pull_requests ADD COLUMN linked_issue_hard_rule_violation_issues_json TEXT;

src/db/repositories.ts

@@ -4229,13 +4229,43 @@ export async function markPullRequestMergeBlocked(env: Env, fullName: string, nu
  *  the body or the linked issue's state changing after this call is a no-op here: the PR already proved itself in
  *  violation once and stays that way for its lifetime. A no-op (0 rows affected) when the PR row doesn't exist yet
  *  is safe -- the caller only reaches this after a live violation was just evaluated against an existing row. */
-export async function markPullRequestLinkedIssueHardRuleViolated(env: Env, fullName: string, number: number, reason: string): Promise<void> {
+export async function markPullRequestLinkedIssueHardRuleViolated(
+  env: Env,
+  fullName: string,
+  number: number,
+  // Nullable: the default lives here rather than at the call site so the fallback is directly unit-testable
+  // (a violation with no reason is not reachable through the live evaluator today, but the column is
+  // NOT NULL-tolerant by contract and a future rule could omit one).
+  reason: string | null | undefined,
+  // #9029: the linked-issue set observed at violation time, COALESCE'd like the siblings so it always describes
+  // the FIRST violation. Without it a later pass cannot tell an issue-side state change (exonerate) from an
+  // author link edit (violation stands) -- see mergeLinkedIssueHardRuleWithPersistedViolation.
+  linkedIssues?: readonly number[] | undefined,
+): Promise<void> {
   const db = getDb(env.DB);
+  const snapshot = jsonString([...new Set(linkedIssues ?? [])].sort((a, b) => a - b));
   await db
     .update(pullRequests)
     .set({
       linkedIssueHardRuleViolatedAt: sql`COALESCE(${pullRequests.linkedIssueHardRuleViolatedAt}, ${nowIso()})`,
-      linkedIssueHardRuleViolationReason: sql`COALESCE(${pullRequests.linkedIssueHardRuleViolationReason}, ${reason.slice(0, 280)})`,
+      linkedIssueHardRuleViolationReason: sql`COALESCE(${pullRequests.linkedIssueHardRuleViolationReason}, ${(reason ?? "the linked issue is not eligible for a community PR").slice(0, 280)})`,
+      linkedIssueHardRuleViolationIssuesJson: sql`COALESCE(${pullRequests.linkedIssueHardRuleViolationIssuesJson}, ${snapshot})`,
+      updatedAt: nowIso(),
+    })
+    .where(and(eq(pullRequests.repoFullName, fullName), eq(pullRequests.number, number)));
+}
+
+/** #9029: clear a remembered linked-issue hard-rule violation once a later pass proves it was an issue-side,
+ *  benign state change rather than an author dodge. Only ever called with that determination already made
+ *  (see mergeLinkedIssueHardRuleWithPersistedViolation's exoneration branch). */
+export async function clearPullRequestLinkedIssueHardRuleViolation(env: Env, fullName: string, number: number): Promise<void> {
+  const db = getDb(env.DB);
+  await db
+    .update(pullRequests)
+    .set({
+      linkedIssueHardRuleViolatedAt: null,
+      linkedIssueHardRuleViolationReason: null,
+      linkedIssueHardRuleViolationIssuesJson: null,
       updatedAt: nowIso(),
     })
     .where(and(eq(pullRequests.repoFullName, fullName), eq(pullRequests.number, number)));
@@ -6844,6 +6874,7 @@ function toPullRequestRecordFromRow(row: typeof pullRequests.$inferSelect): Pull
     lastRegatedAt: row.lastRegatedAt,
     lastPublishedSurfaceSha: row.lastPublishedSurfaceSha,
     linkedIssueHardRuleViolatedAt: row.linkedIssueHardRuleViolatedAt,
+    linkedIssueHardRuleViolationIssues: parseJson<number[]>(row.linkedIssueHardRuleViolationIssuesJson, []),
     linkedIssueHardRuleViolationReason: row.linkedIssueHardRuleViolationReason,
     visualCaptureSatisfiedSha: row.visualCaptureSatisfiedSha,
     screenshotTablePresenceSatisfied: parseJson<{ headSha: string; evidenceFingerprint: string } | null>(row.screenshotTablePresenceSatisfiedJson, null),

src/db/schema.ts

@@ -420,6 +420,10 @@ export const pullRequests = sqliteTable(
     // pairing with merge_blocked_sha) -- so a later close can still cite the concrete rule even if the live
     // re-parse can no longer reproduce it (the issue was unlinked or its state changed).
     linkedIssueHardRuleViolationReason: text("linked_issue_hard_rule_violation_reason"),
+    // #9029: the linked-issue set observed AT violation time. Lets a later pass distinguish "the issue's state
+    // changed underneath the contributor" (same issues still linked, live rules now pass ⇒ exonerate) from
+    // "the author edited the link to dodge the remembered violation" (issue set changed ⇒ violation stands).
+    linkedIssueHardRuleViolationIssuesJson: text("linked_issue_hard_rule_violation_issues_json"),
     // Visual-capture gate satisfaction (#4110): the head SHA at which the bot's before/after capture pipeline
     // (review.visual.enabled) last produced a REAL before+after render pair (not a placeholder/failed/pending
     // shot) for this PR. Lets the deterministic screenshotTableGate treat a successful automated capture as

src/github/backfill.ts

@@ -2877,6 +2877,54 @@ function dedupeLatestCheckRunsByIdentity(checkRuns: ReadonlyArray<LiveCiCheckRun
   return output;
 }
 
+/**
+ * #9053: drop a `cancelled` check-run that a NEWER check suite has already superseded with the same check name.
+ *
+ * CONFIRMED live, not theoretical: `GET /commits/{sha}/check-runs` (even with GitHub's default `filter=latest`)
+ * returns runs from MULTIPLE suites for one SHA — measured 11 runs across 6 suites on a real PR head, 10 of them
+ * from older suites. A same-SHA workflow re-trigger (`reopened`, `ready_for_review`, `labeled` — and ORB ITSELF
+ * applies labels) starts a new suite; `.github/workflows/ci.yml` sets `concurrency: cancel-in-progress: true`,
+ * so the first run is cancelled and its `conclusion: "cancelled"` runs stay attached to the same head SHA in the
+ * OLD suite. Because `cancelled` is in CI_FAILING_CONCLUSIONS and the dedupe key deliberately includes the suite
+ * id (so a genuine failure can never be hidden by a later success), those stale cancelled runs landed in
+ * `failingDetails` alongside the new suite's passing ones → ciState "failed" → `willClose` on ciFailed →
+ * one-shot auto-close of a PR whose CI is actually green. The executor's recheck re-derives the same verdict,
+ * so it confirmed rather than caught it. "CI is failing" is the single most common close reason in the ledger,
+ * which makes any false-positive rate here expensive.
+ *
+ * Scoped as narrowly as possible: ONLY `cancelled` conclusions, and ONLY when a strictly newer suite carries the
+ * same (name, app) — a cancellation with no superseding attempt still counts as failing, and genuine `failure` /
+ * `timed_out` / `startup_failure` conclusions are never dropped, so the cross-suite non-collapse this function
+ * sits beside keeps doing its job. Suite ids are compared numerically (GitHub's are monotonically increasing);
+ * a non-numeric or absent id is not comparable, so such a run is conservatively KEPT.
+ */
+function dropSupersededCancelledCheckRuns(checkRuns: ReadonlyArray<LiveCiCheckRun>): LiveCiCheckRun[] {
+  const suiteIdOf = (run: LiveCiCheckRun): number | null => {
+    const raw = run.check_suite?.id ?? run.check_suite?.databaseId ?? null;
+    if (raw == null || raw === "") return null;
+    const parsed = Number(raw);
+    return Number.isFinite(parsed) ? parsed : null;
+  };
+  const identityOf = (run: LiveCiCheckRun): string => `${run.name}\0${(run.app?.slug ?? "").toLowerCase()}`;
+  // Highest suite id observed per (name, app), across every conclusion — a later suite supersedes an earlier
+  // attempt at the same check regardless of how that later attempt turned out.
+  const newestSuiteByIdentity = new Map<string, number>();
+  for (const run of checkRuns) {
+    const suiteId = suiteIdOf(run);
+    if (suiteId == null) continue;
+    const identity = identityOf(run);
+    const seen = newestSuiteByIdentity.get(identity);
+    if (seen == null || suiteId > seen) newestSuiteByIdentity.set(identity, suiteId);
+  }
+  return checkRuns.filter((run) => {
+    if ((run.conclusion ?? "").toLowerCase() !== "cancelled") return true;
+    const suiteId = suiteIdOf(run);
+    if (suiteId == null) return true; // not comparable → keep (conservative)
+    const newest = newestSuiteByIdentity.get(identityOf(run));
+    return newest == null || suiteId >= newest;
+  });
+}
+
 /**
  * Pure reduction of a head SHA's check-runs + classic statuses (+ a lazily-fetched check-suite backstop) into the
  * gate's LiveCiAggregate. Extracted so the REST fetch path (fetchLiveCiAggregate) and the GraphQL rollup path
@@ -2934,7 +2982,7 @@ async function reduceLiveCiAggregate(
   // current one, without collapsing unrelated checks that merely share a display name.
   const checkRunSummary = (run: LiveCiCheckRun): string | undefined =>
     [run.output?.title, run.output?.summary].find((value): value is string => typeof value === "string" && value.trim().length > 0)?.trim().slice(0, 200);
-  for (const run of dedupeLatestCheckRunsByIdentity(checkRuns)) {
+  for (const run of dropSupersededCancelledCheckRuns(dedupeLatestCheckRunsByIdentity(checkRuns))) {
     seenContextNames.add(run.name); // mark BEFORE bot-check skip: a bot-owned required context is "seen"
     const appSlug = (run.app?.slug ?? "").toLowerCase();
     if (appSlug === "github-actions") sawFirstPartyCheckRun = true;

src/queue/processors.ts

@@ -74,6 +74,7 @@ import {
   isGlobalAgentFrozen,
   markGateOutcomeOverridden,
   markPullRequestLinkedIssueHardRuleViolated,
+  clearPullRequestLinkedIssueHardRuleViolation,
   startActiveReviewTracking,
   terminalizeActiveReviewTracking,
   bumpPullRequestDraftConversionCount,
@@ -3093,16 +3094,26 @@ async function runAgentMaintenancePlanAndExecute(
   // confirmed violation isn't remembered, matching every other loopover-computed marker write in this file
   // (mergeBlockedSha, draftConversionCount, lastRegatedAt).
   if (liveLinkedIssueHardRule?.violated === true) {
-    await markPullRequestLinkedIssueHardRuleViolated(env, repoFullName, pr.number, liveLinkedIssueHardRule.reason ?? "the linked issue is not eligible for a community PR").catch(() => undefined);
+    // #9029: snapshot WHICH issues were linked at violation time, so a later pass can tell an issue-side state
+    // change (exonerate) from an author link edit (violation stands).
+    await markPullRequestLinkedIssueHardRuleViolated(env, repoFullName, pr.number, liveLinkedIssueHardRule.reason, pr.linkedIssues).catch(() => undefined);
   }
   const linkedIssueHardRule = mergeLinkedIssueHardRuleWithPersistedViolation(
     liveLinkedIssueHardRule,
     {
       violatedAt: pr.linkedIssueHardRuleViolatedAt,
       reason: pr.linkedIssueHardRuleViolationReason,
+      issuesAtViolation: pr.linkedIssueHardRuleViolationIssues,
+      currentIssues: pr.linkedIssues,
     },
     anyLinkedIssueHardRuleOn(linkedIssueRulesConfig),
   );
+  // #9029: the merge just exonerated a remembered violation (same issues still linked, live rules now pass), so
+  // drop the marker rather than re-deciding it identically on every future pass. Best-effort: a failed clear
+  // only means the next pass re-derives the same exoneration, never a wrong close.
+  if (pr.linkedIssueHardRuleViolatedAt != null && linkedIssueHardRule?.violated !== true) {
+    await clearPullRequestLinkedIssueHardRuleViolation(env, repoFullName, pr.number).catch(() => undefined);
+  }
 
   // Unlinked-issue guardrail (#unlinked-issue-guardrail, credibility-gate-farming defense): when this PR
   // links NO issue and the repo opted in (settings.unlinkedIssueGuardrail.mode === "hold"), check whether the

src/review/linked-issue-hard-rules.ts

@@ -170,15 +170,46 @@ export function evaluateLinkedIssueHardRules(input: {
  */
 export function mergeLinkedIssueHardRuleWithPersistedViolation(
   live: LinkedIssueHardRuleResult | undefined,
-  persisted: { violatedAt: string | null | undefined; reason: string | null | undefined },
+  persisted: {
+    violatedAt: string | null | undefined;
+    reason: string | null | undefined;
+    // #9029: the linked-issue set observed when the violation was first recorded, and the set linked NOW.
+    // Both absent ⇒ pre-#9029 rows and every caller that has no snapshot behave exactly as before.
+    issuesAtViolation?: readonly number[] | null | undefined;
+    currentIssues?: readonly number[] | null | undefined;
+  },
   anyRuleOn: boolean,
 ): LinkedIssueHardRuleResult | undefined {
   if (live?.violated === true) return live;
   if (!anyRuleOn) return live;
   if (persisted.violatedAt == null) return live;
+  // #9029: the live rules now PASS. Persisting regardless is what let an ephemeral, benign issue state condemn
+  // a PR forever -- a maintainer momentarily self-assigning the linked issue to triage it stamped the violation,
+  // and un-assigning did not exonerate it. The persistence exists to stop a stamp-then-dodge (edit the body to
+  // unlink the ineligible issue AFTER being caught), so the question that actually matters is WHICH SIDE moved:
+  // if the PR still links exactly the issues that were linked at violation time, nothing the author controls
+  // changed and the improvement came from the issue itself ⇒ exonerate. If the linked set changed, the author
+  // edited the link and the remembered violation stands, unchanged from before.
+  if (linkedIssueSetsMatch(persisted.issuesAtViolation, persisted.currentIssues)) return live;
   return { violated: true, reason: persisted.reason ?? "the linked issue is not eligible for a community PR" };
 }
 
+/** True only when BOTH sides are present and describe the same set of issue numbers (order-insensitive). A
+ *  missing/empty snapshot (pre-#9029 rows) is deliberately NOT a match: without knowing what was linked at
+ *  violation time we cannot rule out a dodge, so those rows keep the original permanent-violation behavior. */
+function linkedIssueSetsMatch(
+  atViolation: readonly number[] | null | undefined,
+  current: readonly number[] | null | undefined,
+): boolean {
+  if (!atViolation || atViolation.length === 0) return false;
+  if (!current || current.length === 0) return false;
+  const a = new Set(atViolation);
+  const b = new Set(current);
+  if (a.size !== b.size) return false;
+  for (const value of a) if (!b.has(value)) return false;
+  return true;
+}
+
 /**
  * Orchestrate the per-PR linked-issue hard-rule decision (the testable core of maybeRunAgentMaintenance's
  * linked-issue block). Returns the hard-rule result, or undefined when no rule applies. Takes the raw PR body +

src/types.ts

@@ -694,6 +694,8 @@ export type PullRequestRecord = {
    *  pairing with mergeBlockedSha) — so a later close can still cite the concrete rule even when the live re-parse
    *  can no longer reproduce it. */
   linkedIssueHardRuleViolationReason?: string | null | undefined;
+  /** #9029: linked-issue numbers observed when the violation was first recorded. */
+  linkedIssueHardRuleViolationIssues?: number[] | undefined;
   /** Visual-capture gate satisfaction (#4110): the head SHA at which the bot's before/after capture pipeline
    *  last produced a REAL before+after render pair (not a placeholder/failed/pending shot) for this PR. The
    *  screenshotTableGate treats visualCaptureSatisfiedSha === headSha as evidence equivalent to a hand-authored