[juliusmarminge]

Known forge quota refusals in background PR discovery and settlement now log one Info line without a stack. Unknown failures, defects and compound causes retain warning stacks. Pause, retry, backfill and settlement behavior is unchanged.

Fixes #19781. Replaces #19794.

Original description by @aravhawk

Problem

Known GitHub quota pauses in thread PR discovery and automatic settlement emit warning stacks on every sweep, even though quota admission already blocks those reads until reset.

Change

Log recognized typed quota pauses as compact Debug records with thread context and a retry time when known. Typed rate-limit refusals without a retry time still log at Debug; an inner typed cause can supply the timestamp. Unexpected failures, defects and compound causes retain warning stacks. The helper applies to discovery group reads, per-thread saved-PR summary reads, and settlement group reads; the per-thread settlement dispatch catch stays unchanged because it performs no GitHub read.

Quota admission, retry scheduling, settlement eligibility, interruption handling and discovery backfill behavior remain unchanged.

Scope and approval

Fixes #19781, with maintainer approval. Credit to aravhawk for the original contribution. Contributor commits are retained, and current main is integrated by merge.

Verification

• Reduced nine PR-added cases to six service cases: wrapped discovery pause, unexpected warning, saved-PR summary pause, and settlement pause with a provider timestamp, no timestamp, or an inner typed timestamp. Main's branch-retention tests remain intact.
• Genuine before-fix/inverse probes failed on the expected log-level differences. Both affected service test files pass all 48 tests on the final implementation head.
• Scoped server typecheck, lint, formatting and diff checks pass. Lint retains one pre-existing unused-variable warning.
• This changes server logging only. Tests inject actual typed errors through service dependencies; no live GitHub quota exhaustion, provider calls, user database writes or UI proof is claimed.

Fresh complete independent Claude Opus 5.5 and GPT 6.1 Sol reviews approve 1d98d5a564.

Implementation: Claude Fable 5.1. Minimal tests and independent review: Claude Opus 5.5. Coordination and independent review: GPT 6.1 Sol, through T3 Code.

Takeover

Credit to @aravhawk for the original contribution; the author’s commits are retained. This replacement uses Info and a single text record because the pretty logger spreads structured context over multiple lines.
Forge-owned driver classifiers also cover initial refusals, including Bitbucket 429 body-read failures. Duplicate timestamp tests were replaced with service regressions for previously missed real error types.

Evidence

Before: 76da264a0c. After: bcfa2526f1.
Fixture matrix · Complete paired transcripts and reproduction scripts · Independent audit.

Linux x86_64, Node v26.12.1, real source dev server/workers/pretty logger, fresh temporary homes. APP+STUB fetches loopback 429/502 responses and constructs typed failures; real forge HTTP/admission is bypassed. HARNESS rows call the classifier directly. No live data, real quota exhaustion or browser use.

Matrix row / fixture	Before	After	Proof
1 Wrapped admitted pause, grouped discovery	Warn + stack	One Info line + reset	APP+STUB
2 Unknown grouped discovery failure	Warn + stack	Warn + stack	APP+STUB
3 Paused saved-PR read	Warn + stack	One Info line + reset	APP+STUB
4 Unknown saved-PR failure	Warn + stack	Warn + stack	APP+STUB
5 Quota settlement, branch and saved forms	Warn + stack	One Info line + reset	APP+STUB
6 Unknown settlement, branch and saved forms	Warn + stack	Warn + stack	APP+STUB
7 Refusal without reset	Warn + stack	One Info line, no invented time	APP+STUB
8 Reset on an inner typed cause	Warn + stack	One Info line + inner reset	APP+STUB
9 Generic provider refusal with reset	Warn + stack	One Info line + reset	HARNESS
10 Initial GitHub refusal	Warn + stack	One Info line + reset	APP+STUB
11 Initial GitCafe 429	Warn + stack	One Info line, no invented time	APP+STUB
12 Unknown error mentions rate limit	Warn + stack	Warn + stack	HARNESS
13 Quota failure plus unknown failure	Warn + causes	Warn + causes	HARNESS
14 Quota failure plus defect	Warn + causes	Warn + causes	HARNESS
15 Typed pause as a defect	Warn + cause	Warn + cause	HARNESS
16 Other initial refusals across six forges	Warn + stack	One Info line, optional reset	HARNESS
17 Other forge errors, including body-read 502	Warn + stack	Warn + stack	HARNESS
18 Invalid/out-of-range reset	Warn + stack	One Info line, invalid time omitted	HARNESS
19 Bitbucket body-read 429	Warn + stack	One Info line + reset	APP+STUB

Paired app records: 18 distinct baseline Warn records become 14 single-line Info and 4 unknown Warn records. Repeated sweeps are also retained. Paired classifier records: 25 cases pass. State proof: nine equal thread payloads excluding timestamps, four saved references retained, no settlement or provider runs.

What else changed: expected log level and format only. No UI change. All 19 required matrix rows are covered.

Verification log

• Three independent Codex/Claude review rounds. Fixed initial GitHub/GitCafe refusals, nonthrowing reset formatting, and the Bitbucket body-read variant. Both reviewers returned MERGE in round three at the final head.
• PR-added cases: 6 → 4 after pruning duplicate reset-time cases, then 3 service regressions added for missed initial refusal shapes: 7 final cases. Affected files pass 12 + 23 = 35 tests.
• Final gate: 42/42 tests across the two services, registry and quota admission; formatting and server typecheck pass. Changed-file lint has only the pre-existing unused layer warning.
• Independent proof audit: SUFFICIENT, rows 1–19; corrected publication verified and withdrawn exports return 404. Rebuilt an affected app case with three empty payload deltas and all nine state payloads matching.
• CI: pending on the replacement PR at publication; no merge performed.
• Unverified surfaces: none among changed server logging paths. Real forge HTTP/admission and real reset timing were not exercised; APP+STUB explicitly bypasses them. Web, desktop and mobile rendering are unchanged.
• Temporary servers stopped; all proof scripts uploaded, none committed. Original contributor head stayed unchanged throughout the takeover.
• Models/harness: Claude Opus 5.5 for tests and implementation; GPT 6.1 Sol and Claude Opus 5.5 for independent reviews; GPT 6.1 Sol for evidence and proof audit, coordinated through the Codex harness in T3 Code.

Fixes #19781

[macroscopeapp]

Approvability

Verdict: Approved at bcfa252

Macroscope's review found this PR approvable — This PR narrowly adjusts background pull-request failure logs so recognized forge quota pauses produce compact Info records while unexpected failures retain warning stacks. Request, retry, discovery, settlement, and stored-data behavior remain unchanged.

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

[github-actions]

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	0 B (0.0%)	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	0 B (0.0%)	2.0 KiB	✅
Claude	Live turn WebSocket decoded	21.2 KiB	21.2 KiB	0 B (0.0%)	29.3 KiB	✅
Claude	Live turn messages	2	2	0 (0.0%)	8	✅

Baseline: c4fa462 · PR result: bcfa252 · 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.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The change adds rate-limit refusal recognition to source-control drivers and centralizes failure logging for thread pull-request discovery and settlement. Recognized rate limits log at info level; other failures retain warning-level logs.

Changes: Rate-limit refusal logging

Layer / File(s)	Summary
Refusal contract and driver recognition packages/source-control-core/src/server/driver.ts, packages/source-control-*/src/server/driver.ts	The driver interface adds an optional rateLimitRefusal callback and refusal details with an optional retry time. Provider drivers recognize rate-limit errors, with Bitbucket and GitHub returning retry times when available.
Failure classification and logging apps/server/src/orchestration-v2/threadPullRequestFailure.ts	The shared helper checks paused errors, provider refusals, and driver refusals. Recognized rate limits log at info level with thread IDs and a formatted reset time when available. Other failures log at warning level with the formatted cause.
Thread service integration and tests apps/server/src/orchestration-v2/ThreadPullRequestService.ts, apps/server/src/orchestration-v2/ThreadSettlementService.ts, apps/server/src/orchestration-v2/ThreadPullRequestService.test.ts, apps/server/src/orchestration-v2/ThreadSettlementService.test.ts	Both services use the shared helper for failure logging. Tests cover paused lookups, provider rate limits, and unexpected failures, including the absence of dispatched commands in the tested rate-limit cases.

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

Change: Bug fix · Severity of issue fixed: Low

Possibly related issues

• Issue 23702 — The change logs known quota-pause failures at info level, but does not stop repeated discovery requests during the pause.

Suggested reviewers: maria-rcks, macroscopeapp

Merge Risk: Minimal · up to bcfa2

No confirmed issue blocks merging the quota-logging change after normal checks.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title "fix(server): log known forge quota pauses without warning stacks" matches the main change. The changeset adds logThreadPullRequestFailure, which logs recognized forge rate-limit pauses as o…
Linked Issues check		Issue #19781 asks that expected quota pauses in thread PR discovery and automatic settlement stop emitting repeated full warning stacks. Unexpected failures must keep warning stacks. Evidence at head:…
Out of Scope Changes check		Changes outside apps/server are the optional rateLimitRefusal hook in packages/source-control-core/src/server/driver.ts and per-forge implementations in the GitHub, GitLab, GitCafe, Bitbucket, Forgejo…
Description check		The description covers the problem, changes, scope and approval, and verification. It includes focused test results and states which real forge behavior was not tested.

Full details: Title check

Explanation

The title "fix(server): log known forge quota pauses without warning stacks" matches the main change. The changeset adds logThreadPullRequestFailure, which logs recognized forge rate-limit pauses as one Info line and keeps warning stacks for unknown failures. The title is specific and conventional, and it names the server scope.

Full details: Linked Issues check

Explanation

Issue #19781 asks that expected quota pauses in thread PR discovery and automatic settlement stop emitting repeated full warning stacks. Unexpected failures must keep warning stacks. Evidence at head: apps/server/src/orchestration-v2/threadPullRequestFailure.ts defines logThreadPullRequestFailure. It walks the cause chain for SourceControlRateLimitPausedError, rate-limited PullRequestProviderError, and driver rateLimitRefusal results. For a recognized pause it calls Effect.logInfo once, with thread IDs and the reset time when it parses. Otherwise it calls Effect.logWarning with Cause.pretty. ThreadPullRequestService.ts now routes the per-thread discovery catch (about line 261) and the grouped synchronize catch (about line 341) through this helper. The whole-PR summary states that ThreadSettlementService.ts routes its group-read failures through the same helper, with a settlement test for a GitLab rate limit. Requirement mapping: [#19781] expected pauses are summarized as one Info line per sweep, which the issue allows ('summarized or throttled'). Repeated stacks are removed. Unknown failures, defects, and compound causes (more than one failure reason) still log Warn with stacks. The PR does not add throttling, and the issue does not require it. Unresolved uncertainty: the settlement call-site diff and the driver diffs could not be read directly. This verdict relies on the whole-PR summary for those hunks.

Full details: Out of Scope Changes check

Explanation

Changes outside apps/server are the optional rateLimitRefusal hook in packages/source-control-core/src/server/driver.ts and per-forge implementations in the GitHub, GitLab, GitCafe, Bitbucket, Forgejo, and Azure DevOps drivers. The helper uses these hooks to recognize initial forge refusals that reach the catch before quota admission wraps them. The PR matrix rows 10, 11, 16, and 19 cover these cases, so the hooks support the logging objective of [#19781]. Each hook returns undefined for unrecognized errors, so unknown failures keep their warning stacks. The issue names apps/server as its reported scope. The package changes extend recognition beyond that scope but serve the same log-level behavior. The PR description and summary report no change to quota admission, retry scheduling, settlement eligibility, or backfill. Those claims are not contradicted by the reviewed evidence. Test files and the helper are in scope as supporting tests and implementation. Unresolved uncertainty: the driver diffs could not be read directly. This verdict relies on the summary's description of each hook.

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

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

Recent review info

Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: takeover/19794-quota-pause-logs
• File: .coderabbit.config.ts
• Commit: bcfa252
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Team
• Run ID: aeee974d-438d-47ad-a627-fa1ef9187b13

Commits

Reviewing files that changed from the base of the PR and between c4fa462 and bcfa252.

Files selected for processing: 12

• apps/server/src/orchestration-v2/ThreadPullRequestService.test.ts
• apps/server/src/orchestration-v2/ThreadPullRequestService.ts
• apps/server/src/orchestration-v2/ThreadSettlementService.test.ts
• apps/server/src/orchestration-v2/ThreadSettlementService.ts
• apps/server/src/orchestration-v2/threadPullRequestFailure.ts
• packages/source-control-azure-devops/src/server/driver.ts
• packages/source-control-bitbucket/src/server/driver.ts
• packages/source-control-core/src/server/driver.ts
• packages/source-control-forgejo/src/server/driver.ts
• packages/source-control-gitcafe/src/server/driver.ts
• packages/source-control-github/src/server/driver.ts
• packages/source-control-gitlab/src/server/driver.ts

Limit details: You’ve used all 10 included reviews currently available.