From a939f2ac55d4bf3ecaac8a03daee3f09b76c67be Mon Sep 17 00:00:00 2001 From: Gregory Schier Date: Thu, 10 Sep 2026 17:01:52 -0700 Subject: [PATCH] fix(git): stop polling a sync directory that is not a git repository MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A workspace whose sync directory points at a plain folder still ran the whole git polling loop. `useGitFetchAll` was unconditional, and `cmd_git_fetch_all` shells out twice — once to probe `git --version`, once for the fetch itself — so every window focus spawned two processes to be told fatal: not a git repository (or any of the parent directories): .git `cmd_git_branch_info` failed alongside it, and neither backed off, so the failures repeated for as long as the workspace stayed open. Reported by a user whose console was full of both errors. Branch info (or status, in the dialogs) is now the repository probe, and the fetch only runs once that probe has succeeded. A non-repository sync directory therefore costs one libgit2 lookup rather than a pair of subprocesses. Since branch info is no longer keyed on the fetch timestamp, a completed fetch invalidates what it invalidates instead — which also stops the branch-info cache growing an entry per fetch. Failed git queries now pace themselves: `shouldRetryAfterFailure` holds a query back for 30s after its first failure and doubles that per consecutive failure up to ten minutes. That predicate drives `refetchOnWindowFocus` and `retryOnMount` both: a probe that has never succeeded has no data, so `refetchOnMount` never gets consulted (`shouldLoadOnMount` in query-core short-circuits on `state.data === undefined`), and without `retryOnMount` reopening the commit or remotes dialog on a non-repository re-ran the failed probe immediately. Any git mutation still invalidates the whole `["git"]` key, so an actual `git init` takes effect straight away. Co-Authored-By: Claude Opus 5 --- crates/yaak-git/index.ts | 82 +++++++++++++++++++++++---------- crates/yaak-git/package.json | 5 +- crates/yaak-git/polling.test.ts | 27 +++++++++++ crates/yaak-git/polling.ts | 21 +++++++++ 4 files changed, 109 insertions(+), 26 deletions(-) create mode 100644 crates/yaak-git/polling.test.ts create mode 100644 crates/yaak-git/polling.ts diff --git a/crates/yaak-git/index.ts b/crates/yaak-git/index.ts index 16533421d..02f30ec78 100644 --- a/crates/yaak-git/index.ts +++ b/crates/yaak-git/index.ts @@ -16,6 +16,7 @@ import { PushResult, } from "./bindings/gen_git"; import { showToast } from "@yaakapp/yaak-client/lib/toast"; +import { shouldRetryAfterFailure } from "./polling"; export * from "./bindings/gen_git"; export * from "./bindings/gen_models"; @@ -85,26 +86,56 @@ export function watchGitWorktreeStatus(dir: string, callback: (status: GitWorktr .catch(console.error); } -function useGitFetchAll(dir: string, refreshKey?: string) { - return useQuery({ +function refetchAfterBackoff(query: { + state: { fetchFailureCount: number; errorUpdatedAt: number }; +}) { + return shouldRetryAfterFailure( + query.state.fetchFailureCount, + query.state.errorUpdatedAt, + Date.now(), + ); +} + +/** + * `git fetch` shells out — twice, counting the availability probe — so it only runs once + * something cheaper has established that `dir` is a repository at all. Callers pass that + * verdict in: a sync directory pointed at a plain folder then costs one libgit2 lookup + * instead of a pair of subprocesses on every window focus. + */ +function useGitFetchAll(dir: string, refreshKey: string | undefined, enabled: boolean) { + return useQuery({ + enabled, queryKey: ["git", "fetch_all", dir, refreshKey], - queryFn: () => platform.rpc("cmd_git_fetch_all", { dir }), + async queryFn() { + await platform.rpc("cmd_git_fetch_all", { dir }); + // Fetching moves the remote refs, so anything counted against them is now stale + await queryClient.invalidateQueries({ queryKey: ["git", "branch_info", dir] }); + await queryClient.invalidateQueries({ queryKey: ["git", "status", dir] }); + return null; + }, refetchInterval: 10 * 60_000, + refetchOnWindowFocus: refetchAfterBackoff, + retryOnMount: refetchAfterBackoff, }); } -function useGitBranchInfoQuery(dir: string, refreshKey?: string, fetchAllUpdatedAt?: number) { +function useGitBranchInfoQuery(dir: string, refreshKey?: string) { return useQuery({ refetchOnMount: true, - queryKey: ["git", "branch_info", dir, refreshKey, fetchAllUpdatedAt], + refetchOnWindowFocus: refetchAfterBackoff, + // A probe that has only ever failed has no data, so `refetchOnMount` never gets a + // say; `retryOnMount` is the knob that paces re-opening a dialog on a broken repo + retryOnMount: refetchAfterBackoff, + queryKey: ["git", "branch_info", dir, refreshKey], queryFn: () => platform.rpc("cmd_git_branch_info", { dir }), placeholderData: (prev) => prev, }); } export function useGitBranchInfo(dir: string, refreshKey?: string) { - const fetchAll = useGitFetchAll(dir, refreshKey); - return useGitBranchInfoQuery(dir, refreshKey, fetchAll.dataUpdatedAt); + const branchInfo = useGitBranchInfoQuery(dir, refreshKey); + useGitFetchAll(dir, refreshKey, branchInfo.isSuccess); + return branchInfo; } export function useGitLog(dir: string, refreshKey?: string, relaPath?: string) { @@ -135,25 +166,26 @@ export function useGitFileDiffForCommit( export function useGit(dir: string, callbacks: GitCallbacks, refreshKey?: string) { const mutations = useGitMutations(dir, callbacks); - const fetchAll = useGitFetchAll(dir, refreshKey); + const remotes = useQuery({ + queryKey: ["git", "remotes", dir, refreshKey], + queryFn: () => getRemotes(dir), + placeholderData: (prev) => prev, + }); + const log = useGitLog(dir, refreshKey); + const status = useQuery({ + refetchOnMount: true, + refetchOnWindowFocus: refetchAfterBackoff, + retryOnMount: refetchAfterBackoff, + queryKey: ["git", "status", dir, refreshKey], + queryFn: () => platform.rpc("cmd_git_status", { dir }), + placeholderData: (prev) => prev, + }); - return [ - { - remotes: useQuery({ - queryKey: ["git", "remotes", dir, refreshKey], - queryFn: () => getRemotes(dir), - placeholderData: (prev) => prev, - }), - log: useGitLog(dir, refreshKey), - status: useQuery({ - refetchOnMount: true, - queryKey: ["git", "status", dir, refreshKey, fetchAll.dataUpdatedAt], - queryFn: () => platform.rpc("cmd_git_status", { dir }), - placeholderData: (prev) => prev, - }), - }, - mutations, - ] as const; + // Status doubles as the repository probe here, so nothing shells out to `git fetch` + // until there is a repository to fetch into + useGitFetchAll(dir, refreshKey, status.isSuccess); + + return [{ remotes, log, status }, mutations] as const; } export function useGitMutations(dir: string, callbacks: GitCallbacks) { diff --git a/crates/yaak-git/package.json b/crates/yaak-git/package.json index 671fe49ad..a49504100 100644 --- a/crates/yaak-git/package.json +++ b/crates/yaak-git/package.json @@ -2,5 +2,8 @@ "name": "@yaakapp-internal/git", "version": "1.0.0", "private": true, - "main": "index.ts" + "main": "index.ts", + "scripts": { + "test": "vitest run" + } } diff --git a/crates/yaak-git/polling.test.ts b/crates/yaak-git/polling.test.ts new file mode 100644 index 000000000..1ab16f118 --- /dev/null +++ b/crates/yaak-git/polling.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, test } from "vitest"; +import { shouldRetryAfterFailure } from "./polling"; + +const NOW = 1_000_000_000; + +describe("shouldRetryAfterFailure", () => { + test("a query that has not failed always runs", () => { + expect(shouldRetryAfterFailure(0, 0, NOW)).toBe(true); + }); + + test("waits 30s after the first failure", () => { + expect(shouldRetryAfterFailure(1, NOW - 29_999, NOW)).toBe(false); + expect(shouldRetryAfterFailure(1, NOW - 30_000, NOW)).toBe(true); + }); + + test("doubles the wait for each consecutive failure", () => { + expect(shouldRetryAfterFailure(2, NOW - 59_999, NOW)).toBe(false); + expect(shouldRetryAfterFailure(2, NOW - 60_000, NOW)).toBe(true); + expect(shouldRetryAfterFailure(3, NOW - 119_999, NOW)).toBe(false); + expect(shouldRetryAfterFailure(3, NOW - 120_000, NOW)).toBe(true); + }); + + test("caps the wait at ten minutes", () => { + expect(shouldRetryAfterFailure(50, NOW - (10 * 60_000 - 1), NOW)).toBe(false); + expect(shouldRetryAfterFailure(50, NOW - 10 * 60_000, NOW)).toBe(true); + }); +}); diff --git a/crates/yaak-git/polling.ts b/crates/yaak-git/polling.ts new file mode 100644 index 000000000..ec89a80f9 --- /dev/null +++ b/crates/yaak-git/polling.ts @@ -0,0 +1,21 @@ +/** + * When a git query that failed is allowed to run again. + * + * Nothing here retries on its own; the git queries are polled by window focus and by a + * long interval. The failures worth pacing are the ones that do not clear up on their + * own — a sync directory that is not a repository, a repository that was deleted — where + * every focus event would otherwise re-run the same doomed command. + */ + +const RETRY_BASE_MS = 30_000; +const RETRY_MAX_MS = 10 * 60_000; + +export function shouldRetryAfterFailure( + failureCount: number, + erroredAt: number, + now: number, +): boolean { + if (failureCount <= 0) return true; + const wait = Math.min(RETRY_BASE_MS * 2 ** (failureCount - 1), RETRY_MAX_MS); + return now - erroredAt >= wait; +}