fix(orb): actuation atomicity, close-critical concurrency, and reward-label integrity

migrations/0191_linked_issue_claims.sql

@@ -0,0 +1,33 @@
+-- #9160: durable per-(PR, issue) claim ledger. pull_requests.linked_issue_claimed_at is a single PR-level
+-- timestamp that BLENDS every currently-linked issue into one value -- resolveLinkedIssueClaimedAt preserves
+-- it whenever the new linked-issue set overlaps the old one, so a PR that claimed issue #1 on day 1 and later
+-- adds "Fixes #7" keeps the day-1 timestamp for #7 too. That lets an attacker keep a long-lived PR linking a
+-- throwaway issue, then edit its body to add a victim's valuable issue and inherit a backdated claim time,
+-- stealing the duplicate-cluster winner slot (queue/duplicate-detection.ts, packages/loopover-engine's
+-- duplicate-winner election).
+--
+-- Each row here is written ONCE, immutably, the first time loopover observes THIS SPECIFIC (repo, PR, issue)
+-- triple (INSERT OR IGNORE) -- so a newly-added issue always gets its own fresh claim time regardless of what
+-- the blended pull_requests column says. The duplicate-winner election reads this table, scoped to only the
+-- issue(s) actually contested with a sibling, instead of the blended PR-level value.
+CREATE TABLE IF NOT EXISTS linked_issue_claims (
+  repo_full_name TEXT NOT NULL,
+  pull_number INTEGER NOT NULL,
+  issue_number INTEGER NOT NULL,
+  claimed_at TEXT NOT NULL,
+  PRIMARY KEY (repo_full_name, pull_number, issue_number)
+);
+
+-- Backfill: seed one row per currently-linked issue from the existing blended pull_requests column (same
+-- COALESCE chain 0084_pull_request_linked_issue_claimed_at.sql used), so an issue already linked before this
+-- migration keeps its historical relative ordering intact instead of silently resetting to "whenever this PR
+-- next happens to re-sync" the first time recordLinkedIssueClaims runs post-deploy. This is exactly as
+-- backdated as today's pre-fix behavior for these PRE-EXISTING rows (no worse) -- the fix's actual benefit (a
+-- NEWLY-added issue getting its own fresh, un-backdatable clock) applies going forward, from each PR's next
+-- sync onward.
+INSERT OR IGNORE INTO linked_issue_claims (repo_full_name, pull_number, issue_number, claimed_at)
+SELECT pr.repo_full_name, pr.number, CAST(je.value AS INTEGER), COALESCE(pr.linked_issue_claimed_at, pr.updated_at, pr.created_at)
+FROM pull_requests pr, json_each(pr.linked_issues_json) je
+WHERE pr.linked_issues_json IS NOT NULL
+  AND pr.linked_issues_json != '[]'
+  AND pr.linked_issues_json != '';

src/db/repositories.ts

@@ -33,6 +33,7 @@ import {
   installations,
   issues,
   githubRateLimitObservations,
+  linkedIssueClaims,
   notificationDeliveries,
   issueWatchSubscriptions,
   notificationSubscriptions,
@@ -453,6 +454,11 @@ export async function upsertPullRequestFromGitHub(
     existingClaimRow,
     observedLinkedIssueClaimedAt,
   );
+  // #9160: alongside the blended PR-level column above, durably record each CURRENTLY-linked issue's own
+  // first-observed claim time (immutable, per-issue) -- a brand-new issue gets `syncedAt` here regardless of
+  // what the blended column resolves to, so the duplicate-winner election can read a claim time that a
+  // different, already-linked issue can never backdate. Best-effort: never fails the PR upsert itself.
+  await recordLinkedIssueClaims(env, repoFullName, pr.number, linkedIssues, syncedAt).catch(() => undefined);
   // A genuinely observed body (even an explicitly empty one) proves this PR's linked-issue extraction is
   // trustworthy going forward; a sparse payload (pr.body === undefined) proves nothing either way and must not
   // start the clock. Set once, keep forever (#linked-issue-sparse-first-upsert).
@@ -586,6 +592,39 @@ function linkedIssueSetsOverlap(left: number[], right: number[]): boolean {
   return right.some((value) => leftSet.has(value));
 }
 
+/** #9160: record an IMMUTABLE first-observed claim time for each of `issueNumbers` on (repoFullName,
+ *  pullNumber) -- INSERT ... ON CONFLICT DO NOTHING, so an issue already claimed here keeps its ORIGINAL
+ *  claimedAt forever, while a genuinely NEW issue (never linked to this PR before) gets `observedAt` as its
+ *  own fresh clock -- regardless of what pull_requests.linked_issue_claimed_at's blended value says (see that
+ *  column's own #linked-issue-claim-overlap-preserve doc comment for the backdating bug this ledger exists to
+ *  close). A no-op for an empty `issueNumbers`. Called from upsertPullRequestFromGitHub on every sync; safe to
+ *  call repeatedly with the same issue numbers (idempotent). */
+export async function recordLinkedIssueClaims(env: Env, repoFullName: string, pullNumber: number, issueNumbers: readonly number[], observedAt: string): Promise<void> {
+  if (issueNumbers.length === 0) return;
+  const bounded = boundedString(repoFullName, 200);
+  await getDb(env.DB)
+    .insert(linkedIssueClaims)
+    .values(issueNumbers.map((issueNumber) => ({ repoFullName: bounded, pullNumber, issueNumber, claimedAt: observedAt })))
+    .onConflictDoNothing({ target: [linkedIssueClaims.repoFullName, linkedIssueClaims.pullNumber, linkedIssueClaims.issueNumber] });
+}
+
+/** #9160: the EARLIEST recorded per-issue claim time among `issueNumbers` for (repoFullName, pullNumber), or
+ *  `null` when none of them has ever been recorded (a pre-migration PR, or a caller passing an issue number
+ *  this PR never actually linked) -- fails closed the same way resolveDuplicateClusterWinnerNumber's own
+ *  sparse-row handling does, rather than guessing. Callers scope `issueNumbers` to the issue(s) ACTUALLY
+ *  contested with a specific sibling (see queue/duplicate-detection.ts) so an unrelated issue also linked to
+ *  this PR can never backdate (or postdate) the comparison. */
+export async function getEarliestLinkedIssueClaim(env: Env, repoFullName: string, pullNumber: number, issueNumbers: readonly number[]): Promise<string | null> {
+  if (issueNumbers.length === 0) return null;
+  const [row] = await getDb(env.DB)
+    .select({ claimedAt: linkedIssueClaims.claimedAt })
+    .from(linkedIssueClaims)
+    .where(and(eq(linkedIssueClaims.repoFullName, boundedString(repoFullName, 200)), eq(linkedIssueClaims.pullNumber, pullNumber), inArray(linkedIssueClaims.issueNumber, [...issueNumbers])))
+    .orderBy(asc(linkedIssueClaims.claimedAt))
+    .limit(1);
+  return row?.claimedAt ?? null;
+}
+
 export async function upsertIssueFromGitHub(env: Env, repoFullName: string, issue: GitHubIssuePayload, options: { seenOpenAt?: string } = {}): Promise<IssueRecord> {
   const record = toIssueRecord(repoFullName, issue);
   const db = getDb(env.DB);

src/db/schema.ts

@@ -482,6 +482,29 @@ export const pullRequests = sqliteTable(
   }),
 );
 
+// #9160: durable per-(PR, issue) claim ledger. `pullRequests.linkedIssueClaimedAt` above is a single PR-level
+// timestamp that BLENDS every currently-linked issue into one value -- resolveLinkedIssueClaimedAt preserves it
+// whenever the new linked-issue set overlaps the old one, so a PR that claimed issue #1 on day 1 and later adds
+// "Fixes #7" keeps the day-1 timestamp for #7 too, letting an old placeholder PR backdate a claim on a
+// newly-added, more valuable issue and steal the duplicate-cluster winner slot from whoever actually claimed it
+// first (queue/duplicate-detection.ts). Each row here is written ONCE, immutably, the first time loopover
+// observes THIS SPECIFIC (repo, PR, issue) triple (INSERT ... ON CONFLICT DO NOTHING) -- so a newly-added issue
+// always gets its own fresh claim time here regardless of what the blended column says, and the duplicate-
+// winner election can be scoped to only the issue(s) actually contested with a sibling instead of the blended
+// PR-level value.
+export const linkedIssueClaims = sqliteTable(
+  "linked_issue_claims",
+  {
+    repoFullName: text("repo_full_name").notNull(),
+    pullNumber: integer("pull_number").notNull(),
+    issueNumber: integer("issue_number").notNull(),
+    claimedAt: text("claimed_at").notNull(),
+  },
+  (table) => ({
+    primary: primaryKey({ columns: [table.repoFullName, table.pullNumber, table.issueNumber] }),
+  }),
+);
+
 export const pullRequestFiles = sqliteTable(
   "pull_request_files",
   {

src/queue/duplicate-detection.ts

@@ -12,6 +12,7 @@ import { githubRateLimitAdmissionKeyForToken } from "../github/client";
 import { resolveCopycatDuplicateSibling } from "../signals/copycat-duplicate";
 import { isDuplicateClusterWinnerByClaim, resolveDuplicateClusterWinnerNumber } from "../signals/duplicate-winner";
 import { isDuplicateWinnerEnabledGlobally, resolveDuplicateWinnerEnabled } from "../settings/duplicate-winner-mode";
+import { getEarliestLinkedIssueClaim } from "../db/repositories";
 import type { CopycatGateMode, PullRequestRecord, RepositorySettings } from "../types";
 import { mapWithConcurrency } from "./map-with-concurrency";
 
@@ -134,6 +135,36 @@ export async function reconcileLiveDuplicateSiblings(
   );
 }
 
+/**
+ * #9160: scope `pr`'s claim time to ONLY the issue(s) actually contested with at least one of `openSiblings`,
+ * reading each issue's own IMMUTABLE first-observed claim from the durable per-(PR, issue) ledger
+ * (`db/repositories.ts`'s `linked_issue_claims`, see its own doc comment) instead of `pr`'s blended
+ * `linkedIssueClaimedAt` column. This is what closes the #linked-issue-claim-overlap-preserve-style backdating
+ * attack at the winner-election boundary specifically: an attacker's OTHER, unrelated linked issue (kept
+ * around purely so the blended column's overlap rule preserves an old timestamp) never enters this
+ * computation, because it was never shared with any sibling in `openSiblings`.
+ *
+ * `contestedIssues` empty (no sibling actually overlaps -- defensive; every real caller already filtered
+ * `openSiblings` down to PRs sharing an issue with `pr`) returns `pr.linkedIssueClaimedAt` unchanged, since
+ * there is no backdating angle without a contested issue. Otherwise returns the per-issue ledger's earliest
+ * claim among the CONTESTED issues only, which fails closed to `null` (mirrors this module's existing
+ * sparse-row-fails-closed posture) rather than falling back to the blended value when the ledger has no row
+ * yet for any of them -- a `null` here makes {@link isDuplicateClusterWinnerByClaim} correctly refuse to
+ * elect `pr` the winner from missing data, rather than risk resurrecting the very blended value being scoped
+ * away from.
+ */
+export async function resolveScopedLinkedIssueClaimedAt(
+  env: Env,
+  repoFullName: string,
+  pr: Pick<PullRequestRecord, "number" | "linkedIssues" | "linkedIssueClaimedAt">,
+  openSiblings: readonly Pick<PullRequestRecord, "linkedIssues">[],
+): Promise<string | null | undefined> {
+  const siblingIssues = new Set(openSiblings.flatMap((sibling) => sibling.linkedIssues));
+  const contestedIssues = pr.linkedIssues.filter((issue) => siblingIssues.has(issue));
+  if (contestedIssues.length === 0) return pr.linkedIssueClaimedAt;
+  return getEarliestLinkedIssueClaim(env, repoFullName, pr.number, contestedIssues);
+}
+
 export function linkedIssueDuplicatePullRequestsForGate(
   pr: PullRequestRecord,
   pullRequests: PullRequestRecord[],