Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/db/repositories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5256,6 +5256,11 @@ function toPullRequestRecord(repoFullName: string, pr: GitHubPullRequestPayload)
mergeableState: pr.mergeable_state ?? pr.mergeableState ?? mergeableBooleanState(pr.mergeable),
reviewDecision: pr.reviewDecision,
body: pr.body,
// GitHub's true PR-creation time (#dup-winner true-creation-time). Already persisted into payloadJson via
// compactGitHubPayload below and re-surfaced correctly by toPullRequestRecordFromRow on any later read — this
// populates it on the IMMEDIATE upsert return too, so a caller acting on this same call's result (not a
// subsequent DB round-trip) sees the same value instead of `undefined`.
createdAt: pr.created_at,
labels: (pr.labels ?? []).flatMap((label) => (label.name ? [label.name] : [])),
linkedIssues: extractLinkedIssueNumbers(pr.body ?? ""),
};
Expand Down
43 changes: 38 additions & 5 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ import {
PR_PANEL_RETRIGGER_MARKER,
type ContributorProfile,
} from "../signals/engine";
import { isDuplicateClusterWinnerByClaim } from "../signals/duplicate-winner";
import { isDuplicateClusterWinnerByClaim, resolveDuplicateClusterWinnerNumber } from "../signals/duplicate-winner";
import { buildUnifiedReviewDiff, totalAddedLineCount } from "../review/review-diff";
import { estimateReviewEffort } from "../review/review-effort";
import { buildUnifiedCommentBody } from "../review/unified-comment-bridge";
Expand Down Expand Up @@ -2768,6 +2768,8 @@ async function runAgentMaintenancePlanAndExecute(
const approvalsSatisfied =
autoMaintain.requireApprovals === 0 ||
(liveReviewDecision ?? pr.reviewDecision) === "APPROVED";
const duplicateWinnerEnabled = env.GITTENSORY_DUPLICATE_WINNER === "true";
const openDuplicateSiblings = linkedIssueDuplicatePullRequestRecordsForGate(pr, otherOpenPullRequests);
const planned = planAgentMaintenanceActions({
conclusion: gate.conclusion,
blockerTitles: gate.blockers.map((blocker) => blocker.title),
Expand Down Expand Up @@ -2822,10 +2824,21 @@ async function runAgentMaintenancePlanAndExecute(
// (it can still close on its own merits — CI/conflict/blockers). Flag-OFF short-circuits ⇒ the real
// count is used (byte-identical). Sparse legacy rows fail closed so duplicate evidence remains visible.
linkedDuplicateCount: dupWinnerLinkedDuplicateCount(
linkedIssueDuplicatePullRequestRecordsForGate(pr, otherOpenPullRequests),
openDuplicateSiblings,
pr.number,
pr.linkedIssueClaimedAt,
env.GITTENSORY_DUPLICATE_WINNER === "true",
duplicateWinnerEnabled,
pr.createdAt,
),
// #dup-winner-credit: name the cluster's actual winner in a loser's close comment instead of a generic
// "duplicate of another open PR". `null` (flag off, this PR IS the winner, or an ambiguous election)
// falls back to the pre-existing generic wording in agent-actions.ts, byte-identical to before this existed.
linkedDuplicateWinnerNumber: dupWinnerLinkedDuplicateWinnerNumber(
openDuplicateSiblings,
pr.number,
pr.linkedIssueClaimedAt,
duplicateWinnerEnabled,
pr.createdAt,
),
headSha: pr.headSha,
mergeBlockedSha: pr.mergeBlockedSha,
Expand Down Expand Up @@ -7322,19 +7335,39 @@ export async function runAiSlopForAdvisory(
* when count > 0). Flag-OFF (default) returns the real sibling count — byte-identical to today.
*/
export function dupWinnerLinkedDuplicateCount(
openSiblings: Pick<PullRequestRecord, "number" | "linkedIssueClaimedAt">[],
openSiblings: Pick<PullRequestRecord, "number" | "linkedIssueClaimedAt" | "createdAt">[],
prNumber: number,
linkedIssueClaimedAt: string | null | undefined,
duplicateWinnerEnabled: boolean,
createdAt?: string | null | undefined,
): number {
if (
duplicateWinnerEnabled &&
isDuplicateClusterWinnerByClaim({ number: prNumber, linkedIssueClaimedAt }, openSiblings)
isDuplicateClusterWinnerByClaim({ number: prNumber, linkedIssueClaimedAt, createdAt }, openSiblings)
)
return 0;
return openSiblings.length;
}

/**
* Duplicate-winner adjudication (#dup-winner-credit) seam for naming the cluster's actual winner in a loser's
* close comment. Returns `null` (generic "duplicate of another open PR" wording, byte-identical to before this
* existed) when the flag is off, this PR IS the winner (nothing to name — its close reason omits the cause
* entirely via {@link dupWinnerLinkedDuplicateCount}), or the election is too ambiguous to name a specific
* winner ({@link resolveDuplicateClusterWinnerNumber}'s fail-closed `null`).
*/
export function dupWinnerLinkedDuplicateWinnerNumber(
openSiblings: Pick<PullRequestRecord, "number" | "linkedIssueClaimedAt" | "createdAt">[],
prNumber: number,
linkedIssueClaimedAt: string | null | undefined,
duplicateWinnerEnabled: boolean,
createdAt?: string | null | undefined,
): number | null {
if (!duplicateWinnerEnabled) return null;
const winner = resolveDuplicateClusterWinnerNumber({ number: prNumber, linkedIssueClaimedAt, createdAt }, openSiblings);
return winner === null || winner === prNumber ? null : winner;
}

/**
* Live-reconcile the duplicate cluster's open siblings before the winner is elected (#dup-winner / audit #15).
*
Expand Down
11 changes: 10 additions & 1 deletion src/settings/agent-actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,10 @@ export type AgentActionPlanInput = {
slopRisk?: number | null | undefined;
labels: string[];
linkedDuplicateCount?: number | undefined;
// #dup-winner-credit: the elected winner's PR number, when the election is confident enough to name one (see
// dupWinnerLinkedDuplicateWinnerNumber). Only read below when linkedDuplicateCount > 0; null/absent falls
// back to the pre-existing generic "duplicate of another open PR" wording.
linkedDuplicateWinnerNumber?: number | null | undefined;
// RC3 terminal-fail merges: the live head SHA + the SHA at which a prior merge was terminally blocked
// (perms/required-check/conflict). When they match, the merge can't complete for this commit → suppress it.
headSha?: string | null | undefined;
Expand Down Expand Up @@ -1085,7 +1089,12 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne
if (isConflict) closeReasons.push("conflicts with the base branch — resolve and open a fresh PR");
for (const blockerTitle of input.blockerTitles) closeReasons.push(blockerTitle);
if (input.pr.slopRisk != null && input.pr.slopRisk >= slopGateMinScore) closeReasons.push(`slop score ${input.pr.slopRisk} ≥ ${slopGateMinScore}`);
if ((input.pr.linkedDuplicateCount ?? 0) > 0) closeReasons.push("duplicate of another open PR");
if ((input.pr.linkedDuplicateCount ?? 0) > 0)
closeReasons.push(
input.pr.linkedDuplicateWinnerNumber != null
? `duplicate of open PR #${input.pr.linkedDuplicateWinnerNumber}`
: "duplicate of another open PR",
);
if (closeReasons.length === 0) closeReasons.push("the review gate is not satisfied");
// Tagged "heuristic": a verdict-driven close (gate-verdict / duplicate / slop / CI). The close-precision
// breaker downgrades this to a hold when close precision has dropped — UNLESS it is also backed by concrete,
Expand Down
65 changes: 55 additions & 10 deletions src/signals/duplicate-winner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,23 @@
*
* When several OPEN PRs link the same issue (a duplicate cluster), the legacy behavior gate-blocks +
* auto-closes EVERY sibling as a duplicate — no winner survives. With the flag ON, exactly ONE winner is
* spared: the earliest observed linked-issue claimant. Sparse legacy rows that do not yet have claim timing
* fail closed so unknown ordering cannot arbitrarily suppress duplicate evidence. Only the LOSERS are
* blocked/closed; the winner still must pass CI / conflict / gate / linked-issue / slop on its OWN merits.
* spared: the earliest claimant. Sparse legacy rows that do not yet have claim timing fail closed so unknown
* ordering cannot arbitrarily suppress duplicate evidence. Only the LOSERS are blocked/closed; the winner
* still must pass CI / conflict / gate / linked-issue / slop on its OWN merits.
*
* This module is PURE — no IO, no Date, no random — so the same inputs always yield the same verdict and the
* 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.
*
* 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.
Expand All @@ -19,6 +28,8 @@
export type DuplicateClaimMember = {
number: number;
linkedIssueClaimedAt?: string | null | undefined;
/** GitHub's true PR creation time. See the module doc's "ELECTION ORDER" note. */
createdAt?: string | null | undefined;
};

/**
Expand All @@ -37,20 +48,54 @@ export function isDuplicateClusterWinner(prNumber: number, openSiblingNumbers: n
}

/**
* True iff `pr` is the earliest known linked-issue claimant in the open duplicate cluster. Sparse legacy rows
* fail closed; ties between known claim times use PR number.
* 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.
*/
export function isDuplicateClusterWinnerByClaim(pr: DuplicateClaimMember, openSiblings: DuplicateClaimMember[]): boolean {
if (openSiblings.length === 0) return true;
for (const sibling of openSiblings) {
if (!prPrecedesSibling(pr, sibling)) return false;
}
return true;
}

/**
* 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.
*/
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);
if (siblingClaim === null) return false;
if (siblingClaim < prClaim) return false;
if (siblingClaim === prClaim && sibling.number < pr.number) return false;
return true;
}

/**
* The winning PR number among `pr` and its open duplicate siblings, or `null` when the election is not
* determinable (mirrors {@link isDuplicateClusterWinnerByClaim}'s fail-closed semantics — this never guesses a
* specific winner when the ordering data is too sparse/ambiguous to be sure). Used only for DISPLAY (naming the
* winner in a loser's close comment, #dup-winner-credit) — the close/hold decision for any given PR is still
* driven directly by {@link isDuplicateClusterWinnerByClaim}, not by this function's return value.
*/
export function resolveDuplicateClusterWinnerNumber(pr: DuplicateClaimMember, openSiblings: DuplicateClaimMember[]): number | null {
if (isDuplicateClusterWinnerByClaim(pr, openSiblings)) return pr.number;
for (const sibling of openSiblings) {
const siblingClaim = claimTimeMs(sibling.linkedIssueClaimedAt);
if (siblingClaim === null) return false;
if (siblingClaim < prClaim) return false;
if (siblingClaim === prClaim && sibling.number < pr.number) return false;
const rest = openSiblings.filter((other) => other.number !== sibling.number);
if (isDuplicateClusterWinnerByClaim(sibling, [pr, ...rest])) return sibling.number;
}
return true;
return null;
}

function claimTimeMs(value: string | null | undefined): number | null {
Expand Down
7 changes: 6 additions & 1 deletion src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -476,11 +476,16 @@ export type PullRequestRecord = {
mergeableState?: string | null | undefined;
reviewDecision?: string | null | undefined;
body?: string | null | undefined;
/** GitHub's own PR creation time (`pull_request.created_at`) — the ground-truth order contributors actually
* opened their PRs in, independent of when gittensory's own webhook/sweep pipeline happened to observe or
* process this PR. NOT the same as {@link linkedIssueClaimedAt} (gittensory's own sync-time). Preferred for
* duplicate-cluster winner election when present on both sides being compared (#dup-winner). */
createdAt?: string | null | undefined;
updatedAt?: string | null | undefined;
closedAt?: string | null | undefined;
/** First time Gittensory observed this PR claiming one or more linked issues. Used to elect same-issue
* duplicate winners by claim order instead of PR number. */
* duplicate winners by claim order instead of PR number ONLY when {@link createdAt} is unavailable on either
* side of a comparison. */
linkedIssueClaimedAt?: string | null | undefined;
labels: string[];
linkedIssues: number[];
Expand Down
13 changes: 13 additions & 0 deletions test/unit/agent-actions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,19 @@ describe("planAgentMaintenanceActions (#778)", () => {
expect(winnerClose.reason).not.toContain("duplicate of another open PR");
});

it("#dup-winner-credit: names the actual winning PR in the close reason when the election is confident enough", () => {
const named = planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { close: "auto" }, blockerTitles: ["x"], pr: { labels: [], linkedDuplicateCount: 1, linkedDuplicateWinnerNumber: 99 } }));
const namedClose = named.find((a) => a.actionClass === "close")!;
expect(namedClose.reason).toContain("duplicate of open PR #99");
expect(namedClose.reason).not.toContain("duplicate of another open PR");
});

it("#dup-winner-credit: falls back to the generic wording when no winner number is known (null, the nullish arm)", () => {
const generic = planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { close: "auto" }, blockerTitles: ["x"], pr: { labels: [], linkedDuplicateCount: 1, linkedDuplicateWinnerNumber: null } }));
const genericClose = generic.find((a) => a.actionClass === "close")!;
expect(genericClose.reason).toContain("duplicate of another open PR");
});

it("keeps every close cause as a structured closeReasons list for historical audit accuracy", () => {
const plan = planAgentMaintenanceActions(
input({
Expand Down
Loading