Skip to content

fix(server): v2 background GitHub checks spend less quota - #13202

Closed
juliusmarminge wants to merge 640 commits into
t3code/codex-turn-mappingfrom
v2-github-quota
Closed

juliusmarminge wants to merge 640 commits into
t3code/codex-turn-mappingfrom
v2-github-quota

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Sep 23, 2026 •

Copy link
Copy Markdown
Member

Ports the GitHub quota stack (#13189, #13198, #13200) onto the v2 orchestrator branch.

Problem

v2 has the same background GitHub loops as main, so it drains the shared 5000/h GraphQL quota the same way:

  • branch PR lookups re-ask every minute for branches that have no open PR;
  • the settlement sweep does an uncached branch recheck for every terminal link, even when the group would not settle;
  • PR sync reads summaries one gh pr view at a time;
  • fork lookups probe owner:branch heads, which gh pr list never matches.

Fix

  • GitManager: answers without an open PR are cached for 5 minutes instead of 1. This cherry-picked cleanly.
  • orchestration-v2/ThreadSettlementService: replaces main's ThreadSettlementReactor, so it gets a hand port of the wouldSettle gate. The uncached branch recheck runs only when the linked PR is terminal and the group would actually settle. A group that becomes eligible only after that check waits for the next sweep instead of settling on an unverified link. The zero-lookup test is ported too.
  • GitHubPullRequestCli: summary reads are batched into one aliased GraphQL query per host and credential, with a per-PR gh pr view fallback. PullRequestSyncReactor's sweep goes to concurrency 25. v2's ThreadPullRequestService reads through the same summary path at concurrency 8, so it batches as well. This cherry-picked cleanly.
  • GitManager: GitHub head lookups skip owner:/remote: selectors. This cherry-picked cleanly.

Estimates are in the main PRs. The per-call savings are the same here.

Verification

  • vp test run over ThreadSettlementService, ThreadPullRequestService, GitManager, GitHubPullRequestCli, gitHubPullRequestJson and PullRequestSyncReactor: 362 passed.
  • apps/server typecheck is clean.

Made by Claude Opus 5.5 in Claude Code.

🤖 Generated with Claude Code


Devin Review

juliusmarminge and others added 30 commits September 22, 2026 17:46
Accumulate owned step usage once and preserve partial or unavailable telemetry for failed, interrupted, or reconnected turns.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Mike Olson <mwolson@member.fsf.org>
Co-authored-by: Julius Marminge <julius0216@outlook.com>
Co-authored-by: Julius Marminge <julius0216@outlook.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…8464)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Julius Marminge <julius0216@outlook.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
juliusmarminge and others added 23 commits September 22, 2026 17:46
…12946)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Co-authored-by: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Co-authored-by: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com>
)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Background-task notifications and delegated completions queued behind a
held queue are hidden from the queue UI, so the Resume queue control never
appeared, yet thread.settle counted them as active work. Settle now cancels
those automatic runs in the same transaction; user-queued messages still
block it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Branch PR lookups without an open PR now stay cached for 5 minutes instead
of 1, so idle branches stop costing a GraphQL call on every sweep. In-app
paths that can open a PR (turn end, push, create PR, user refresh) still
bypass the cache.

The v2 settlement sweep only re-checks a branch uncached when the linked PR
is terminal and the group would actually settle. A group that only becomes
eligible after that check waits for the next sweep.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Concurrent summary reads on one host and credential now share aliased
GraphQL requests of 25 through a RequestResolver, falling back to
`gh pr view` for anything the batch could not answer. The sync sweep runs
25 groups at once so a sweep's reads land in the same batch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`gh pr list --head` filters on the head ref name alone, so `owner:branch`
and `remote:branch` selectors always list nothing while spending a GraphQL
call. Fork lookups on GitHub now query the bare branch and let
matchesBranchHeadContext pick the right fork.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Sep 23, 2026
limit: 1,
// A bare branch name also lists same-named branches on other forks,
// so leave room for the owner match below.
limit: OPEN_PR_PROBE_LIMIT,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High git/GitManager.ts:1633

findOpenPr can return null for an existing PR when more than 10 open PRs share the branch name across forks: limit: OPEN_PR_PROBE_LIMIT truncates the provider response before matchesBranchHeadContext filters it to the checkout's fork. runPrStep then creates a duplicate instead of returning opened_existing. Paginate the provider results or query with an owner-qualified selector before applying the local filter.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/git/GitManager.ts around line 1633:

`findOpenPr` can return `null` for an existing PR when more than 10 open PRs share the branch name across forks: `limit: OPEN_PR_PROBE_LIMIT` truncates the provider response before `matchesBranchHeadContext` filters it to the checkout's fork. `runPrStep` then creates a duplicate instead of returning `opened_existing`. Paginate the provider results or query with an owner-qualified selector before applying the local filter.

@macroscopeapp

macroscopeapp Bot commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — The PR substantially changes production GitHub polling, caching, batching, and settlement behavior, including a five-minute default cache for terminal/no-PR results. It also adds a static-analysis suppression and leaves an unresolved high-severity risk where the ten-result probe limit can miss an existing PR and create a duplicate.

Not approved because:

  • 1 blocking correctness issue found at or above your repo's Minimum Blocking Severity

Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.

@github-actions

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire — 4.9 KiB — 6.8 KiB ✅
Codex Thread snapshot wire — 3.7 KiB — 4.9 KiB ✅
Codex Live turn WebSocket wire — 1.2 KiB — 2.0 KiB ✅
Codex Live turn WebSocket decoded — 20.4 KiB — 29.3 KiB ✅
Codex Live turn messages — 2 — 8 ✅
Claude Total thread wire — 4.9 KiB — 6.8 KiB ✅
Claude Thread snapshot wire — 3.7 KiB — 4.9 KiB ✅
Claude Live turn WebSocket wire — 1.2 KiB — 2.0 KiB ✅
Claude Live turn WebSocket decoded — 20.8 KiB — 29.3 KiB ✅
Claude Live turn messages — 2 — 8 ✅

Baseline: unavailable · PR result: e18ee51 · Source CI: failure

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: 106.1 KiB
  • Claude decoded thread snapshot: 106.4 KiB

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.