mirror of
https://github.com/mountain-loop/yaak.git
synced 2026-11-13 12:13:42 +01:00
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>
This commit is contained in:
1 parent
d2d4b80a09
commit
a939f2ac55
4 files changed
+109
-26
No files matched your search
+57
-25
@@ -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) {
|
||||
|
||||
@@ -2,5 +2,8 @@
|
||||
"name": "@yaakapp-internal/git",
|
||||
"version": "1.0.0",
|
||||
"private": true,
|
||||
"main": "index.ts"
|
||||
"main": "index.ts",
|
||||
"scripts": {
|
||||
"test": "vitest run"
|
||||
}
|
||||
}
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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;
|
||||
}
|
||||
Reference in new issue
Block a user