fix(review): elect duplicate-cluster winner by true PR creation time - #3819
Merged
Conversation
…3816) linkedIssueClaimedAt is gittensory's own sync-observation time, not GitHub's real pull_request.created_at. When processing falls out of creation order (a stalled sweep catching up on a backlog, backfill reordering, delayed webhook delivery), the winner election could crown whichever PR gittensory happened to observe first instead of whoever actually opened their PR first, silently mis-crediting a later contributor over an earlier one. Thread GitHub's true creation time (already persisted in payloadJson, already surfaced by toPullRequestRecordFromRow, just never wired into the election) into isDuplicateClusterWinnerByClaim, preferring it over claim time whenever both sides of a comparison have a valid one and falling back to the legacy claim-time comparison unchanged otherwise. Also name the actual winning PR number in a loser's close comment (#3817) instead of the generic "duplicate of another open PR" wording, so a closed contributor can verify their work wasn't silently discarded.
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3819 +/- ##
=======================================
Coverage 93.38% 93.39%
=======================================
Files 323 323
Lines 32727 32746 +19
Branches 11982 11990 +8
=======================================
+ Hits 30563 30582 +19
Misses 1530 1530
Partials 634 634
🚀 New features to boost your workflow:
|
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
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
isDuplicateClusterWinnerByClaimelected a duplicate-cluster's surviving PR bylinkedIssueClaimedAt— gittensory's own sync-observation time, not GitHub's realpull_request.created_at. When processing falls out of creation order (a stalled sweep catching up on a backlog, backfill reordering, delayed webhook delivery), the election could crown whichever PR gittensory happened to observe first instead of whoever actually opened their PR first, silently mis-crediting a later contributor over an earlier one.payloadJson, already surfaced bytoPullRequestRecordFromRow, just never wired into the election) into the winner comparison, preferring it whenever both sides of a comparison have a valid one and falling back to the legacy claim-time comparison unchanged otherwise — so sparse/legacy rows keep their existing fail-closed behavior exactly.toPullRequestRecord(the raw-webhook-payload constructor) to populatecreatedAton the immediate upsert return, matching whattoPullRequestRecordFromRowalready did on a later DB read.duplicate of open PR #N) instead of the generic "duplicate of another open PR" wording, when the election is confident enough to name one — so a closed contributor can verify their work wasn't silently discarded.created_atwas already being persisted; this only wires already-stored data into the election logic that never looked at it.Closes #3816, closes #3817.
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.npm 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=moderateSafety
ui:openapi:check.)UI Evidencesection below. (N/A — no visible UI change.)Notes
created_atwas already persisted intopayloadJsonfor every PR row and already surfaced bytoPullRequestRecordFromRow; this PR only threads that existing field into the duplicate-winner election and one previously-missing constructor.