diff --git a/apps/code/src/main/di/container.ts b/apps/code/src/main/di/container.ts index 0f94543ab8..99efbb5c0a 100644 --- a/apps/code/src/main/di/container.ts +++ b/apps/code/src/main/di/container.ts @@ -503,10 +503,10 @@ 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, - forceRefresh: true, + 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/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..6e7600835d 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,44 @@ 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); + + // 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/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..ff94d5a4f7 100644 --- a/packages/workspace-server/src/services/git/service.ts +++ b/packages/workspace-server/src/services/git/service.ts @@ -432,25 +432,40 @@ 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 (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 { 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); } } private async getGitSyncStatusInternal( directoryPath: string, - forceRefresh = false, + fetchFromRemote = false, ): Promise { - if (forceRefresh) { - this.lastFetchTime.delete(directoryPath); + if (fetchFromRemote) { + await this.forceFetch(directoryPath); } - await this.fetchIfStale(directoryPath); const status = await getSyncStatus(directoryPath); return { @@ -483,7 +498,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 +523,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, ), ),