diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e71da6..b6b75ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 into fresh packages showed 4 changed files in the footer and on the session card, while the Review tab listed all of them. The count now asks git for every untracked file; the line totals were already right. +- **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 704a9de..f245c2e 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 192f46d..bce5ff2 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 1be4b06..cc7efa0 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 13494a3..c966537 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])