Skip to content

Fix stale merged PR badge on the default-branch worktree - #720

Open
Snirpini wants to merge 1 commit into
supabitapp:mainfrom
Snirpini:fix/default-branch-stale-pr
Open

Fix stale merged PR badge on the default-branch worktree#720
Snirpini wants to merge 1 commit into
supabitapp:mainfrom
Snirpini:fix/default-branch-stale-pr

Conversation

@Snirpini

Copy link
Copy Markdown

Closes #695

Summary

A worktree checked out on the repository's default branch showed a stale merged PR badge. Any PR whose headRefName is the default branch is a sync PR (e.g. main → dev), not the default-branch worktree's own work — and because the default branch persists after such a PR merges (unlike a feature branch, which is typically deleted), the newest merged head=<default> PR kept matching the default-branch worktree indefinitely.

The fix skips PR association for the worktree whose name equals the repository's default branch, in refreshRepositoryPullRequests — the point where PRs are mapped to worktrees. The default branch is fetched defensively with try?: if that lookup fails, the refresh falls back to today's behavior rather than dropping every worktree's badge on a transient gh hiccup.

Scope — Option A. This suppresses all PR badges on the default-branch worktree, merged or open. It's the smallest, clearest change and matches the report: a clean default-branch worktree should show just the branch, with no PR attached. A narrower Option B — suppress only merged/closed head=<default> PRs while still surfacing a currently-open sync PR — is also reasonable. I went with A, but the guard is localized to one spot, so switching to B (or adding it) would be a small follow-up there. Happy to change it if you'd prefer to keep open sync PRs visible on the default branch.

Type of change

  • Bug fix (the linked issue is a bug report)
  • Feature (the linked issue is a feature request marked ready)
  • Documentation
  • Other (please describe)

How was this tested?

Two reducer tests added in supacodeTests/RepositoriesFeatureTests.swift:

  • repositoryPullRequestRefreshSkipsDefaultBranchWorktree — the default-branch worktree gets no PR even when batchPullRequests returns a merged head=main PR for it, while a feature worktree still gets its PR. This reproduces Default branch worktree shows a stale merged PR #695 and fails without the fix.

  • repositoryPullRequestRefreshAssignsWhenDefaultBranchLookupFails — when the default-branch lookup throws, the refresh still assigns by branch name, guarding the defensive fallback against a future regression.

  • make check passes (format + lint)

  • make test passes (run with TEST_PARALLEL=NO)

  • I built and ran the app to confirm the change works — verified via the automated tests above; I did not run the GUI app.

AI tool disclosure (optional)

  • Model(s): Claude Opus 4.8
  • Harness / tools: Claude Code

Checklist

  • This pull request is linked to an issue with Closes # above.
  • For a feature, the linked issue is labeled ready. (This is a bug fix; the issue is labeled ready.)
  • I am the author of this work and accountable for it; no commit is authored or co-authored by an AI agent.
  • I have read the Contributing guide and the Code of Conduct.

A PR whose head is the repository's default branch is always a sync PR
(e.g. main -> dev), never the default-branch worktree's own work. Because
the default branch persists after such a PR merges, the newest merged
head=<default> PR permanently matched the default-branch worktree and
showed a stale merged PR badge.

Skip PR association for the worktree on the repository's default branch in
refreshRepositoryPullRequests. Fetch the default branch defensively so a
transient gh failure falls back to the previous behavior instead of
dropping every worktree's badge. Add reducer tests for both the skip and
the defensive fallback.
@Snirpini
Snirpini force-pushed the fix/default-branch-stale-pr branch from 95ad7a4 to a78e330 Compare August 4, 2026 11:55
@Snirpini

Snirpini commented Aug 4, 2026

Copy link
Copy Markdown
Author

Rebased on main (now at 5b32953b) — no conflicts, make check and make test still pass (410 tests).

Still reproduces on current main: refreshRepositoryPullRequests maps prsByBranch[worktree.name] with no default-branch guard, so the newest merged head=main sync PR stays matched to the default-branch worktree indefinitely.

On the open question in the description — I've settled on Option A (suppress all PR badges on the default-branch worktree). It matches the report in #695 and keeps the guard in one spot. If you'd rather keep a currently-open sync PR visible there, that's a small follow-up on the same line and I'm happy to do it.

No rush — just flagging that it's rebased and ready whenever it fits your queue.

@sbertix

sbertix commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Sorry if I'm stalling on this. A bit unsure on the best way to tackle the problem, cause this solution might not be enough.

@Snirpini

Snirpini commented Aug 5, 2026

Copy link
Copy Markdown
Author

Your hesitation was justified — I think my patch fixes this in the wrong place. I've put an alternative up as a branch rather than force-pushing over your review:

compare: main...Snirpini:fix/sync-pr-matching

The bottom line: instead of skipping the default-branch worktree in the reducer, drop any candidate in pullRequestsByBranch where head == defaultBranch && base != defaultBranch. A PR flowing out of the default branch into something else is a sync PR, never the head branch's own work.

That keeps matching policy in one place (next to the baseRefName guards from 4a15af54), drops the gh repo view call my version added to every refresh, and removes the Option A/B question I left you with — as a property of the PR rather than of the worktree, there's nothing to decide. It also covers what #465 reported.

make check and make test pass; the 6 suppression tests fail with the rule disabled, so they're real regression tests.

Why the reducer was the wrong place

The gh repo view --json defaultBranchRef I added was uncached, so it ran per repo per refresh — a second login-shell gh spawn alongside resolveRemoteInfo. It also answers for the wrong repository: in a fork clone gh repo view resolves to upstream, so it returns upstream's default branch, not the one being displayed. This version resolves from local refs via automaticWorktreeBaseRef and caches per repository; an unresolvable default disables the rule rather than dropping badges.

Why head and base, and how #465 fits

Checking both keeps it principled rather than "hide things on main": with dev as the default branch, main → dev is a real contribution and still matches main, while dev → main is dropped from dev. There's a test for each direction.

On #465 — same root cause, reported earlier. One thing I couldn't make work: its suggested fix is to add the missing baseRefName != branch guard to the upstream tier, but as far as I can tell that condition can never hold there, since GitHub won't accept a PR whose head and base are the same ref within one repo. So the tier asymmetry looks real but harmless, and the default-branch signal is doing the actual work. Let me know if I've misread that.

I've kept Closes #695 as the only link since #465 isn't ready — happy to do whatever you prefer with it.

Two gaps I left out on purpose
  • A feature branch whose PR merged, wasn't deleted, and then got new commits still shows the merged badge. That needs headRefOid plus the local tip SHA — git worktree list --porcelain already parses and discards that SHA, so it's cheap, but it's a different change. Happy to open an issue.
  • A branch with two open PRs picks an arbitrary winner by updatedAt; baseRefName is fetched but not used for ranking.

I don't want to assume this is the right direction — you know the sidebar's constraints far better than I do. If you'd rather keep the original, or take neither and shape it differently, that's completely fine. And if there's something I haven't covered or a concern still sitting at the back of your mind, I'd genuinely like to hear it and help get it all covered properly.

Happy to force-push into this PR, open a fresh one, or leave it — whichever suits you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Default branch worktree shows a stale merged PR

2 participants