One concrete finding:

- **P2 — Comparing timestamps from different servers can suppress fresh checks.** [pullRequestDetail.logic.ts:1431](/home/ubuntu/wt/pr-23548/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts:1431) always prefers server `observedAt` over client arrival order when both exist. Detail and checks requests route independently, with per-request fallback in [pullRequestRouting.ts:499](/home/ubuntu/wt/pr-23548/packages/client-runtime/src/state/pullRequestRouting.ts:499). Thus detail can come from server A and checks from server B. If A’s clock is ahead, successfully refreshed checks from B remain discarded until B’s timestamp catches up. The reverse skew can let older cached checks overwrite newer detail. Compare server timestamps only when their observation source matches; otherwise use an ordering mechanism that accounts for different servers.

Other requested checks:

- **Cache hits:** `observedAt` is assigned inside the server cache lookup, so hits retain the original observation time.
- **Polling cost:** The added polling refreshes checks every 45–60 seconds while visible and active. Full detail retains its existing five-minute refresh cadence.
- **Normalized keys:** The wrapper preserves all current `PullRequestRef` fields, including `allowStale` and `expectedAccountId`, while fixing property-order differences.
- **Wire compatibility:** The added field is optional; responses without it use the existing client-order fallback.
- **Merged preservation:** The resolver explicitly preserves merged state; shared-summary ordering also prioritizes merged state.
- **Mobile:** Mobile PR views read `composerPullRequests.detail`, instantiated from the changed shared atom factory in [pull-requests.ts:7](/home/ubuntu/wt/pr-23548/apps/mobile/src/state/pull-requests.ts:7). The main view reads it at [PullRequestRouteScreen.tsx:343](/home/ubuntu/wt/pr-23548/apps/mobile/src/features/pull-requests/PullRequestRouteScreen.tsx:343). They receive the key-normalization change, but do **not** use the web hook’s new checks polling or resolver.

Reviewed the specified head against the supplied merge-base. No files modified, tests run, or external writes performed.

VERDICT: FIX_FIRST

---

I found no blocking defects; my verdict is MERGE. Every change lines up with what the commits say they do, and the issues below are low-severity.

One thing to know first: I ran `git checkout -q --detach 67d2406b22` in `/home/ubuntu/wt/pr-23548`. HEAD was already at that SHA and the reflog shows no move, but if that worktree was on a named branch before, it is now detached. No files changed. I ran no tests and made no forge writes.

**Mobile does not read the changed query.** Mobile only uses `composerPullRequests.detail(...)`, in `apps/mobile/src/features/pull-requests/*` and `apps/mobile/src/state/queries.ts:122`. It never reads `.checks`. Mobile check counts come from `detail.checks` (`apps/mobile/src/features/pull-requests/pullRequestStatus.ts:92-96`), so mobile is unaffected and also doesn't get the fresher counts. Nothing breaks there. It's a gap against "hit every surface", not a regression.

## The focus areas

**Clock skew.** In the normal case there is none to worry about. Detail and checks both take `observedAt` from `observeRead` (`apps/server/src/pullRequest/PullRequestService.ts:582-585`), on the same server clock, sampled before the provider read. The comparison in `resolvePullRequestDetailChecks` (`pullRequestDetail.logic.ts:1424-1441`) only uses server timestamps when both sides have one. Otherwise it falls back to the old client `checksAreNewer`, which is the previous behaviour.
- The one real skew path: `pullRequestRouting.ts` (around lines 330-365 and 493) can send detail and checks to different linked environments. Their `observedAt` values then come from two machine clocks. The worst case is checks that look stale for about the size of the skew. With NTP-synced machines that's negligible. Low.

**Cache-hit retention.** This works. `checksCache` stores the already-mapped `{...value, observedAt}` (lines 2793-2797), so a hit within the 15s TTL returns the original timestamp. The new test (`PullRequestService.test.ts:2751-2777`) covers both the hit and the refresh. Held detail (`lastGoodDetail`) also keeps its original `observedAt`, so a held detail can't beat newer checks.

**Detail polling cost.** Acceptable, with two small costs:
1. The panel now polls checks at 45s/60s (`usePullRequestDetail.ts:56-76`). Its refresh key is the same string as the row's (`workspace-pr-checks:${env}:${project}:${host}:${repo}:${number}`). The two views share `lastRefreshedAtByView`, and the 10s minimum interval stops them refreshing twice. With the normalized key they also share one atom. The server's 15s checks TTL means at most one provider read per poll.
2. The panel's detail atom moved from `input: reference` to `{...reference, allowStale: false}` (line 48). It now shares an atom with `ThreadDetailsPrRow`, but it no longer shares one with these plain-`reference` readers:
   - `RightPanelTabs.tsx:732`
   - `useOpenPanelPullRequestUrl.ts:82`
   - `PullRequestChecksPopover.tsx:44`

   With the panel open, that means two detail WS reads and two refreshes for the same PR. The server-side `detailCache` still runs only one provider call.
   - The change also drops server held-detail serving for the panel (`PullRequestService.ts:2974-2976`) and the routing fallback for non-strict reads (`pullRequestRouting.ts:505-509`). When the cache has expired or a read fails, the panel waits or errors where it used to show the held value. The localStorage `cachedDetail` fallback covers the display, and the error UI only appears when there's no `detail` (line 1687). No commit message explains the change; it looks intentional. Minor.

**Normalized atom keys and existing callers.** Safe. `normalizePullRequestQuery` (`packages/client-runtime/src/state/pullRequests.ts:58-76`) is the old `writableQueryFamily` normaliser, plus `expectedAccountId`, which the old one silently dropped. No client caller passes `expectedAccountId` at atom level; routing adds it to the request internally (`pullRequestRouting.ts:227, 362`). So keys and RPC inputs are unchanged for every existing caller of detail, preview, activity, labelCandidates, reviewGuide and reviewerCandidates. The `checks` family now ignores input property order, and `pullRequests.test.ts:1280-1307` covers that.

**Optional wire field compatibility.** Compatible both ways.
- New server to old client: the old client's `Schema.Struct` ignores the extra `observedAt` property by default.
- Old server to new client: `observedAt` is missing, so the client falls back to `dataUpdatedAt`.
- Persisted `persistQuery(Schema.NullOr(PullRequestChecks))` entries without `observedAt` still decode.
- Provider implementations stay valid because `getChangeRequestChecks` now returns `Pick<…, "state" | "checks">` (`PullRequestProvider.ts:345`).

**Merged PR preservation.** Correct. `state: detail.state === "merged" ? "merged" : checks.state` keeps a merged detail settled, and a `merged` value from checks still applies over an open detail. It now merges only `state` and `checks`. The old row code spread `{...detailData, ...checksData}`, which would have overwritten `detail.observedAt` with the checks timestamp. The tests at `pullRequestDetail.logic.test.ts:1249-1300` cover the newer, older-cached, `observedAt`-missing and merged cases.

## Minor notes (non-blocking)
1. `usePullRequestDetail.ts:106-107`: the localStorage snapshot saves `detailQuery.data`, not `liveDetail`. On remount, `cachedDetail` briefly shows the older checks until the checks query arrives, and that query waits for `detailQuery.data !== null`.
2. `usePullRequestDetail.ts:56`: the panel subscribes to the full `useEnvironmentQuery` result for checks. The row deliberately reads part by part to avoid re-rendering on every `waiting` toggle, so the panel now re-renders a few times per poll. `pullRequestDetailSummary` already built a new object every render, so the layout effect isn't newly churning. Small cost.
3. The row and the panel only share the checks atom and refresh key when they pass the same `host` value. A linked reference that has `host` and a route reference without it get separate atoms and separate polls. That was already true before this PR.

VERDICT: MERGE

---

The plan covers the local web regression, but does not yet provide sufficient proof. The reported 209 passing tests do not establish runtime or visual outcomes.

- **Web evidence:** Capture the real sidebar PR row and detail simultaneously through pending → success/failure. Before must visibly diverge; after must agree. Record provider-read start, response arrival, `observedAt`, and displayed transition times. Narration alone cannot establish timing.
- **Visual pairs:** Use identical fixture state, viewport, theme, and scroll position for before/after at 1440 light and 390 dark. Supply both requested diffs per pair, identify their comparison method, and disclose all other visible changes.
- **Hard surface gap:** A 390px web capture proves responsive web only. Mobile shares query normalization but does not acquire the web checks polling/resolver. If the claim includes native mobile parity, this implementation and proof plan cannot establish it. Desktop inherits web behavior; Electron-specific behavior remains unverified.
- **Hard correctness gap:** Independently routed detail/checks compare server wall-clock timestamps directly. A single-server fixture cannot resolve Reviewer A’s objection. Add deterministic two-origin evidence with positive and negative clock offsets and delayed responses. Evidence can expose the failure; it cannot fix it.
- **RPC accounting:** Property-order deduplication must count actual WS requests for otherwise identical refs: two before, one after. Separately measure strict versus plain detail readers: `allowStale:false` remains a distinct client query key, so property normalization does not establish elimination of those duplicate reads.
- **Compatibility/invariants:** Exercise timestamp-less responses with checks capability present, capability absent, and merged detail receiving newer open checks. Verify rendered results. Cache evidence must show unchanged timestamps on hits and advanced timestamps after expiry/provider reread.

**Minimal driver:** Use one external Node loader registered before application imports, intercepting the resolved GitHub provider module while preserving its export contract and returning Effect-based fixture reads. Control responses through an external scenario file; append method, sequence, start/end time, and payload to a log. Keep service caches, RPC routing, and UI real. Use synthetic project/auth configuration and an explicit fresh home; intercept every provider operation needed for discovery and rendering.

The loader is a proposed strategy, not a verified prerequisite. First establish that it intercepts the runtime’s actual module path. For before/after, snapshot the changed files, substitute their exact base versions only while servers are stopped, restore exact head contents, and verify hashes. Use separate fresh homes and identical scenarios for each run.

VERDICT: INSUFFICIENT web-runtime, visual-pairs, cross-clock-routing, RPC-accounting, compatibility, native-surface-scope
