Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions apps/ade-cli/src/services/sync/syncRemoteCommandService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -412,13 +412,35 @@ describe("createSyncRemoteCommandService", () => {

expect(getGithubSnapshot).toHaveBeenCalledWith({
force: false,
automaticRefresh: false,
includeExternalClosed: true,
historyPageLimit: 4,
revalidate: false,
includeStateCounts: true,
});
});

it("forwards the automatic-refresh opt-out for remote GitHub snapshot reads", async () => {
const getGithubSnapshot = vi.fn().mockResolvedValue({ repoPullRequests: [] });
const { service } = createService({ prService: { getGithubSnapshot } });

// A remote poller asking for a forced snapshot must still respect the
// host's GitHub failure ladder; a remote client that omits the field (an
// older mobile build) keeps today's user-initiated bypass.
await service.execute(makePayload("prs.getGitHubSnapshot", {
force: true,
automaticRefresh: true,
}));
expect(getGithubSnapshot).toHaveBeenLastCalledWith(
expect.objectContaining({ force: true, automaticRefresh: true }),
);

await service.execute(makePayload("prs.getGitHubSnapshot", { force: true }));
expect(getGithubSnapshot).toHaveBeenLastCalledWith(
expect.objectContaining({ force: true, automaticRefresh: false }),
);
});

it("routes GitHub stack reads and mutations with typed repository arguments", async () => {
const listGithubStacks = vi.fn().mockReturnValue([]);
const createGithubStack = vi.fn().mockResolvedValue({ number: 12 });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5464,6 +5464,7 @@ function registerPrAndDeeplinkRemoteCommands({ args, register }: RemoteCommandRe
register("prs.getGitHubSnapshot", { viewerAllowed: true, observesAbort: true }, async (payload) =>
args.prService.getGithubSnapshot({
force: payload.force === true,
automaticRefresh: payload.automaticRefresh === true,
includeExternalClosed: payload.includeExternalClosed === true,
revalidate: payload.revalidate !== false,
includeStateCounts: payload.includeStateCounts === true,
Expand Down
3 changes: 2 additions & 1 deletion apps/desktop/src/main/services/ipc/registerIpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9872,10 +9872,11 @@ export function registerIpc({
return ctx.prService.listSnapshots({ prId: typeof arg?.prId === "string" ? arg.prId : undefined });
});

ipcMain.handle(IPC.prsGetGitHubSnapshot, async (_event, arg?: { force?: boolean; includeExternalClosed?: boolean; historyPageLimit?: number }): Promise<GitHubPrSnapshot> => {
ipcMain.handle(IPC.prsGetGitHubSnapshot, async (_event, arg?: { force?: boolean; includeExternalClosed?: boolean; historyPageLimit?: number; automaticRefresh?: boolean }): Promise<GitHubPrSnapshot> => {
const ctx = ensurePrReadContext();
return await ctx.prService.getGithubSnapshot({
force: arg?.force === true,
automaticRefresh: arg?.automaticRefresh === true,
includeExternalClosed: arg?.includeExternalClosed === true,
historyPageLimit: typeof arg?.historyPageLimit === "number" ? arg.historyPageLimit : undefined,
});
Expand Down
131 changes: 131 additions & 0 deletions apps/desktop/src/main/services/prs/githubReadBackoff.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
import { describe, expect, it } from "vitest";

import {
createGithubReadBackoff,
githubReadFailureBackoffMs,
GITHUB_READ_FAILURE_BACKOFF_BASE_MS,
GITHUB_READ_FAILURE_BACKOFF_MAX_MS,
} from "./githubReadBackoff";

// Deliberate mirrors of prService.ts's exported GITHUB_SNAPSHOT_TTL_MS /
// GITHUB_CLOSED_SNAPSHOT_TTL_MS — that module is canonical. Copied rather than
// imported to keep this pure-function suite free of the 12k-line Electron-
// coupled service; drift here mislabels the "pins the rungs" test, nothing more.
const OPEN_SNAPSHOT_TTL_MS = 120_000;
const CLOSED_SNAPSHOT_TTL_MS = 600_000;

describe("githubReadFailureBackoffMs", () => {
// The invariant the whole module exists for. If a failure ever bought less
// quiet than a success, a throttled GitHub would make ADE ask FASTER than
// when healthy: every caller a warm cache would have served locally becomes
// a live request the moment the cache goes stale and cannot be refreshed.
it("never lets a failure buy less quiet than a success", () => {
const ttls = [0, 1_000, OPEN_SNAPSHOT_TTL_MS, CLOSED_SNAPSHOT_TTL_MS, GITHUB_READ_FAILURE_BACKOFF_MAX_MS * 2];
for (const successTtlMs of ttls) {
for (let attempts = 1; attempts <= 12; attempts += 1) {
expect(githubReadFailureBackoffMs(attempts, successTtlMs))
.toBeGreaterThanOrEqual(successTtlMs);
}
}
});

it("climbs a doubling ladder and caps it", () => {
expect(githubReadFailureBackoffMs(1, 0)).toBe(GITHUB_READ_FAILURE_BACKOFF_BASE_MS);
expect(githubReadFailureBackoffMs(2, 0)).toBe(GITHUB_READ_FAILURE_BACKOFF_BASE_MS * 2);
expect(githubReadFailureBackoffMs(3, 0)).toBe(GITHUB_READ_FAILURE_BACKOFF_BASE_MS * 4);
expect(githubReadFailureBackoffMs(50, 0)).toBe(GITHUB_READ_FAILURE_BACKOFF_MAX_MS);
});

it("pins the rungs the PR service actually gets", () => {
// The success-TTL floor swallows the early rungs, so the doubling only
// becomes visible once the ladder climbs past the TTL. Pinned because the
// module doc describes the raw 20s/40s/80s ladder, not these values.
expect([1, 2, 3, 4, 5].map((n) => githubReadFailureBackoffMs(n, OPEN_SNAPSHOT_TTL_MS)))
.toEqual([120_000, 120_000, 120_000, 160_000, 320_000]);
expect([1, 2, 3, 4, 5, 6].map((n) => githubReadFailureBackoffMs(n, CLOSED_SNAPSHOT_TTL_MS)))
.toEqual([600_000, 600_000, 600_000, 600_000, 600_000, 640_000]);
});

it("treats a zeroth attempt as the first rung", () => {
expect(githubReadFailureBackoffMs(0, 0)).toBe(GITHUB_READ_FAILURE_BACKOFF_BASE_MS);
expect(githubReadFailureBackoffMs(-5, 0)).toBe(GITHUB_READ_FAILURE_BACKOFF_BASE_MS);
});
});

describe("createGithubReadBackoff", () => {
it("arms, expires, and climbs per key", () => {
const backoff = createGithubReadBackoff();
const at = 1_000_000;

expect(backoff.isBackedOff("a", at)).toBe(false);
expect(backoff.lastError("a", at)).toBeNull();

const first = backoff.record("a", new Error("boom"), 0, at);
expect(first).toEqual({ attempts: 1, cooldownMs: GITHUB_READ_FAILURE_BACKOFF_BASE_MS });
expect(backoff.isBackedOff("a", at)).toBe(true);
expect(backoff.isBackedOff("b", at)).toBe(false);

const second = backoff.record("a", new Error("boom"), 0, at);
expect(second.attempts).toBe(2);
expect(second.cooldownMs).toBe(GITHUB_READ_FAILURE_BACKOFF_BASE_MS * 2);
});

it("ends the cooldown exactly at untilMs", () => {
const backoff = createGithubReadBackoff();
backoff.record("a", new Error("boom"), 0, 0);
expect(backoff.isBackedOff("a", GITHUB_READ_FAILURE_BACKOFF_BASE_MS - 1)).toBe(true);
expect(backoff.isBackedOff("a", GITHUB_READ_FAILURE_BACKOFF_BASE_MS)).toBe(false);
});

it("restarts the ladder for a key that expired before failing again", () => {
const backoff = createGithubReadBackoff();
backoff.record("a", new Error("boom"), 0, 0);
// Reading past the cooldown drops the entry, so the next failure is a
// first attempt rather than an inherited rung.
expect(backoff.isBackedOff("a", GITHUB_READ_FAILURE_BACKOFF_BASE_MS + 1)).toBe(false);
expect(backoff.record("a", new Error("boom"), 0, GITHUB_READ_FAILURE_BACKOFF_BASE_MS + 1).attempts)
.toBe(1);
});

it("replays the recorded failure and normalizes a non-Error rejection", () => {
const backoff = createGithubReadBackoff();
const cause = new Error("rate limited");
backoff.record("a", cause, 0, 0);
expect(backoff.lastError("a", 0)).toBe(cause);

backoff.record("b", "string rejection", 0, 0);
const normalized = backoff.lastError("b", 0);
expect(normalized).toBeInstanceOf(Error);
expect(normalized?.message).toBe("string rejection");

// A nullish rejection must still produce something to replay, or the
// caller falls through and issues the request the ladder exists to stop.
backoff.record("c", undefined, 0, 0);
expect(backoff.lastError("c", 0)).toBeInstanceOf(Error);
});

it("clears one key or every key", () => {
const backoff = createGithubReadBackoff();
backoff.record("a", new Error("boom"), 60_000, 0);
backoff.record("b", new Error("boom"), 60_000, 0);

backoff.clear("a");
expect(backoff.isBackedOff("a", 0)).toBe(false);
expect(backoff.isBackedOff("b", 0)).toBe(true);

backoff.clear();
expect(backoff.isBackedOff("b", 0)).toBe(false);
});

it("sweeps entries nothing ever asks about again", () => {
const backoff = createGithubReadBackoff();
// A branch that failed and then had its lane archived is never re-checked,
// so lazy expiry alone would keep its entry for the process lifetime.
backoff.record("archived-lane", new Error("boom"), 0, 0);
backoff.record("live", new Error("boom"), 0, GITHUB_READ_FAILURE_BACKOFF_BASE_MS + 1);
// The sweep dropped the stale entry, so the next failure on that key starts
// the ladder over rather than resuming at attempt 2.
expect(backoff.record("archived-lane", new Error("boom"), 0, GITHUB_READ_FAILURE_BACKOFF_BASE_MS + 1).attempts)
.toBe(1);
});
});
125 changes: 125 additions & 0 deletions apps/desktop/src/main/services/prs/githubReadBackoff.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
/**
* Failure backoff for GitHub reads.
*
* ADE caches successful GitHub reads, so a success buys a known window of quiet.
* Failures used to buy nothing: once GitHub throttled us, every UI-driven caller
* that a warm cache would have served locally turned back into a live request
* and ADE asked *faster* while degraded than while healthy. This module is the
* shared ladder that closes that gap, used both for the whole-repo PR snapshot
* and for per-lane-branch PR lookups.
*/

export const GITHUB_READ_FAILURE_BACKOFF_BASE_MS = 20_000;
export const GITHUB_READ_FAILURE_BACKOFF_MAX_MS = 15 * 60_000;

/**
* How long a failed GitHub read buys before the same read may be attempted
* again: a 20s -> 40s -> 80s -> ... ladder capped at 15 minutes, then floored at
* the window a *success* would have bought.
*
* That floor is the load-bearing part, not a rounding detail — without it a
* failure buys less quiet than a success and the amplification spiral above is
* exactly what you get. Because the floor usually dominates the early rungs
* (with a 120s success TTL the first four rungs are 120s, 120s, 120s, 160s),
* the doubling only becomes visible once the ladder climbs past the TTL.
*/
export function githubReadFailureBackoffMs(attempts: number, successTtlMs: number): number {
const ladder = GITHUB_READ_FAILURE_BACKOFF_BASE_MS * 2 ** Math.max(0, attempts - 1);
return Math.max(successTtlMs, Math.min(ladder, GITHUB_READ_FAILURE_BACKOFF_MAX_MS));
}

const GITHUB_REQUEST_ERROR = Symbol.for("ade.prs.githubRequestError");

/**
* Tag an error as "GitHub said no", so the ladder is only armed by GitHub.
*
* A snapshot rebuild also reads lanes and the local database. Without this tag
* a transient `laneService.list()` or SQLite failure would silence GitHub reads
* for minutes and replay a local error to the UI as though the API had failed.
*/
export function markGithubRequestError<T>(error: T): T {
if (error && (typeof error === "object" || typeof error === "function")) {
try {
Object.defineProperty(error, GITHUB_REQUEST_ERROR, {
value: true,
configurable: true,
enumerable: false,
});
} catch {
// Frozen or exotic error object — fall through untagged, which only costs
// us the backoff, never correctness.
}
}
return error;
}

export function isGithubRequestError(error: unknown): boolean {
return Boolean(
error
&& (typeof error === "object" || typeof error === "function")
&& (error as Record<PropertyKey, unknown>)[GITHUB_REQUEST_ERROR] === true,
);
}

export type GithubReadBackoff = {
/** True while `key` is inside its cooldown. Expired entries are dropped. */
isBackedOff: (key: string, nowMs?: number) => boolean;
/** The failure to replay for `key`, or null when it is not backed off. */
lastError: (key: string, nowMs?: number) => Error | null;
/** Arms (or climbs) the ladder for `key`. Returns the applied cooldown. */
record: (key: string, error: unknown, successTtlMs: number, nowMs?: number) => {
attempts: number;
cooldownMs: number;
};
/** Clears one key, or every key when called with no argument. */
clear: (key?: string) => void;
};

type BackoffEntry = {
attempts: number;
untilMs: number;
error: Error;
};

function toError(error: unknown): Error {
if (error instanceof Error) return error;
return new Error(typeof error === "string" && error ? error : String(error));
}

export function createGithubReadBackoff(): GithubReadBackoff {
const entries = new Map<string, BackoffEntry>();

const read = (key: string, nowMs: number): BackoffEntry | null => {
const entry = entries.get(key);
if (!entry) return null;
if (entry.untilMs <= nowMs) {
entries.delete(key);
return null;
}
return entry;
};

return {
isBackedOff: (key, nowMs = Date.now()) => read(key, nowMs) !== null,
lastError: (key, nowMs = Date.now()) => read(key, nowMs)?.error ?? null,
record: (key, error, successTtlMs, nowMs = Date.now()) => {
// Keys are per repo and per branch, so a long-lived process accumulates
// one entry per branch it ever failed on. Expired entries are only
// dropped when something asks about that exact key again, which never
// happens once a lane is archived — so sweep on every write instead.
// The sweep includes `key`: a lapsed cooldown must restart the ladder at
// rung 1 whether or not anything happened to read it first.
for (const [existingKey, existing] of entries) {
if (existing.untilMs <= nowMs) entries.delete(existingKey);
}
const attempts = (entries.get(key)?.attempts ?? 0) + 1;
const cooldownMs = githubReadFailureBackoffMs(attempts, successTtlMs);
entries.set(key, { attempts, untilMs: nowMs + cooldownMs, error: toError(error) });
return { attempts, cooldownMs };
},
clear: (key) => {
if (key === undefined) entries.clear();
else entries.delete(key);
},
};
}
Loading
Loading