Sign up
View stargazersSign up

[Bug]: Threads whose branch PR is merged or closed re-read git state every minute#24098

opened23d agobyBBatuhan1999

Area

apps/server

Steps to reproduce

  1. Have a thread on a feature branch whose pull request is merged or closed, and keep the thread unsettled. Any of these does it:
    • the user wrote in the thread after the merge, so the merge does not settle it
    • sidebarAutoSettleOnMerge is off
    • sidebarAutoSettleKeepPullRequests is on, or inactivity settling is off
  2. Leave the server running. The settlement sweep and the PR-sync sweep each run once a minute.
  3. Count git processes, for example with runGitCommand spans in server.trace.ndjson or a process monitor.

Expected behavior

Once a branch's PR answer and git state are cached, a sweep that only finds "merged, but this thread stays" starts no git process. That is what happens for an open PR, and #17063's 5-minute git-state cache (PR_LOOKUP_GIT_STATE_TTL) is meant to allow it. Fresh git reads should happen only where an answer is about to settle a thread or be saved as a link.

Actual behavior

GitManager.branchPullRequest skips the git-state cache for every merged or closed answer, unless the caller already asked for fresh state (apps/server/src/git/GitManager.ts:2693 on main):

ts
1if (pullRequest === null || pullRequest.state === "open" || freshGitState) return pullRequest;
2return yield* lookupBranchPullRequest(cacheCwd, branch, { refresh: false, freshGitState: true });

The fresh lookup runs git remote, git for-each-ref, the remote default-branch read and one or two git config reads. That is 4 to 6 processes; a real-git test with an origin-tracking branch counts 4.

Two once-a-minute sweeps call it without refresh, so each pays that confirm every minute:

  • ThreadSettlementService.ts:503 (sweep scheduled at :622) passes the branch lookup's answer to settleThread. When the answer is terminal and the thread stays, nothing happens, and the confirm was wasted.
  • ThreadPullRequestService.ts:197 (loop at :400) uses the answer to plan link updates. When the thread already holds that PR, nothing happens. When it would write, it already rereads git state with { freshGitState: true } at :275 before dispatching.

So each unsettled checkout and branch with a terminal PR costs about 8 to 12 git processes a minute, for as long as the thread exists. Open PRs and branches without a PR cost nothing on a warm cache.

#17063 added this confirm on purpose: a merged or closed answer can settle a thread or remove a worktree, so that decision must not rest on remembered git state. Neither sweep needs the confirm before it decides whether to act:

  • Settlement's linked-PR path already checks wouldSettle first and confirms with refresh: true only when it would settle (ThreadSettlementService.ts:482-485). The branch path can do the same.
  • PR sync confirms with freshGitState: true (:275) before every write.

Impact

Minor bug or occasional failure

There are no wrong results, only steady background load. Each git process costs more than a git run alone: on Windows #17063 measured about 90 ms of CPU each, and Node starts every one synchronously on the main thread. One user's telemetry from the Oct 11 nightly showed PR-lookup git work as the largest server load, about 100 git processes a minute while active. #17063's cache removed most of that. The terminal-state path described here is what remains. Its cost on current main is worked out from reading the code and from the test's spawn count. I have not measured it on a running server.

Version or commit

main @ 16aaf5b

Environment

Any OS. Server-side background sweeps. The cost per process is highest on Windows.

Workaround

Settle or archive threads whose PR is merged or closed. Settled and archived threads leave both sweeps.

Bbrandus115d ago
Synced from GitHub

Seen at scale on 0.1.6-nightly.20261030.3917, Linux arm64 headless server: about 390 worktrees and 1,466 settled threads with a worktree and branch, which together link 4,146 PRs, nearly all merged or closed.

t3 trace summary over 28 minutes:

  • runGitCommand: 832/min, p50 477 ms (about 6–7 git processes alive at any moment);
  • GitVcsDriver.readConfigValue: 349/min;
  • GitManager.rememberPrLookupGitState: 319/min;
  • lookupBranchPullRequest / branchPullRequest: 64/min, p50 6.5–7.2 s.

The service log over one day: 24,509 "pull request sync skipped" and 3,033 "thread branch pull request lookup failed".

As a workaround we are unlinking merged and closed PRs from settled threads, and removing worktrees whose branch has merged.

HHarshSharma9x99d ago
Synced from GitHub

Same on 0.1.7 stable, Linux x64 headless (6 vCPU / 12 GB, ~100 worktrees). In a 60 s trace window: 228 runGitCommand spans totalling 407 s; 149 rememberPrLookupGitState, 88 readConfigValue. On a loaded host a single git config read took ~2 s, so the PR sweep alone keeps the event loop at 100% utilization and pushes message.dispatch to 11–36 s.

EElliotDrel5d ago
Synced from GitHub

Note

🤖 Claude Opus 5.5 responding on behalf of Elliot

Still present on 0.1.8-nightly.20261109.4399. This is a Windows 11 desktop host with 148 worktrees and 403 threads, none archived, monitoring about 20 open PRs.

Server traces, 68 minutes, mostly idle host:

  • runGitCommand: about 309/min, which is about 5 git processes per second.
  • GitManager.rememberPrLookupGitState: about 430/min.
  • GitVcsDriver.readConfigValue: about 135/min. It takes 676 ms at the median, likely mostly queueing for the 2-slot background git lane.
  • branchPullRequest / lookupBranchPullRequest: about 85/min. The median is about 3 s and the worst was 38 s.

On this build the settled-PR path (GitManager.ts:2736-2768) caches the confirmation for SETTLED_PR_LOOKUP_RECHECK_INTERVAL. However, readSettledPrLookupFingerprint (:2743) still runs for-each-ref with fresh state plus config --get-regexp on every call, before the cache check. That is 2 git processes per settled thread per sweep.

Separately, the prLookupCache loader goes through resolveLookupHeadContext → resolveBranchHeadContext / resolveRemoteRepositoryContext (:1521-1610). Those call readConfigValueNullable, which bypasses the rememberPrLookupGitState cache. That is 2–4 git config --get processes per cache miss, so the 60 s open-PR TTL turns into a steady stream.

Possible fixes:

  • Read each repository's config once per TTL, for example with git config --list -z keyed on the git common dir, instead of one process per key.
  • Skip the fingerprint read while the settled entry is still within its recheck interval.

Windows impact: see #25245. Each of these starts blocks the event loop, and under host load each start costs hundreds of ms.