feat(review): report a superseded pull request as superseded, not unlinked - #10205
Merged
Conversation
A contributor whose linked issue is closed by a rival PR that merged first is currently told "No linked issue detected — link it explicitly in the PR body". They did link one, correctly, and the advice cannot work: re-linking a closed issue changes nothing. metagraphed#8886 linked issue #8829 at 09:22:36; rival #8881 merged at 09:30:24 and the issue closed one second later, and from then on every evaluation produced the same unactionable hold. confirmedNoOpenLinkedIssue collapses gaming (citing an already-dead issue to clear linkedIssueGateMode: block) with supersession (linking a genuinely open issue that a rival then closed). Only the first is a linking failure. The two separate on facts already in hand, with no new GitHub call: the issue's closed_at postdates the PR's created_at, and a MERGED sibling citing the same issue landed in the window ending at that close. GitHub's issue payload already carries closed_at (#4528) and the linked-issue pass already fetches every linked issue -- it discarded everything but a boolean. The rival comes from our own pull_requests rows; the duplicate machinery cannot see it, because that keys on OPEN siblings and the rival stopped being one when it merged. Every uncertain case resolves to "not superseded": a supersession verdict closes a contributor's PR, so a missing timestamp, an unparseable date, a still-open issue, or an absent rival all leave the existing disposition alone. Ordering is pinned on both axes (ascending issue number, then ascending PR number within one issue) because a result that closes a PR must not depend on database row order. Refs #10168
… checks The supersession resolver (#10168) needs the rivals that merged between a PR's creation and its linked issue's close. Selects only number/merged_at/linked issues, bounded by the window and capped, ordered by ascending merge time so the cap keeps the earliest merges -- the resolver elects the earliest qualifying rival, so dropping those would mis-name the cause. Reads pull_requests rather than the purpose-shaped recent_merged_pull_requests table: that table is no longer written -- its newest row on the Orb is three weeks old -- so a rival that merged minutes ago is absent from it entirely, and a check reading it would silently never fire. Refs #10168
…inked Splits the confirmedNoOpenLinkedIssue verdict in two. A PR that cited an already-dead issue keeps reading as missing_linked_issue -- that is the gaming case the guardrail exists for. A PR whose genuinely-open issue a rival closed after it opened now gets its own code, its own message naming the rival, and an action a contributor can actually take. The two facts are proven before the split fires: the issue outlived this PR's creation, and a merged sibling citing it landed in the window ending at its close. resolveLinkedIssueHasOpenReference already fetched every linked issue and discarded all but a boolean, so the closure facts now ride out of that same pass -- no extra GitHub call. The rival comes from a bounded pull_requests read. Wired through all four gate-evaluating call sites via the shared resolveLinkedIssueAdvisoryContext, so the sweep, the webhook path, the heavy re-review and authorized PR actions cannot disagree about it. Applied to BOTH advisory twins (src/rules/advisory.ts and the engine's gate-advisory.ts), each with its own suite, since a fix to only one side is exactly the drift that pair is kept apart to expose. linked_issue_superseded rides the same linkedIssueGateMode knob it was split out of: a repo that opted into block already asked for this PR to be acted on, and splitting the message must not quietly change WHETHER it is acted on -- only what the contributor is told and which code the ledger records. It joins CONCRETE_EVIDENCE_BLOCKER_CODES on the same footing as its sibling (two recorded timestamps and a merged PR's own linked-issue set; no AI judgment) and CONFIGURED_GATE_BLOCKER_SIGNAL_CODES so its reversals record under their own id. Gated OFF by LOOPOVER_SUPERSEDED_CLOSE: recognising supersession CLOSES the pull request, so it ships dark until shadow-checked against the held backlog. Closes #10168
Drives the split through reReviewStoredPullRequest rather than against buildPullRequestAdvisory directly, so the whole seam is proven: the linked-issue verification pass carrying closure facts out, the bounded merged-rival read, the flag, and the finding the contributor actually sees. Reproduces the Orb's own collision -- PR 8886 citing issue 8829, rival 8881 merging behind it, the issue closing one second later -- and pins that the feature is byte-identical with the flag off. The candidate-window derivation moves into the pure module beside the resolver it serves, so 'can this even be superseded' is one tested unit instead of branches stranded in processors.ts that only an integration test could reach. Refs #10168
Contributor
|
Important 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏳ LoopOver is waiting…LoopOver has seen this pull request and is waiting on CI checks to finish before reviewing it. This comment will update once the review runs. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed · 🟨 Waiting |
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | 67ae721 | Commit Preview URL Branch Preview URL |
Jul 31 2026, 12:33 PM |
This was referenced Jul 31, 2026
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10205 +/- ##
==========================================
- Coverage 92.21% 92.21% -0.01%
==========================================
Files 934 936 +2
Lines 114178 114249 +71
Branches 27593 27618 +25
==========================================
+ Hits 105294 105359 +65
Misses 7582 7582
- Partials 1302 1308 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
|
loopover-orb Bot
pushed a commit
that referenced
this pull request
Jul 31, 2026
… of a 12-argument tail (#10214) addPullRequestFindings exists twice, in the two deliberately-divergent advisory twins, and both took the same POSITIONAL tail. Every new signal had to be threaded in identical ORDER through two files -- a shape where transposing two same-typed arguments compiles cleanly and silently changes a verdict. #10205 added the twelfth argument and paid that tax in both signatures and both call sites; the next one would too. Both copies now take one named object. Each declares its own local type rather than sharing one: keeping these files free of a common import is precisely what the divergence exists for (#4518, keep-divergent recorded for #4881), so the engine still never reaches into the host's graph. Pure refactor, and the evidence is that NOTHING ELSE MOVED: no finding added or removed, no message text touched, no test edited. The diff contains zero changed lines matching findings.push or a finding code literal. The existing advisory suites on both sides, test:engine-parity and engine-parity:drift-check all pass unmodified, which is the whole regression proof -- if an assertion had needed editing, that would have meant behaviour changed. Closes #10210
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A contributor whose linked issue is closed by a rival PR that merged first is currently told:
They did link one, correctly. The advice cannot work — re-linking a closed issue changes nothing. This splits that verdict in two.
confirmedNoOpenLinkedIssuecollapses two different situations:linkedIssueGateMode: block. Real, and unchanged by this PR.The superseded case now gets its own code (
linked_issue_superseded), its own message naming the rival, and an action a contributor can act on.The two facts are proven before the split fires, both from data already in hand:
closed_atpostdates the PR'screated_at— it was open when the PR was written;resolveLinkedIssueHasOpenReferencealready fetched every linked issue and discarded all but a boolean, so the closure facts now ride out of that same pass — no extra GitHub call. The rival comes from a boundedpull_requestsread; the duplicate/overlap machinery cannot see it, because that keys on open siblings and the rival stopped being one the moment it merged.Reproduced from the Orb (
JSONbored/metagraphed): PR 8886 opened 09:22:36 citing issue 8829; PR 8881 merged 09:30:24 citing the same issue; issue 8829 closed 09:30:25. 8886 then produced 56 identicalhold | missing_linked_issueverdicts on one head SHA.Notable decisions
pull_requests, notrecent_merged_pull_requests. That purpose-shaped table stopped being written — its newest row on the Orb is three weeks old — so a rival that merged minutes ago is absent from it and the check would silently never fire. Filed separately.linkedIssueGateModeknob it was split out of. A repo that opted intoblockalready asked for this PR to be acted on; splitting the message must not quietly change whether it is acted on, only what the contributor is told and which code the ledger records.src/rules/advisory.tsand the engine'sgate-advisory.ts— each with its own suite, since a fix to only one side is exactly the drift that pair is kept apart to expose.LOOPOVER_SUPERSEDED_CLOSE. Recognising supersession closes the PR, so it ships dark until shadow-checked against the held backlog.Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #123) — a linked open issue is required for every contributor PR.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changednpm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateFull
npx vitest run: 1357 files, 26465 tests, 0 failures.@loopover/engine: 942 pass.test:engine-parity: 15 pass. Also green:dead-source-files,dead-exports,cf-typegen,import-specifiers,coverage-boltons,engine-parity:drift,docs:drift,db:migrations,typecheck-coverage.src/review/linked-issue-superseded.tsis at 100% statements, branches, functions and lines, and every guard in it was mutation-tested — 12 mutants applied (each guard inverted or removed, both sort keys dropped), 12 killed. The first sweep left 3 survivors; fixing them exposed a real nondeterminism, where two rivals merged in the same second resolved by whichever row the database returned first.If any required check was skipped, explain why:
actionlint,test:workers,test:mcp-pack,ui:lint,ui:buildwere not run locally — this PR touches no workflow, worker-pool, MCP-pack, or UI file. CI runs them all.npm auditreports 13 pre-existing advisories onmain; this PR changes no dependency (nopackage.json/ lockfile diff).Safety
UI Evidencesection below.The one new contributor-visible string names a public PR number and a public issue number, and nothing else.
UI Evidence
Not applicable — no UI, frontend, docs, or extension files are touched.
Notes
The new finding reads:
Per the disposition decision on the issue, this treats a proven supersession as a close, not a hold. The countervailing risk — a superseded PR that also carries work the rival did not cover — is why the action text points the contributor at opening a fresh issue for the remainder, and why the whole thing ships behind a flag that is off.
Closes #10168