fix(server): PR sync stops reading missing pull requests every minute

[abiassi]

Problem

PR sync reads a linked pull request on every one-minute sweep while the link has no snapshot. A link never gets a snapshot if GitHub returns NOT_FOUND. The sweep keeps reading the pull request, even on settled threads. On my machine, 9 such links made 10 failed GraphQL requests a minute. Fixes #17799.

Change

• The no-snapshot rule applies only to unsettled threads, as fix(server): settled threads stop polling their pull requests #16762 intended.
• A not-found answer finishes the read and marks the pull request missing. On unsettled threads, sweeps retry it every 15 minutes. A successful read clears the mark.
• Neither a pending stack retry nor another link's sync bypasses the 15-minute pace. A reader's refresh still reads the pull request immediately.

Scope and approval

I believe this fits the exception for a very small, focused fix to an obvious bug. The user guide says "A settled thread's reviews stop refreshing until you unsettle it," but links without snapshots are still read every minute after the thread settles. Only PullRequestSyncReactor.ts and its tests change. There is no new setting, contract, or UI change. Retries use the existing 15-minute pace for closed pull requests. The batch fallback, the read cache, and auto-settle stay as they are. #17609 (open) changes the same event handler. If #17609 merges first, I will rebase and apply the same skip in its requestUnsyncedLinks. Linking or relinking a pull request that is still marked missing does not force a read. A reader's refresh does.

Verification

• vp test run apps/server/src/orchestration-v2/PullRequestSyncReactor.test.ts. The test commit fails 5 cases on the current reactor. The fix passes all 23.
• Dev server on a copy of my data, with 9 missing pull requests on settled threads. main made 50 failed requests in 5 sweeps. With the fix and one thread unsettled, 18 sweeps made 4 failed requests: 2 at startup and 2 after 15 minutes.
• vp run --filter t3 typecheck, and vp lint on both files. I did not verify not-found handling with other hosts.

Made with Claude Code (Claude Opus 5.5).

[macroscopeapp]

Approvability

Verdict: Approved at 58fd275

Macroscope's review found this PR approvable — This is a small, localized server bug fix that backs off repeated reads for pull requests confirmed missing while preserving explicit refreshes and recovery polling. The accompanying tests cover the changed cadence and event behavior, with no schema, deployment, security, billing, default, or static-analysis 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: 7ae0ab1f-cb5a-4644-8078-e991ef9a2992

---

📥 Commits

Reviewing files that changed from the base of the PR and between 91474e5 and 58fd275.

---

📒 Files selected for processing (2)

• apps/server/src/orchestration-v2/PullRequestSyncReactor.test.ts
• apps/server/src/orchestration-v2/PullRequestSyncReactor.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

The pull-request sync reactor now bases automatic reads on thread and pull-request state. It tracks pull requests that return not-found, retries them on a 15-minute cadence, and limits event-triggered refreshes for known-missing links.

Changes

Pull-request sync polling

Layer / File(s)	Summary
Due checks and missing-request cadence apps/server/src/orchestration-v2/PullRequestSyncReactor.ts, apps/server/src/orchestration-v2/PullRequestSyncReactor.test.ts	Automatic sync eligibility now considers active links and their snapshots. Missing pull requests use a 15-minute cadence. A not-found summary records the missing key and advances its sync timestamp; a successful summary clears the marker. Tests cover ordinary host failures, missing-request retries, and stack-read failures.
Event-triggered refreshes apps/server/src/orchestration-v2/PullRequestSyncReactor.ts, apps/server/src/orchestration-v2/PullRequestSyncReactor.test.ts	A synced-thread event requests a refresh for an unsynced link only when its key is not marked missing. Tests cover settled-thread links, sibling-link syncs, and explicit state-change requests.

Priority: ⬆️ High

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: t3dotgg

---

Merge Risk | ⚪ Minimal · up to 58fd2

Merge Risk: ⚪ Minimal · up to 58fd2

No actionable merge-blocking risk remains; the change is ready for normal merge checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 58fd2

The change reduces repeated requests without adding permissions or public functionality. One bounded concern remains: a missing result obtained through one project's credentials can delay refreshes after another project becomes responsible for reading the same pull request. Explicit refresh and periodic retries limit the impact.

Retained concerns

• Low · reliability · inferred: A credential-relative not-found result can remain authoritative after the serving project changes. The new missing marker is shared by normalized pull-request identity and is removed only when that identity disappears entirely or a summary succeeds. If the original project's link is removed while another project's link remains, automatic and synced-thread refreshes can stay suppressed until the 15-minute retry, despite the next read using a different project context. This weakens failure containment across project credentials. Existing cross-project grouping predates the PR; the additional recovery delay is new. The consequence is inferred from source, not a demonstrated authorization exploit.

Security review details

Security Blast Radius

• observed — A missing decision affects every visible link grouped under the same normalized pull-request key in one reactor. The projection input spans projects, so that scope is not inherently project-local. The marker does not suppress unrelated pull-request keys or an entire host.

Security Findings and Attack Paths

• inferred — The supported adverse path is delayed refresh after a not-found result under an earlier serving context, followed by a project-context change that leaves the same pull-request key present. This establishes a bounded failure-containment concern, not verified attacker reachability, cross-tenant exposure, credential disclosure, or privilege escalation.

Trust Boundaries and Controls

• observed — The provider-selection boundary is unchanged. Only the explicit not-found reason creates negative state; other summary failures do not enter that branch. Explicit requestSync remains ahead of missing-state eligibility, and the existing project/host rate-limit pause still gates reads.

Resilience and Maintainability Implications

• observed — The existing worker processes sweeps sequentially. Request generations preserve refreshes arriving during an in-flight read, and missing-state updates occur together in one synchronous block. Successful reads clear negative state; disappearance of all matching links prunes it. Scope shutdown interrupts the worker and drops queued work rather than leaving a durable missing-state transition.

Hardening Proposals

• proposed — Bind negative-result ownership to the serving access context, or invalidate the marker when that context changes. This would preserve the reduced polling cadence without treating one project's visibility result as authoritative after a credential or serving-project transition.

---

Pre-merge checks | 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main fix: preventing repeated one-minute reads of missing pull requests.
Description check		The description covers the problem, implementation, scope rationale, linked issue, verification results, and known verification limitation. It follows the required template and provides sufficient det…
Linked Issues check		Issue #17799 is an active direct link. PullRequestSyncReactor.ts filters settled threads before periodic due checks, records not-found keys with a 15-minute cadence timestamp, clears the request, …
Out of Scope Changes check		The change is limited to apps/server/src/orchestration-v2/PullRequestSyncReactor.ts and its tests. The reactor changes directly implement issue #17799. The test changes provide regression coverage f…

---

✨ 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 58fd275