From a4dbc1f62d3b935e970356f8ce477a103572be43 Mon Sep 17 00:00:00 2001 From: omartelo Date: Sat, 25 Jul 2026 19:32:47 -0300 Subject: [PATCH] fix: retire the pull request badge when the merge lands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The footer badge, the session cards and the sidebar's "Pull request" entry only looked the pull request up again when the checkout's HEAD moved or the window lost focus and got it back. A merge does neither — the commit lands on the base branch, on the remote — so merging from the Pulls screen emptied that screen while every badge around it went on reading "#N Open" until you clicked away to another window and back. The shared lookup now takes subscribers: invalidatePullRequests drops every cached answer and asks the badges to read again. Merging, opening a pull request and the header's reload button call it alongside the screen's own refresh. The check poll and the focus re-read stay the screen's alone — they change nothing the badges show. --- CHANGELOG.md | 8 ++++++ frontend/src/components/pulls/Pulls.tsx | 16 +++++++++-- .../src/lib/pulls/pull-request-lookup.test.ts | 24 +++++++++++++++- frontend/src/lib/pulls/pull-request-lookup.ts | 28 +++++++++++++++++++ frontend/src/lib/pulls/use-pull-request.ts | 13 +++++---- 5 files changed, 80 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 323cf3b7a..f426f3efc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Merging a pull request now clears its badge everywhere, at once.** The + footer, the session cards and the sidebar's "Pull request" entry only looked + the pull request up again when the checkout's HEAD moved, or when the window + lost focus and got it back — and a merge does neither, since the commit lands + on the base branch, on the remote. Merging from the Pulls screen emptied that + screen while every badge around it went on reading `#N Open` until you clicked + away to another window and back. Merging, opening a pull request and the + header's reload button now retire the shared answer and re-read it. - **A model newer than your lich no longer reads its context window as 200k.** Opus 5 sessions showed five times their real usage in the footer, because the window came from a table of the 1M models and anything missing from it fell diff --git a/frontend/src/components/pulls/Pulls.tsx b/frontend/src/components/pulls/Pulls.tsx index 704a9de4b..f245c2e32 100644 --- a/frontend/src/components/pulls/Pulls.tsx +++ b/frontend/src/components/pulls/Pulls.tsx @@ -22,6 +22,7 @@ import { Notice } from "@/components/common/Notice" import { closePulls } from "@/lib/pulls-card-store" import { activeTarget, sessionsOf } from "@/lib/session/sessions" import { useGitStatus } from "@/lib/git/use-git-status" +import { invalidatePullRequests } from "@/lib/pulls/pull-request-lookup" import { usePullRequestDetail } from "@/lib/pulls/use-pull-request-detail" import { useInject } from "@/lib/use-inject" import { cn, errorText } from "@/lib/utils" @@ -68,6 +69,15 @@ export function Pulls() { const { detail, loading, error, refresh } = usePullRequestDetail(path, branch, head) const inject = useInject(sessionId) + // This screen and the badges around it read the same pull request through two + // separate lookups, so a change with HEAD standing still — a merge, a PR + // opened, whatever the reload button was pressed for — has to retire both. + // The check poll and the focus re-read stay this screen's own. + const reload = () => { + invalidatePullRequests() + refresh() + } + // A merged branch leaves its checkout behind — the one cleanup the user would // otherwise walk back to the sidebar for. Refuse a dirty worktree: whatever // was never committed lives only there, and the sidebar's flow is the one that @@ -97,7 +107,7 @@ export function Pulls() { } const onMerged = () => { - refresh() + reload() const merged = `Merged #${detail?.number} into ${detail?.baseRefName}` // Only a worktree checkout has something to clean up; the project's own // directory stays where it is. @@ -122,7 +132,7 @@ export function Pulls() { path={path} head={head} detail={detail} - onRefresh={refresh} + onRefresh={reload} onMerged={onMerged} onInject={inject} /> @@ -130,7 +140,7 @@ export function Pulls() { } else if (loading) { body = } else { - body = + body = } return
{body}
diff --git a/frontend/src/lib/pulls/pull-request-lookup.test.ts b/frontend/src/lib/pulls/pull-request-lookup.test.ts index 192f46dc3..bce5ff229 100644 --- a/frontend/src/lib/pulls/pull-request-lookup.test.ts +++ b/frontend/src/lib/pulls/pull-request-lookup.test.ts @@ -1,5 +1,9 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" -import { lookupPullRequest } from "./pull-request-lookup" +import { + invalidatePullRequests, + lookupPullRequest, + onPullRequestInvalidated, +} from "./pull-request-lookup" // The RPC is the boundary here; stub it so the test asserts how many gh calls // the lookup actually makes. Each test uses its own checkout path, so the @@ -70,6 +74,24 @@ describe("pull-request-lookup", () => { expect(pullRequest).toHaveBeenCalledTimes(2) }) + // Merging moves no HEAD and changes no branch, so an invalidation is the only + // thing that can retire the answer the badge is still showing. + it("asks gh again after an invalidation, inside the share window", async () => { + await lookupPullRequest("/g", "main", "sha1") + invalidatePullRequests() + await lookupPullRequest("/g", "main", "sha1") + expect(pullRequest).toHaveBeenCalledTimes(2) + }) + + it("notifies subscribers until they unsubscribe", () => { + const reload = vi.fn() + const unsubscribe = onPullRequestInvalidated(reload) + invalidatePullRequests() + unsubscribe() + invalidatePullRequests() + expect(reload).toHaveBeenCalledTimes(1) + }) + it("resolves a failed lookup to no pull request", async () => { pullRequest.mockRejectedValueOnce(new Error("gh: not found")) await expect(lookupPullRequest("/e", "main", "sha1")).resolves.toBeNull() diff --git a/frontend/src/lib/pulls/pull-request-lookup.ts b/frontend/src/lib/pulls/pull-request-lookup.ts index 1be4b06f6..cc7efa019 100644 --- a/frontend/src/lib/pulls/pull-request-lookup.ts +++ b/frontend/src/lib/pulls/pull-request-lookup.ts @@ -16,6 +16,34 @@ interface Shared { const shared = new Map() +const listeners = new Set<() => void>() + +// onPullRequestInvalidated registers a badge's re-read, returning its +// unsubscribe. Every badge of every checkout is called, not just the one that +// changed: a merge is a rare, deliberate action, and the callers of one +// checkout still collapse into a single gh call. Key the listeners by path if +// that ever stops being cheap enough. +export function onPullRequestInvalidated(reload: () => void): () => void { + listeners.add(reload) + return () => { + listeners.delete(reload) + } +} + +// invalidatePullRequests drops every shared answer and asks the badges to look +// up again — for a change only lich itself can announce. Merging does not move +// the checkout's HEAD (the commit lands on the base branch, on the remote), so +// without this the footer, the session cards and the sidebar's pull request +// entry keep reading "Open" until the window loses focus and gets it back. +// Clearing the map matters as much as the notify: a re-read inside the share +// window would otherwise replay the pre-merge answer. +export function invalidatePullRequests(): void { + shared.clear() + for (const reload of listeners) { + reload() + } +} + // lookupPullRequest resolves a checkout's open PR, sharing one gh call across // every caller asking about the same checkout+branch+commit at the same time. // It never rejects: a failed lookup (no gh, not a GitHub repo) reads as "no diff --git a/frontend/src/lib/pulls/use-pull-request.ts b/frontend/src/lib/pulls/use-pull-request.ts index 13494a39b..c96653794 100644 --- a/frontend/src/lib/pulls/use-pull-request.ts +++ b/frontend/src/lib/pulls/use-pull-request.ts @@ -1,5 +1,5 @@ import { useEffect, useState } from "react" -import { lookupPullRequest } from "./pull-request-lookup" +import { lookupPullRequest, onPullRequestInvalidated } from "./pull-request-lookup" import type { PullRequest } from "@/lib/api-types" export type { PullRequest } @@ -9,10 +9,11 @@ export type { PullRequest } // round-trip — but it refetches whenever the checkout's HEAD moves, so a PR the // session just opened (or a merge that closed one) reaches the badge without // waiting for a window focus. It also refetches on focus, for a PR opened or -// merged in the browser. Callers asking about the same checkout share one gh -// call (pull-request-lookup). Returns null while loading, on any error, or when -// the branch has no open PR (a merged or closed one is filtered server-side), -// so the caller hides the badge. +// merged in the browser, and on invalidatePullRequests, for a merge landed from +// the Pulls screen — which moves no HEAD at all. Callers asking about the same +// checkout share one gh call (pull-request-lookup). Returns null while loading, +// on any error, or when the branch has no open PR (a merged or closed one is +// filtered server-side), so the caller hides the badge. export function usePullRequest(path: string, branch: string, head: string): PullRequest | null { const [pr, setPr] = useState(null) @@ -35,9 +36,11 @@ export function usePullRequest(path: string, branch: string, head: string): Pull } load() window.addEventListener("focus", load) + const unsubscribe = onPullRequestInvalidated(load) return () => { alive = false window.removeEventListener("focus", load) + unsubscribe() } }, [path, branch, head])