fix(server): first pull request open no longer fails as interrupted

[whoisaldo]

Replaces #14122, which was closed because its description did not name the model and harness. That is now at the end. The branch is merged with current main.

Closes #14113.

The first time the PR panel opened after a turn, it often showed "Could not load pull requests — All fibers interrupted without error", and Retry fixed it. The client cancels its first detail read and immediately sends the same read again. When a cached read's last caller leaves, effect/Cache interrupts the lookup but keeps the entry until the lookup has stopped. The resent read lands in that window, joins the dying lookup, and gets its interrupt back as the answer.

As the triage suggests, the fix is in the cache read. getCached wraps Cache.get and asks once more when a read comes back with only an interrupt. By then the dying entry is gone, so the retry starts a fresh lookup. catchCauseIf does not run for a caller that was itself interrupted, so a real cancel still stops without spawning gh again. Every Cache.get in PullRequestService now goes through it. Detail, activity, preview, list, diff, stats, files-viewed, and viewer reads all share this shape, and the macOS reproduction on the issue saw activity fail too.

main already retries this way for the list and list-stats reads since #15851. This puts that retry in one helper and routes every cache read in the service through it, including detail, the read #14113 reports.

Evidence

• getCached.test.ts reproduces the race from the issue. With plain Cache.get, the second read fails with a bare interrupt; with getCached it gets a fresh value. A second test checks that an interrupted read does not start another lookup.
• The getCached, PullRequestService, and PullRequestReadCache tests pass on the merged branch (206 tests). Server tsc --noEmit, lint, and format are clean.
• In-app: the reporter verified #14122's branch with the reproduction from [Bug] PR panel shows "Could not load pull requests" on the first open after a turn #14113, on Windows 11 with WSL2. Before, the first open failed 4/4; on this branch it loaded 4/4. The WebSocket trace shows the client still interrupting and resending detail, but the resent read no longer fails. Removing the retry made the race test fail again. Since that check the helper is unchanged. The branch has only merged main and routed two caches main added (checks and watch fingerprint) through the helper.
• Not rerun on this branch: the reporter's unmodified macOS reproduction (16/16 failures before).

Model: Claude Opus 5.5 via Claude Code, in T3 Code.

[macroscopeapp]

Approvability

Verdict: Approved at 36603e0

Macroscope's review found this PR approvable — The PR fixes a narrowly scoped cache-interruption race by centralizing a one-time retry and applies it consistently across pull-request reads, with focused tests covering both recovery and cancellation behavior. It introduces no schema, deployment, security-sensitive, default, or static-analysis configuration changes.

Notes:

• Diff unchanged. Approvability was decided on eligibility alone.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Path: .coderabbit.config.ts
• Review profile: CHILL
• Plan: Advanced
• Run ID: fc9f39ec-ca5a-4c68-b00d-1be96bd3e6c8

---

📥 Commits

Reviewing files that changed from the base of the PR and between ed0f7b9 and 36603e0.

---

📒 Files selected for processing (3)

• apps/server/src/pullRequest/PullRequestService.ts
• apps/server/src/utils/getCached.test.ts
• apps/server/src/utils/getCached.ts

---

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

---

📝 Walkthrough

Walkthrough

A new getCached helper retries a cache read once when it fails only with interruption. PullRequestService now uses the helper across its cache reads, replacing local retry logic for list and list-stats reads.

Changes

Cache read recovery

Layer / File(s)	Summary
Add interrupt-only cache retry apps/server/src/utils/getCached.ts, apps/server/src/utils/getCached.test.ts	getCached retries a cache read once when its failure contains only interrupts. Tests cover a second read joining an interrupted lookup and confirm that interrupting a read does not start another lookup.
Use helper for pull request cache reads apps/server/src/pullRequest/PullRequestService.ts	Viewer-flight, list, detail, preview, activity, diff, viewed-files, list-stats, checks, and watch-fingerprint reads use getCached. The local retry logic for list and list-stats reads is removed.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant PullRequestService
  participant getCached
  participant Cache
  participant Lookup
  PullRequestService->>getCached: Read cache key
  getCached->>Cache: Cache.get
  Cache->>Lookup: Start or join lookup
  Lookup-->>Cache: Interrupt-only failure
  Cache-->>getCached: Interrupt-only failure
  getCached->>Cache: Retry Cache.get once
  Cache->>Lookup: Start fresh lookup
  Lookup-->>Cache: Fresh value
  Cache-->>getCached: Fresh value
  getCached-->>PullRequestService: Fresh value

Loading

---

Merge Risk | ⚪ Minimal · up to 36603

Merge Risk: ⚪ Minimal · up to 36603

The cache-read retry is mergeable with normal checks; no concrete remaining issue is identified.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 36603

Recovery is limited to one retry and preserves existing access controls. No introduced security issue was identified, but interruption and cleanup behavior is not fully verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The broadened policy can repeat existing provider or local read work after interruption, but the inspected changes do not add another host, project, credential namespace, or persistence authority. Viewer coalescing remains scoped to the existing host, provider, and workspace-root key.

Trust Boundaries and Controls

• observed — Provider calls retain their rate-limit wrapper on each attempt. Normal viewer reads also retain the check before cache access; explicitly paused viewer reads keep their pre-existing interactive allowance. Typed provider failures do not satisfy the helper's interrupt-only retry condition.

Hardening Proposals

• proposed — Document the helper's replay-safe loader requirement and validate cleanup ordering, non-caller interruption, concurrent replacement readers, and second-attempt termination against the deployed cache-library version.

---

Pre-merge checks | 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check		The title clearly and concisely describes the server fix for the first pull request open failing with an interruption.
Description check		The description explains the problem, root cause, implementation, affected cache reads, tests, verification results, reproduction evidence, and model and harness. It is sufficiently complete, although…
Linked Issues check		The active directly linked issue is #14113. The PR addresses its server-side cache race by adding getCached, which retries one interrupt-only Cache.get result and does not retry a caller interrupt…
Out of Scope Changes check		The diff adds one cache-read helper, focused tests, and applies the helper to cached reads in PullRequestService. The broader routing covers the same cache race identified in #14113, including activ…

---

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}

Dismissing prior approval to re-evaluate 36603e0