fix(review): prevent backdated duplicate-winner claims (#3956)

Author: JSONbored

packages/gittensory-engine/src/duplicate-winner.ts

@@ -11,14 +11,11 @@
  * caller can compute the winner ONCE per review run and thread the result boolean consistently into every
  * surface (advisory finding, close reason, slop, panels), so they agree by construction.
  *
- * ELECTION ORDER (#dup-winner true-creation-time): prefer each PR's true GitHub `pull_request.created_at` —
- * the real order contributors opened their PRs in — over `linkedIssueClaimedAt` (gittensory's own sync-time,
- * i.e. whenever a webhook/sweep/backfill pass happened to OBSERVE the linked issue). Sync order and creation
- * order diverge whenever processing isn't strictly FIFO (a stalled sweep catching up on a backlog, backfill
- * reordering, webhook delivery delay), under the old claim-time-only rule, that divergence could crown a
- * LATER contributor the winner and close the PR of whoever actually opened first. `createdAt` is compared
- * only when BOTH sides of a given comparison have a valid one; otherwise this falls back to the legacy
- * claim-time comparison unchanged, so sparse/legacy rows keep their existing fail-closed behavior exactly.
+ * ELECTION ORDER: compare `linkedIssueClaimedAt`, the time gittensory first observed the PR claiming
+ * the issue. GitHub `pull_request.created_at` is intentionally not an ordering signal here: contributors can
+ * edit an old placeholder PR to add a linked issue later, so creation time would let backdated claims steal
+ * duplicate-winner credit from the PR that actually claimed the issue first. Sparse legacy rows that lack
+ * claim timing keep failing closed so unknown ordering cannot suppress duplicate evidence.
  *
  * INVARIANT (the caller MUST honor it): {@link openSiblingNumbers} carries OPEN-only sibling PR numbers. The
  * existing sources already exclude closed/merged PRs. Once the winner closes (e.g. red CI), it leaves the open
@@ -34,7 +31,7 @@
 export type DuplicateClaimMember = {
   number: number;
   linkedIssueClaimedAt?: string | null | undefined;
-  /** GitHub's true PR creation time. See the module doc's "ELECTION ORDER" note. */
+  /** GitHub's true PR creation time. Retained for caller compatibility; not used for winner ordering. */
   createdAt?: string | null | undefined;
 };
 
@@ -55,8 +52,7 @@ export function isDuplicateClusterWinner(prNumber: number, openSiblingNumbers: n
 
 /**
  * True iff `pr` is the earliest-elected claimant in the open duplicate cluster (see the module doc's
- * "ELECTION ORDER" note for the createdAt-vs-claim-time precedence). Sparse legacy rows fail closed; ties
- * between equally-ordered members use PR number.
+ * "ELECTION ORDER" note). Sparse legacy rows fail closed; ties between equally-ordered members use PR number.
  */
 export function isDuplicateClusterWinnerByClaim(pr: DuplicateClaimMember, openSiblings: DuplicateClaimMember[]): boolean {
   if (openSiblings.length === 0) return true;
@@ -67,18 +63,11 @@ export function isDuplicateClusterWinnerByClaim(pr: DuplicateClaimMember, openSi
 }
 
 /**
- * True iff `pr` is ordered at or ahead of `sibling` for cluster-winner purposes. Prefers `createdAt` when BOTH
- * sides have a valid one (the true creation-time order); otherwise falls back to the legacy `linkedIssueClaimedAt`
- * comparison unchanged (including its fail-closed-on-missing/invalid-timestamp behavior), so a mixed
- * legacy/modern cluster never silently guesses using two different clocks for the two sides of one comparison.
+ * True iff `pr` is ordered at or ahead of `sibling` for cluster-winner purposes. Only the observed linked-issue
+ * claim time participates in the election; `createdAt` is deliberately ignored because an older PR can claim a
+ * linked issue later by editing its body.
  */
 function prPrecedesSibling(pr: DuplicateClaimMember, sibling: DuplicateClaimMember): boolean {
-  const prCreated = claimTimeMs(pr.createdAt);
-  const siblingCreated = claimTimeMs(sibling.createdAt);
-  if (prCreated !== null && siblingCreated !== null) {
-    if (prCreated !== siblingCreated) return prCreated < siblingCreated;
-    return pr.number <= sibling.number;
-  }
   const prClaim = claimTimeMs(pr.linkedIssueClaimedAt);
   if (prClaim === null) return false;
   const siblingClaim = claimTimeMs(sibling.linkedIssueClaimedAt);

packages/gittensory-engine/src/signals/duplicate-winner.ts

@@ -11,24 +11,27 @@
  * caller can compute the winner ONCE per review run and thread the result boolean consistently into every
  * surface (advisory finding, close reason, slop, panels), so they agree by construction.
  *
- * ELECTION ORDER (#dup-winner true-creation-time): prefer each PR's true GitHub `pull_request.created_at` —
- * the real order contributors opened their PRs in — over `linkedIssueClaimedAt` (gittensory's own sync-time,
- * i.e. whenever a webhook/sweep/backfill pass happened to OBSERVE the linked issue). Sync order and creation
- * order diverge whenever processing isn't strictly FIFO (a stalled sweep catching up on a backlog, backfill
- * reordering, webhook delivery delay), under the old claim-time-only rule, that divergence could crown a
- * LATER contributor the winner and close the PR of whoever actually opened first. `createdAt` is compared
- * only when BOTH sides of a given comparison have a valid one; otherwise this falls back to the legacy
- * claim-time comparison unchanged, so sparse/legacy rows keep their existing fail-closed behavior exactly.
+ * ELECTION ORDER: compare `linkedIssueClaimedAt`, the time gittensory first observed the PR claiming
+ * the issue. GitHub `pull_request.created_at` is intentionally not an ordering signal here: contributors can
+ * edit an old placeholder PR to add a linked issue later, so creation time would let backdated claims steal
+ * duplicate-winner credit from the PR that actually claimed the issue first. Sparse legacy rows that lack
+ * claim timing keep failing closed so unknown ordering cannot suppress duplicate evidence.
  *
  * INVARIANT (the caller MUST honor it): {@link openSiblingNumbers} carries OPEN-only sibling PR numbers. The
  * existing sources already exclude closed/merged PRs. Once the winner closes (e.g. red CI), it leaves the open
  * set and the next-earliest OPEN claimant becomes the winner on re-eval — no permanently-orphaned cluster.
+ *
+ * SECOND CONSUMER (#2278): this module is intentionally engine-hosted (not `src/`-only) because its election
+ * logic is reusable for the miner's own soft-claim adjudication — deciding which of several miners claiming
+ * the same issue proceeds. A future contributor wiring the miner's local claim ledger should import this
+ * module rather than reimplementing the election rule, so both the maintainer gate and the miner agree on
+ * exactly one winner by construction.
  */
 
 export type DuplicateClaimMember = {
   number: number;
   linkedIssueClaimedAt?: string | null | undefined;
-  /** GitHub's true PR creation time. See the module doc's "ELECTION ORDER" note. */
+  /** GitHub's true PR creation time. Retained for caller compatibility; not used for winner ordering. */
   createdAt?: string | null | undefined;
 };
 
@@ -49,8 +52,7 @@ export function isDuplicateClusterWinner(prNumber: number, openSiblingNumbers: n
 
 /**
  * True iff `pr` is the earliest-elected claimant in the open duplicate cluster (see the module doc's
- * "ELECTION ORDER" note for the createdAt-vs-claim-time precedence). Sparse legacy rows fail closed; ties
- * between equally-ordered members use PR number.
+ * "ELECTION ORDER" note). Sparse legacy rows fail closed; ties between equally-ordered members use PR number.
  */
 export function isDuplicateClusterWinnerByClaim(pr: DuplicateClaimMember, openSiblings: DuplicateClaimMember[]): boolean {
   if (openSiblings.length === 0) return true;
@@ -61,18 +63,11 @@ export function isDuplicateClusterWinnerByClaim(pr: DuplicateClaimMember, openSi
 }
 
 /**
- * True iff `pr` is ordered at or ahead of `sibling` for cluster-winner purposes. Prefers `createdAt` when BOTH
- * sides have a valid one (the true creation-time order); otherwise falls back to the legacy `linkedIssueClaimedAt`
- * comparison unchanged (including its fail-closed-on-missing/invalid-timestamp behavior), so a mixed
- * legacy/modern cluster never silently guesses using two different clocks for the two sides of one comparison.
+ * True iff `pr` is ordered at or ahead of `sibling` for cluster-winner purposes. Only the observed linked-issue
+ * claim time participates in the election; `createdAt` is deliberately ignored because an older PR can claim a
+ * linked issue later by editing its body.
  */
 function prPrecedesSibling(pr: DuplicateClaimMember, sibling: DuplicateClaimMember): boolean {
-  const prCreated = claimTimeMs(pr.createdAt);
-  const siblingCreated = claimTimeMs(sibling.createdAt);
-  if (prCreated !== null && siblingCreated !== null) {
-    if (prCreated !== siblingCreated) return prCreated < siblingCreated;
-    return pr.number <= sibling.number;
-  }
   const prClaim = claimTimeMs(pr.linkedIssueClaimedAt);
   if (prClaim === null) return false;
   const siblingClaim = claimTimeMs(sibling.linkedIssueClaimedAt);

src/signals/duplicate-winner.ts

@@ -2,7 +2,7 @@
  * Duplicate-winner adjudication (#dup-winner), extracted to `@jsonbored/gittensory-engine` (#2278) so the
  * maintainer gate and the miner's own soft-claim adjudication (a later Phase-0 issue) import the identical,
  * versioned election logic instead of drifting apart. See the engine module's doc comment for the full
- * election-order rationale (createdAt-vs-claim-time precedence, fail-closed semantics).
+ * election-order rationale (claim-time election, anti-backdating semantics).
  *
  * packages/gittensory-engine/src/duplicate-winner.ts (imported via relative source path, not the published
  * module, matching the #2282 scoring-preview extraction) is the source of truth.

test/unit/duplicate-winner.test.ts

@@ -92,76 +92,50 @@ describe("isDuplicateClusterWinnerByClaim (#dup-winner claim election)", () => {
   });
 });
 
-describe("isDuplicateClusterWinnerByClaim createdAt precedence (#dup-winner true-creation-time)", () => {
+describe("isDuplicateClusterWinnerByClaim claim-time election with createdAt present (#dup-winner anti-backdating)", () => {
   const member = (number: number, createdAt: string | null, linkedIssueClaimedAt: string | null) => ({ number, createdAt, linkedIssueClaimedAt });
 
-  it("REGRESSION: elects the PR that GitHub says opened first, even when gittensory OBSERVED (claimed) the later-opened sibling first", () => {
-    // PR 13 truly opened first (10:00) but gittensory's stalled sweep only got around to syncing/claiming it at
-    // 11:00. PR 14 opened later (10:05) but was claimed immediately (10:06) because the sweep happened to reach
-    // it first. Under the old claim-time-only rule, 14 would wrongly win and 13 (the real first mover) would be
-    // closed as the "duplicate." createdAt must override that.
+  it("REGRESSION: does not let an older placeholder PR steal winner credit by adding the issue later", () => {
+    // PR 12 was opened first but only edited in the linked issue after PR 13 had already claimed it. The
+    // anti-backdating signal is linkedIssueClaimedAt, so createdAt must not override the actual claim order.
     expect(
       isDuplicateClusterWinnerByClaim(
-        member(13, "2026-06-29T10:00:00.000Z", "2026-06-29T11:00:00.000Z"),
-        [member(14, "2026-06-29T10:05:00.000Z", "2026-06-29T10:06:00.000Z")],
-      ),
-    ).toBe(true);
-    // And symmetrically, the later-created PR no longer wins just because it was claimed first.
-    expect(
-      isDuplicateClusterWinnerByClaim(
-        member(14, "2026-06-29T10:05:00.000Z", "2026-06-29T10:06:00.000Z"),
-        [member(13, "2026-06-29T10:00:00.000Z", "2026-06-29T11:00:00.000Z")],
+        member(12, "2026-06-29T09:00:00.000Z", "2026-06-29T10:05:00.000Z"),
+        [member(13, "2026-06-29T09:30:00.000Z", "2026-06-29T10:00:00.000Z")],
       ),
     ).toBe(false);
-  });
-
-  it("falls back to claim-time comparison when only ONE side has a valid createdAt (mixed legacy/modern cluster)", () => {
-    // pr has createdAt; sibling (a legacy row) does not — never mix clocks across the two sides of one
-    // comparison. pr's claim (10:00) is earlier than sibling's claim (10:05) ⇒ pr still wins via the fallback.
     expect(
       isDuplicateClusterWinnerByClaim(
-        { number: 12, createdAt: "2026-06-29T09:00:00.000Z", linkedIssueClaimedAt: "2026-06-29T10:00:00.000Z" },
-        [{ number: 13, createdAt: null, linkedIssueClaimedAt: "2026-06-29T10:05:00.000Z" }],
+        member(13, "2026-06-29T09:30:00.000Z", "2026-06-29T10:00:00.000Z"),
+        [member(12, "2026-06-29T09:00:00.000Z", "2026-06-29T10:05:00.000Z")],
       ),
     ).toBe(true);
-    // Same mixed case, but pr's own claim is later than the sibling's ⇒ pr loses via the fallback.
-    expect(
-      isDuplicateClusterWinnerByClaim(
-        { number: 12, createdAt: "2026-06-29T09:00:00.000Z", linkedIssueClaimedAt: "2026-06-29T10:05:00.000Z" },
-        [{ number: 13, createdAt: null, linkedIssueClaimedAt: "2026-06-29T10:00:00.000Z" }],
-      ),
-    ).toBe(false);
   });
 
-  it("falls back to claim-time comparison when a createdAt value is present but unparseable", () => {
+  it("ignores createdAt even when both sides have valid values", () => {
     expect(
       isDuplicateClusterWinnerByClaim(
-        member(12, "not-a-date", "2026-06-29T10:00:00.000Z"),
-        [member(13, "2026-06-29T09:00:00.000Z", "2026-06-29T10:05:00.000Z")],
+        member(14, "2026-06-29T10:05:00.000Z", "2026-06-29T10:00:00.000Z"),
+        [member(13, "2026-06-29T10:00:00.000Z", "2026-06-29T11:00:00.000Z")],
       ),
     ).toBe(true);
   });
 
-  it("tie-breaks equal createdAt values by PR number, mirroring the claim-time tie-break", () => {
-    expect(isDuplicateClusterWinnerByClaim(member(12, "2026-06-29T10:00:00.000Z", null), [member(13, "2026-06-29T10:00:00.000Z", null)])).toBe(true);
-    expect(isDuplicateClusterWinnerByClaim(member(13, "2026-06-29T10:00:00.000Z", null), [member(12, "2026-06-29T10:00:00.000Z", null)])).toBe(false);
-  });
-
-  it("createdAt-based cases are unaffected by (and do not require) a claim timestamp at all", () => {
-    expect(isDuplicateClusterWinnerByClaim(member(12, "2026-06-29T10:00:00.000Z", null), [member(13, "2026-06-29T10:05:00.000Z", null)])).toBe(true);
+  it("still fails closed when createdAt is present but claim timing is missing", () => {
+    expect(isDuplicateClusterWinnerByClaim(member(12, "2026-06-29T10:00:00.000Z", null), [member(13, "2026-06-29T10:05:00.000Z", null)])).toBe(false);
   });
 });
 
 describe("resolveDuplicateClusterWinnerNumber (#dup-winner-credit)", () => {
   it("returns this PR's own number when it is the winner", () => {
-    expect(resolveDuplicateClusterWinnerNumber({ number: 12, createdAt: "2026-06-29T10:00:00.000Z" }, [{ number: 13, createdAt: "2026-06-29T10:05:00.000Z" }])).toBe(12);
+    expect(resolveDuplicateClusterWinnerNumber({ number: 12, linkedIssueClaimedAt: "2026-06-29T10:00:00.000Z" }, [{ number: 13, linkedIssueClaimedAt: "2026-06-29T10:05:00.000Z" }])).toBe(12);
   });
 
   it("returns the actual winning sibling's number when this PR is a loser, even with multiple siblings", () => {
     expect(
-      resolveDuplicateClusterWinnerNumber({ number: 14, createdAt: "2026-06-29T10:10:00.000Z" }, [
-        { number: 13, createdAt: "2026-06-29T10:00:00.000Z" },
-        { number: 15, createdAt: "2026-06-29T10:05:00.000Z" },
+      resolveDuplicateClusterWinnerNumber({ number: 14, linkedIssueClaimedAt: "2026-06-29T10:10:00.000Z" }, [
+        { number: 13, linkedIssueClaimedAt: "2026-06-29T10:00:00.000Z" },
+        { number: 15, linkedIssueClaimedAt: "2026-06-29T10:05:00.000Z" },
       ]),
     ).toBe(13);
   });
@@ -223,9 +197,9 @@ describe("dupWinnerLinkedDuplicateCount (#dup-winner close-reason seam)", () =>
     expect(dupWinnerLinkedDuplicateCount([], 12, "2026-06-29T10:00:00.000Z", false)).toBe(0);
   });
 
-  it("REGRESSION (#dup-winner true-creation-time): createdAt overrides a claim-time-only verdict when passed through", () => {
-    // By claim time alone this PR (12) would lose to sibling 13 (claimed earlier, 10:00 vs 10:05). But 12's true
-    // createdAt (09:00) precedes 13's (09:30), so passing createdAt flips the verdict to a win (count 0).
+  it("REGRESSION (#dup-winner anti-backdating): createdAt does not override claim-time ordering when passed through", () => {
+    // PR 12 is older, but sibling 13 claimed the linked issue first; passing createdAt must not suppress the
+    // duplicate count for the later claimant.
     expect(
       dupWinnerLinkedDuplicateCount(
         [{ number: 13, linkedIssueClaimedAt: "2026-06-29T10:00:00.000Z", createdAt: "2026-06-29T09:30:00.000Z" }],
@@ -234,21 +208,21 @@ describe("dupWinnerLinkedDuplicateCount (#dup-winner close-reason seam)", () =>
         true,
         "2026-06-29T09:00:00.000Z",
       ),
-    ).toBe(0);
+    ).toBe(1);
   });
 });
 
 describe("dupWinnerLinkedDuplicateWinnerNumber (#dup-winner-credit close-reason naming seam)", () => {
   it("flag OFF ⇒ null regardless of who would win (generic wording, byte-identical to before this existed)", () => {
-    expect(dupWinnerLinkedDuplicateWinnerNumber([{ number: 13, createdAt: "2026-06-29T10:05:00.000Z" }], 12, undefined, false, "2026-06-29T10:00:00.000Z")).toBeNull();
+    expect(dupWinnerLinkedDuplicateWinnerNumber([{ number: 13, linkedIssueClaimedAt: "2026-06-29T10:05:00.000Z" }], 12, "2026-06-29T10:00:00.000Z", false, "2026-06-29T10:00:00.000Z")).toBeNull();
   });
 
   it("winner + flag ON ⇒ null (nothing to name — its own close reason omits the duplicate cause entirely)", () => {
-    expect(dupWinnerLinkedDuplicateWinnerNumber([{ number: 13, createdAt: "2026-06-29T10:05:00.000Z" }], 12, undefined, true, "2026-06-29T10:00:00.000Z")).toBeNull();
+    expect(dupWinnerLinkedDuplicateWinnerNumber([{ number: 13, linkedIssueClaimedAt: "2026-06-29T10:05:00.000Z" }], 12, "2026-06-29T10:00:00.000Z", true, "2026-06-29T10:00:00.000Z")).toBeNull();
   });
 
   it("loser + flag ON ⇒ the actual winning sibling's number", () => {
-    expect(dupWinnerLinkedDuplicateWinnerNumber([{ number: 12, createdAt: "2026-06-29T10:00:00.000Z" }], 14, undefined, true, "2026-06-29T10:10:00.000Z")).toBe(12);
+    expect(dupWinnerLinkedDuplicateWinnerNumber([{ number: 12, linkedIssueClaimedAt: "2026-06-29T10:00:00.000Z" }], 14, "2026-06-29T10:10:00.000Z", true, "2026-06-29T10:10:00.000Z")).toBe(12);
   });
 
   it("loser + flag ON, but the election is too ambiguous ⇒ null (falls back to generic wording)", () => {
@@ -326,7 +300,7 @@ describe("listOtherOpenPullRequests ordering (#audit-3.9)", () => {
   });
 });
 
-describe("upsertPullRequestFromGitHub createdAt threading (#dup-winner true-creation-time)", () => {
+describe("upsertPullRequestFromGitHub createdAt threading", () => {
   it("populates createdAt from GitHub's true pull_request.created_at on the IMMEDIATE upsert return, not just on a later DB round-trip", async () => {
     const env = createTestEnv();
     const record = await upsertPullRequestFromGitHub(env, "owner/repo", {