diff --git a/src/lib/components/PrereqBanner.svelte b/src/lib/components/PrereqBanner.svelte index 6fe8a31..efc5228 100644 --- a/src/lib/components/PrereqBanner.svelte +++ b/src/lib/components/PrereqBanner.svelte @@ -8,11 +8,12 @@ onMount(() => { void prerequisites.load(); - // Re-probe when the user returns to the app while a prerequisite is still missing — - // e.g. after finishing the async Command Line Tools installer — so the banner clears - // and the editing controls re-enable without a restart. Stops once everything's met. + // Re-probe when the user returns to the app and prerequisites aren't confirmed met — + // after finishing the async Command Line Tools installer, OR to retry a probe that + // transiently failed (check === null) — so the banner clears and the editing controls + // re-enable without a restart. Stops once everything's met (check set + editable). const onReturn = () => { - if (prerequisites.check && !prerequisites.canEditHistory) void prerequisites.refresh(); + if (!prerequisites.check || !prerequisites.canEditHistory) void prerequisites.refresh(); }; window.addEventListener("focus", onReturn); return () => window.removeEventListener("focus", onReturn); diff --git a/src/lib/github/activity.test.ts b/src/lib/github/activity.test.ts index 2893dc7..65426a6 100644 --- a/src/lib/github/activity.test.ts +++ b/src/lib/github/activity.test.ts @@ -15,14 +15,12 @@ describe("sparklinePoints", () => { }); describe("isRetryableActivityError", () => { - it("retries cold-stats / transient failures", () => { - for (const kind of ["Other", "RateLimited", "Forbidden"]) { - expect(isRetryableActivityError({ kind })).toBe(true); - } + it("retries only transient cold-cache failures", () => { + expect(isRetryableActivityError({ kind: "Other", message: "parse activity" })).toBe(true); expect(isRetryableActivityError("network blip")).toBe(true); // non-GithubError → retry }); - it("does not retry permanent failures", () => { - for (const kind of ["NotFound", "NotAuthed", "NoRemote", "NotInstalled"]) { + it("does not retry explicit rate limits or permanent failures", () => { + for (const kind of ["RateLimited", "Forbidden", "NotFound", "NotAuthed", "NoRemote", "NotInstalled"]) { expect(isRetryableActivityError({ kind })).toBe(false); } }); @@ -70,15 +68,17 @@ describe("fetchActivityWithRetry", () => { expect(fetchOnce).toHaveBeenCalledTimes(3); }); - it("aborts immediately on a PERMANENT error without burning retries", async () => { - const fetchOnce = vi.fn().mockRejectedValue({ kind: "NotFound" }); - await expect(fetchActivityWithRetry(fetchOnce, noSleep, 5)).rejects.toEqual({ kind: "NotFound" }); - expect(fetchOnce).toHaveBeenCalledTimes(1); + it("aborts immediately on a non-retryable error (permanent OR explicit rate limit)", async () => { + for (const kind of ["NotFound", "RateLimited"]) { + const fetchOnce = vi.fn().mockRejectedValue({ kind }); + await expect(fetchActivityWithRetry(fetchOnce, noSleep, 5)).rejects.toEqual({ kind }); + expect(fetchOnce).toHaveBeenCalledTimes(1); + } }); - it("propagates the last error when a transient failure never clears", async () => { - const fetchOnce = vi.fn().mockRejectedValue({ kind: "RateLimited" }); - await expect(fetchActivityWithRetry(fetchOnce, noSleep, 3)).rejects.toEqual({ kind: "RateLimited" }); + it("propagates the last error when a transient (Other) failure never clears", async () => { + const fetchOnce = vi.fn().mockRejectedValue({ kind: "Other", message: "parse activity" }); + await expect(fetchActivityWithRetry(fetchOnce, noSleep, 3)).rejects.toMatchObject({ kind: "Other" }); expect(fetchOnce).toHaveBeenCalledTimes(3); }); }); diff --git a/src/lib/github/activity.ts b/src/lib/github/activity.ts index cf4b41a..1531e5d 100644 --- a/src/lib/github/activity.ts +++ b/src/lib/github/activity.ts @@ -11,19 +11,18 @@ export function sparklinePoints(values: number[], w: number, h: number): string .join(" "); } -/** A cold-stats failure is worth retrying; a missing repo / auth / remote problem is not. - * GitHub computes /stats/commit_activity lazily (HTTP 202 the first time), so the initial - * request can fail transiently — a rate limit, a 202 surfaced as an error, an unparseable - * placeholder body. Those clear on their own; NotFound/NotAuthed/NoRemote/NotInstalled won't. */ +/** Only a transient cold-cache failure is worth the short fixed backoff. GitHub computes + * /stats/commit_activity lazily (HTTP 202 the first time), which can surface as an + * unparseable placeholder body — an `Other`/parse error — or a non-GithubError network + * blip. Explicit terminal states are NOT retried: `RateLimited` won't clear inside the + * backoff window (and re-hitting it prolongs the throttle), and Forbidden / NotFound / + * NotAuthed / NoRemote / NotInstalled are permanent. Reserve the retries for the + * computing/parse case; a real rate limit should surface (and wait for its reset) instead. */ export function isRetryableActivityError(e: unknown): boolean { - const kind = - e && typeof e === "object" && "kind" in e ? (e as GithubError).kind : "Other"; - return ( - kind !== "NotFound" && - kind !== "NotAuthed" && - kind !== "NoRemote" && - kind !== "NotInstalled" - ); + if (e && typeof e === "object" && "kind" in e) { + return (e as GithubError).kind === "Other"; + } + return true; // non-GithubError throw → treat as a transient blip } /** Fetch commit activity, retrying through GitHub's lazy-computation window. diff --git a/src/lib/prerequisites.svelte.ts b/src/lib/prerequisites.svelte.ts index c42c375..522e224 100644 --- a/src/lib/prerequisites.svelte.ts +++ b/src/lib/prerequisites.svelte.ts @@ -6,15 +6,34 @@ import { api } from "./api"; import type { PrerequisiteCheck } from "./types"; +function isTauri(): boolean { + return typeof window !== "undefined" && "__TAURI_INTERNALS__" in window; +} + function makePrerequisites() { let check = $state(null); let loading: Promise | null = null; + // Monotonic probe id. refresh() can start a probe while another is still in flight, so + // only the LATEST probe may commit its result — otherwise a stale/out-of-order completion + // (e.g. an older failure resolving after a newer success) would clobber the current state + // and wrongly re-disable editing. + let probeSeq = 0; async function probe() { + // Non-Tauri (browser preview) has no backend to probe — leave the state "unknown" + // (not a failure) so canEditHistory stays optimistic there. + if (!isTauri()) return; + const seq = ++probeSeq; try { - check = await api.checkPrerequisites(); + const result = await api.checkPrerequisites(); + if (seq !== probeSeq) return; // superseded by a newer probe — drop this result + check = result; } catch { + if (seq !== probeSeq) return; // superseded by a newer probe — drop this stale failure check = null; + // Clear the cached promise so a later load()/refresh() re-probes a transient failure + // instead of no-op'ing on this (resolved) promise for the rest of the session. + loading = null; } } @@ -22,11 +41,14 @@ function makePrerequisites() { get check() { return check; }, - // Commit-time editing needs git + python3 + a working bundled filter-repo. Unknown - // (not yet probed / non-Tauri) → treated as available so we never block prematurely; - // a genuinely-missing tool still fails loudly at run time. + // Commit-time editing needs a SUCCESSFUL probe confirming git + python3 + a working + // bundled filter-repo. Until one arrives, the DESKTOP app keeps the destructive "Rewrite + // history" action DISABLED — a slow OR failed probe must never leave it invokable on + // unverified prerequisites. A non-Tauri preview has nothing to verify (editing is inert + // there anyway), so it stays optimistic. A later successful probe enables it. get canEditHistory() { - return check ? check.git && check.python3 && check.filterRepo : true; + if (check) return check.git && check.python3 && check.filterRepo; + return !isTauri(); }, // Probe once, then share the result — deduped so the banner and ApplyPanel don't // double-invoke.