From 90d8868bfca4dbc58c67172111355e994c8c1ce3 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Thu, 20 Aug 2026 01:38:38 +0300 Subject: [PATCH] fix(ui): keep work status PR checks current --- .../chat/work-status/DOCUMENTATION.md | 6 +- .../work-status/WorkStatusPrimaryGroup.tsx | 8 +-- .../src/stores/useGitHubPrStatusStore.test.ts | 30 +++++++++- .../ui/src/stores/useGitHubPrStatusStore.ts | 60 +++++++++++++------ 4 files changed, 78 insertions(+), 26 deletions(-) diff --git a/packages/ui/src/components/chat/work-status/DOCUMENTATION.md b/packages/ui/src/components/chat/work-status/DOCUMENTATION.md index 16c9e9a4..0d9b9712 100644 --- a/packages/ui/src/components/chat/work-status/DOCUMENTATION.md +++ b/packages/ui/src/components/chat/work-status/DOCUMENTATION.md @@ -89,7 +89,7 @@ which requests only providers enabled for this panel. | Context + cost | `contextUsage.ts` over `useSessionMessages`, `Session.cost` | see below — the store getters cannot serve this | | Branch, ahead/behind, attention | `useGitStore` directory state | warmed via `runBackgroundNetworkTask(ensureStatus)` and refreshed from Git mutation hints | | Changed files | `useGitStore` status `files` + `diffStats` | working tree, not session-authored edits | -| PR + checks | `usePrVisualSummary` | **read-only** | +| PR + checks | `useFreshestPrVisualSummaryForBranch` | **read-only**; follows the freshest remote-keyed entry for the branch | | Subagents | child sessions from `useAllLiveSessions` (`parentID`) + `useAllSessionStatuses` | | | Subagent blockers | directory `permission` / `question` maps | one subscription covers every child | | Usage | `components/usage/usageGroups.ts` over `useQuotaStore` | grouping shared with the mobile popover; presentation is not | @@ -145,6 +145,10 @@ The panel never calls `startWatching`. PR watching is owned by the background tracker, and its concurrency gate exists because per-consumer PR fetches once saturated the browser's connection pool and stalled startup for ~20s. A panel that started a watch per open session would reintroduce exactly that fan-out. +The PR surface can watch a concrete remote while passive readers initially know +only the automatic remote key, so the panel reads the freshest entry for the +directory and branch across remote keys. This keeps its PR and checks rows in +sync with the live PR surface without adding another request owner. ### Changed files come from git status, not the session diff --git a/packages/ui/src/components/chat/work-status/WorkStatusPrimaryGroup.tsx b/packages/ui/src/components/chat/work-status/WorkStatusPrimaryGroup.tsx index a5f12e15..4db88c6c 100644 --- a/packages/ui/src/components/chat/work-status/WorkStatusPrimaryGroup.tsx +++ b/packages/ui/src/components/chat/work-status/WorkStatusPrimaryGroup.tsx @@ -3,7 +3,7 @@ import { useI18n } from '@/lib/i18n'; import { useGitStore } from '@/stores/useGitStore'; import { useRuntimeAPIs } from '@/hooks/useRuntimeAPIs'; import { runBackgroundNetworkTask } from '@/lib/background-network'; -import { getGitHubPrStatusKey, usePrVisualSummary } from '@/stores/useGitHubPrStatusStore'; +import { useFreshestPrVisualSummaryForBranch } from '@/stores/useGitHubPrStatusStore'; import { useSession, useSessionMessages } from '@/sync/sync-context'; import { useConfigStore } from '@/stores/useConfigStore'; import { useUIStore } from '@/stores/useUIStore'; @@ -107,11 +107,7 @@ export const WorkStatusPrimaryGroup: React.FC = ({ sessionId, directory, // Read-only: PR watching is owned by the background tracker. Starting a watch // here would multiply GitHub requests per open session, which is exactly the // fan-out the PR-status concurrency gate exists to prevent. - const prKey = React.useMemo( - () => (directory && branch ? getGitHubPrStatusKey(directory, branch) : null), - [directory, branch], - ); - const prSummary = usePrVisualSummary(prKey); + const prSummary = useFreshestPrVisualSummaryForBranch(directory, branch); // `getCurrentModel` is an imperative getter: its reference never changes, so // calling it in render subscribes to nothing. Subscribe to the selected model diff --git a/packages/ui/src/stores/useGitHubPrStatusStore.test.ts b/packages/ui/src/stores/useGitHubPrStatusStore.test.ts index fb376750..f336da6e 100644 --- a/packages/ui/src/stores/useGitHubPrStatusStore.test.ts +++ b/packages/ui/src/stores/useGitHubPrStatusStore.test.ts @@ -4,7 +4,7 @@ import type { GitHubPullRequestStatus, RuntimeAPIs } from "@/lib/api/types" let runtimeKey = "runtime-a" mock.module("@/lib/runtime-switch", () => ({ getRuntimeKey: () => runtimeKey })) -const { getGitHubPrStatusKey, useGitHubPrStatusStore } = await import("./useGitHubPrStatusStore") +const { getFreshestPrStatusForBranch, getGitHubPrStatusKey, useGitHubPrStatusStore } = await import("./useGitHubPrStatusStore") const deferred = () => { let resolve!: (value: T) => void @@ -38,6 +38,34 @@ describe("GitHub PR status cache ownership", () => { expect(new Set([originA, upstreamA, originB]).size).toBe(3) }) + test("passive branch readers follow the freshest remote-keyed status", () => { + const automatic = getGitHubPrStatusKey("/repo", "feature") + const origin = getGitHubPrStatusKey("/repo", "feature", "origin") + useGitHubPrStatusStore.getState().ensureEntry(automatic) + useGitHubPrStatusStore.getState().ensureEntry(origin) + useGitHubPrStatusStore.getState().updateStatus(automatic, () => ({ + connected: true, + pr: { number: 7, title: "old", url: "u7", state: "open", draft: false, base: "main", head: "feature" }, + checks: { state: "pending", total: 3, success: 1, failure: 0, pending: 2 }, + })) + useGitHubPrStatusStore.getState().updateStatus(origin, () => ({ + connected: true, + pr: { number: 7, title: "current", url: "u7", state: "open", draft: false, base: "main", head: "feature" }, + checks: { state: "success", total: 3, success: 3, failure: 0, pending: 0 }, + })) + useGitHubPrStatusStore.setState((state) => ({ + entries: { + ...state.entries, + [automatic]: { ...state.entries[automatic], lastRefreshAt: 1 }, + [origin]: { ...state.entries[origin], lastRefreshAt: 2 }, + }, + })) + + const freshest = getFreshestPrStatusForBranch(useGitHubPrStatusStore.getState().entries, "/repo", "feature") + expect(freshest?.pr?.title).toBe("current") + expect(freshest?.checks?.pending).toBe(0) + }) + test("rejects a response after params change", async () => { const request = deferred() const github = { prStatus: () => request.promise } as unknown as RuntimeAPIs["github"] diff --git a/packages/ui/src/stores/useGitHubPrStatusStore.ts b/packages/ui/src/stores/useGitHubPrStatusStore.ts index a2e82053..737ec45e 100644 --- a/packages/ui/src/stores/useGitHubPrStatusStore.ts +++ b/packages/ui/src/stores/useGitHubPrStatusStore.ts @@ -238,11 +238,11 @@ const findResolvedSiblingEntry = ( * instead of a single key: the entry being actively watched/refreshed may be * keyed by a concrete remote while the 'auto' entry goes stale. */ -export const getFreshestPrStatusForBranch = ( +const getFreshestPrEntryForBranch = ( entries: Record, directory: string, branch: string, -): GitHubPullRequestStatus | null => { +): PrStatusEntry | null => { const runtimeKey = getRuntimeKey(); let best: PrStatusEntry | null = null; for (const [key, entry] of Object.entries(entries)) { @@ -260,7 +260,15 @@ export const getFreshestPrStatusForBranch = ( best = entry; } } - return best?.status ?? null; + return best; +}; + +export const getFreshestPrStatusForBranch = ( + entries: Record, + directory: string, + branch: string, +): GitHubPullRequestStatus | null => { + return getFreshestPrEntryForBranch(entries, directory, branch)?.status ?? null; }; const getKeysBySignature = (entries: Record, signature: string): string[] => { @@ -951,23 +959,39 @@ const summarySignature = (s: PrVisualSummary): string => const PR_SUMMARY_CACHE_MAX_ENTRIES = 300; const prSummaryCacheByKey = new Map(); +const getCachedPrSummary = (cacheKey: string, entry: PrStatusEntry | null | undefined): PrVisualSummary | null => { + const summary = entry ? deriveSummary(entry) : null; + if (!summary) { + prSummaryCacheByKey.delete(cacheKey); + return null; + } + + const sig = summarySignature(summary); + const cached = prSummaryCacheByKey.get(cacheKey); + if (cached?.sig === sig) return cached.summary; + + if (!cached && prSummaryCacheByKey.size >= PR_SUMMARY_CACHE_MAX_ENTRIES) { + const oldestKey = prSummaryCacheByKey.keys().next().value; + if (oldestKey !== undefined) prSummaryCacheByKey.delete(oldestKey); + } + prSummaryCacheByKey.set(cacheKey, { sig, summary }); + return summary; +}; + export const usePrVisualSummary = (key: string | null): PrVisualSummary | null => { return useGitHubPrStatusStore((state) => { if (!key) return null; - const entry = state.entries[key]; - const summary = entry ? deriveSummary(entry) : null; - if (!summary) { - prSummaryCacheByKey.delete(key); - return null; - } - const sig = summarySignature(summary); - const cached = prSummaryCacheByKey.get(key); - if (cached && cached.sig === sig) return cached.summary; - if (!cached && prSummaryCacheByKey.size >= PR_SUMMARY_CACHE_MAX_ENTRIES) { - const oldestKey = prSummaryCacheByKey.keys().next().value; - if (oldestKey !== undefined) prSummaryCacheByKey.delete(oldestKey); - } - prSummaryCacheByKey.set(key, { sig, summary }); - return summary; + return getCachedPrSummary(key, state.entries[key]); + }); +}; + +export const useFreshestPrVisualSummaryForBranch = ( + directory: string | null, + branch: string | null, +): PrVisualSummary | null => { + const cacheKey = directory && branch ? JSON.stringify(['branch', getRuntimeKey(), directory, branch]) : null; + return useGitHubPrStatusStore((state) => { + if (!directory || !branch || !cacheKey) return null; + return getCachedPrSummary(cacheKey, getFreshestPrEntryForBranch(state.entries, directory, branch)); }); };