fix(web): pull request checks stay current after a checks refresh

apps/web/src/components/RightPanelTabs.tsx

@@ -735,7 +735,7 @@ function PullRequestSurfaceIcon({
     repository: surface.repository,
     number: surface.number,
   };
-  const sharedSummary = useSharedPullRequestSummary(resolvedEnvironmentId, reference, null);
+  const sharedSummary = useSharedPullRequestSummary(resolvedEnvironmentId, reference);
   const status = linkedSnapshot ?? newestPullRequestSummary(detail, sharedSummary) ?? seed ?? null;
   if (status === null) {
     return <PullRequestGlyph.pullRequest className="size-3 shrink-0 text-muted-foreground" />;

apps/web/src/components/ThreadStatusIndicators.tsx

@@ -95,17 +95,13 @@ export function useLinkedThreadPullRequest(
     current === null ? ((!supportsLinks ? linkedPullRequest : null) ?? branchPullRequest) : null;
   // Preserve reference identity so sidebar renders do not touch the summary cache.
   const reference = fallback == null ? null : linkedPullRequestReference(fallback);
-  const queried = useEnvironmentQuery(
+  // Mounted for its answers, which land in the shared summary read below.
+  useEnvironmentQuery(
     !enabled || environmentId === null || reference === null
       ? null
       : linkedPullRequestDetailAtom({ environmentId, input: reference }),
   );
-  const detail = useSharedPullRequestSummary(
-    environmentId,
-    reference,
-    queried.data,
-    queried.dataUpdatedAt,
-  );
+  const detail = useSharedPullRequestSummary(environmentId, reference);
 
   if (current !== null) return currentStatus;
   return detail === null

apps/web/src/components/pullRequest/PullRequestsPage.tsx

@@ -205,7 +205,6 @@ export function PullRequestsPage() {
           ...renderedPullRequestSurface,
           projectId: renderedPullRequestSurface.projectId as ProjectId,
         },
-    null,
   );
   const updateSearch = (patch: {
     [Key in keyof PullRequestsSearch]?: PullRequestsSearch[Key] | undefined;

apps/web/src/components/pullRequest/usePullRequestDetail.ts

@@ -2,11 +2,10 @@ import { resolvePullRequestDetailChecks } from "@t3tools/client-runtime/pull-req
 import {
   AuthSourceControlWriteScope,
   type EnvironmentId,
-  type PullRequestDetail,
   type PullRequestListEntry,
   type PullRequestRef,
 } from "@t3tools/contracts";
-import { useEffect, useEffectEvent, useState } from "react";
+import { useState } from "react";
 
 import { usePullRequestChecksRefresh } from "~/hooks/usePullRequestChecksRefresh";
 import { useServerConfigs } from "~/state/entities";
@@ -21,18 +20,12 @@ import {
 import {
   readPullRequestDetailSnapshot,
   resolveDisplayedPullRequestDetail,
-  writePullRequestDetailSnapshot,
 } from "./pullRequestDetail.logic";
-import { pullRequestChecksState } from "./pullRequestPresentation";
 
 export function pullRequestRefKey(reference: PullRequestRef) {
   return `${reference.projectId}:${reference.host ?? ""}:${reference.repository}#${reference.number}`;
 }
 
-function pullRequestDetailSummary(data: PullRequestDetail | null) {
-  return data === null ? null : { ...data, checksState: pullRequestChecksState(data.checks) };
-}
-
 /**
  * Loads a pull request's detail and activity and merges them, with the snapshot cache, the shared
  * summary and the list row, into the detail the panel shows.
@@ -95,24 +88,7 @@ export function usePullRequestDetail(
   } else if (detailQuery.data !== null && detailQuery.data !== cachedDetail) {
     setCachedDetail(detailQuery.data);
   }
-  const saveDetailSnapshot = useEffectEvent((data: PullRequestDetail) => {
-    writePullRequestDetailSnapshot(
-      typeof window === "undefined" ? undefined : window.localStorage,
-      environmentId,
-      reference,
-      data,
-    );
-  });
-  useEffect(() => {
-    if (detailQuery.data !== null) saveDetailSnapshot(detailQuery.data);
-  }, [detailQuery.data]);
-  const detailSummary = pullRequestDetailSummary(liveDetail);
-  const observedSummary = useSharedPullRequestSummary(
-    environmentId,
-    reference,
-    detailSummary,
-    Math.max(detailQuery.dataUpdatedAt, checksQuery.dataUpdatedAt),
-  );
+  const observedSummary = useSharedPullRequestSummary(environmentId, reference);
   const activity = activityQuery.data;
   const resolvedCoreDetail = resolveDisplayedPullRequestDetail({
     live: liveDetail,
@@ -122,8 +98,7 @@ export function usePullRequestDetail(
   // Every hook runs above this line: a hook between the merge and the panel's plain readers of
   // `detail` would cost it its cache scope.
   const listSummary = listEntry === null ? null : pullRequestListEntryToSummary(listEntry);
-  // The list row is also published to the shared cache, but only after this commit's layout
-  // effects run, so it is compared directly rather than trusted to be there already.
+  // The row can come from the page's stored snapshot, which no answer has published yet.
   const sharedSummary = newestPullRequestSummary(
     resolvedCoreDetail,
     newestPullRequestSummary(observedSummary, listSummary),

apps/web/src/state/pullRequests.test.ts

@@ -4,6 +4,9 @@ import { describe, expect, it } from "vite-plus/test";
 
 import {
   newestPullRequestObservation,
+  observedPullRequestSummary,
+  observePullRequestChecks,
+  observePullRequestMetadata,
   newestPullRequestSummary,
   threadPullRequestLinkToSummary,
 } from "./pullRequests";
@@ -114,3 +117,73 @@ describe("pull request summary cache", () => {
     ).toBeNull();
   });
 });
+
+describe("pull request observations", () => {
+  const fromSummary = (value: PullRequestSummary, observedAt: number) =>
+    observePullRequestMetadata({ metadata: null, checks: null }, observed(value, observedAt));
+  const checksRead = (
+    checksState: "passing" | "failing" | "pending" | null,
+    readAt: number | undefined,
+    observedAt: number,
+  ) => ({ checksState, readAt, observedAt });
+
+  it("keeps a newer list's mergeability when a checks read lands after it", () => {
+    const detail = summary({
+      mergeability: "conflicting",
+      checksState: "pending",
+      observedAt: 100,
+    });
+    let state = fromSummary(detail, 1);
+    state = observePullRequestMetadata(
+      state,
+      observed(summary({ mergeability: "mergeable", checksState: "pending", observedAt: 200 }), 2),
+    );
+    state = observePullRequestChecks(state, checksRead("passing", 300, 3));
+    expect(observedPullRequestSummary(state)).toMatchObject({
+      mergeability: "mergeable",
+      checksState: "passing",
+    });
+  });
+
+  it("keeps a newer checks read when an older list read lands after it", () => {
+    let state = fromSummary(
+      summary({ mergeability: "conflicting", checksState: "pending", observedAt: 100 }),
+      1,
+    );
+    state = observePullRequestChecks(state, checksRead("passing", 300, 2));
+    state = observePullRequestMetadata(
+      state,
+      observed(summary({ mergeability: "mergeable", checksState: "pending", observedAt: 250 }), 3),
+    );
+    expect(observedPullRequestSummary(state)).toMatchObject({
+      mergeability: "mergeable",
+      checksState: "passing",
+    });
+  });
+
+  it("lets a newer read's checks replace an older checks read", () => {
+    let state = fromSummary(summary({ checksState: "pending", observedAt: 100 }), 1);
+    state = observePullRequestChecks(state, checksRead("passing", 300, 2));
+    state = observePullRequestMetadata(
+      state,
+      observed(summary({ checksState: "failing", observedAt: 400 }), 3),
+    );
+    expect(observedPullRequestSummary(state)?.checksState).toBe("failing");
+  });
+
+  it("orders checks by arrival when the server reports no read time", () => {
+    let state = fromSummary(summary({ checksState: "pending" }), 1);
+    state = observePullRequestChecks(state, checksRead("passing", undefined, 2));
+    expect(observedPullRequestSummary(state)?.checksState).toBe("passing");
+    state = observePullRequestChecks(state, checksRead("failing", undefined, 1));
+    expect(observedPullRequestSummary(state)?.checksState).toBe("passing");
+  });
+
+  it("shows nothing until some read has described the pull request", () => {
+    const state = observePullRequestChecks(
+      { metadata: null, checks: null },
+      checksRead("failing", 1, 1),
+    );
+    expect(observedPullRequestSummary(state)).toBeNull();
+  });
+});