From ef9161bfe1ae005f78a8077dee9fc3438d7f562e Mon Sep 17 00:00:00 2001 From: dmarticus Date: Fri, 26 Jun 2026 14:21:41 -0700 Subject: [PATCH 1/3] fix(git): stop background sync-status polls from hitting the network MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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) --- apps/code/src/main/di/container.ts | 2 +- packages/core/src/git/git-host.ts | 5 +++- .../host-router/src/routers/git.router.ts | 4 +-- .../src/services/git/git.integration.test.ts | 26 +++++++++++++++++++ .../src/services/git/schemas.ts | 8 +++++- .../src/services/git/service.ts | 13 +++++----- packages/workspace-server/src/trpc.ts | 2 +- 7 files changed, 47 insertions(+), 13 deletions(-) diff --git a/apps/code/src/main/di/container.ts b/apps/code/src/main/di/container.ts index 97971fc4cf..b455aab074 100644 --- a/apps/code/src/main/di/container.ts +++ b/apps/code/src/main/di/container.ts @@ -504,7 +504,7 @@ container.bind(GIT_DIFF_SOURCE).toDynamicValue((ctx) => { fetchIfStale: async (directoryPath: string) => { await git().getGitSyncStatus.query({ directoryPath, - forceRefresh: true, + fetchFromRemote: true, }); }, }; diff --git a/packages/core/src/git/git-host.ts b/packages/core/src/git/git-host.ts index 86df6e6ad7..0084145d07 100644 --- a/packages/core/src/git/git-host.ts +++ b/packages/core/src/git/git-host.ts @@ -129,7 +129,10 @@ export class GitHostService extends TypedEventEmitter { await Promise.allSettled([ this.git.getChangedFilesHead.query({ directoryPath }), this.git.getDiffStats.query({ directoryPath }), - this.git.getGitSyncStatus.query({ directoryPath, forceRefresh: true }), + this.git.getGitSyncStatus.query({ + directoryPath, + fetchFromRemote: true, + }), this.git.getLatestCommit.query({ directoryPath }), this.git.getPrStatus.query({ directoryPath }), ]); diff --git a/packages/host-router/src/routers/git.router.ts b/packages/host-router/src/routers/git.router.ts index aa27f3e2d7..35c98088cd 100644 --- a/packages/host-router/src/routers/git.router.ts +++ b/packages/host-router/src/routers/git.router.ts @@ -319,14 +319,14 @@ export const gitRouter = router({ .input( z.object({ directoryPath: z.string(), - forceRefresh: z.boolean().optional(), + fetchFromRemote: z.boolean().optional(), }), ) .output(getGitSyncStatusOutput) .query(({ ctx, input }) => getWorkspaceClient(ctx.container).git.getGitSyncStatus.query({ directoryPath: input.directoryPath, - forceRefresh: input.forceRefresh, + fetchFromRemote: input.fetchFromRemote, }), ), diff --git a/packages/workspace-server/src/services/git/git.integration.test.ts b/packages/workspace-server/src/services/git/git.integration.test.ts index b4b4d21391..e00a7521e2 100644 --- a/packages/workspace-server/src/services/git/git.integration.test.ts +++ b/packages/workspace-server/src/services/git/git.integration.test.ts @@ -300,5 +300,31 @@ describe("GitService integration (git-read + git-mutate)", () => { const result = await git.sync(work, "origin"); expect(result.success).toBe(true); }); + + it("getGitSyncStatus does not fetch by default, fetches when fetchFromRemote=true", async () => { + // Another clone pushes a commit. The original `work` repo only learns + // about it after a fetch — so this lets us tell whether a sync-status + // read touched the network or not. + const otherClone = await fs.mkdtemp(path.join(os.tmpdir(), "git-other-")); + dirs.push(otherClone); + run(`git clone ${bare} ${otherClone}`, os.tmpdir()); + run("git config user.email 'other@test.com'", otherClone); + run("git config user.name 'Other'", otherClone); + run("git config commit.gpgsign false", otherClone); + await fs.writeFile( + path.join(otherClone, "remote-only.txt"), + "from-other\n", + ); + commitAll(otherClone, "from other clone"); + run("git push origin main", otherClone); + + // Default: no fetch, so `work` still thinks it is at the remote tip. + const stale = await git.getGitSyncStatus(work); + expect(stale.behind).toBe(0); + + // Explicit fetch: `work` learns it is one commit behind. + const fresh = await git.getGitSyncStatus(work, true); + expect(fresh.behind).toBe(1); + }, 15_000); }); }); diff --git a/packages/workspace-server/src/services/git/schemas.ts b/packages/workspace-server/src/services/git/schemas.ts index 17314d0c13..6cc2cd86f9 100644 --- a/packages/workspace-server/src/services/git/schemas.ts +++ b/packages/workspace-server/src/services/git/schemas.ts @@ -169,7 +169,13 @@ export type DiscardFileChangesOutput = z.infer; export const getGitSyncStatusInput = z.object({ directoryPath: z.string(), - forceRefresh: z.boolean().optional(), + /** + * Whether to run `git fetch` before reading sync status. Defaults to false: + * background pollers should read local refs only so that idle UI does not + * keep hitting the network (and, transitively, ssh-agent). Set true at the + * few callsites that genuinely need an up-to-date view of `origin/*`. + */ + fetchFromRemote: z.boolean().optional(), }); export const gitBusyStateInput = directoryPathInput; diff --git a/packages/workspace-server/src/services/git/service.ts b/packages/workspace-server/src/services/git/service.ts index 5cb3b3148f..b108626656 100644 --- a/packages/workspace-server/src/services/git/service.ts +++ b/packages/workspace-server/src/services/git/service.ts @@ -445,12 +445,11 @@ export class GitService extends TypedEventEmitter { private async getGitSyncStatusInternal( directoryPath: string, - forceRefresh = false, + fetchFromRemote = false, ): Promise { - if (forceRefresh) { - this.lastFetchTime.delete(directoryPath); + if (fetchFromRemote) { + await this.fetchIfStale(directoryPath); } - await this.fetchIfStale(directoryPath); const status = await getSyncStatus(directoryPath); return { @@ -483,7 +482,7 @@ export class GitService extends TypedEventEmitter { includeChangedFiles ? this.getChangedFilesHead(directoryPath) : null, includeDiffStats ? this.getDiffStats(directoryPath) : null, includeSyncStatus - ? this.getGitSyncStatusInternal(directoryPath, true) + ? this.getGitSyncStatusInternal(directoryPath, false) : null, includeLatestCommit ? this.getLatestCommit(directoryPath) : null, ]); @@ -508,9 +507,9 @@ export class GitService extends TypedEventEmitter { async getGitSyncStatus( directoryPath: string, - forceRefresh = false, + fetchFromRemote = false, ): Promise { - return this.getGitSyncStatusInternal(directoryPath, forceRefresh); + return this.getGitSyncStatusInternal(directoryPath, fetchFromRemote); } async createBranch(directoryPath: string, branchName: string): Promise { diff --git a/packages/workspace-server/src/trpc.ts b/packages/workspace-server/src/trpc.ts index 629923dd30..7885d44c27 100644 --- a/packages/workspace-server/src/trpc.ts +++ b/packages/workspace-server/src/trpc.ts @@ -426,7 +426,7 @@ export function createAppRouter({ .query(({ input }) => gitService().getGitSyncStatus( input.directoryPath, - input.forceRefresh, + input.fetchFromRemote, ), ), From e96784093afd561cecffdc3c6cf1725ac0224e32 Mon Sep 17 00:00:00 2001 From: dmarticus Date: Fri, 26 Jun 2026 15:02:44 -0700 Subject: [PATCH 2/3] fix(git): make fetchFromRemote=true bypass the staleness throttle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../src/services/git/git.integration.test.ts | 13 ++++++++++++ .../src/services/git/service.ts | 21 ++++++++++++++----- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/packages/workspace-server/src/services/git/git.integration.test.ts b/packages/workspace-server/src/services/git/git.integration.test.ts index e00a7521e2..6e7600835d 100644 --- a/packages/workspace-server/src/services/git/git.integration.test.ts +++ b/packages/workspace-server/src/services/git/git.integration.test.ts @@ -325,6 +325,19 @@ describe("GitService integration (git-read + git-mutate)", () => { // Explicit fetch: `work` learns it is one commit behind. const fresh = await git.getGitSyncStatus(work, true); expect(fresh.behind).toBe(1); + + // A second fetch immediately after must still hit the network — the + // staleness throttle must never silently swallow an opt-in + // fetchFromRemote=true call. + await fs.writeFile( + path.join(otherClone, "remote-only-2.txt"), + "from-other-2\n", + ); + commitAll(otherClone, "from other clone (second)"); + run("git push origin main", otherClone); + + const fresher = await git.getGitSyncStatus(work, true); + expect(fresher.behind).toBe(2); }, 15_000); }); }); diff --git a/packages/workspace-server/src/services/git/service.ts b/packages/workspace-server/src/services/git/service.ts index b108626656..7d80ff56e4 100644 --- a/packages/workspace-server/src/services/git/service.ts +++ b/packages/workspace-server/src/services/git/service.ts @@ -432,14 +432,25 @@ export class GitService extends TypedEventEmitter { private readonly lastFetchTime = new Map(); + /** + * Always runs `git fetch`, bypassing the staleness throttle. Use when the + * caller has explicitly asked for a fresh view of the remote (e.g., + * `fetchFromRemote: true`) — otherwise a fetch triggered by a preceding + * mutation can silently swallow this one and leave the snapshot stale at + * exactly the moment it mattered. + */ + private async forceFetch(directoryPath: string): Promise { + try { + await gitFetch(directoryPath); + this.lastFetchTime.set(directoryPath, Date.now()); + } catch {} + } + private async fetchIfStale(directoryPath: string): Promise { const now = Date.now(); const lastFetch = this.lastFetchTime.get(directoryPath) ?? 0; if (now - lastFetch > FETCH_THROTTLE_MS) { - try { - await gitFetch(directoryPath); - this.lastFetchTime.set(directoryPath, now); - } catch {} + await this.forceFetch(directoryPath); } } @@ -448,7 +459,7 @@ export class GitService extends TypedEventEmitter { fetchFromRemote = false, ): Promise { if (fetchFromRemote) { - await this.fetchIfStale(directoryPath); + await this.forceFetch(directoryPath); } const status = await getSyncStatus(directoryPath); From f4c40b935393fc19e6adfa767bde613f35551544 Mon Sep 17 00:00:00 2001 From: dmarticus Date: Tue, 30 Jun 2026 08:47:51 -0700 Subject: [PATCH 3/3] fix(git): log fetch failures and rename misleading fetchIfStale adapter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- apps/code/src/main/di/container.ts | 2 +- packages/core/src/git-pr/git-pr.test.ts | 4 ++-- packages/core/src/git-pr/git-pr.ts | 2 +- packages/core/src/git-pr/identifiers.ts | 2 +- packages/workspace-server/src/services/git/service.ts | 7 ++++++- 5 files changed, 11 insertions(+), 6 deletions(-) diff --git a/apps/code/src/main/di/container.ts b/apps/code/src/main/di/container.ts index 1e8139d988..99efbb5c0a 100644 --- a/apps/code/src/main/di/container.ts +++ b/apps/code/src/main/di/container.ts @@ -503,7 +503,7 @@ container.bind(GIT_DIFF_SOURCE).toDynamicValue((ctx) => { }), getPrTemplate: (directoryPath: string) => git().getPrTemplate.query({ directoryPath }), - fetchIfStale: async (directoryPath: string) => { + fetchFromRemote: async (directoryPath: string) => { await git().getGitSyncStatus.query({ directoryPath, fetchFromRemote: true, diff --git a/packages/core/src/git-pr/git-pr.test.ts b/packages/core/src/git-pr/git-pr.test.ts index e9d632e3d6..29f50cc915 100644 --- a/packages/core/src/git-pr/git-pr.test.ts +++ b/packages/core/src/git-pr/git-pr.test.ts @@ -26,7 +26,7 @@ function makeDiffSource(over: Partial = {}): GitDiffSource { getDiffAgainstRemote: vi.fn().mockResolvedValue(""), getCommitsBetweenBranches: vi.fn().mockResolvedValue([]), getPrTemplate: vi.fn().mockResolvedValue({ template: null }), - fetchIfStale: vi.fn().mockResolvedValue(undefined), + fetchFromRemote: vi.fn().mockResolvedValue(undefined), ...over, }; } @@ -97,7 +97,7 @@ describe("GitPrService.generatePrTitleAndBody", () => { expect(result.title).toBe("feat: add widget"); expect(result.body).toBe("TL;DR: adds a widget."); - expect(diffSource.fetchIfStale).toHaveBeenCalledWith("/repo"); + expect(diffSource.fetchFromRemote).toHaveBeenCalledWith("/repo"); }); }); diff --git a/packages/core/src/git-pr/git-pr.ts b/packages/core/src/git-pr/git-pr.ts index cbccae75f3..3d67958c6f 100644 --- a/packages/core/src/git-pr/git-pr.ts +++ b/packages/core/src/git-pr/git-pr.ts @@ -110,7 +110,7 @@ ${truncatedDiff}${contextSection}`; directoryPath: string, conversationContext?: string, ): Promise<{ title: string; body: string }> { - await this.gitDiff.fetchIfStale(directoryPath); + await this.gitDiff.fetchFromRemote(directoryPath); const [defaultBranch, currentBranch, prTemplate] = await Promise.all([ this.gitDiff.getDefaultBranch(directoryPath), diff --git a/packages/core/src/git-pr/identifiers.ts b/packages/core/src/git-pr/identifiers.ts index d288e71282..ed4e415a46 100644 --- a/packages/core/src/git-pr/identifiers.ts +++ b/packages/core/src/git-pr/identifiers.ts @@ -40,7 +40,7 @@ export interface GitDiffSource { limit: number, ): Promise; getPrTemplate(directoryPath: string): Promise; - fetchIfStale(directoryPath: string): Promise; + fetchFromRemote(directoryPath: string): Promise; } export interface GitPrLogger extends SagaLogger {} diff --git a/packages/workspace-server/src/services/git/service.ts b/packages/workspace-server/src/services/git/service.ts index 7d80ff56e4..ff94d5a4f7 100644 --- a/packages/workspace-server/src/services/git/service.ts +++ b/packages/workspace-server/src/services/git/service.ts @@ -443,7 +443,12 @@ export class GitService extends TypedEventEmitter { try { await gitFetch(directoryPath); this.lastFetchTime.set(directoryPath, Date.now()); - } catch {} + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + process.stderr.write( + `[git-service] fetch failed for ${directoryPath}; using local refs: ${message}\n`, + ); + } } private async fetchIfStale(directoryPath: string): Promise {