Compare commits

...
Author SHA1 Message Date
Gregory SchierandClaude Opus 5 a939f2ac55 fix(git): stop polling a sync directory that is not a git repository
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 <noreply@anthropic.com>
2026-09-10 17:01:52 -07:00
4 changed files with 109 additions and 26 deletions

No files matched your search

+57 -25
View File
@@ -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<void, string>({
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<null, string>({
enabled,
queryKey: ["git", "fetch_all", dir, refreshKey],
queryFn: () => platform.rpc("cmd_git_fetch_all", { dir }),
async queryFn() {
await platform.rpc<null>("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<GitBranchInfo, string>({
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<GitRemote[], string>({
queryKey: ["git", "remotes", dir, refreshKey],
queryFn: () => getRemotes(dir),
placeholderData: (prev) => prev,
});
const log = useGitLog(dir, refreshKey);
const status = useQuery<GitStatusSummary, string>({
refetchOnMount: true,
refetchOnWindowFocus: refetchAfterBackoff,
retryOnMount: refetchAfterBackoff,
queryKey: ["git", "status", dir, refreshKey],
queryFn: () => platform.rpc("cmd_git_status", { dir }),
placeholderData: (prev) => prev,
});
return [
{
remotes: useQuery<GitRemote[], string>({
queryKey: ["git", "remotes", dir, refreshKey],
queryFn: () => getRemotes(dir),
placeholderData: (prev) => prev,
}),
log: useGitLog(dir, refreshKey),
status: useQuery<GitStatusSummary, string>({
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) {
+4 -1
View File
@@ -2,5 +2,8 @@
"name": "@yaakapp-internal/git",
"version": "1.0.0",
"private": true,
"main": "index.ts"
"main": "index.ts",
"scripts": {
"test": "vitest run"
}
}
+27
View File
@@ -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);
});
});
+21
View File
@@ -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;
}