Skip to content

refactor(advisory): give addPullRequestFindings named signals instead of a 12-argument tail #10214

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
loopover-orb merged 1 commit into from
Jul 31, 2026
Merged

refactor(advisory): give addPullRequestFindings named signals instead of a 12-argument tail #10214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
38 changes: 25 additions & 13 deletions packages/loopover-engine/src/advisory/gate-advisory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,14 @@ export function buildPullRequestAdvisory(
action: "Re-deliver the webhook or wait for the next sync.",
});
} else {
addPullRequestFindings(repo, pr, findings, context.otherOpenPullRequests ?? [], Boolean(context.requireLinkedIssue), Boolean(context.duplicateWinnerEnabled), context.linkedIssueAuthorLogins ?? [], Boolean(context.confirmedNoOpenLinkedIssue), context.supersededBy);
addPullRequestFindings(repo, pr, findings, {
otherOpenPullRequests: context.otherOpenPullRequests ?? [],
requireLinkedIssue: Boolean(context.requireLinkedIssue),
duplicateWinnerEnabled: Boolean(context.duplicateWinnerEnabled),
linkedIssueAuthorLogins: context.linkedIssueAuthorLogins ?? [],
confirmedNoOpenLinkedIssue: Boolean(context.confirmedNoOpenLinkedIssue),
supersededBy: context.supersededBy,
});
}
return advisory("pull_request", targetKey, repoFullName, findings, "Pull request advisory generated.", pr?.number, undefined, pr?.headSha ?? undefined);
}
Expand Down Expand Up @@ -295,18 +302,23 @@ function hasDuplicateOverlapCorroboration(pr: PullRequestRecord, otherPr: PullRe
return Boolean(theirsFiles && theirsFiles.length > 0);
}

function addPullRequestFindings(
repo: RepositoryRecord | null,
pr: PullRequestRecord,
findings: AdvisoryFinding[],
otherOpenPullRequests: PullRequestRecord[],
requireLinkedIssue: boolean,
duplicateWinnerEnabled: boolean,
linkedIssueAuthorLogins: (string | null | undefined)[],
confirmedNoOpenLinkedIssue: boolean,
// #10168: present only when the caller proved a rival merged after this PR opened and closed its issue.
supersededBy?: { issueNumber: number; rivalPullNumber: number } | null | undefined,
): void {
/** #10210 (host-parity): the resolved per-PR signals {@link addPullRequestFindings} evaluates, as ONE object
* rather than a positional tail. See the host copy (src/rules/advisory.ts) for the full rationale — in short,
* the tail had to be threaded in identical ORDER through both twins on every addition, and transposing two
* same-typed arguments compiled cleanly. Declared locally rather than shared with the host: keeping these two
* files free of a common import is precisely what the divergence exists for (#4518/#4881). */
type PullRequestFindingSignals = {
otherOpenPullRequests: PullRequestRecord[];
requireLinkedIssue: boolean;
duplicateWinnerEnabled: boolean;
linkedIssueAuthorLogins: (string | null | undefined)[];
confirmedNoOpenLinkedIssue: boolean;
/** #10168: present only when the caller proved a rival merged after this PR opened and closed its issue. */
supersededBy?: { issueNumber: number; rivalPullNumber: number } | null | undefined;
};

function addPullRequestFindings(repo: RepositoryRecord | null, pr: PullRequestRecord, findings: AdvisoryFinding[], signals: PullRequestFindingSignals): void {
const { otherOpenPullRequests, requireLinkedIssue, duplicateWinnerEnabled, linkedIssueAuthorLogins, confirmedNoOpenLinkedIssue, supersededBy } = signals;
if (pr.state !== "open") {
findings.push({
code: "pr_not_open",
Expand Down
79 changes: 49 additions & 30 deletions src/rules/advisory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,20 +448,17 @@ export function buildPullRequestAdvisory(
action: "Re-deliver the webhook or wait for the next sync.",
});
} else {
addPullRequestFindings(
repo,
pr,
findings,
context.otherOpenPullRequests ?? [],
Boolean(context.requireLinkedIssue),
Boolean(context.duplicateWinnerEnabled),
context.linkedIssueAuthorLogins ?? [],
Boolean(context.confirmedNoOpenLinkedIssue),
context.copycatGateMode,
context.copycatGateMinScore,
context.scopedLinkedIssueClaimedAt,
context.supersededBy,
);
addPullRequestFindings(repo, pr, findings, {
otherOpenPullRequests: context.otherOpenPullRequests ?? [],
requireLinkedIssue: Boolean(context.requireLinkedIssue),
duplicateWinnerEnabled: Boolean(context.duplicateWinnerEnabled),
linkedIssueAuthorLogins: context.linkedIssueAuthorLogins ?? [],
confirmedNoOpenLinkedIssue: Boolean(context.confirmedNoOpenLinkedIssue),
copycatGateMode: context.copycatGateMode,
copycatGateMinScore: context.copycatGateMinScore,
scopedLinkedIssueClaimedAt: context.scopedLinkedIssueClaimedAt,
supersededBy: context.supersededBy,
});
}
return advisory("pull_request", targetKey, repoFullName, findings, "Pull request advisory generated.", pr?.number, undefined, pr?.headSha ?? undefined);
}
Expand Down Expand Up @@ -1049,27 +1046,49 @@ function hasDuplicateOverlapCorroboration(pr: PullRequestRecord, otherPr: PullRe
return Boolean(theirsFiles && theirsFiles.length > 0);
}

/** #10210: the resolved per-PR signals {@link addPullRequestFindings} evaluates, as ONE object rather than a
* positional tail. The tail had reached twelve arguments and had to be threaded in the identical ORDER
* through this file and its deliberately-divergent engine twin
* (packages/loopover-engine/src/advisory/gate-advisory.ts) on every addition -- a shape where transposing
* two same-typed arguments compiles cleanly and silently changes the verdict. Named members make that
* class of mistake unrepresentable, and cost nothing at the single call site, which already had a
* `context` object to unpack. Declared locally, NOT shared with the twin: keeping the two files free of a
* common import is the whole point of the divergence (#4518/#4881). */
type PullRequestFindingSignals = {
otherOpenPullRequests: PullRequestRecord[];
requireLinkedIssue: boolean;
duplicateWinnerEnabled: boolean;
linkedIssueAuthorLogins: (string | null | undefined)[];
confirmedNoOpenLinkedIssue: boolean;
copycatGateMode: CopycatGateMode | null | undefined;
copycatGateMinScore: number | null | undefined;
/** #9160: pr's claim time, ALREADY SCOPED by the caller (queue/duplicate-detection.ts's
* resolveScopedLinkedIssueClaimedAt) to only the issue(s) actually contested with an open sibling, instead
* of pr.linkedIssueClaimedAt's blended-across-every-linked-issue value -- see that function's own doc
* comment for why the blended column lets an unrelated, already-linked issue backdate a newly-added one's
* claim. `undefined` (every non-DB caller, like decision-replay.ts) falls back to pr.linkedIssueClaimedAt. */
scopedLinkedIssueClaimedAt?: string | null | undefined;
/** #10168: present only when the caller proved a rival merged after this PR opened and closed its issue. */
supersededBy?: SupersededByRival | null | undefined;
};

function addPullRequestFindings(
repo: RepositoryRecord | null,
pr: PullRequestRecord,
findings: AdvisoryFinding[],
otherOpenPullRequests: PullRequestRecord[],
requireLinkedIssue: boolean,
duplicateWinnerEnabled: boolean,
linkedIssueAuthorLogins: (string | null | undefined)[],
confirmedNoOpenLinkedIssue: boolean,
copycatGateMode: CopycatGateMode | null | undefined,
copycatGateMinScore: number | null | undefined,
// #9160: pr's claim time, ALREADY SCOPED by the caller (queue/duplicate-detection.ts's
// resolveScopedLinkedIssueClaimedAt) to only the issue(s) actually contested with an open sibling, instead of
// pr.linkedIssueClaimedAt's blended-across-every-linked-issue value -- see that function's own doc comment
// for why the blended column lets an unrelated, already-linked issue backdate a newly-added one's claim.
// `undefined` (every caller that hasn't been updated, and every non-DB caller like decision-replay.ts) falls
// back to pr.linkedIssueClaimedAt, byte-identical to before this existed.
scopedLinkedIssueClaimedAt?: string | null | undefined,
// #10168: present only when the caller proved a rival merged after this PR opened and closed its issue.
supersededBy?: SupersededByRival | null | undefined,
signals: PullRequestFindingSignals,
): void {
const {
otherOpenPullRequests,
requireLinkedIssue,
duplicateWinnerEnabled,
linkedIssueAuthorLogins,
confirmedNoOpenLinkedIssue,
copycatGateMode,
copycatGateMinScore,
scopedLinkedIssueClaimedAt,
supersededBy,
} = signals;
if (pr.state !== "open") {
findings.push({
code: "pr_not_open",
Expand Down
Loading