[Bug]: Threads whose branch PR is merged or closed re-read git state every minute#24098
Area
apps/server
Steps to reproduce
- 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
sidebarAutoSettleOnMergeis offsidebarAutoSettleKeepPullRequestsis on, or inactivity settling is off
- Leave the server running. The settlement sweep and the PR-sync sweep each run once a minute.
- Count git processes, for example with
runGitCommandspans inserver.trace.ndjsonor 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):
| 1 | if (pullRequest === null || pullRequest.state === "open" || freshGitState) return pullRequest; |
| 2 | return 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 tosettleThread. 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:275before 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
wouldSettlefirst and confirms withrefresh: trueonly 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.