Repository navigation
fix(github): automatic linked-PR clears unselect instead of deleting references - #31989
thefrikidude wants to merge 1 commit into
Conversation
…references The branch-mismatch and merged-divergence refresh clears wrote `linkedPR: null`, which the attachment normalizer treats as removing the item from `linkedItems`. References added via `orca reference add` on a workspace whose own branch has no PR were therefore deleted one per refresh. Clear only the review selection and keep the items. Fixes stablyai#31962 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to
|
|
Closing as superseded by #32263, which already fixes this on main: automatic linked-PR clears now unselect the review via |
ELI5
When you attach pull requests to a workspace whose own branch has no PR, Orca kept quietly forgetting them, one per refresh. Orca was using "stop highlighting this PR" and "remove this PR from the list" as the same action. Now the automatic cleanup only stops highlighting it, and the PR stays in the list.
What Changed
Before: on a workspace whose branch has no PR of its own, each PR refresh (for example opening the hover card) cleared the selected PR. That deleted it from the workspace's references. The next
orca reference addauto-selected the first remaining PR, so the next refresh deleted that one too, until the workspace lost them all.After: the same refresh unselects the PR (
linkedPRbecomesnull) and every reference stays inlinkedItems.Mechanism: the branch-mismatch and merged-divergence clears wrote
{ linkedPR: null }. InnormalizeWorkspaceAttachmentUpdate, a null legacy slot removes the matching item fromlinkedItems. A newbuildUnselectLinkedReviewUpdatehelper (src/renderer/src/store/github/linked-review-unselect.ts) nulls every review slot and passes the currentlinkedItems, so the normalizer unselects without removing. All five automatic clear call sites use it (pull-request-actions.ts,pull-request-execution.tsx2,refresh-event-actions.tsx2).Why
References added with
orca reference addor "Manage links…" should stay until the user removes them. The clears exist so Checks does not stay pinned to a PR from another branch, and unselecting satisfies that.Alternatives considered:
reviews[0]is untouched. That auto-select is now harmless, since the follow-up clear only unselects.Behavior note: a stale branch-linked PR on a normal branch now stays in the workspace's reference list, unselected, instead of vanishing.
Linked Issue
Fixes #31962
Visual Proof
N/A. No visible UI change; the fix changes which metadata the automatic refresh writes. Verified end to end in a running app (see Testing).
Testing
Real app (isolated dev instance via
ORCA_BACKGROUND_LAUNCH=1, driven over CDP; throwaway repo on branchumbrellawithorigin=stablyai/orca; PRs #31982, #31981, #31980 attached withorca reference add), calling the renderer'sfetchPRForBranchfour times:linkedPRafter refreshlinkedItems31981[31981, 31980](#31982 deleted)null[31982, 31981, 31980](unchanged over 4 refreshes)Automated:
linked-review-unselect.test.ts: unselecting keeps both references.github-pr-branch-linked-pr-divergence.test.tsandgithub-pr-branch-coordinator-events.test.tsthat expected the old{ linkedPR: null }payload.pnpm test src/renderer/src/storepasses (490 files) andpnpm tc:webpasses. Fullpnpm lint,pnpm testandpnpm buildwere not run locally; CI will cover them.Platform tested: macOS only.
AI Disclosure
Written with Claude (Sonnet 5.5) via Claude Code. I reviewed the diff and ran the verification above.
Author: X @thefrikidude
Review
AI code-review summary (self-review):
updateWorktreeMetapath, which already handles remote and runtime hosts. ExistingshouldApply*guards are unchanged. Not tested against SSH.Agent skill upstream boundary
Notes
Security, cross-platform (Linux, Windows, macOS), Remote SSH, Mobile, backwards compatibility and performance considered; see Review.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)🤖 Generated with Claude Code