fix(server): pause periodic PR discovery without foreground clients #23852 · pingdotgg/t3code

Proof	Before	After
Deterministic service test	10 extra lookups; exit 1	6 tests pass; exit 0
Real server	Recurring idle branch requests	No recurring branch discovery

Added 3 commits18h ago

test(server): reproduce idle branch pull request sweeps (#23702)1e1b2b9
fix(server): idle servers no longer sweep branch pull requests every minute (#23702)29ce300
Merge main into fix/issue-23702ab14184

MmacroscopeappBotapproved these changes18h ago

Approvability

Verdict: Approved at ab14184

Macroscope's review found this PR approvable — This focused bug fix pauses only the periodic pull-request discovery sweep when no foreground client or permitted background demand exists, while preserving initial backfill and event-driven refreshes. It adds targeted regression coverage and does not alter schemas, defaults, deployments, or security-sensitive code.

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

CcoderabbitaiBot18h ago

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Source mode: Auto
Repository: pingdotgg/t3code
Branch: fix/issue-23702
File: .coderabbit.config.ts
Commit: ab14184
Layers: Repository TS config
Review profile: CHILL
Plan: Team
Run ID: 9040e83e-fe1c-40f6-90ce-b76336d510aa

📥 Commits

Reviewing files that changed from the base of the PR and between 2c8bdf6177159d8fcd12a897d7c9311f6eb93237 and ab1418478c65754a3637796ff61657c26aae2e56.

📒 Files selected for processing: 2

apps/server/src/orchestration-v2/ThreadPullRequestService.test.ts
apps/server/src/orchestration-v2/ThreadPullRequestService.ts

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

📝 Walkthrough

The periodic pull-request sweep now checks BackgroundPolicy.shouldRunOpportunisticWork before proceeding. A regression test verifies that an idle interval does not trigger additional lookups, while a checkpoint event still triggers a lookup.

Changes: Pull-request discovery

Layer / File(s)	Summary
Policy-gated periodic sweep `apps/server/src/orchestration-v2/ThreadPullRequestService.ts`, `apps/server/src/orchestration-v2/ThreadPullRequestService.test.ts`	The service skips the periodic sweep when `shouldRunOpportunisticWork` is false. The test verifies that checkpoint events still trigger lookups and that no additional lookups occur during a ten-minute idle interval.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: t3dotgg, brechard

Merge Risk: ⚪ Minimal · up to ab141

The change appears ready to merge after normal checks: idle periodic discovery is paused, while checkpoint-triggered discovery remains available.

Pre-merge checks: ^Passed 3 · ^Failed 1

^Failed Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Linked Issues check	Warning	The change addresses `#23702`'s recurring idle discovery requirement. `ThreadPullRequestServiceV2` gates periodic sweeps on `shouldRunOpportunisticWork`, and the regression test covers idle minutes and …	Honor a known source-control quota pause until its reset before retrying periodic discovery, and add a regression test that verifies no retry occurs during the pause.

^Passed Passed checks: 3 passed

Check name	Status	Explanation
Title check	Passed	The title clearly summarizes the main change: periodic PR discovery pauses when no foreground clients are active.
Description check	Passed	The description covers the problem, change, scope and approval, and verification. It also states limitations and distinguishes recurring branch discovery from startup backfill, thread-event refreshes,…
Out of Scope Changes check	Passed	The service change and regression test relate directly to `#23702`'s idle branch-discovery behavior. The supplied change summary shows no unrelated changes.

Full details: Description check

Explanation

The description covers the problem, change, scope and approval, and verification. It also states limitations and distinguishes recurring branch discovery from startup backfill, thread-event refreshes, and PR-summary polling.

Full details: Linked Issues check

Explanation

The change addresses #23702's recurring idle discovery requirement. ThreadPullRequestServiceV2 gates periodic sweeps on shouldRunOpportunisticWork, and the regression test covers idle minutes and checkpoint-triggered discovery. However, #23702 also requires a known quota pause to be honored until reset. The reviewed change does not add that behavior; BackgroundPolicy.shouldRunOpportunisticWork checks foreground activity and power constraints, not quota state. A paused provider can still be retried on a later sweep when opportunistic work is allowed.

Resolution

Honor a known source-control quota pause until its reset before retrying periodic discovery, and add a regression test that verifies no retry occurs during the pause.

✨ Finishing Touches

📝 Generate docstrings

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

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

Ggithub-actionsBot18h ago

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider	Metric	Main baseline	This PR	Impact	PR ceiling
Codex	Total thread wire	5.0 KiB	5.0 KiB	−7 B (−0.1%)	6.8 KiB	✅
Codex	Thread snapshot wire	3.8 KiB	3.8 KiB	0 B (0.0%)	4.9 KiB	✅
Codex	Live turn WebSocket wire	1.2 KiB	1.2 KiB	−7 B (−0.6%)	2.0 KiB	✅
Codex	Live turn WebSocket decoded	20.9 KiB	20.9 KiB	0 B (0.0%)	29.3 KiB	✅
Codex	Live turn messages	2	2	0 (0.0%)	8	✅
Claude	Total thread wire	5.0 KiB	5.0 KiB	+31 B (+0.6%)	6.8 KiB	✅
Claude	Thread snapshot wire	3.8 KiB	3.8 KiB	0 B (0.0%)	4.9 KiB	✅
Claude	Live turn WebSocket wire	1.2 KiB	1.2 KiB	+31 B (+2.6%)	2.0 KiB	✅
Claude	Live turn WebSocket decoded	21.2 KiB	21.2 KiB	+41 B (+0.2%)	29.3 KiB	✅
Claude	Live turn messages	1	2	+1 (+100.0%)	8	✅

Baseline: 2c8bdf6 · PR result: ab14184 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

Codex decoded thread snapshot: 108.5 KiB
Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

JJulius Marminge18h ago

The known quota pause already blocks requests locally before network I/O in GitHubRequestPolicy.ts and SourceControlRateLimit.ts. This PR stops the recurring idle sweep entirely, including re-entry during a known pause. Foreground local retries and warning stacks remain tracked in #19781, which the report explicitly separates. The linked-issue warning needs no additional change in this PR. All 28 current-head checks pass; CodeRabbit reports no actionable code comments, and Macroscope approved.

fix(server): pause periodic PR discovery without foreground clients#23852

Problem

Change

Scope and approval

Verification

Checks

Approvability

Thread transfer impact