fix: solve #4513 — wrong PR shown next to V2 sidebar workspaces#4514
Draft
github-actions[bot] wants to merge 1 commit into
Draft
fix: solve #4513 — wrong PR shown next to V2 sidebar workspaces#4514github-actions[bot] wants to merge 1 commit into
github-actions[bot] wants to merge 1 commit into
Conversation
V2's PR resolution attached the first matching candidate from the GitHub REST list endpoint (sorted by updated desc), so a recently-closed PR could shadow an open one on the same branch, and a long-closed historical PR could linger next to `master` after the branch head had moved past it. Root cause: - `normalizePullRequestCandidates` used `.find` and accepted the first matching head, with no preference for OPEN over CLOSED/MERGED. - The per-workspace assignment in `performProjectRefresh` attached any matched PR unconditionally; V1's `shouldAcceptPRMatch` historical-vs-headSha gate had no V2 counterpart. Fix: - Sort candidates by state rank (OPEN > MERGED > CLOSED), tiebreak on updatedAt, before returning the picked node. - Introduce `shouldAttachHistoricalMatch` and use it before linking — a CLOSED or MERGED PR only attaches when its headRefOid matches the workspace headSha (mirrors V1). - Surface the picked node's state + headSha through `fetchRepoPullRequests` so the per-workspace decision has the data it needs. Tests: - `github-query.test.ts`: open PR is selected even when the closed one has a newer updatedAt. - `pull-requests.test.ts`: stale closed PR on `master` does not attach when workspace HEAD has moved past the PR's head commit. Closes #4513
|
would love to see this merged 🙏🏼 |
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.
Root cause
V2's PR resolution in
packages/host-service/src/runtime/pull-requests/had two gaps relative to V1:normalizePullRequestCandidatesused.findagainst the RESTpulls?head=...&state=allresponse (sorted byupdated desc). A recently-closed PR could be picked over an older open PR on the same head.performProjectRefreshattached any matched PR unconditionally. V1 hasshouldAcceptPRMatch— historical (CLOSED/MERGED) PRs only attach whenheadRefOid === workspace.headSha. V2 had no counterpart, so a long-closed PR onmasterkept reappearing after the branch HEAD had moved on.Fix
github-query.ts: sort candidates by state rank (OPEN > MERGED > CLOSED), tiebreak onupdatedAtdesc. This fixes the "multiple PRs, closed wins" case.pull-requests.ts: introduceshouldAttachHistoricalMatchand gate the per-workspace link assignment. CLOSED/MERGED matches only attach when theirheadShaequalsworkspace.headSha. Surfacestate+headShafromfetchRepoPullRequestsso the per-workspace decision has the data.Mirrors the V1 logic in
apps/desktop/src/lib/trpc/routers/workspaces/utils/github/pr-resolution.ts(shouldAcceptPRMatch+sortPRCandidates) — which is exactly the spread the issue cites as "not present in V1".Tests proving repro + resolution
github-query.test.ts: open PR (new desktop #101, olderupdatedAt) is selected over closed PR (better persistence #100, newerupdatedAt) on the same head.pull-requests.test.ts: a stale closed PR onmasterwhoseheadRefOiddiffers fromworkspace.headShadoes not attach afterrefreshPullRequestsByWorkspaces.Both tests fail on
mainand pass with this patch.Test plan
bun test packages/host-service/src/runtime/pull-requests/— 12 pass, 0 failbun run lint— cleantsc --noEmitfor host-service — cleanCloses #4513
Summary by cubic
Fixes the wrong PR showing next to V2 sidebar workspaces by preferring open PRs and preventing stale closed/merged PRs from attaching after the branch moves. Closes #4513.
updatedAtdesc, ingithub-query.ts.shouldAttachHistoricalMatch: CLOSED/MERGED only attach when PRheadShaequalsworkspace.headSha.stateandheadShathrough the fetch for linking decisions; added targeted tests for both scenarios.Written for commit 3ce2dbb. Summary will update on new commits.