authorgravatar for 97427849+flamboh@users.noreply.github.comoliver <97427849+flamboh@users.noreply.github.com> 2026-09-20 13:20:10-07:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-09-20 13:20:10-07:00
log9f0c9f725d4553d34e3c91c13d67ebcc58be5b0f
tree8d78aa961c58affcd17e9b09b83d87bda936c07c
parentead1dee22916b414906eafdae5f5a1293f16e637
signature Signed by PGP key B5690EEEBB952194

fix(web): reuse current PR status in the sidebar (#12545)

15 files changed, 604 insertions(+), 171 deletions(-)

apps/server/src/pullRequest/PullRequestService.test.ts+61
...@@ -3404,6 +3404,67 @@ it.effect("a listing narrowed to some projects is its own cache entry", () =>...@@ -3404,6 +3404,67 @@ it.effect("a listing narrowed to some projects is its own cache entry", () =>
3404 }),3404 }),
3405);3405);
34063406
3407it.effect(
3408 "keeps listing freshness tied to read start when filtered reads finish out of order",
3409 () =>
3410 Effect.gen(function* () {
3411 const olderStarted = yield* Deferred.make<void>();
3412 const releaseOlder = yield* Deferred.make<void>();
3413 let reads = 0;
3414 const updatedAt = "2026-07-02T00:00:00Z";
3415 const service = yield* makeService({
3416 projects: [
3417 project({ id: "p1", title: "web", workspaceRoot: "/a", repository: "acme/web" }),
3418 ],
3419 providers: [
3420 fakeProvider("github", {
3421 listChangeRequests: ({ filters }) =>
3422 Effect.gen(function* () {
3423 reads += 1;
3424 const older = filters?.checks === "failing";
3425 if (older) {
3426 yield* Deferred.succeed(olderStarted, undefined);
3427 yield* Deferred.await(releaseOlder);
3428 }
3429 return {
3430 items: [
3431 {
3432 ...changeRequest(1, updatedAt),
3433 checksState: older ? ("failing" as const) : ("passing" as const),
3434 mergeability: older ? ("mergeable" as const) : ("conflicting" as const),
3435 },
3436 ],
3437 truncated: false,
3438 continues: false,
3439 };
3440 }),
3441 }),
3442 ],
3443 });
3444 const olderInput = { state: "open" as const, filters: { checks: "failing" as const } };
3445 const newerInput = { state: "open" as const, filters: { checks: "passing" as const } };
3446
3447 const olderRead = yield* service.list(olderInput).pipe(Effect.forkChild());
3448 yield* Deferred.await(olderStarted);
3449 yield* TestClock.adjust("1 second");
3450 const newer = yield* service.list(newerInput);
3451 yield* Deferred.succeed(releaseOlder, undefined);
3452 const older = yield* Fiber.join(olderRead);
3453
3454 assert.strictEqual(older.entries[0]?.checksState, "failing");
3455 assert.strictEqual(older.entries[0]?.mergeability, "mergeable");
3456 assert.strictEqual(newer.entries[0]?.checksState, "passing");
3457 assert.strictEqual(newer.entries[0]?.mergeability, "conflicting");
3458 assert.strictEqual(typeof older.entries[0]?.observedAt, "number");
3459 assert.strictEqual(typeof newer.entries[0]?.observedAt, "number");
3460 assert.isBelow(older.entries[0]!.observedAt!, newer.entries[0]!.observedAt!);
3461
3462 const cachedOlder = yield* service.list(olderInput);
3463 assert.strictEqual(cachedOlder.entries[0]?.observedAt, older.entries[0]?.observedAt);
3464 assert.strictEqual(reads, 2);
3465 }),
3466);
3467
3407it.effect("keeps unrelated PRs warm after a mutation, explicit refresh, and project turn", () =>3468it.effect("keeps unrelated PRs warm after a mutation, explicit refresh, and project turn", () =>
3408 Effect.gen(function* () {3469 Effect.gen(function* () {
3409 const calls: string[] = [];3470 const calls: string[] = [];
apps/server/src/pullRequest/PullRequestService.ts+23-8
...@@ -611,6 +611,12 @@ function withRateLimitBackoff(...@@ -611,6 +611,12 @@ function withRateLimitBackoff(
611 Record<Exclude<keyof PullRequestProviderApi, keyof typeof wrapped>, never>;611 Record<Exclude<keyof PullRequestProviderApi, keyof typeof wrapped>, never>;
612}612}
613613
614// Capture before the provider read so a slow response keeps its original freshness through caches.
615const observeRead = Effect.fnUntraced(function* <A, E, R>(read: Effect.Effect<A, E, R>) {
616 const observedAt = yield* Clock.currentTimeMillis;
617 return { value: yield* read, observedAt };
618});
619
614export const make = Effect.gen(function* () {620export const make = Effect.gen(function* () {
615 const mergedPullRequests = yield* PubSub.sliding<PullRequestMergeEvent>(64);621 const mergedPullRequests = yield* PubSub.sliding<PullRequestMergeEvent>(64);
616 const pullRequestRefreshes = yield* SubscriptionRef.make(0);622 const pullRequestRefreshes = yield* SubscriptionRef.make(0);
...@@ -1076,6 +1082,7 @@ export const make = Effect.gen(function* () {...@@ -1076,6 +1082,7 @@ export const make = Effect.gen(function* () {
1076 readonly project: SupportedProject;1082 readonly project: SupportedProject;
1077 readonly item: ProviderChangeRequest;1083 readonly item: ProviderChangeRequest;
1078 readonly viewer: string;1084 readonly viewer: string;
1085 readonly observedAt: number;
1079 }): PullRequestListEntry => {1086 }): PullRequestListEntry => {
1080 const viewer = input.viewer.toLowerCase();1087 const viewer = input.viewer.toLowerCase();
1081 return {1088 return {
...@@ -1098,6 +1105,7 @@ export const make = Effect.gen(function* () {...@@ -1098,6 +1105,7 @@ export const make = Effect.gen(function* () {
1098 deletions: input.item.deletions,1105 deletions: input.item.deletions,
1099 createdAt: input.item.createdAt,1106 createdAt: input.item.createdAt,
1100 updatedAt: input.item.updatedAt,1107 updatedAt: input.item.updatedAt,
1108 observedAt: input.observedAt,
1101 ...(input.item.checksState === undefined || input.item.checksState === null1109 ...(input.item.checksState === undefined || input.item.checksState === null
1102 ? {}1110 ? {}
1103 : { checksState: input.item.checksState }),1111 : { checksState: input.item.checksState }),
...@@ -1246,7 +1254,8 @@ export const make = Effect.gen(function* () {...@@ -1246,7 +1254,8 @@ export const make = Effect.gen(function* () {
1246 }),1254 }),
1247 })1255 })
1248 .pipe(1256 .pipe(
1249 Effect.map((page): RepositoryBatch => {1257 observeRead,
1258 Effect.map(({ value: page, observedAt }): RepositoryBatch => {
1250 // The boundary instant was asked for inclusively, so the rows already sent at it1259 // The boundary instant was asked for inclusively, so the rows already sent at it
1251 // come back with the slice. Dropping them here rather than asking for strictly1260 // come back with the slice. Dropping them here rather than asking for strictly
1252 // older is what keeps their neighbours at the same instant from being skipped.1261 // older is what keeps their neighbours at the same instant from being skipped.
...@@ -1262,7 +1271,7 @@ export const make = Effect.gen(function* () {...@@ -1262,7 +1271,7 @@ export const make = Effect.gen(function* () {
1262 key,1271 key,
1263 entries: items1272 entries: items
1264 .filter((item) => matchesRowFilters(item, input.filters, viewer))1273 .filter((item) => matchesRowFilters(item, input.filters, viewer))
1265 .map((item) => toEntry({ project, item, viewer })),1274 .map((item) => toEntry({ project, item, viewer, observedAt })),
1266 errors: [],1275 errors: [],
1267 truncated: page.truncated,1276 truncated: page.truncated,
1268 nextCursor:1277 nextCursor:
...@@ -1323,7 +1332,8 @@ export const make = Effect.gen(function* () {...@@ -1323,7 +1332,8 @@ export const make = Effect.gen(function* () {
1323 ? {}1332 ? {}
1324 : { cursor: { updatedBefore: cursor.updatedBefore, delivered: cursor.delivered } }),1333 : { cursor: { updatedBefore: cursor.updatedBefore, delivered: cursor.delivered } }),
1325 }).pipe(1334 }).pipe(
1326 Effect.flatMap((page) =>1335 observeRead,
1336 Effect.flatMap(({ value: page, observedAt }) =>
1327 Effect.flatMap(Clock.currentTimeMillis, (now) => {1337 Effect.flatMap(Clock.currentTimeMillis, (now) => {
1328 const rows = new Map<string, Array<ProviderChangeRequest>>();1338 const rows = new Map<string, Array<ProviderChangeRequest>>();
1329 for (const [key, visibleAt] of searchVisibleAt) {1339 for (const [key, visibleAt] of searchVisibleAt) {
...@@ -1384,7 +1394,7 @@ export const make = Effect.gen(function* () {...@@ -1384,7 +1394,7 @@ export const make = Effect.gen(function* () {
1384 key: project.cursorKey,1394 key: project.cursorKey,
1385 entries: items1395 entries: items
1386 .filter((item) => matchesRowFilters(item, input.filters, viewer))1396 .filter((item) => matchesRowFilters(item, input.filters, viewer))
1387 .map((item) => toEntry({ project, item, viewer })),1397 .map((item) => toEntry({ project, item, viewer, observedAt })),
1388 errors: [],1398 errors: [],
1389 truncated: page.truncated,1399 truncated: page.truncated,
1390 nextCursor:1400 nextCursor:
...@@ -1551,7 +1561,8 @@ export const make = Effect.gen(function* () {...@@ -1551,7 +1561,8 @@ export const make = Effect.gen(function* () {
1551 : project.api.getChangeRequestSummary(providerInput);1561 : project.api.getChangeRequestSummary(providerInput);
1552 return read.pipe(1562 return read.pipe(
1553 Effect.mapError(toPullRequestError("summary")),1563 Effect.mapError(toPullRequestError("summary")),
1554 Effect.map((changeRequest): PullRequestSummary => ({1564 observeRead,
1565 Effect.map(({ value: changeRequest, observedAt }): PullRequestSummary => ({
1555 provider: project.api.kind,1566 provider: project.api.kind,
1556 projectId: project.project.id,1567 projectId: project.project.id,
1557 repository: project.repository,1568 repository: project.repository,
...@@ -1564,6 +1575,7 @@ export const make = Effect.gen(function* () {...@@ -1564,6 +1575,7 @@ export const make = Effect.gen(function* () {
1564 closedAt: changeRequest.closedAt ?? null,1575 closedAt: changeRequest.closedAt ?? null,
1565 mergedAt: changeRequest.mergedAt ?? null,1576 mergedAt: changeRequest.mergedAt ?? null,
1566 updatedAt: changeRequest.updatedAt,1577 updatedAt: changeRequest.updatedAt,
1578 observedAt,
1567 ...(changeRequest.isDraft === undefined ? {} : { isDraft: changeRequest.isDraft }),1579 ...(changeRequest.isDraft === undefined ? {} : { isDraft: changeRequest.isDraft }),
1568 ...(changeRequest.author === undefined ? {} : { author: changeRequest.author }),1580 ...(changeRequest.author === undefined ? {} : { author: changeRequest.author }),
1569 ...(changeRequest.additions === undefined1581 ...(changeRequest.additions === undefined
...@@ -1634,12 +1646,12 @@ export const make = Effect.gen(function* () {...@@ -1634,12 +1646,12 @@ export const make = Effect.gen(function* () {
1634 host: project.host,1646 host: project.host,
1635 number: input.number,1647 number: input.number,
1636 })1648 })
1637 .pipe(Effect.mapError(toPullRequestError("detail"))),1649 .pipe(Effect.mapError(toPullRequestError("detail")), observeRead),
1638 viewerOf(project),1650 viewerOf(project),
1639 ],1651 ],
1640 { concurrency: 2 },1652 { concurrency: 2 },
1641 ).pipe(1653 ).pipe(
1642 Effect.map(([changeRequest, viewer]): PullRequestDetail => ({1654 Effect.map(([{ value: changeRequest, observedAt }, viewer]): PullRequestDetail => ({
1643 provider: project.api.kind,1655 provider: project.api.kind,
1644 capabilities: project.api.capabilities,1656 capabilities: project.api.capabilities,
1645 projectId: project.project.id,1657 projectId: project.project.id,
...@@ -1664,6 +1676,7 @@ export const make = Effect.gen(function* () {...@@ -1664,6 +1676,7 @@ export const make = Effect.gen(function* () {
1664 baseBranch: changeRequest.baseBranch,1676 baseBranch: changeRequest.baseBranch,
1665 createdAt: changeRequest.createdAt,1677 createdAt: changeRequest.createdAt,
1666 updatedAt: changeRequest.updatedAt,1678 updatedAt: changeRequest.updatedAt,
1679 observedAt,
1667 mergedAt: changeRequest.mergedAt,1680 mergedAt: changeRequest.mergedAt,
1668 closedAt: changeRequest.closedAt,1681 closedAt: changeRequest.closedAt,
1669 reviewers: changeRequest.reviewers,1682 reviewers: changeRequest.reviewers,
...@@ -2894,12 +2907,14 @@ export const make = Effect.gen(function* () {...@@ -2894,12 +2907,14 @@ export const make = Effect.gen(function* () {
2894 closedAt: detail.closedAt,2907 closedAt: detail.closedAt,
2895 mergedAt: detail.mergedAt,2908 mergedAt: detail.mergedAt,
2896 updatedAt: detail.updatedAt,2909 updatedAt: detail.updatedAt,
2910 observedAt: detail.observedAt,
2897 });2911 });
2898 const shouldReplaceHeldSummary = (key: string, next: PullRequestSummary) => {2912 const shouldReplaceHeldSummary = (key: string, next: PullRequestSummary) => {
2899 const current = lastGoodSummary.peek(key);2913 const current = lastGoodSummary.peek(key);
2900 if (current === undefined) return true;2914 if (current === undefined) return true;
2901 if (current.state === "merged" && next.state !== "merged") return false;2915 if (current.state === "merged" && next.state !== "merged") return false;
2902 return next.updatedAt >= current.updatedAt;2916 if (next.updatedAt !== current.updatedAt) return next.updatedAt > current.updatedAt;
2917 return (next.observedAt ?? -Infinity) >= (current.observedAt ?? -Infinity);
2903 };2918 };
2904 const detail: PullRequestService["Service"]["detail"] = (input) => {2919 const detail: PullRequestService["Service"]["detail"] = (input) => {
2905 const key = refCacheKey(input);2920 const key = refCacheKey(input);
apps/server/src/pullRequest/gitHubPullRequestJson.test.ts+8
...@@ -121,6 +121,13 @@ describe("pull request list decoding", () => {...@@ -121,6 +121,13 @@ describe("pull request list decoding", () => {
121 { statusCheckRollup: [{ context: "ci/legacy", state: "ERROR" }] },121 { statusCheckRollup: [{ context: "ci/legacy", state: "ERROR" }] },
122 // Neither a pass, a failure nor a wait is no verdict rather than a green tick.122 // Neither a pass, a failure nor a wait is no verdict rather than a green tick.
123 { statusCheckRollup: [{ name: "lint", status: "COMPLETED", conclusion: "SKIPPED" }] },123 { statusCheckRollup: [{ name: "lint", status: "COMPLETED", conclusion: "SKIPPED" }] },
124 // Cancelled reads as failing here and in the detail header, so the two never flap.
125 {
126 statusCheckRollup: [
127 { name: "lint", status: "COMPLETED", conclusion: "SUCCESS" },
128 { name: "test", status: "COMPLETED", conclusion: "CANCELLED" },
129 ],
130 },
124 { statusCheckRollup: [] },131 { statusCheckRollup: [] },
125 {},132 {},
126 ]),133 ]),
...@@ -132,6 +139,7 @@ describe("pull request list decoding", () => {...@@ -132,6 +139,7 @@ describe("pull request list decoding", () => {
132 "passing",139 "passing",
133 "failing",140 "failing",
134 null,141 null,
142 "failing",
135 null,143 null,
136 null,144 null,
137 ]);145 ]);
apps/server/src/pullRequest/gitHubPullRequestJson.ts+4-3
...@@ -1447,8 +1447,9 @@ function toCheckEntries(...@@ -1447,8 +1447,9 @@ function toCheckEntries(
1447 * GitHub's own indicator reads: a run that has already gone red will not go green by finishing.1447 * GitHub's own indicator reads: a run that has already gone red will not go green by finishing.
1448 *1448 *
1449 * Null rather than "passing" for a head commit with no checks at all, so a repository that runs1449 * Null rather than "passing" for a head commit with no checks at all, so a repository that runs
1450 * none shows nothing instead of a green tick it never earned. Checks whose verdict is neither a1450 * none shows nothing instead of a green tick it never earned. A cancelled run is a failure, as
1451 * pass, a failure nor a wait — skipped, cancelled, neutral — count towards neither.1451 * GitHub's own rollup and the client's detail rollup both read it; skipped and neutral count
1452 * towards neither, so the row and the detail header never disagree about one head commit.
1452 *1453 *
1453 * Counted off the deduped checks rather than the raw rollup, so the word and the list under it1454 * Counted off the deduped checks rather than the raw rollup, so the word and the list under it
1454 * cannot disagree: the run a re-run replaced is not a verdict twice. A row with no name at all is1455 * cannot disagree: the run a re-run replaced is not a verdict twice. A row with no name at all is
...@@ -1463,7 +1464,7 @@ function rollupChecksState(...@@ -1463,7 +1464,7 @@ function rollupChecksState(
1463 ...(raw ?? []).filter(isNamelessCheck).map((check) => toCheckStatus(check)),1464 ...(raw ?? []).filter(isNamelessCheck).map((check) => toCheckStatus(check)),
1464 ];1465 ];
1465 if (statuses.length === 0) return null;1466 if (statuses.length === 0) return null;
1466 if (statuses.includes("failure")) return "failing";1467 if (statuses.includes("failure") || statuses.includes("cancelled")) return "failing";
1467 if (statuses.includes("pending") || statuses.includes("action-required")) return "pending";1468 if (statuses.includes("pending") || statuses.includes("action-required")) return "pending";
1468 return statuses.includes("success") ? "passing" : null;1469 return statuses.includes("success") ? "passing" : null;
1469}1470}
apps/web/src/components/RightPanelTabs.tsx+20-8
...@@ -34,6 +34,7 @@ import {...@@ -34,6 +34,7 @@ import {
34 type ReactNode,34 type ReactNode,
35 useCallback,35 useCallback,
36 useEffect,36 useEffect,
37 useMemo,
37 useRef,38 useRef,
38 useState,39 useState,
39} from "react";40} from "react";
...@@ -62,7 +63,11 @@ import { ScrollArea } from "~/components/ui/scroll-area";...@@ -62,7 +63,11 @@ import { ScrollArea } from "~/components/ui/scroll-area";
62import { PanelTabCloseButton } from "~/components/ui/panel-tab-close-button";63import { PanelTabCloseButton } from "~/components/ui/panel-tab-close-button";
63import { faviconUrlForOrigin } from "~/lib/favicon";64import { faviconUrlForOrigin } from "~/lib/favicon";
64import { useTheme } from "~/hooks/useTheme";65import { useTheme } from "~/hooks/useTheme";
65import { pullRequestEnvironment } from "~/state/pullRequests";66import {
67 newestPullRequestSummary,
68 pullRequestEnvironment,
69 useSharedPullRequestSummary,
70} from "~/state/pullRequests";
66import { useEnvironmentQuery } from "~/state/query";71import { useEnvironmentQuery } from "~/state/query";
67import { COLLAPSED_SIDEBAR_TITLEBAR_INSET_CLASS } from "~/workspaceTitlebar";72import { COLLAPSED_SIDEBAR_TITLEBAR_INSET_CLASS } from "~/workspaceTitlebar";
6873
...@@ -800,19 +805,26 @@ function PullRequestSurfaceIcon({...@@ -800,19 +805,26 @@ function PullRequestSurfaceIcon({
800 },805 },
801 }),806 }),
802 ).data;807 ).data;
808 const reference = useMemo(
809 () => ({
810 projectId: surface.projectId as ProjectId,
811 repository: surface.repository,
812 number: surface.number,
813 }),
814 [surface.projectId, surface.repository, surface.number],
815 );
816 const sharedSummary = useSharedPullRequestSummary(resolvedEnvironmentId, reference, null);
803 // The compact tab intentionally shows lifecycle and draft state only. Conflict warnings have817 // The compact tab intentionally shows lifecycle and draft state only. Conflict warnings have
804 // their own presentation on surfaces that have mergeability, while this tab stays stable as818 // their own presentation on surfaces that have mergeability, while this tab stays stable as
805 // detail data arrives.819 // detail data arrives.
806 const status =820 const status = linkedSnapshot ?? newestPullRequestSummary(detail, sharedSummary) ?? seed ?? null;
807 linkedSnapshot !== null
808 ? linkedSnapshot
809 : detail === null
810 ? (seed ?? null)
811 : { state: detail.state, isDraft: detail.isDraft };
812 if (status === null) {821 if (status === null) {
813 return <PullRequestGlyph.pullRequest className="size-3 shrink-0 text-muted-foreground" />;822 return <PullRequestGlyph.pullRequest className="size-3 shrink-0 text-muted-foreground" />;
814 }823 }
815 const presentation = resolvePullRequestState({ state: status.state, isDraft: status.isDraft });824 const presentation = resolvePullRequestState({
825 state: status.state,
826 isDraft: status.isDraft ?? detail?.isDraft ?? seed?.isDraft ?? false,
827 });
816 return <presentation.Icon className={cn("size-3 shrink-0", presentation.toneClassName)} />;828 return <presentation.Icon className={cn("size-3 shrink-0", presentation.toneClassName)} />;
817}829}
818830
apps/web/src/components/ThreadStatusIndicators.tsx+14-5
...@@ -76,15 +76,24 @@ export function useLinkedThreadPullRequest(...@@ -76,15 +76,24 @@ export function useLinkedThreadPullRequest(
76 );76 );
77 const fallback =77 const fallback =
78 current === null ? ((!supportsLinks ? linkedPullRequest : null) ?? branchPullRequest) : null;78 current === null ? ((!supportsLinks ? linkedPullRequest : null) ?? branchPullRequest) : null;
79 const host = fallback == null ? undefined : parseChangeRequestUrl(fallback.url)?.host;79 // Stable per link: the shared summary effect keys on this object, and a sidebar row must not
80 const reference =80 // touch the cache on every render.
81 fallback == null ? null : { ...fallback, ...(host === undefined ? {} : { host }) };81 const reference = useMemo(() => {
82 if (fallback == null) return null;
83 const host = parseChangeRequestUrl(fallback.url)?.host;
84 return { ...fallback, ...(host === undefined ? {} : { host }) };
85 }, [fallback]);
82 const queried = useEnvironmentQuery(86 const queried = useEnvironmentQuery(
83 !enabled || environmentId === null || reference === null87 !enabled || environmentId === null || reference === null
84 ? null88 ? null
85 : linkedPullRequestDetailAtom({ environmentId, input: reference }),89 : linkedPullRequestDetailAtom({ environmentId, input: reference }),
86 ).data;90 );
87 const detail = useSharedPullRequestSummary(environmentId, reference, queried);91 const detail = useSharedPullRequestSummary(
92 environmentId,
93 reference,
94 queried.data,
95 queried.dataUpdatedAt,
96 );
8897
89 return useMemo(() => {98 return useMemo(() => {
90 if (current !== null) return linkedPullRequestSnapshotStatus(current);99 if (current !== null) return linkedPullRequestSnapshotStatus(current);
apps/web/src/components/pullRequest/PullRequestChecksPopover.tsx+8-2
...@@ -109,6 +109,7 @@ function ChecksBody({...@@ -109,6 +109,7 @@ function ChecksBody({
109export function PullRequestChecksPopover({109export function PullRequestChecksPopover({
110 checksState,110 checksState,
111 checks,111 checks,
112 stale = false,
112 environmentId,113 environmentId,
113 reference,114 reference,
114 threadRef = null,115 threadRef = null,
...@@ -117,6 +118,7 @@ export function PullRequestChecksPopover({...@@ -117,6 +118,7 @@ export function PullRequestChecksPopover({
117 checksState: PullRequestChecksState;118 checksState: PullRequestChecksState;
118 /** The checks already in hand, for the detail header. Absent on a listing row. */119 /** The checks already in hand, for the detail header. Absent on a listing row. */
119 checks?: ReadonlyArray<PullRequestCheck>;120 checks?: ReadonlyArray<PullRequestCheck>;
121 stale?: boolean;
120 environmentId?: EnvironmentId;122 environmentId?: EnvironmentId;
121 reference?: PullRequestRef;123 reference?: PullRequestRef;
122 /** Thread the popover sits beside; a listing row has none. */124 /** Thread the popover sits beside; a listing row has none. */
...@@ -125,7 +127,7 @@ export function PullRequestChecksPopover({...@@ -125,7 +127,7 @@ export function PullRequestChecksPopover({
125}) {127}) {
126 const presentation = pullRequestChecksStatePresentation(checksState);128 const presentation = pullRequestChecksStatePresentation(checksState);
127 // Counts beat the rollup's own wording where they are known, the way GitHub's own header reads.129 // Counts beat the rollup's own wording where they are known, the way GitHub's own header reads.
128 const summary = checks === undefined ? null : summarizePullRequestChecks(checks);130 const summary = checks === undefined || stale ? null : summarizePullRequestChecks(checks);
129 return (131 return (
130 <Popover>132 <Popover>
131 {/* A listing row is itself a button, so the trigger renders as a span: a nested button is133 {/* A listing row is itself a button, so the trigger renders as a span: a nested button is
...@@ -148,7 +150,11 @@ export function PullRequestChecksPopover({...@@ -148,7 +150,11 @@ export function PullRequestChecksPopover({
148 <PopoverPopup align="start" className="w-80 max-w-full" side="bottom">150 <PopoverPopup align="start" className="w-80 max-w-full" side="bottom">
149 <p className="mb-2 font-medium text-sm">{presentation.label}</p>151 <p className="mb-2 font-medium text-sm">{presentation.label}</p>
150 {summary === null ? null : <p className="mb-2 text-muted-foreground text-xs">{summary}</p>}152 {summary === null ? null : <p className="mb-2 text-muted-foreground text-xs">{summary}</p>}
151 {checks !== undefined ? (153 {stale ? (
154 <p className="text-muted-foreground text-xs">
155 Check details are out of date. Refresh the pull request to update them.
156 </p>
157 ) : checks !== undefined ? (
152 <ChecksBody checks={checks} threadRef={threadRef} />158 <ChecksBody checks={checks} threadRef={threadRef} />
153 ) : environmentId !== undefined && reference !== undefined ? (159 ) : environmentId !== undefined && reference !== undefined ? (
154 <LazyChecksBody160 <LazyChecksBody
apps/web/src/components/pullRequest/PullRequestDetailPanel.test.tsx+2-1
...@@ -41,7 +41,8 @@ vi.mock("~/lib/sourceControlActions", () => ({...@@ -41,7 +41,8 @@ vi.mock("~/lib/sourceControlActions", () => ({
41 usePreparePullRequestThreadAction: () => ({ run: prepareThread }),41 usePreparePullRequestThreadAction: () => ({ run: prepareThread }),
42}));42}));
43vi.mock("~/state/use-atom-command", () => ({ useAtomCommand: () => vi.fn() }));43vi.mock("~/state/use-atom-command", () => ({ useAtomCommand: () => vi.fn() }));
44vi.mock("~/state/pullRequests", () => ({44vi.mock("~/state/pullRequests", async (importOriginal) => ({
45 ...(await importOriginal<typeof import("~/state/pullRequests")>()),
45 pullRequestEnvironment: { detail: () => "detail", activity: () => "activity" },46 pullRequestEnvironment: { detail: () => "detail", activity: () => "activity" },
46 usePullRequestTurnRefresh: () => 0,47 usePullRequestTurnRefresh: () => 0,
47 useSharedPullRequestSummary: () => null,48 useSharedPullRequestSummary: () => null,
apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx+189-111
...@@ -78,8 +78,13 @@ import { useProjects, useServerConfigs } from "~/state/entities";...@@ -78,8 +78,13 @@ import { useProjects, useServerConfigs } from "~/state/entities";
78import { useEnvironments, usePrimaryEnvironmentId } from "~/state/environments";78import { useEnvironments, usePrimaryEnvironmentId } from "~/state/environments";
79import { useEnvironmentQuery } from "~/state/query";79import { useEnvironmentQuery } from "~/state/query";
80import { useLiveRefresh } from "~/hooks/useLiveRefresh";80import { useLiveRefresh } from "~/hooks/useLiveRefresh";
81import { pullRequestEnvironment } from "~/state/pullRequests";81import {
82import { usePullRequestTurnRefresh, useSharedPullRequestSummary } from "~/state/pullRequests";82 pullRequestEnvironment,
83 pullRequestListEntryToSummary,
84 newestPullRequestSummary,
85 usePullRequestTurnRefresh,
86 useSharedPullRequestSummary,
87} from "~/state/pullRequests";
83import { useAtomCommand } from "~/state/use-atom-command";88import { useAtomCommand } from "~/state/use-atom-command";
84import { PullRequestStackMenu } from "./PullRequestStackMenu";89import { PullRequestStackMenu } from "./PullRequestStackMenu";
85import { PullRequestThreadLinks } from "./PullRequestThreadLinks";90import { PullRequestThreadLinks } from "./PullRequestThreadLinks";
...@@ -167,6 +172,7 @@ import {...@@ -167,6 +172,7 @@ import {
167 PullRequestMetaLine,172 PullRequestMetaLine,
168 PullRequestReviewOutcomeIcon,173 PullRequestReviewOutcomeIcon,
169 pullRequestChecksState,174 pullRequestChecksState,
175 pullRequestChecksStatePresentation,
170 pullRequestReviewOutcomeToneClassName,176 pullRequestReviewOutcomeToneClassName,
171 resolvePullRequestState,177 resolvePullRequestState,
172 summarizePullRequestChecks,178 summarizePullRequestChecks,
...@@ -521,10 +527,11 @@ export function PullRequestDetailPanel({...@@ -521,10 +527,11 @@ export function PullRequestDetailPanel({
521}) {527}) {
522 const environmentConfigs = useServerConfigs();528 const environmentConfigs = useServerConfigs();
523 const projects = useProjects();529 const projects = useProjects();
524 const repositoryIdentity = projects.find(530 const project = projects.find(
525 (project) =>531 (project) =>
526 project.id === requestedReference.projectId && project.environmentId === environmentId,532 project.id === requestedReference.projectId && project.environmentId === environmentId,
527 )?.repositoryIdentity;533 );
534 const repositoryIdentity = project?.repositoryIdentity;
528 const supportsThreadPullRequests =535 const supportsThreadPullRequests =
529 environmentConfigs.get(environmentId)?.environment.capabilities.threadPullRequests === true;536 environmentConfigs.get(environmentId)?.environment.capabilities.threadPullRequests === true;
530 const reference = useMemo(537 const reference = useMemo(
...@@ -542,6 +549,8 @@ export function PullRequestDetailPanel({...@@ -542,6 +549,8 @@ export function PullRequestDetailPanel({
542 const matchingListEntry =549 const matchingListEntry =
543 listEntry?.projectId === reference.projectId &&550 listEntry?.projectId === reference.projectId &&
544 listEntry.repository.toLowerCase() === reference.repository.toLowerCase() &&551 listEntry.repository.toLowerCase() === reference.repository.toLowerCase() &&
552 (reference.host === undefined ||
553 listEntry.host.toLowerCase() === reference.host.toLowerCase()) &&
545 listEntry.number === reference.number554 listEntry.number === reference.number
546 ? listEntry555 ? listEntry
547 : null;556 : null;
...@@ -675,14 +684,47 @@ export function PullRequestDetailPanel({...@@ -675,14 +684,47 @@ export function PullRequestDetailPanel({
675 cached: cachedDetail,684 cached: cachedDetail,
676 reference,685 reference,
677 });686 });
678 const sharedSummary = useSharedPullRequestSummary(environmentId, reference, resolvedCoreDetail);687 const listSummary = useMemo(
688 () => (matchingListEntry === null ? null : pullRequestListEntryToSummary(matchingListEntry)),
689 [matchingListEntry],
690 );
691 const detailSummary = useMemo(
692 () =>
693 detailQuery.data === null
694 ? null
695 : {
696 ...detailQuery.data,
697 checksState: pullRequestChecksState(detailQuery.data.checks),
698 },
699 [detailQuery.data],
700 );
701 const observedSummary = useSharedPullRequestSummary(
702 environmentId,
703 reference,
704 detailSummary,
705 detailQuery.dataUpdatedAt,
706 );
707 // The list row is also published to the shared cache, but only after this commit's layout
708 // effects run, so it is compared directly rather than trusted to be there already.
709 const sharedSummary = useMemo(
710 () =>
711 newestPullRequestSummary(
712 resolvedCoreDetail,
713 newestPullRequestSummary(observedSummary, listSummary),
714 ),
715 [resolvedCoreDetail, observedSummary, listSummary],
716 );
679 const coreDetail = useMemo(717 const coreDetail = useMemo(
680 () =>718 () =>
681 resolvedCoreDetail === null || sharedSummary === null || sharedSummary === resolvedCoreDetail719 resolvedCoreDetail === null || sharedSummary === null || sharedSummary === resolvedCoreDetail
682 ? resolvedCoreDetail720 ? resolvedCoreDetail
683 : {721 : {
684 ...resolvedCoreDetail,722 ...resolvedCoreDetail,
685 ...sharedSummary,723 title: sharedSummary.title,
724 state: sharedSummary.state,
725 headBranch: sharedSummary.headBranch,
726 baseBranch: sharedSummary.baseBranch,
727 updatedAt: sharedSummary.updatedAt,
686 author: sharedSummary.author ?? resolvedCoreDetail.author,728 author: sharedSummary.author ?? resolvedCoreDetail.author,
687 additions: sharedSummary.additions ?? resolvedCoreDetail.additions,729 additions: sharedSummary.additions ?? resolvedCoreDetail.additions,
688 deletions: sharedSummary.deletions ?? resolvedCoreDetail.deletions,730 deletions: sharedSummary.deletions ?? resolvedCoreDetail.deletions,
...@@ -720,6 +762,7 @@ export function PullRequestDetailPanel({...@@ -720,6 +762,7 @@ export function PullRequestDetailPanel({
720 },762 },
721 [activity, coreDetail],763 [activity, coreDetail],
722 );764 );
765 const handoffSummary = detail ?? sharedSummary;
723 const keybindings = useAtomValue(primaryServerKeybindingsAtom);766 const keybindings = useAtomValue(primaryServerKeybindingsAtom);
724 const { copyToClipboard: copyReference } = useCopyToClipboard<string>({767 const { copyToClipboard: copyReference } = useCopyToClipboard<string>({
725 target: "pull request reference",768 target: "pull request reference",
...@@ -945,9 +988,11 @@ export function PullRequestDetailPanel({...@@ -945,9 +988,11 @@ export function PullRequestDetailPanel({
945 const acting =988 const acting =
946 pickableEnvironments.find((entry) => entry.environmentId === chosenEnvironmentId) ?? null;989 pickableEnvironments.find((entry) => entry.environmentId === chosenEnvironmentId) ?? null;
947 const actingEnvironmentId = acting?.environmentId ?? environmentId;990 const actingEnvironmentId = acting?.environmentId ?? environmentId;
991 const checkoutRoot =
992 acting?.workspaceRoot ?? detail?.workspaceRoot ?? project?.workspaceRoot ?? null;
948 const prepareThread = usePreparePullRequestThreadAction({993 const prepareThread = usePreparePullRequestThreadAction({
949 environmentId: actingEnvironmentId,994 environmentId: actingEnvironmentId,
950 cwd: acting?.workspaceRoot ?? detail?.workspaceRoot ?? null,995 cwd: checkoutRoot,
951 });996 });
952997
953 const finishAction = async (998 const finishAction = async (
...@@ -1178,7 +1223,7 @@ export function PullRequestDetailPanel({...@@ -1178,7 +1223,7 @@ export function PullRequestDetailPanel({
1178 // already work — and it moves the branch under everything else that is open there.1223 // already work — and it moves the branch under everything else that is open there.
1179 mode: "worktree" | "local" = "worktree",1224 mode: "worktree" | "local" = "worktree",
1180 ) => {1225 ) => {
1181 if (!detail || handoff !== null) return;1226 if (!handoffSummary || handoff !== null) return;
1182 if (attachTarget !== null && task !== null) {1227 if (attachTarget !== null && task !== null) {
1183 writeTaskToComposer(attachTarget, task);1228 writeTaskToComposer(attachTarget, task);
1184 toastManager.add({1229 toastManager.add({
...@@ -1188,6 +1233,7 @@ export function PullRequestDetailPanel({...@@ -1188,6 +1233,7 @@ export function PullRequestDetailPanel({
1188 });1233 });
1189 return;1234 return;
1190 }1235 }
1236 if (checkoutRoot === null) return;
1191 setHandoff(kind);1237 setHandoff(kind);
1192 // The menu closes on the press and takes its "Preparing..." label with it, so this is the1238 // The menu closes on the press and takes its "Preparing..." label with it, so this is the
1193 // only thing answering for the checkout. It carries no timeout of its own: a loading toast1239 // only thing answering for the checkout. It carries no timeout of its own: a loading toast
...@@ -1198,7 +1244,10 @@ export function PullRequestDetailPanel({...@@ -1198,7 +1244,10 @@ export function PullRequestDetailPanel({
1198 });1244 });
1199 // Wherever the reader chose to act: the thread, the checkout it is pointed at and the composer1245 // Wherever the reader chose to act: the thread, the checkout it is pointed at and the composer
1200 // the task lands in are all one server's, and picking another one moves all three.1246 // the task lands in are all one server's, and picking another one moves all three.
1201 const projectRef = scopeProjectRef(actingEnvironmentId, acting?.projectId ?? detail.projectId);1247 const projectRef = scopeProjectRef(
1248 actingEnvironmentId,
1249 acting?.projectId ?? handoffSummary.projectId,
1250 );
1202 // The thread is opened before the checkout rather than after it, because the project's setup1251 // The thread is opened before the checkout rather than after it, because the project's setup
1203 // script only runs for a checkout that knows which thread it is for — and a worktree with no1252 // script only runs for a checkout that knows which thread it is for — and a worktree with no
1204 // dependencies installed is not something anyone can test.1253 // dependencies installed is not something anyone can test.
...@@ -1219,7 +1268,7 @@ export function PullRequestDetailPanel({...@@ -1219,7 +1268,7 @@ export function PullRequestDetailPanel({
1219 return;1268 return;
1220 }1269 }
1221 const prepared = await prepareThread.run({1270 const prepared = await prepareThread.run({
1222 reference: detail.url,1271 reference: handoffSummary.url,
1223 mode,1272 mode,
1224 threadId: opened.threadId,1273 threadId: opened.threadId,
1225 });1274 });
...@@ -1348,7 +1397,7 @@ export function PullRequestDetailPanel({...@@ -1348,7 +1397,7 @@ export function PullRequestDetailPanel({
1348 };1397 };
13491398
1350 const startCheckout = (mode: "worktree" | "local") => {1399 const startCheckout = (mode: "worktree" | "local") => {
1351 if (!detail) return;1400 if (!handoffSummary) return;
1352 void startHandoff(`checkout:${mode}`, null, mode);1401 void startHandoff(`checkout:${mode}`, null, mode);
1353 };1402 };
13541403
...@@ -1380,20 +1429,20 @@ export function PullRequestDetailPanel({...@@ -1380,20 +1429,20 @@ export function PullRequestDetailPanel({
1380 baseBranch: detail.baseBranch,1429 baseBranch: detail.baseBranch,
1381 reviewThreads: detail.reviewThreads,1430 reviewThreads: detail.reviewThreads,
1382 comments: detail.comments,1431 comments: detail.comments,
1383 checks: detail.checks,1432 checks: checksStale ? [] : detail.checks,
1384 commentsTruncated: detail.commentsTruncated,1433 commentsTruncated: detail.commentsTruncated,
1385 }),1434 }),
1386 );1435 );
1387 };1436 };
13881437
1389 const startResolveConflicts = () => {1438 const startResolveConflicts = () => {
1390 if (!detail) return;1439 if (!handoffSummary) return;
1391 void startHandoff("conflicts", {1440 void startHandoff("conflicts", {
1392 prompt: buildResolveConflictsPrompt({1441 prompt: buildResolveConflictsPrompt({
1393 number: detail.number,1442 number: handoffSummary.number,
1394 url: detail.url,1443 url: handoffSummary.url,
1395 headBranch: detail.headBranch,1444 headBranch: handoffSummary.headBranch,
1396 baseBranch: detail.baseBranch,1445 baseBranch: handoffSummary.baseBranch,
1397 }),1446 }),
1398 });1447 });
1399 };1448 };
...@@ -1443,7 +1492,17 @@ export function PullRequestDetailPanel({...@@ -1443,7 +1492,17 @@ export function PullRequestDetailPanel({
1443 const can = (action: PullRequestAction) =>1492 const can = (action: PullRequestAction) =>
1444 detail?.capabilities.actions.includes(action) === true &&1493 detail?.capabilities.actions.includes(action) === true &&
1445 detail.viewerPermissions.actions.includes(action);1494 detail.viewerPermissions.actions.includes(action);
1446 const checksState = detail ? pullRequestChecksState(detail.checks) : null;1495 const detailChecksState = detail ? pullRequestChecksState(detail.checks) : null;
1496 const latestChecksState =
1497 sharedSummary?.checksState === undefined ? detailChecksState : sharedSummary.checksState;
1498 // List rollups can omit workflows awaiting approval. Only refreshed detail can clear those.
1499 const checksState =
1500 latestChecksState !== "failing" &&
1501 detail?.checks.some((check) => check.status === "action-required")
1502 ? "pending"
1503 : latestChecksState;
1504 // A newer rollup cannot tell us which runs changed or how many passed.
1505 const checksStale = checksState !== detailChecksState;
1447 // The merge state remains in one stable slot from waiting through completion. Conflicts take1506 // The merge state remains in one stable slot from waiting through completion. Conflicts take
1448 // the slot while they need a person; the armed badge remains beside them so that state is not lost.1507 // the slot while they need a person; the armed badge remains beside them so that state is not lost.
1449 const primaryAction = detail1508 const primaryAction = detail
...@@ -1494,7 +1553,13 @@ export function PullRequestDetailPanel({...@@ -1494,7 +1553,13 @@ export function PullRequestDetailPanel({
1494 const statePresentation = detail1553 const statePresentation = detail
1495 ? resolvePullRequestState({ state: detail.state, isDraft: detail.isDraft })1554 ? resolvePullRequestState({ state: detail.state, isDraft: detail.isDraft })
1496 : null;1555 : null;
1497 const checksSummary = detail ? summarizePullRequestChecks(detail.checks) : null;1556 const checksSummary = checksStale
1557 ? checksState === null
1558 ? "No checks reported"
1559 : pullRequestChecksStatePresentation(checksState).label
1560 : detail
1561 ? summarizePullRequestChecks(detail.checks)
1562 : null;
1498 // Approvals that still stand, and only those. A superseded one is dimmed beside the reviewer1563 // Approvals that still stand, and only those. A superseded one is dimmed beside the reviewer
1499 // who gave it, so counting it here would have the header assert in a number what the row next1564 // who gave it, so counting it here would have the header assert in a number what the row next
1500 // to it has just qualified.1565 // to it has just qualified.
...@@ -1509,10 +1574,108 @@ export function PullRequestDetailPanel({...@@ -1509,10 +1574,108 @@ export function PullRequestDetailPanel({
1509 ).length1574 ).length
1510 : 0;1575 : 0;
15111576
1577 const checkoutControl =
1578 context === "page" ? (
1579 <Menu>
1580 <Tooltip>
1581 <TooltipTrigger
1582 render={
1583 <MenuTrigger
1584 disabled={handoff !== null || checkoutRoot === null}
1585 render={
1586 <Button
1587 size="xs"
1588 variant="outline"
1589 aria-label={handoff?.startsWith("checkout") ? "Checking out..." : "Check out"}
1590 >
1591 <GitBranchIcon aria-hidden className="size-3.5" />
1592 <span className="@max-[35rem]/pr-header:hidden">
1593 {handoff?.startsWith("checkout") ? "Checking out..." : "Check out"}
1594 </span>
1595 <ChevronDownIcon aria-hidden className="size-3.5 text-muted-foreground" />
1596 </Button>
1597 }
1598 />
1599 }
1600 />
1601 <TooltipPopup>Check out this pull request</TooltipPopup>
1602 </Tooltip>
1603 <MenuPopup align="end" side="bottom" className="min-w-72">
1604 <MenuItem onClick={() => startCheckout("worktree")}>
1605 <GitBranchIcon className="mt-1 size-3.5 shrink-0 self-start" />
1606 <span className="flex min-w-0 flex-col">
1607 <span>In a separate worktree</span>
1608 <span className="text-xs text-muted-foreground">
1609 Its own folder and thread. Nothing you have open moves.
1610 </span>
1611 </span>
1612 </MenuItem>
1613 <MenuItem onClick={() => startCheckout("local")}>
1614 <FolderGit2Icon className="mt-1 size-3.5 shrink-0 self-start" />
1615 <span className="flex min-w-0 flex-col">
1616 <span>In this repository</span>
1617 <span className="text-xs text-muted-foreground">
1618 Switches the branch you are working in, like `gh pr checkout`.
1619 </span>
1620 </span>
1621 </MenuItem>
1622 {pickableEnvironments.length > 0 ? (
1623 <ActOnEnvironmentPicker
1624 environments={pickableEnvironments}
1625 value={actingEnvironmentId}
1626 onChange={(next) => setActingScope({ pullRequestKey, environmentId: next })}
1627 disabled={handoff !== null}
1628 />
1629 ) : null}
1630 </MenuPopup>
1631 </Menu>
1632 ) : null;
1633
1634 const resolveConflictsControl = (
1635 <Tooltip>
1636 <TooltipTrigger
1637 render={
1638 <span className="inline-flex shrink-0">
1639 <Button
1640 size="xs"
1641 variant="destructive-outline"
1642 disabled={handoff !== null || (attachTarget === null && checkoutRoot === null)}
1643 onClick={startResolveConflicts}
1644 aria-label={handoff === "conflicts" ? "Preparing..." : "Resolve conflicts"}
1645 >
1646 <PullRequestGlyph.conflicting aria-hidden className="size-3.5" />
1647 <span className="@max-[30rem]/pr-header:hidden">
1648 {handoff === "conflicts" ? "Preparing..." : "Resolve conflicts"}
1649 </span>
1650 </Button>
1651 </span>
1652 }
1653 />
1654 <TooltipPopup side="top">
1655 {handoff === "conflicts" ? "Preparing..." : "Resolve conflicts"}
1656 </TooltipPopup>
1657 </Tooltip>
1658 );
1659
1512 // The list already has the pull request's identity and summary. Keep them on screen1660 // The list already has the pull request's identity and summary. Keep them on screen
1513 // and let the richer detail read replace the remaining placeholders in place.1661 // and let the richer detail read replace the remaining placeholders in place.
1514 if (detailQuery.isPending && !detail) {1662 if (detailQuery.isPending && !detail) {
1515 return <PullRequestDetailGhost seed={matchingListEntry} />;1663 return (
1664 <PullRequestDetailGhost
1665 seed={matchingListEntry}
1666 summary={sharedSummary}
1667 actions={
1668 handoffSummary ? (
1669 <TooltipProvider delay={150} closeDelay={150} timeout={400}>
1670 {checkoutControl}
1671 {handoffSummary.state === "open" && handoffSummary.mergeability === "conflicting"
1672 ? resolveConflictsControl
1673 : null}
1674 </TooltipProvider>
1675 ) : undefined
1676 }
1677 />
1678 );
1516 }1679 }
15171680
1518 return (1681 return (
...@@ -1720,71 +1883,7 @@ export function PullRequestDetailPanel({...@@ -1720,71 +1883,7 @@ export function PullRequestDetailPanel({
1720 threadRef={null}1883 threadRef={null}
1721 />1884 />
1722 ) : null}1885 ) : null}
1723 {/* Checking a pull request out is the reason to open one here at all, so it is a1886 {checkoutControl}
1724 button of its own rather than a side effect of asking an agent for something.
1725 It asks where, because the two answers are not interchangeable: one leaves your
1726 work where it is, the other moves the repository you are standing in. Only on
1727 the page: beside a thread the branch is already checked out right there. */}
1728 {context === "page" ? (
1729 <Menu>
1730 <Tooltip>
1731 <TooltipTrigger
1732 render={
1733 <MenuTrigger
1734 disabled={handoff !== null}
1735 render={
1736 <Button
1737 size="xs"
1738 variant="outline"
1739 aria-label={
1740 handoff?.startsWith("checkout") ? "Checking out..." : "Check out"
1741 }
1742 >
1743 <GitBranchIcon aria-hidden className="size-3.5" />
1744 <span className="@max-[35rem]/pr-header:hidden">
1745 {handoff?.startsWith("checkout") ? "Checking out..." : "Check out"}
1746 </span>
1747 <ChevronDownIcon
1748 aria-hidden
1749 className="size-3.5 text-muted-foreground"
1750 />
1751 </Button>
1752 }
1753 />
1754 }
1755 />
1756 <TooltipPopup>Check out this pull request</TooltipPopup>
1757 </Tooltip>
1758 <MenuPopup align="end" side="bottom" className="min-w-72">
1759 <MenuItem onClick={() => startCheckout("worktree")}>
1760 <GitBranchIcon className="mt-1 size-3.5 shrink-0 self-start" />
1761 <span className="flex min-w-0 flex-col">
1762 <span>In a separate worktree</span>
1763 <span className="text-xs text-muted-foreground">
1764 Its own folder and thread. Nothing you have open moves.
1765 </span>
1766 </span>
1767 </MenuItem>
1768 <MenuItem onClick={() => startCheckout("local")}>
1769 <FolderGit2Icon className="mt-1 size-3.5 shrink-0 self-start" />
1770 <span className="flex min-w-0 flex-col">
1771 <span>In this repository</span>
1772 <span className="text-xs text-muted-foreground">
1773 Switches the branch you are working in, like `gh pr checkout`.
1774 </span>
1775 </span>
1776 </MenuItem>
1777 {pickableEnvironments.length > 0 ? (
1778 <ActOnEnvironmentPicker
1779 environments={pickableEnvironments}
1780 value={actingEnvironmentId}
1781 onChange={(next) => setActingScope({ pullRequestKey, environmentId: next })}
1782 disabled={handoff !== null}
1783 />
1784 ) : null}
1785 </MenuPopup>
1786 </Menu>
1787 ) : null}
1788 {/* Said where the Merge button is, because it is the answer to why nobody has1887 {/* Said where the Merge button is, because it is the answer to why nobody has
1789 pressed it: the merge is already asked for, and the host is holding it. */}1888 pressed it: the merge is already asked for, and the host is holding it. */}
1790 {autoMergeArmed && primaryAction !== "auto-merge-armed" ? (1889 {autoMergeArmed && primaryAction !== "auto-merge-armed" ? (
...@@ -1809,31 +1908,7 @@ export function PullRequestDetailPanel({...@@ -1809,31 +1908,7 @@ export function PullRequestDetailPanel({
1809 </Tooltip>1908 </Tooltip>
1810 ) : null}1909 ) : null}
1811 {primaryAction === "resolve" ? (1910 {primaryAction === "resolve" ? (
1812 <Tooltip>1911 resolveConflictsControl
1813 <TooltipTrigger
1814 render={
1815 <span className="inline-flex shrink-0">
1816 <Button
1817 size="xs"
1818 variant="destructive-outline"
1819 disabled={handoff !== null}
1820 onClick={startResolveConflicts}
1821 aria-label={
1822 handoff === "conflicts" ? "Preparing..." : "Resolve conflicts"
1823 }
1824 >
1825 <PullRequestGlyph.conflicting aria-hidden className="size-3.5" />
1826 <span className="@max-[30rem]/pr-header:hidden">
1827 {handoff === "conflicts" ? "Preparing..." : "Resolve conflicts"}
1828 </span>
1829 </Button>
1830 </span>
1831 }
1832 />
1833 <TooltipPopup side="top">
1834 {handoff === "conflicts" ? "Preparing..." : "Resolve conflicts"}
1835 </TooltipPopup>
1836 </Tooltip>
1837 ) : primaryAction === "ready" ? (1912 ) : primaryAction === "ready" ? (
1838 <Tooltip>1913 <Tooltip>
1839 <TooltipTrigger1914 <TooltipTrigger
...@@ -2474,7 +2549,7 @@ export function PullRequestDetailPanel({...@@ -2474,7 +2549,7 @@ export function PullRequestDetailPanel({
2474 </ToggleGroup>2549 </ToggleGroup>
2475 {tab === "summary" ? (2550 {tab === "summary" ? (
2476 <span className="ml-auto inline-flex shrink-0 items-center">2551 <span className="ml-auto inline-flex shrink-0 items-center">
2477 {workflowApprovalsRequired > 0 && can("approve-workflows") ? (2552 {workflowApprovalsRequired > 0 && !checksStale && can("approve-workflows") ? (
2478 <Tooltip>2553 <Tooltip>
2479 <TooltipTrigger2554 <TooltipTrigger
2480 render={2555 render={
...@@ -2516,6 +2591,7 @@ export function PullRequestDetailPanel({...@@ -2516,6 +2591,7 @@ export function PullRequestDetailPanel({
2516 {checksState !== null ? (2591 {checksState !== null ? (
2517 <PullRequestChecksPopover2592 <PullRequestChecksPopover
2518 checks={detail.checks}2593 checks={detail.checks}
2594 stale={checksStale}
2519 checksState={checksState}2595 checksState={checksState}
2520 threadRef={threadRef}2596 threadRef={threadRef}
2521 />2597 />
...@@ -2649,12 +2725,14 @@ export function PullRequestDetailPanel({...@@ -2649,12 +2725,14 @@ export function PullRequestDetailPanel({
2649 reference={reference}2725 reference={reference}
2650 detail={detail}2726 detail={detail}
2651 activityPending={activityPending}2727 activityPending={activityPending}
2728 checksStale={checksStale}
2652 activityError={activityError}2729 activityError={activityError}
2653 pendingFinding={handoff}2730 pendingFinding={handoff}
2654 fixFindingLabel={handoffLabels.fixFinding}2731 fixFindingLabel={handoffLabels.fixFinding}
2655 fixCheckLabel={handoffLabels.fixCheck}2732 fixCheckLabel={handoffLabels.fixCheck}
2656 onFixFinding={startFixFinding}2733 onFixFinding={startFixFinding}
2657 onRefresh={refreshDetail}2734 onRefresh={refreshDetail}
2735 onRefreshChecks={refreshFromHost}
2658 />2736 />
2659 </div>2737 </div>
2660 ) : null}2738 ) : null}
apps/web/src/components/pullRequest/PullRequestGhosts.tsx+31-13
...@@ -7,8 +7,9 @@...@@ -7,8 +7,9 @@
7 * both themes) and the single `animate-skeleton` pulse, applied once on the container so any7 * both themes) and the single `animate-skeleton` pulse, applied once on the container so any
8 * number of bars costs one opacity animation.8 * number of bars costs one opacity animation.
9 */9 */
10import type { PullRequestListEntry } from "@t3tools/contracts";10import type { PullRequestListEntry, PullRequestSummary } from "@t3tools/contracts";
11import { ArrowLeftIcon } from "lucide-react";11import { ArrowLeftIcon } from "lucide-react";
12import type { ReactNode } from "react";
1213
13import { cn } from "~/lib/utils";14import { cn } from "~/lib/utils";
14import { formatRelativeTimeLabel } from "~/timestampFormat";15import { formatRelativeTimeLabel } from "~/timestampFormat";
...@@ -72,16 +73,33 @@ export function PullRequestListGhost({...@@ -72,16 +73,33 @@ export function PullRequestListGhost({
72 * boundaries in the ghost prevents the loaded pull request from replacing one layout with73 * boundaries in the ghost prevents the loaded pull request from replacing one layout with
73 * another a moment later.74 * another a moment later.
74 */75 */
75export function PullRequestDetailGhost({ seed }: { seed?: PullRequestListEntry | null }) {76export function PullRequestDetailGhost({
77 seed: entry,
78 summary,
79 actions,
80}: {
81 seed?: PullRequestListEntry | null;
82 summary?: PullRequestSummary | null;
83 actions?: ReactNode;
84}) {
85 const seed = summary
86 ? {
87 ...entry,
88 ...summary,
89 isDraft: summary.isDraft ?? entry?.isDraft,
90 }
91 : entry;
76 const statePresentation = seed92 const statePresentation = seed
77 ? resolvePullRequestState({93 ? resolvePullRequestState({
78 state: seed.state,94 state: seed.state,
79 isDraft: seed.isDraft,95 isDraft: seed.isDraft ?? false,
80 })96 })
81 : null;97 : null;
82 const checksPresentation = seed?.checksState98 // Passing list rollups can omit workflows awaiting approval; wait for detail to claim success.
83 ? pullRequestChecksStatePresentation(seed.checksState)99 const checksPresentation =
84 : null;100 seed?.checksState === "failing" || seed?.checksState === "pending"
101 ? pullRequestChecksStatePresentation(seed.checksState)
102 : null;
85103
86 return (104 return (
87 <div105 <div
...@@ -92,7 +110,7 @@ export function PullRequestDetailGhost({ seed }: { seed?: PullRequestListEntry |...@@ -92,7 +110,7 @@ export function PullRequestDetailGhost({ seed }: { seed?: PullRequestListEntry |
92 !seed && "motion-safe:animate-skeleton",110 !seed && "motion-safe:animate-skeleton",
93 )}111 )}
94 >112 >
95 <div className="shrink-0 border-b border-border/60">113 <div className="@container/pr-header shrink-0 border-b border-border/60">
96 <div className="flex h-7 items-center justify-between gap-3 px-4">114 <div className="flex h-7 items-center justify-between gap-3 px-4">
97 <div className="flex min-w-0 flex-1 items-center gap-1.5">115 <div className="flex min-w-0 flex-1 items-center gap-1.5">
98 {seed && statePresentation ? (116 {seed && statePresentation ? (
...@@ -114,8 +132,8 @@ export function PullRequestDetailGhost({ seed }: { seed?: PullRequestListEntry |...@@ -114,8 +132,8 @@ export function PullRequestDetailGhost({ seed }: { seed?: PullRequestListEntry |
114 )}132 )}
115 </div>133 </div>
116 <div className="flex shrink-0 items-center gap-1">134 <div className="flex shrink-0 items-center gap-1">
117 <GhostBar className="h-5 w-16 rounded-md" />135 {actions ?? <GhostBar className="h-5 w-16 rounded-md" />}
118 <GhostBar className="size-5 rounded-md" />136 <GhostBar className="size-6 rounded-md" />
119 </div>137 </div>
120 </div>138 </div>
121139
...@@ -129,7 +147,7 @@ export function PullRequestDetailGhost({ seed }: { seed?: PullRequestListEntry |...@@ -129,7 +147,7 @@ export function PullRequestDetailGhost({ seed }: { seed?: PullRequestListEntry |
129 {seed ? (147 {seed ? (
130 <>148 <>
131 <PullRequestActorLabel149 <PullRequestActorLabel
132 actor={seed.author}150 actor={seed.author ?? null}
133 className="font-medium"151 className="font-medium"
134 tooltip={false}152 tooltip={false}
135 />153 />
...@@ -165,8 +183,8 @@ export function PullRequestDetailGhost({ seed }: { seed?: PullRequestListEntry |...@@ -165,8 +183,8 @@ export function PullRequestDetailGhost({ seed }: { seed?: PullRequestListEntry |
165 <GhostBar className="w-10" />183 <GhostBar className="w-10" />
166 {seed ? (184 {seed ? (
167 <PullRequestDiffStat185 <PullRequestDiffStat
168 additions={seed.additions}186 additions={seed.additions ?? 0}
169 deletions={seed.deletions}187 deletions={seed.deletions ?? 0}
170 className="font-mono text-xs"188 className="font-mono text-xs"
171 />189 />
172 ) : (190 ) : (
...@@ -217,7 +235,7 @@ export function PullRequestDetailGhost({ seed }: { seed?: PullRequestListEntry |...@@ -217,7 +235,7 @@ export function PullRequestDetailGhost({ seed }: { seed?: PullRequestListEntry |
217 <GhostBar className="w-10" />235 <GhostBar className="w-10" />
218 </div>236 </div>
219 <div className="flex items-center gap-1">237 <div className="flex items-center gap-1">
220 {seed ? (238 {seed?.labels ? (
221 seed.labels.slice(0, 3).map((label) => {239 seed.labels.slice(0, 3).map((label) => {
222 const color = pullRequestLabelColor(label.color);240 const color = pullRequestLabelColor(label.color);
223 return (241 return (
apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx+12-1
...@@ -461,18 +461,21 @@ export function PullRequestSummaryTab({...@@ -461,18 +461,21 @@ export function PullRequestSummaryTab({
461 reference,461 reference,
462 detail,462 detail,
463 activityPending,463 activityPending,
464 checksStale = false,
464 activityError,465 activityError,
465 pendingFinding,466 pendingFinding,
466 fixFindingLabel = "Fix in a thread",467 fixFindingLabel = "Fix in a thread",
467 fixCheckLabel = "Fix",468 fixCheckLabel = "Fix",
468 onFixFinding,469 onFixFinding,
469 onRefresh,470 onRefresh,
471 onRefreshChecks = onRefresh,
470}: {472}: {
471 environmentId: EnvironmentId;473 environmentId: EnvironmentId;
472 threadRef: ScopedThreadRef | null;474 threadRef: ScopedThreadRef | null;
473 reference: PullRequestRef;475 reference: PullRequestRef;
474 detail: PullRequestDetailView;476 detail: PullRequestDetailView;
475 activityPending: boolean;477 activityPending: boolean;
478 checksStale?: boolean;
476 activityError: string | null;479 activityError: string | null;
477 /** The hand-off currently preparing, if any, so only the finding it belongs to says so. */480 /** The hand-off currently preparing, if any, so only the finding it belongs to says so. */
478 pendingFinding?: string | null;481 pendingFinding?: string | null;
...@@ -480,6 +483,7 @@ export function PullRequestSummaryTab({...@@ -480,6 +483,7 @@ export function PullRequestSummaryTab({
480 fixCheckLabel?: string;483 fixCheckLabel?: string;
481 onFixFinding?: (finding: PullRequestFinding) => void;484 onFixFinding?: (finding: PullRequestFinding) => void;
482 onRefresh: () => void;485 onRefresh: () => void;
486 onRefreshChecks?: () => void;
483}) {487}) {
484 // Keyed by the pull request, so opening another one starts at the end of its conversation488 // Keyed by the pull request, so opening another one starts at the end of its conversation
485 // rather than wherever the last one had been read back to.489 // rather than wherever the last one had been read back to.
...@@ -872,7 +876,14 @@ export function PullRequestSummaryTab({...@@ -872,7 +876,14 @@ export function PullRequestSummaryTab({
872 </Section>876 </Section>
873877
874 <Section key={`checks:${detail.url}`} title="Checks" defaultOpen={false}>878 <Section key={`checks:${detail.url}`} title="Checks" defaultOpen={false}>
875 {detail.checks.length === 0 ? (879 {checksStale ? (
880 <div className="flex items-center gap-2 text-xs text-muted-foreground">
881 <span>Check details are out of date.</span>
882 <Button size="xs" variant="ghost" onClick={onRefreshChecks}>
883 Refresh
884 </Button>
885 </div>
886 ) : detail.checks.length === 0 ? (
876 <p className="text-xs text-muted-foreground">No checks reported.</p>887 <p className="text-xs text-muted-foreground">No checks reported.</p>
877 ) : (888 ) : (
878 detail.checks.map((check, index) => {889 detail.checks.map((check, index) => {
apps/web/src/state/pullRequests.test.ts created+83
...@@ -0,0 +1,83 @@
1import { ProjectId, type PullRequestSummary } from "@t3tools/contracts";
2import { describe, expect, it } from "vite-plus/test";
3
4import { newestPullRequestObservation, newestPullRequestSummary } from "./pullRequests";
5
6function summary(overrides: Partial<PullRequestSummary> = {}): PullRequestSummary {
7 return {
8 provider: "github",
9 projectId: ProjectId.make("pull-request-cache-test"),
10 repository: "acme/widget",
11 number: 7,
12 title: "Improve widget",
13 url: "https://github.com/acme/widget/pull/7",
14 state: "open",
15 isDraft: false,
16 headBranch: "improve-widget",
17 baseBranch: "main",
18 updatedAt: "2026-09-10T00:00:00Z",
19 author: { login: "oliver", name: null, avatarUrl: null },
20 mergeability: "mergeable",
21 checksState: "passing",
22 ...overrides,
23 };
24}
25
26const observed = (value: PullRequestSummary, observedAt: number) => ({
27 summary: value,
28 observedAt,
29});
30
31describe("pull request summary cache", () => {
32 it("orders same-dated snapshots by the server's read time, not by arrival", () => {
33 const stale = observed(summary({ observedAt: 200 }), 500);
34 const held = observed(
35 summary({ mergeability: "conflicting", checksState: "failing", observedAt: 300 }),
36 100,
37 );
38 expect(newestPullRequestObservation(stale, held)?.summary).toMatchObject({
39 mergeability: "conflicting",
40 checksState: "failing",
41 observedAt: 300,
42 });
43 // An older filtered or server-cached response finishing last must not roll status back.
44 expect(newestPullRequestObservation(held, stale)).toBe(held);
45 // The same read seen again is not a new observation.
46 expect(newestPullRequestObservation(held, { ...held, observedAt: 900 })).toBe(held);
47 });
48
49 it("uses arrival order only when neither snapshot carries a server read time", () => {
50 const first = observed(summary(), 100);
51 const later = observed(summary({ checksState: "failing" }), 200);
52 expect(newestPullRequestObservation(first, later)).toMatchObject({ observedAt: 200 });
53 expect(newestPullRequestObservation(later, first)).toBe(later);
54 // A stamped read beats an unstamped one regardless of which arrived last.
55 const stamped = observed(summary({ observedAt: 50 }), 1);
56 expect(newestPullRequestObservation(later, stamped)?.summary.observedAt).toBe(50);
57 expect(newestPullRequestObservation(stamped, later)).toBe(stamped);
58 });
59
60 it("keeps known status when a newer snapshot omits it, and clears it when told to", () => {
61 const list = observed(summary({ reviewDecision: "approved", observedAt: 100 }), 100);
62 const sparse = observed(
63 summary({ checksState: undefined, reviewDecision: undefined, observedAt: 200 }),
64 200,
65 );
66 expect(newestPullRequestObservation(list, sparse)?.summary).toMatchObject({
67 checksState: "passing",
68 reviewDecision: "approved",
69 });
70 const cleared = observed(summary({ checksState: null, observedAt: 300 }), 300);
71 expect(newestPullRequestObservation(list, cleared)?.summary.checksState).toBeNull();
72 });
73
74 it("treats merged as final and otherwise prefers the later host update", () => {
75 const merged = summary({ state: "merged", updatedAt: "2026-09-01T00:00:00Z" });
76 const reopened = summary({ updatedAt: "2026-09-12T00:00:00Z", observedAt: 900 });
77 expect(newestPullRequestSummary(merged, reopened)).toBe(merged);
78 expect(newestPullRequestSummary(reopened, merged)).toBe(merged);
79 const older = summary({ updatedAt: "2026-09-11T00:00:00Z", observedAt: 999 });
80 expect(newestPullRequestSummary(older, reopened)).toBe(reopened);
81 expect(newestPullRequestSummary(reopened, older)).toBe(reopened);
82 });
83});
apps/web/src/state/pullRequests.ts+141-19
...@@ -8,6 +8,7 @@ import type {...@@ -8,6 +8,7 @@ import type {
8 EnvironmentId,8 EnvironmentId,
9 PullRequestListInput,9 PullRequestListInput,
10 PullRequestListStatsInput,10 PullRequestListStatsInput,
11 PullRequestListEntry,
11 PullRequestRef,12 PullRequestRef,
12 PullRequestSummary,13 PullRequestSummary,
13} from "@t3tools/contracts";14} from "@t3tools/contracts";
...@@ -30,49 +31,147 @@ export const linkedPullRequestDetailAtom = createLinkedPullRequestSummaryAtomFam...@@ -30,49 +31,147 @@ export const linkedPullRequestDetailAtom = createLinkedPullRequestSummaryAtomFam
30 pullRequestEnvironment.refreshes,31 pullRequestEnvironment.refreshes,
31);32);
3233
34export interface ObservedPullRequestSummary {
35 readonly summary: PullRequestSummary;
36 /** Client arrival time, the only ordering older servers leave us for same-dated snapshots. */
37 readonly observedAt: number;
38}
39
33const observedPullRequestSummaryAtom = Atom.family((key: string) =>40const observedPullRequestSummaryAtom = Atom.family((key: string) =>
34 Atom.make<PullRequestSummary | null>(null).pipe(41 Atom.make<ObservedPullRequestSummary | null>(null).pipe(
35 Atom.setIdleTTL(5 * 60_000),42 Atom.setIdleTTL(5 * 60_000),
36 Atom.withLabel(`web-pull-requests:observed-summary:${key}`),43 Atom.withLabel(`web-pull-requests:observed-summary:${key}`),
37 ),44 ),
38);45);
3946
47/**
48 * Positive when `incoming` is the newer snapshot. Merged is final. Then the host's own update
49 * time, then the server's read-start time, which survives its caches; a snapshot without one
50 * never beats a stamped read. Zero when neither side carries a read time.
51 */
52function compareSummaries(current: PullRequestSummary, incoming: PullRequestSummary): number {
53 const merged = Number(incoming.state === "merged") - Number(current.state === "merged");
54 if (merged !== 0) return merged;
55 const updated = Date.parse(incoming.updatedAt) - Date.parse(current.updatedAt);
56 if (updated !== 0) return updated;
57 if (current.observedAt === undefined && incoming.observedAt === undefined) return 0;
58 return (incoming.observedAt ?? -Infinity) - (current.observedAt ?? -Infinity);
59}
60
40export function newestPullRequestSummary(61export function newestPullRequestSummary(
41 current: PullRequestSummary | null,62 current: PullRequestSummary | null,
42 observed: PullRequestSummary | null,63 observed: PullRequestSummary | null,
43): PullRequestSummary | null {64): PullRequestSummary | null {
44 if (current === null) return observed;65 if (current === null) return observed;
45 if (observed === null) return current;66 if (observed === null) return current;
46 if (current.state === "merged") return current;67 return compareSummaries(current, observed) >= 0 ? observed : current;
47 if (observed.state === "merged") return observed;68}
48 return Date.parse(observed.updatedAt) >= Date.parse(current.updatedAt) ? observed : current;69
70/** Reuse list status without treating its deferred line-count placeholders as real stats. */
71export function pullRequestListEntryToSummary(entry: PullRequestListEntry): PullRequestSummary {
72 return {
73 provider: entry.provider,
74 projectId: entry.projectId,
75 repository: entry.repository,
76 number: entry.number,
77 title: entry.title,
78 url: entry.url,
79 state: entry.state,
80 isDraft: entry.isDraft,
81 headBranch: entry.headBranch,
82 baseBranch: entry.baseBranch,
83 updatedAt: entry.updatedAt,
84 ...(entry.observedAt === undefined ? {} : { observedAt: entry.observedAt }),
85 author: entry.author,
86 ...(entry.reviewDecision === undefined ? {} : { reviewDecision: entry.reviewDecision }),
87 ...(entry.checksState === undefined ? {} : { checksState: entry.checksState }),
88 mergeability: entry.mergeability,
89 };
90}
91
92// A project has one remote, so its id already pins the host. Leaving the host out lets a list
93// row, a hostless legacy reference and a URL-derived thread reference share one entry.
94function pullRequestSummaryKey(environmentId: EnvironmentId, reference: PullRequestRef): string {
95 return JSON.stringify([
96 environmentId,
97 reference.projectId,
98 reference.repository.toLowerCase(),
99 reference.number,
100 ]);
101}
102
103/** The observation to hold after `incoming` arrives. Returns `current` itself on a tie. */
104export function newestPullRequestObservation(
105 current: ObservedPullRequestSummary | null,
106 incoming: ObservedPullRequestSummary | null,
107): ObservedPullRequestSummary | null {
108 if (current === null) return incoming;
109 if (incoming === null) return current;
110 let order = compareSummaries(current.summary, incoming.summary);
111 // Client clocks only break ties between snapshots that both lack a server read time.
112 if (
113 order === 0 &&
114 current.summary.observedAt === undefined &&
115 incoming.summary.observedAt === undefined
116 ) {
117 order = incoming.observedAt - current.observedAt;
118 }
119 if (!(order > 0)) return current;
120 // A sparse summary must not erase known status, or carry old detail stats into a new list read.
121 return {
122 ...incoming,
123 summary: {
124 ...incoming.summary,
125 isDraft: incoming.summary.isDraft ?? current.summary.isDraft,
126 mergeability: incoming.summary.mergeability ?? current.summary.mergeability,
127 reviewDecision:
128 incoming.summary.reviewDecision === undefined
129 ? current.summary.reviewDecision
130 : incoming.summary.reviewDecision,
131 checksState:
132 incoming.summary.checksState === undefined
133 ? current.summary.checksState
134 : incoming.summary.checksState,
135 },
136 };
137}
138
139function observePullRequestSummary(
140 environmentId: EnvironmentId,
141 reference: PullRequestRef,
142 summary: PullRequestSummary,
143 observedAt: number,
144): void {
145 const atom = observedPullRequestSummaryAtom(pullRequestSummaryKey(environmentId, reference));
146 appAtomRegistry.modify(atom, (previous) => {
147 const next = newestPullRequestObservation(previous, { summary, observedAt });
148 return next === previous ? [false, previous] : [true, next];
149 });
49}150}
50151
51export function useSharedPullRequestSummary(152export function useSharedPullRequestSummary(
52 environmentId: EnvironmentId | null,153 environmentId: EnvironmentId | null,
53 reference: PullRequestRef | null,154 reference: PullRequestRef | null,
54 current: PullRequestSummary | null,155 current: PullRequestSummary | null,
156 observedAt: number | null = null,
55): PullRequestSummary | null {157): PullRequestSummary | null {
56 const key =158 const key =
57 environmentId === null || reference === null159 environmentId === null || reference === null
58 ? "none"160 ? "none"
59 : JSON.stringify([161 : pullRequestSummaryKey(environmentId, reference);
60 environmentId,
61 reference.projectId,
62 reference.host?.toLowerCase() ?? null,
63 reference.repository.toLowerCase(),
64 reference.number,
65 ]);
66 const atom = observedPullRequestSummaryAtom(key);162 const atom = observedPullRequestSummaryAtom(key);
67 const observed = useAtomValue(atom);163 const observed = useAtomValue(atom);
68 useLayoutEffect(() => {164 useLayoutEffect(() => {
69 if (environmentId === null || current === null) return;165 if (environmentId === null || reference === null || current === null || observedAt === null)
70 appAtomRegistry.modify(atom, (previous) => {166 return;
71 const next = newestPullRequestSummary(previous, current);167 observePullRequestSummary(environmentId, reference, current, observedAt);
72 return next === previous ? [false, previous] : [true, next];168 }, [current, environmentId, reference, observedAt]);
73 });169 return (
74 }, [atom, current, environmentId]);170 newestPullRequestObservation(
75 return newestPullRequestSummary(current, observed);171 observed,
172 current === null || observedAt === null ? null : { summary: current, observedAt },
173 )?.summary ?? current
174 );
76}175}
77export const pullRequestStackAtom = createPullRequestStackAtomFamily(176export const pullRequestStackAtom = createPullRequestStackAtomFamily(
78 connectionAtomRuntime,177 connectionAtomRuntime,
...@@ -90,6 +189,7 @@ interface MergedEnvironmentQueryView<A> {...@@ -90,6 +189,7 @@ interface MergedEnvironmentQueryView<A> {
90 /** The first environment that failed. Others may still have answered — this is not fatal. */189 /** The first environment that failed. Others may still have answered — this is not fatal. */
91 readonly error: string | null;190 readonly error: string | null;
92 readonly isPending: boolean;191 readonly isPending: boolean;
192 readonly observations: ReadonlyArray<readonly [EnvironmentId, A, number]>;
93}193}
94194
95/**195/**
...@@ -110,6 +210,7 @@ function createMergedEnvironmentQuery<Input, A>(...@@ -110,6 +210,7 @@ function createMergedEnvironmentQuery<Input, A>(
110 Atom.make((get): MergedEnvironmentQueryView<A> => {210 Atom.make((get): MergedEnvironmentQueryView<A> => {
111 const targets = JSON.parse(key) as ReadonlyArray<EnvironmentQueryTarget<Input>>;211 const targets = JSON.parse(key) as ReadonlyArray<EnvironmentQueryTarget<Input>>;
112 const values: Array<readonly [EnvironmentId, A]> = [];212 const values: Array<readonly [EnvironmentId, A]> = [];
213 const observations: Array<readonly [EnvironmentId, A, number]> = [];
113 let error: string | null = null;214 let error: string | null = null;
114 let isPending = false;215 let isPending = false;
115 for (const target of targets) {216 for (const target of targets) {
...@@ -120,12 +221,16 @@ function createMergedEnvironmentQuery<Input, A>(...@@ -120,12 +221,16 @@ function createMergedEnvironmentQuery<Input, A>(
120 }221 }
121 const value = Option.getOrNull(AsyncResult.value(result));222 const value = Option.getOrNull(AsyncResult.value(result));
122 if (value !== null) values.push([target.environmentId, value]);223 if (value !== null) values.push([target.environmentId, value]);
224 if (result._tag === "Success") {
225 observations.push([target.environmentId, result.value, result.timestamp]);
226 }
123 }227 }
124 return { values, error, isPending };228 return { values, error, isPending, observations };
125 }).pipe(Atom.withLabel(`${label}:${key}`)),229 }).pipe(Atom.withLabel(`${label}:${key}`)),
126 );230 );
127 const empty = Atom.make<MergedEnvironmentQueryView<A>>({231 const empty = Atom.make<MergedEnvironmentQueryView<A>>({
128 values: [],232 values: [],
233 observations: [],
129 error: null,234 error: null,
130 isPending: false,235 isPending: false,
131 }).pipe(Atom.withLabel(`${label}:empty`));236 }).pipe(Atom.withLabel(`${label}:empty`));
...@@ -187,6 +292,23 @@ export function usePullRequestList(...@@ -187,6 +292,23 @@ export function usePullRequestList(
187 targets: ReadonlyArray<EnvironmentQueryTarget<PullRequestListInput>>,292 targets: ReadonlyArray<EnvironmentQueryTarget<PullRequestListInput>>,
188): MergedPullRequestListView {293): MergedPullRequestListView {
189 const query = usePullRequestListsQuery(targets);294 const query = usePullRequestListsQuery(targets);
295 useLayoutEffect(() => {
296 for (const [environmentId, answer, observedAt] of query.observations) {
297 for (const entry of answer.entries) {
298 observePullRequestSummary(
299 environmentId,
300 {
301 projectId: entry.projectId,
302 host: entry.host,
303 repository: entry.repository,
304 number: entry.number,
305 },
306 pullRequestListEntryToSummary(entry),
307 observedAt,
308 );
309 }
310 }
311 }, [query.observations]);
190 const data = useMemo(() => mergePullRequestLists(query.values), [query.values]);312 const data = useMemo(() => mergePullRequestLists(query.values), [query.values]);
191 return { data, error: query.error, isPending: query.isPending, refresh: query.refresh };313 return { data, error: query.error, isPending: query.isPending, refresh: query.refresh };
192}314}
apps/web/src/state/query.ts+2
...@@ -9,6 +9,7 @@ const EMPTY_ASYNC_RESULT_ATOM = Atom.make(AsyncResult.initial<never, never>(fals...@@ -9,6 +9,7 @@ const EMPTY_ASYNC_RESULT_ATOM = Atom.make(AsyncResult.initial<never, never>(fals
99
10export interface EnvironmentQueryView<A> {10export interface EnvironmentQueryView<A> {
11 readonly data: A | null;11 readonly data: A | null;
12 readonly dataUpdatedAt: number | null;
12 readonly error: string | null;13 readonly error: string | null;
13 readonly isPending: boolean;14 readonly isPending: boolean;
14 readonly isSuccess: boolean;15 readonly isSuccess: boolean;
...@@ -30,6 +31,7 @@ export function useEnvironmentQuery<A, E>(...@@ -30,6 +31,7 @@ export function useEnvironmentQuery<A, E>(
30 const refresh = useAtomRefresh(selectedAtom);31 const refresh = useAtomRefresh(selectedAtom);
31 return {32 return {
32 data: Option.getOrNull(AsyncResult.value(result)),33 data: Option.getOrNull(AsyncResult.value(result)),
34 dataUpdatedAt: result._tag === "Success" ? result.timestamp : null,
33 error: result._tag === "Failure" ? formatEnvironmentQueryError(result.cause) : null,35 error: result._tag === "Failure" ? formatEnvironmentQueryError(result.cause) : null,
34 isPending: atom !== null && result.waiting,36 isPending: atom !== null && result.waiting,
35 isSuccess: result._tag === "Success",37 isSuccess: result._tag === "Success",
packages/contracts/src/pullRequest.ts+6
...@@ -525,6 +525,8 @@ export const PullRequestListEntry = Schema.Struct({...@@ -525,6 +525,8 @@ export const PullRequestListEntry = Schema.Struct({
525 deletions: NonNegativeInt,525 deletions: NonNegativeInt,
526 createdAt: IsoDateTime,526 createdAt: IsoDateTime,
527 updatedAt: IsoDateTime,527 updatedAt: IsoDateTime,
528 /** Server epoch milliseconds when the provider read started; preserved on cache hits. */
529 observedAt: Schema.optional(Schema.Finite),
528 viewerReviewRequested: Schema.Boolean,530 viewerReviewRequested: Schema.Boolean,
529 labels: Schema.Array(PullRequestLabel),531 labels: Schema.Array(PullRequestLabel),
530 /** Absent where the host does not summarise its reviews, which is every host but GitHub. */532 /** Absent where the host does not summarise its reviews, which is every host but GitHub. */
...@@ -736,6 +738,8 @@ export const PullRequestSummary = Schema.Struct({...@@ -736,6 +738,8 @@ export const PullRequestSummary = Schema.Struct({
736 closedAt: Schema.optional(Schema.NullOr(Schema.String)),738 closedAt: Schema.optional(Schema.NullOr(Schema.String)),
737 mergedAt: Schema.optional(Schema.NullOr(Schema.String)),739 mergedAt: Schema.optional(Schema.NullOr(Schema.String)),
738 updatedAt: IsoDateTime,740 updatedAt: IsoDateTime,
741 /** Server epoch milliseconds when the provider read started; preserved on cache hits. */
742 observedAt: Schema.optional(Schema.Finite),
739 author: Schema.optional(Schema.NullOr(PullRequestActor)),743 author: Schema.optional(Schema.NullOr(PullRequestActor)),
740 additions: Schema.optional(NonNegativeInt),744 additions: Schema.optional(NonNegativeInt),
741 deletions: Schema.optional(NonNegativeInt),745 deletions: Schema.optional(NonNegativeInt),
...@@ -841,6 +845,8 @@ export const PullRequestDetail = Schema.Struct({...@@ -841,6 +845,8 @@ export const PullRequestDetail = Schema.Struct({
841 baseBranch: TrimmedNonEmptyString,845 baseBranch: TrimmedNonEmptyString,
842 createdAt: IsoDateTime,846 createdAt: IsoDateTime,
843 updatedAt: IsoDateTime,847 updatedAt: IsoDateTime,
848 /** Server epoch milliseconds when the provider read started; preserved on cache hits. */
849 observedAt: Schema.optional(Schema.Finite),
844 mergedAt: Schema.NullOr(IsoDateTime),850 mergedAt: Schema.NullOr(IsoDateTime),
845 closedAt: Schema.NullOr(IsoDateTime),851 closedAt: Schema.NullOr(IsoDateTime),
846 reviewers: Schema.Array(PullRequestActor),852 reviewers: Schema.Array(PullRequestActor),