fix(git): stop background sync-status polls from hitting the network#2955
Conversation
`useGitQueries` runs `getGitSyncStatus` on a 60s `refetchInterval` for every open repo/worktree. The server handler always calls `git fetch` before returning, throttled to one fetch per 30s per directory. Over an SSH remote that means a `git fetch` → `ssh` → ssh-agent signing request roughly every minute per worktree, even when the user is idle. With Secretive (or any hardware-backed SSH agent), each signing request triggers a Touch ID / password prompt; concurrent worktrees produce overlapping prompts that cancel each other (`SecretAgent: Canceled by another authentication`). Rename the existing `forceRefresh` input on `getGitSyncStatus` to `fetchFromRemote` and flip its semantics: the procedure no longer runs `git fetch` by default. Callsites that genuinely need an up-to-date view of `origin/*` opt in: - `GitHost.getPrStateSnapshot` (PR-create flow) - the agent service's explicit `fetchIfStale` adapter Everything else — the UI sync-status poll, the post-mutation `getStateSnapshot`, `TaskPrStatusService` revalidations, the PR-create saga's `hasRemote` check — reads local refs only. They were either already reading purely local fields (`hasRemote`, `aheadOfDefault`, `currentBranch`, `isFeatureBranch`) or were happy with the local-only view of `aheadOfRemote`/`behind` that the existing cache invalidations from commit/push/pull/branch-switch already keep fresh. Added integration test that pushes from another clone and asserts `getGitSyncStatus` reports `behind: 0` by default but `behind: 1` with `fetchFromRemote: true`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
|
Reviews (1): Last reviewed commit: "fix(git): stop background sync-status po..." | Re-trigger Greptile |
Greptile flagged that the previous `forceRefresh: true` path explicitly cleared the throttle entry before calling `fetchIfStale`, so an opt-in fetch was guaranteed to hit the network. The renamed `fetchFromRemote: true` path just called `fetchIfStale`, which still honored the 30s throttle — meaning a PR-create snapshot fired right after a push could silently see stale ahead/behind counts at exactly the moment freshness matters. Extract a `forceFetch` helper that always runs `git fetch` and stamps the throttle clock. `fetchIfStale` now delegates to it; the opt-in `fetchFromRemote: true` path on `getGitSyncStatus` calls it directly. Added an assertion in the existing integration test that a second back-to-back `getGitSyncStatus(work, true)` (well within the throttle window) still picks up a new remote commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jonathanlab
left a comment
There was a problem hiding this comment.
LGTM, left some nits, thank u!
| } catch {} | ||
| } | ||
|
|
||
| private async fetchIfStale(directoryPath: string): Promise<void> { |
There was a problem hiding this comment.
this now always force fetches right, e.g. still calling it fetchIfStale is a lie
There was a problem hiding this comment.
Good nit — the private fetchIfStale inside GitService still respects the throttle, so the name's accurate there. The actually-lying one was the agent adapter exposed as GitDiffSource.fetchIfStale (in apps/code/src/main/di/container.ts) — it goes through getGitSyncStatus.query({ fetchFromRemote: true }), which since the throttle-bypass fix unconditionally hits forceFetch. Renamed that interface method (and its caller in git-pr.ts) to fetchFromRemote in f4c40b9 so the public contract matches the behavior.
| try { | ||
| await gitFetch(directoryPath); | ||
| this.lastFetchTime.set(directoryPath, Date.now()); | ||
| } catch {} |
There was a problem hiding this comment.
Why are we silently catching here?
There was a problem hiding this comment.
I just saw that the old code did the same lol, but let's make this catch non-silent while we're at it
There was a problem hiding this comment.
Done in f4c40b9 — forceFetch now writes the error to process.stderr before falling back to local refs (matches the existing pattern in packages/git/src/signed-commit.ts and workspace-server/src/serve.ts). I tried injecting ROOT_LOGGER first, but @inject(...) parameter decorators on GitService blow up workspace-client's typecheck — its tsconfig has no experimentalDecorators and it transitively imports GitService's type via the AppRouter — so I went with the stderr write to avoid a DI restructure for a nit.
The `forceFetch` helper previously swallowed errors in `catch {}`, so a
failed network fetch left no trace in the host's stderr. Surface the
failure via `process.stderr.write` (matches the pattern used in
`packages/git/src/signed-commit.ts` and `workspace-server/src/serve.ts`)
while still falling back gracefully to local refs.
Also rename the `GitDiffSource.fetchIfStale` adapter (exposed by the
agent service in `apps/code/src/main/di/container.ts`) to
`fetchFromRemote`. Its implementation calls
`getGitSyncStatus.query({ fetchFromRemote: true })`, which always force
fetches and bypasses the staleness throttle — the old name lied about
the behavior. The private `fetchIfStale` helper inside `GitService` keeps
its name; it still respects the throttle.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
useGitQueriesrunsgetGitSyncStatuson a 60srefetchIntervalfor every open repo/worktree. The server handler always calledgit fetchbefore returning (throttled to one fetch per 30s per directory). Over an SSH remote that means agit fetch→ssh→ ssh-agent signing request roughly every minute per worktree, even when the user is idle. With Secretive (or any hardware-backed SSH agent), each signing request triggers a Touch ID / password prompt; concurrent worktrees produce overlapping prompts that cancel each other:This PR renames the existing
forceRefreshinput ongetGitSyncStatustofetchFromRemoteand flips its semantics: the procedure no longer runsgit fetchby default. Callsites that genuinely need an up-to-date view oforigin/*opt in:GitHost.getPrStateSnapshot(PR-create flow) —packages/core/src/git/git-host.ts:132fetchIfStaleadapter —apps/code/src/main/di/container.ts:504Everything else — the UI sync-status poll, the post-mutation
getStateSnapshot,TaskPrStatusServicerevalidations, the PR-create saga'shasRemotecheck — reads local refs only. They were either already reading purely local fields (hasRemote,aheadOfDefault,currentBranch,isFeatureBranch) or were happy with the local-only view ofaheadOfRemote/behindthat existing cache invalidations from commit/push/pull/branch-switch already keep fresh.Trade-off
Background "ahead of origin / behind origin" badges no longer self-update when an outside party pushes to upstream. They refresh whenever the user does a local action (commit, push, pull, branch switch — all already trigger invalidations) or opens the PR-create flow. The user-visible effect is missing teammate-side drift between idle interactions, which feels like an acceptable price for not hammering ssh-agent.
Test plan
packages/workspace-server/src/services/git/git.integration.test.ts: another clone pushes to a shared bare remote;getGitSyncStatus(work)reportsbehind: 0by default,getGitSyncStatus(work, true)reportsbehind: 1.pnpm lintclean.claude-cli-sessions/taskCreationSagafailures onmainare unrelated).🤖 Generated with Claude Code