Skip to content

chore(fork): carry the settled-pr-lookups patch - #180

Merged
fireboltdude1357 merged 1 commit into
patch/releasefrom
chore/carry-settled-pr-lookups
Nov 3, 2026
Merged

fireboltdude1357 merged 1 commit into
patch/releasefrom
chore/carry-settled-pr-lookups

Conversation

@fireboltdude1357

Copy link
Copy Markdown
Owner

Problem

The stl-wsl server has been slow again. A 20-minute CPU profile of the live server showed the main thread spent 272 s inside spawn. That's 48% of its busy time. Each spawn blocks the event loop while the kernel copies the ~900 MB server process, and the server ran about 7,150 of them in those 20 minutes. The settlement sweep started about 59% of them. When the cached PR for a branch is closed or merged, the sweep reruns the whole git lookup every minute.

Change

Adds settled-pr-lookups patch/settled-pr-lookups pingdotgg/t3code#29721 to .github/fork-patches.txt.

patch/settled-pr-lookups is the head of upstream pingdotgg#29721 (ca5698a) with nothing added. That PR is from Julius. Once a fresh lookup confirms a PR is closed or merged, later sweeps check a 2-process fingerprint of the branch and its remotes instead of the ~6-process lookup, and still do a full fresh lookup at most every 15 minutes. CodeRabbit and Macroscope reviewed it upstream, and its CI is green. Drop this line when pingdotgg#29721 merges.

Verification

  • node scripts/fork-sync.ts --force --patch-list <this list> --workdir /tmp/fork-sync-spl merges every patch onto upstream cefb544af3 and builds 0.1.7-nightly.20261103.411901 (853af108ff). The only warning is the existing wal-pinned-writes one.
  • On that merged tree, vp test run apps/server/src/git/GitManager.test.ts passes 116 of 116, including the PR's new cases, and the server typecheck passes.
  • Not yet measured: spawns per minute on stl-wsl after the build installs. The plan is to rerun the same profile capture and compare.

Review tier: 7/10, changes PR-state caching that background auto-settle acts on

Two-way door: removing the line drops the patch from the next build

Blast radius: PR status and auto-settle on every fork server

Written by Claude Opus 5.5 in Claude Code, running in T3 Code.

🤖 Generated with Claude Code

The settlement sweep re-spawns git for every closed or merged PR lookup
each minute, and those spawns block the server's main thread. Carry
upstream pingdotgg#29721 until it lands.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Nov 3, 2026

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Walkthrough

The nightly fork-patch list now includes the settled-pr-lookups branch and its upstream tracking reference.

Changes: Nightly fork-patch list

Layer / File(s) Summary
Add settled-pr-lookups entry
.github/fork-patches.txt
The nightly patch list adds branch patch/settled-pr-lookups and tracks pingdotgg/t3code#29721.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Merge Risk: Minimal · up to 7b294

The patch-list update is narrowly scoped and presents no identified merge-blocking behavior.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: carrying the settled-pr-lookups fork patch.
Description check Passed The description includes Problem, Change, and Verification sections. It explains the performance issue, the patch behavior, the scope, review context, verification results, and the remaining productio…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description includes Problem, Change, and Verification sections. It explains the performance issue, the patch behavior, the scope, review context, verification results, and the remaining production measurement. The required Scope and approval heading is not present, but the needed scope and approval context is substantially included.

Finishing Touches
Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.
Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 95339ce and 7b2946c.

Files selected for processing: 1
  • .github/fork-patches.txt

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

@fireboltdude1357 fireboltdude1357 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Manual review round 1: crev (claude-opus-5-5 + gpt-6.1-sol) (tier 7/10, deep review)

No verified findings.

Reviewed 1 files (+1 -0), 95339cee..7b2946ca against origin/patch/release.

Uncertain: the verifier could not decide (1)
  • .github/fork-patches.txt:29 Settled PR fingerprint may skip the remote-tracking-ref ambiguity check for deleted branches [minor]: I couldn't read the patch's code. This diff only adds the list line; the patch/settled-pr-lookups code isn't in this tree or any checked-out worktree I could find, and Bash/git was denied. What the base code shows:
  • apps/server/src/git/GitManager.ts:2515-2547: when the local branch is gone, it lists refs/remotes and throws "Multiple remotes track ${branch}. Its pull request is ambiguous." when more than one remote has the branch.
  • GitManager.ts:2381-2396 and line 173: these git reads are kept for PR_LOOKUP_GIT_STATE_TTL (5 minutes) unless fresh is set.
  • GitManager.ts:2644-2648: branchPullRequest reruns the lookup with freshGitState: true whenever the result isn't open. So before this patch, a closed or merged result always rereads the tracking refs, and the ambiguity in the trigger gets caught. The trigger is not reachable in the base code.
  • The PR description says the patch replaces that full lookup with "a 2-process fingerprint of the branch and its remotes", which fits the claim but doesn't prove it. A single for-each-ref refs/heads/<b> refs/remotes/*/<b> call would cover tracking refs within 2 processes.
    To settle it, run git show origin/patch/settled-pr-lookups:apps/server/src/git/GitManager.ts and check whether the settled fingerprint includes refs/remotes/*/<branch> (or the full refs/remotes listing) when the local branch doesn't exist.

Tier 7/10 from claude-opus-5-5: Adds a patch to the fleet build list: PR-state caching code that background auto-settle acts on, on every fork server. Stale caching could fail quietly.

Complexity medium: The diff is one config line, but it pulls in upstream caching and fingerprint logic for PR-state lookups. That logic is hard to get right.

@fireboltdude1357 fireboltdude1357 added the coderabbit-queue Waiting for a CodeRabbit review from crev queue label Nov 3, 2026
@fireboltdude1357
fireboltdude1357 merged commit b634ab7 into patch/release Nov 3, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

coderabbit-queue Waiting for a CodeRabbit review from crev queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant