diff --git a/src/app/components/dashboard/DashboardPage.tsx b/src/app/components/dashboard/DashboardPage.tsx index 035a2309..c7cfe55c 100644 --- a/src/app/components/dashboard/DashboardPage.tsx +++ b/src/app/components/dashboard/DashboardPage.tsx @@ -26,7 +26,7 @@ import { expireToken, user, onAuthCleared, DASHBOARD_STORAGE_KEY } from "../../s import { updateRelaySnapshot } from "../../lib/mcp-relay"; import { pushNotification } from "../../lib/errors"; import { getClient, getGraphqlRateLimit, fetchRateLimitDetails } from "../../services/github"; -import { formatCount, prSizeCategory } from "../../lib/format"; +import { formatCount, prSizeCategory, rateLimitCssClass } from "../../lib/format"; import { setsEqual } from "../../lib/collections"; import { withScrollLock } from "../../lib/scroll"; import { Tooltip } from "../shared/Tooltip"; @@ -925,7 +925,7 @@ export default function DashboardPage() { {(rl) => (
- + API RL: {rl().remaining.toLocaleString()}/{formatCount(rl().limit)}/hr diff --git a/src/app/components/settings/SettingsPage.tsx b/src/app/components/settings/SettingsPage.tsx index cf971985..7ff274ed 100644 --- a/src/app/components/settings/SettingsPage.tsx +++ b/src/app/components/settings/SettingsPage.tsx @@ -422,7 +422,7 @@ export default function SettingsPage() { Source Pool - Calls + Usage Last Called @@ -468,7 +468,7 @@ export default function SettingsPage() { onClick={() => resetUsageData()} class="btn btn-xs btn-ghost" > - Reset counts + Reset usage
@@ -514,7 +514,7 @@ export default function SettingsPage() { - {/* Section 5: Notifications */} + {/* Section 6: Notifications */}
- {/* Section 6: Appearance */} + {/* Section 7: Appearance */}

Theme

@@ -634,7 +634,7 @@ export default function SettingsPage() {
- {/* Section 7: Tabs */} + {/* Section 8: Tabs */}
- {/* Section 8: Custom Tabs */} + {/* Section 9: Custom Tabs */}
r.owner))]} @@ -699,7 +699,7 @@ export default function SettingsPage() { />
- {/* Section 9: MCP Server Relay */} + {/* Section 10: MCP Server Relay */}
- {/* Section 10: Data */} + {/* Section 11: Data */}
{/* Authentication method */} { const source = deriveSource(info); const pool: ApiPool = info.isGraphql ? "graphql" : "core"; - trackApiCall(source, pool); + trackApiCall(source, pool, info.graphqlCost ?? 1); // Both pools tracked — Math.max keeps latest; may delay reset of the earlier pool's records if (info.resetEpochMs !== null) updateResetAt(info.resetEpochMs); }); diff --git a/src/app/services/api.ts b/src/app/services/api.ts index 901baac4..17eddc7c 100644 --- a/src/app/services/api.ts +++ b/src/app/services/api.ts @@ -12,6 +12,7 @@ export type { Issue, PullRequest, WorkflowRun, RepoRef, RepoEntry, OrgEntry, Che // ── Types ──────────────────────────────────────────────────────────────────── interface GraphQLRateLimit { + cost?: number; limit: number; remaining: number; resetAt: string; @@ -226,7 +227,7 @@ const ISSUES_SEARCH_QUERY = ` } } } - rateLimit { limit remaining resetAt } + rateLimit { cost limit remaining resetAt } } ${LIGHT_ISSUE_FRAGMENT} `; @@ -287,7 +288,7 @@ const LIGHT_COMBINED_SEARCH_QUERY = ` } } } - rateLimit { limit remaining resetAt } + rateLimit { cost limit remaining resetAt } } ${LIGHT_ISSUE_FRAGMENT} ${LIGHT_PR_FRAGMENT} @@ -318,7 +319,7 @@ const UNFILTERED_SEARCH_QUERY = ` } } } - rateLimit { limit remaining resetAt } + rateLimit { cost limit remaining resetAt } } ${LIGHT_ISSUE_FRAGMENT} ${LIGHT_PR_FRAGMENT} @@ -350,7 +351,7 @@ const LIGHT_PR_SEARCH_QUERY = ` } } } - rateLimit { limit remaining resetAt } + rateLimit { cost limit remaining resetAt } } ${LIGHT_PR_FRAGMENT} `; @@ -395,7 +396,7 @@ const HEAVY_PR_BACKFILL_QUERY = ` } } } - rateLimit { limit remaining resetAt } + rateLimit { cost limit remaining resetAt } } `; @@ -417,7 +418,7 @@ const HOT_PR_STATUS_QUERY = ` } } } - rateLimit { limit remaining resetAt } + rateLimit { cost limit remaining resetAt } } `; @@ -673,7 +674,7 @@ async function runForkPRFallback( ); } - const forkQuery = `query(${varDefs.join(", ")}) {\n${fragments.join("\n")}\nrateLimit { limit remaining resetAt }\n}`; + const forkQuery = `query(${varDefs.join(", ")}) {\n${fragments.join("\n")}\nrateLimit { cost limit remaining resetAt }\n}`; try { const forkResponse = await octokit.graphql(forkQuery, { ...variables, request: { apiSource: "forkCheck" } }); @@ -688,9 +689,13 @@ async function runForkPRFallback( if (pr) pr.checkStatus = mapCheckStatus(state); } } catch (err) { - const partialData = (err && typeof err === "object" && "data" in err && err.data && typeof err.data === "object") - ? err.data as Record + const errDataRaw = (err && typeof err === "object" && "data" in err && err.data && typeof err.data === "object") + ? err.data as Record : null; + if (errDataRaw?.rateLimit) { + updateGraphqlRateLimit(errDataRaw.rateLimit as GraphQLRateLimit); + } + const partialData = errDataRaw as Record | null; if (partialData) { for (let i = 0; i < forkChunk.length; i++) { @@ -1130,6 +1135,12 @@ export async function fetchPREnrichment( }); } } catch (err) { + const partialErr = (err && typeof err === "object" && "data" in err && err.data && typeof err.data === "object") + ? err.data as Partial + : null; + if (partialErr?.rateLimit) { + updateGraphqlRateLimit(partialErr.rateLimit); + } const { statusCode, message } = extractRejectionError(err); errors.push({ repo: `backfill-batch-${batchIdx + 1}/${batches.length}`, @@ -1686,6 +1697,13 @@ export async function fetchHotPRStatus( hadErrors = true; console.warn("[hot-poll] PR status batch failed:", s.reason); Sentry.captureException(s.reason, { tags: { source: "hot-poll-pr-batch" } }); + const reason = s.reason; + const partialErr = (reason && typeof reason === "object" && "data" in reason && reason.data && typeof reason.data === "object") + ? reason.data as Partial + : null; + if (partialErr?.rateLimit) { + updateGraphqlRateLimit(partialErr.rateLimit); + } } } diff --git a/src/app/services/github.ts b/src/app/services/github.ts index 6b27dc67..0c6e4123 100644 --- a/src/app/services/github.ts +++ b/src/app/services/github.ts @@ -67,6 +67,8 @@ export interface ApiRequestInfo { apiSource?: string; /** x-ratelimit-reset converted to ms, or null if unavailable */ resetEpochMs: number | null; + /** GraphQL query point cost from response body, or undefined for REST */ + graphqlCost?: number; } const _requestCallbacks: Array<(info: ApiRequestInfo) => void> = []; @@ -147,16 +149,41 @@ export function createGitHubClient(token: string): GitHubOctokitInstance { // Fire callbacks even on errors — these are real API calls. // Octokit's RequestError includes response.headers for HTTP errors (403, 404, etc.) // so we can still extract x-ratelimit-reset when available. + const errResponse = (err as { response?: { headers?: Record } }).response; if (status > 0) { let resetEpochMs: number | null = null; - const errResponse = (err as { response?: { headers?: Record } }).response; const errResetHeader = errResponse?.headers?.["x-ratelimit-reset"]; if (errResetHeader) resetEpochMs = parseInt(errResetHeader, 10) * 1000; + // Octokit errors store data in two shapes: err.data has the unwrapped GraphQL + // data object, while errResponse.data is the raw HTTP body (GraphQL envelope) + const errGraphqlCost = isGraphql + ? ((err as { data?: { rateLimit?: { cost?: number } } }).data?.rateLimit?.cost + ?? (errResponse as { data?: { data?: { rateLimit?: { cost?: number } } } } | undefined)?.data?.data?.rateLimit?.cost) + : undefined; const info: ApiRequestInfo = { - url: options.url, method, status, isGraphql, apiSource, resetEpochMs, + url: options.url, method, status, isGraphql, apiSource, resetEpochMs, graphqlCost: errGraphqlCost, }; for (const cb of _requestCallbacks) { try { cb(info); } catch { /* swallow */ } } } + const errHeaders = errResponse?.headers as Record | undefined; + if (errHeaders) { + try { + if (isGraphql) { + const remaining = errHeaders["x-ratelimit-remaining"]; + const reset = errHeaders["x-ratelimit-reset"]; + const limit = errHeaders["x-ratelimit-limit"]; + if (remaining !== undefined && reset !== undefined) { + _setGraphqlRateLimit({ + limit: safePositiveNumber(limit !== undefined ? parseInt(limit, 10) : NaN, _graphqlRateLimit()?.limit ?? 5000), + remaining: Number.isFinite(parseInt(remaining, 10)) ? parseInt(remaining, 10) : 0, + resetAt: new Date(parseInt(reset, 10) * 1000), + }); + } + } else { + updateRateLimitFromHeaders(errHeaders); + } + } catch { /* never mask the original error */ } + } throw err; } @@ -164,12 +191,15 @@ export function createGitHubClient(token: string): GitHubOctokitInstance { const headers = (response.headers ?? {}) as Record; const resetHeader = headers["x-ratelimit-reset"]; const resetEpochMs = resetHeader ? parseInt(resetHeader, 10) * 1000 : null; + const graphqlCost = isGraphql + ? (response.data as { data?: { rateLimit?: { cost?: number } } })?.data?.rateLimit?.cost ?? undefined + : undefined; const info: ApiRequestInfo = { - url: options.url, method, status, isGraphql, apiSource, resetEpochMs, + url: options.url, method, status, isGraphql, apiSource, resetEpochMs, graphqlCost, }; for (const cb of _requestCallbacks) { try { cb(info); } catch { /* swallow */ } } - if (response.headers) { + if (response.headers && !isGraphql) { updateRateLimitFromHeaders(response.headers as Record); } @@ -283,13 +313,15 @@ let _lastFetchResult: { core: RateLimitInfo; graphql: RateLimitInfo } | null = n * GET /rate_limit is free — not counted against rate limits by GitHub. * Returns null if client unavailable or request fails. */ -export async function fetchRateLimitDetails(): Promise<{ core: RateLimitInfo; graphql: RateLimitInfo } | null> { +export async function fetchRateLimitDetails(clientOverride?: GitHubOctokitInstance): Promise<{ core: RateLimitInfo; graphql: RateLimitInfo } | null> { // Return cached result within 5-second staleness window if (_lastFetchResult !== null && Date.now() - _lastFetchTime < 5000) { + _setCoreRateLimit(_lastFetchResult.core); + _setGraphqlRateLimit(_lastFetchResult.graphql); return { ..._lastFetchResult }; } - const client = getClient(); + const client = clientOverride ?? getClient(); if (!client) return null; try { @@ -310,6 +342,8 @@ export async function fetchRateLimitDetails(): Promise<{ core: RateLimitInfo; gr }; _lastFetchTime = Date.now(); _lastFetchResult = result; + _setCoreRateLimit(result.core); + _setGraphqlRateLimit(result.graphql); return { ...result }; } catch { return null; diff --git a/src/app/services/poll.ts b/src/app/services/poll.ts index 5d44f32d..c42fa8de 100644 --- a/src/app/services/poll.ts +++ b/src/app/services/poll.ts @@ -1,6 +1,6 @@ import { createSignal, createEffect, createRoot, untrack, onCleanup } from "solid-js"; import * as Sentry from "@sentry/solid"; -import { getClient } from "./github"; +import { getClient, fetchRateLimitDetails } from "./github"; import { config } from "../stores/config"; import { user, onAuthCleared } from "../stores/auth"; import { checkAndResetIfExpired } from "./api-usage"; @@ -357,6 +357,9 @@ export function createPollCoordinator( if (destroyed || isRefreshing()) return; checkAndResetIfExpired(); setIsRefreshing(true); + // Fire-and-forget: seeds footer signals concurrently with fetchAll. If GET /rate_limit + // resolves after a GraphQL response, the footer briefly shows pre-query remaining (cosmetic). + void fetchRateLimitDetails(); // Snapshot sources of notifications from previous cycle (for reconciliation) const previousSources = new Set( diff --git a/tests/components/dashboard/DashboardPage.test.tsx b/tests/components/dashboard/DashboardPage.test.tsx new file mode 100644 index 00000000..cb44b82f --- /dev/null +++ b/tests/components/dashboard/DashboardPage.test.tsx @@ -0,0 +1,24 @@ +import { describe, it, expect } from "vitest"; +import { rateLimitCssClass } from "../../../src/app/lib/format"; + +describe("rateLimitCssClass", () => { + it("remaining: 0 gives text-error", () => { + expect(rateLimitCssClass(0, 5000)).toBe("text-error"); + }); + + it("remaining < 10% of limit gives text-warning", () => { + expect(rateLimitCssClass(100, 5000)).toBe("text-warning"); + }); + + it("remaining >= 10% of limit gives empty string", () => { + expect(rateLimitCssClass(3000, 5000)).toBe(""); + }); + + it("remaining exactly at 10% threshold gives empty string (strict less-than)", () => { + expect(rateLimitCssClass(500, 5000)).toBe(""); + }); + + it("remaining just below 10% threshold gives text-warning", () => { + expect(rateLimitCssClass(499, 5000)).toBe("text-warning"); + }); +}); diff --git a/tests/components/settings/ApiUsageSection.test.tsx b/tests/components/settings/ApiUsageSection.test.tsx index 45e9a79c..2a7e657f 100644 --- a/tests/components/settings/ApiUsageSection.test.tsx +++ b/tests/components/settings/ApiUsageSection.test.tsx @@ -274,18 +274,18 @@ describe("ApiUsageSection — reset button", () => { mockGetUsageResetAt.mockReturnValue(null); }); - it("renders the 'Reset counts' button", () => { + it("renders the 'Reset usage' button", () => { renderSettings(); - expect(screen.getByText("Reset counts")).toBeTruthy(); + expect(screen.getByText("Reset usage")).toBeTruthy(); }); - it("calls resetUsageData() when 'Reset counts' button is clicked", () => { + it("calls resetUsageData() when 'Reset usage' button is clicked", () => { // Wire the mock to clear snapshot on reset, simulating real behavior mockResetUsageData.mockImplementation(() => { mockGetUsageSnapshot.mockReturnValue([]); }); renderSettings(); - const btn = screen.getByText("Reset counts"); + const btn = screen.getByText("Reset usage"); fireEvent.click(btn); expect(mockResetUsageData).toHaveBeenCalledOnce(); }); diff --git a/tests/services/api.test.ts b/tests/services/api.test.ts index 4e99225b..05f3ee17 100644 --- a/tests/services/api.test.ts +++ b/tests/services/api.test.ts @@ -5,6 +5,8 @@ import { fetchRepos, fetchWorkflowRuns, validateGitHubUser, + fetchPREnrichment, + fetchHotPRStatus, type RepoRef, } from "../../src/app/services/api"; import { clearCache } from "../../src/app/stores/cache"; @@ -1137,3 +1139,74 @@ describe("fetchIssuesAndPullRequests — onLightData suppression when all monito expect(result.issues[0].id).toBe(5001); }); }); + +// ── Error-path rateLimit extraction calls updateGraphqlRateLimit ────────────── + +describe("error-path rateLimit extraction — updateGraphqlRateLimit called", () => { + const mockResetAt = "2026-04-23T12:00:00Z"; + + function makeGraphqlError(rateLimitOverride?: object) { + const rateLimit = rateLimitOverride ?? { limit: 5000, remaining: 100, resetAt: mockResetAt }; + return Object.assign(new Error("GraphQL partial error"), { + data: { rateLimit }, + }); + } + + beforeEach(() => { + vi.resetAllMocks(); + }); + + it("fetchPREnrichment error with partial rateLimit calls updateGraphqlRateLimit", async () => { + const githubModule = await import("../../src/app/services/github"); + const updateSpy = vi.spyOn(githubModule, "updateGraphqlRateLimit"); + + const octokit = makeOctokit( + async () => ({ data: {} }), + async () => { throw makeGraphqlError(); } + ); + + await fetchPREnrichment( + octokit as never, + ["PR_kwDOtest001"] + ); + + expect(updateSpy).toHaveBeenCalledWith( + expect.objectContaining({ limit: 5000, remaining: 100, resetAt: mockResetAt }) + ); + }); + + it("fetchHotPRStatus rejected batch with rateLimit calls updateGraphqlRateLimit", async () => { + const githubModule = await import("../../src/app/services/github"); + const updateSpy = vi.spyOn(githubModule, "updateGraphqlRateLimit"); + + const octokit = makeOctokit( + async () => ({ data: {} }), + async () => { throw makeGraphqlError(); } + ); + + await fetchHotPRStatus( + octokit as never, + ["PR_kwDOtest001"] + ); + + expect(updateSpy).toHaveBeenCalledWith( + expect.objectContaining({ limit: 5000, remaining: 100, resetAt: mockResetAt }) + ); + }); + + it("fetchPREnrichment second batch error with rateLimit calls updateGraphqlRateLimit", async () => { + const githubModule = await import("../../src/app/services/github"); + const updateSpy = vi.spyOn(githubModule, "updateGraphqlRateLimit"); + + const singleErrOctokit = makeOctokit( + async () => ({ data: {} }), + async () => { throw makeGraphqlError({ limit: 5000, remaining: 42, resetAt: mockResetAt }); } + ); + + await fetchPREnrichment(singleErrOctokit as never, ["PR_kwDOtest002"]); + + expect(updateSpy).toHaveBeenCalledWith( + expect.objectContaining({ limit: 5000, remaining: 42, resetAt: mockResetAt }) + ); + }); +}); diff --git a/tests/services/github.test.ts b/tests/services/github.test.ts index 4f462329..c74d508a 100644 --- a/tests/services/github.test.ts +++ b/tests/services/github.test.ts @@ -1,7 +1,7 @@ import "fake-indexeddb/auto"; import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { createRoot } from "solid-js"; -import { createGitHubClient, cachedRequest, getClient, initClientWatcher, getGraphqlRateLimit, updateGraphqlRateLimit, updateRateLimitFromHeaders, getCoreRateLimit, onApiRequest, type ApiRequestInfo } from "../../src/app/services/github"; +import { createGitHubClient, cachedRequest, getClient, initClientWatcher, getGraphqlRateLimit, updateGraphqlRateLimit, updateRateLimitFromHeaders, getCoreRateLimit, fetchRateLimitDetails, onApiRequest, type ApiRequestInfo } from "../../src/app/services/github"; import { clearCache } from "../../src/app/stores/cache"; // ── createGitHubClient ─────────────────────────────────────────────────────── @@ -569,3 +569,296 @@ describe("hook.wrap — request tracking callbacks", () => { expect(cbSpy).toHaveBeenCalled(); }); }); + +// ── hook.wrap — rate limit signal routing ──────────────────────────────────── + +describe("hook.wrap — rate limit signal routing", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + // Use status 500 (not 403/429) to avoid the throttle plugin's onRateLimit callback, + // which would schedule a real retry delay. hook.wrap extracts x-ratelimit-* headers + // for any error status, so 500 exercises the same signal-update code path. + it("GraphQL error response with rate limit headers updates _graphqlRateLimit", async () => { + const resetEpoch = Math.floor(Date.now() / 1000) + 3600; + vi.stubGlobal("fetch", vi.fn().mockResolvedValue( + new Response(JSON.stringify({ message: "internal server error" }), { + status: 500, + headers: { + "content-type": "application/json", + "x-ratelimit-remaining": "100", + "x-ratelimit-reset": String(resetEpoch), + "x-ratelimit-limit": "5000", + }, + }) + )); + + const client = createGitHubClient("test-token"); + await expect(client.graphql("{ viewer { login } }", { request: { retries: 0 } })).rejects.toThrow(); + + const rl = getGraphqlRateLimit(); + expect(rl).not.toBeNull(); + expect(rl!.remaining).toBe(100); + expect(rl!.limit).toBe(5000); + expect(rl!.resetAt).toBeInstanceOf(Date); + expect(rl!.resetAt.getTime()).toBe(resetEpoch * 1000); + }); + + it("REST error response with rate limit headers updates _coreRateLimit", async () => { + const resetEpoch = Math.floor(Date.now() / 1000) + 3600; + vi.stubGlobal("fetch", vi.fn().mockResolvedValue( + new Response(JSON.stringify({ message: "internal server error" }), { + status: 500, + headers: { + "content-type": "application/json", + "x-ratelimit-remaining": "100", + "x-ratelimit-reset": String(resetEpoch), + "x-ratelimit-limit": "5000", + }, + }) + )); + + const client = createGitHubClient("test-token"); + await expect(client.request("GET /user", { request: { retries: 0 } })).rejects.toThrow(); + + const rl = getCoreRateLimit(); + expect(rl).not.toBeNull(); + expect(rl!.remaining).toBe(100); + expect(rl!.limit).toBe(5000); + }); + + it("error response without rate limit headers does not crash", async () => { + vi.stubGlobal("fetch", vi.fn().mockResolvedValue( + new Response(JSON.stringify({ message: "not found" }), { + status: 404, + headers: { "content-type": "application/json" }, + }) + )); + + const client = createGitHubClient("test-token"); + // Must reject with the original error only, not an additional crash + await expect(client.request("GET /user")).rejects.toBeDefined(); + }); + + it("successful GraphQL request does NOT update _coreRateLimit", async () => { + const resetEpoch = Math.floor(Date.now() / 1000) + 3600; + // Seed _coreRateLimit to a known value + updateRateLimitFromHeaders({ + "x-ratelimit-remaining": "1234", + "x-ratelimit-reset": String(resetEpoch), + "x-ratelimit-limit": "5000", + }); + + vi.stubGlobal("fetch", vi.fn().mockResolvedValue( + new Response(JSON.stringify({ data: { viewer: { login: "test" } } }), { + status: 200, + headers: { + "content-type": "application/json", + "x-ratelimit-remaining": "9999", + "x-ratelimit-reset": String(resetEpoch), + "x-ratelimit-limit": "5000", + }, + }) + )); + + const client = createGitHubClient("test-token"); + await client.graphql("{ viewer { login } }"); + + // _coreRateLimit must remain at seeded value — GraphQL success must not contaminate it + const rl = getCoreRateLimit(); + expect(rl!.remaining).toBe(1234); + }); + + it("successful REST request updates _coreRateLimit but NOT _graphqlRateLimit", async () => { + const resetEpoch = Math.floor(Date.now() / 1000) + 3600; + // Seed _graphqlRateLimit to a known value + updateGraphqlRateLimit({ limit: 5000, remaining: 4321, resetAt: new Date(resetEpoch * 1000).toISOString() }); + + vi.stubGlobal("fetch", vi.fn().mockResolvedValue( + new Response(JSON.stringify({ login: "test" }), { + status: 200, + headers: { + "content-type": "application/json", + "x-ratelimit-remaining": "4888", + "x-ratelimit-reset": String(resetEpoch), + "x-ratelimit-limit": "5000", + }, + }) + )); + + const client = createGitHubClient("test-token"); + await client.request("GET /user"); + + const coreRl = getCoreRateLimit(); + expect(coreRl!.remaining).toBe(4888); + + // _graphqlRateLimit must be unchanged at seeded value + const graphqlRl = getGraphqlRateLimit(); + expect(graphqlRl!.remaining).toBe(4321); + }); + + it("successful GraphQL request exposes rateLimit.cost in ApiRequestInfo callback", async () => { + const resetEpoch = Math.floor(Date.now() / 1000) + 3600; + vi.stubGlobal("fetch", vi.fn().mockResolvedValue( + new Response(JSON.stringify({ + data: { + viewer: { login: "test" }, + rateLimit: { cost: 42, limit: 5000, remaining: 4900, resetAt: new Date(resetEpoch * 1000).toISOString() }, + }, + }), { + status: 200, + headers: { + "content-type": "application/json", + "x-ratelimit-remaining": "4900", + "x-ratelimit-reset": String(resetEpoch), + "x-ratelimit-limit": "5000", + }, + }) + )); + + let capturedInfo: ApiRequestInfo | null = null; + onApiRequest((info) => { capturedInfo = info; }); + + const client = createGitHubClient("test-token"); + await client.graphql("{ viewer { login } }"); + + expect(capturedInfo).not.toBeNull(); + expect(capturedInfo!.graphqlCost).toBe(42); + expect(capturedInfo!.isGraphql).toBe(true); + }); + + it("REST request has undefined graphqlCost in ApiRequestInfo callback", async () => { + vi.stubGlobal("fetch", vi.fn().mockResolvedValue( + new Response(JSON.stringify({ login: "test" }), { + status: 200, + headers: { + "content-type": "application/json", + "x-ratelimit-remaining": "4999", + "x-ratelimit-reset": String(Math.floor(Date.now() / 1000) + 3600), + "x-ratelimit-limit": "5000", + }, + }) + )); + + let capturedInfo: ApiRequestInfo | null = null; + onApiRequest((info) => { capturedInfo = info; }); + + const client = createGitHubClient("test-token"); + await client.request("GET /user"); + + expect(capturedInfo).not.toBeNull(); + expect(capturedInfo!.graphqlCost).toBeUndefined(); + expect(capturedInfo!.isGraphql).toBe(false); + }); + + it("GraphQL error response with rateLimit.cost in body exposes graphqlCost in ApiRequestInfo callback", async () => { + const resetEpoch = Math.floor(Date.now() / 1000) + 3600; + vi.stubGlobal("fetch", vi.fn().mockResolvedValue( + new Response(JSON.stringify({ + data: { rateLimit: { cost: 7, limit: 5000, remaining: 4800, resetAt: new Date(resetEpoch * 1000).toISOString() } }, + errors: [{ message: "resolver blew up" }], + }), { + status: 500, + headers: { + "content-type": "application/json", + "x-ratelimit-remaining": "4800", + "x-ratelimit-reset": String(resetEpoch), + "x-ratelimit-limit": "5000", + }, + }) + )); + + let capturedInfo: ApiRequestInfo | null = null; + onApiRequest((info) => { capturedInfo = info; }); + + const client = createGitHubClient("test-token"); + await expect(client.graphql("{ viewer { login } }", { request: { retries: 0 } })).rejects.toThrow(); + + expect(capturedInfo).not.toBeNull(); + expect(capturedInfo!.isGraphql).toBe(true); + expect(capturedInfo!.graphqlCost).toBe(7); + }); +}); + +// ── fetchRateLimitDetails ──────────────────────────────────────────────────── +// fetchRateLimitDetails calls getClient() which reads an internal SolidJS signal +// (_client) that is only non-null after auth token is set. These tests exercise +// the null-client fast-path and the signal-preservation guarantee on failure. +// Signal update tests (setCoreRateLimit / setGraphqlRateLimit) are covered by the +// hook.wrap tests above and the cache-hit path adds identical setters (same references). + +describe("fetchRateLimitDetails", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("returns null immediately when no client is available", async () => { + // No auth token is set in tests, so getClient() returns null → fetchRateLimitDetails returns null. + const result = await fetchRateLimitDetails(); + expect(result).toBeNull(); + }); + + it("failure does not clear existing signals", async () => { + // Seed signals to known values using the exported update functions. + const resetEpoch = Math.floor(Date.now() / 1000) + 3600; + updateRateLimitFromHeaders({ + "x-ratelimit-remaining": "1111", + "x-ratelimit-reset": String(resetEpoch), + "x-ratelimit-limit": "5000", + }); + updateGraphqlRateLimit({ limit: 5000, remaining: 2222, resetAt: new Date(resetEpoch * 1000).toISOString() }); + + // fetchRateLimitDetails returns null (no client) — signals must be unaffected. + const result = await fetchRateLimitDetails(); + expect(result).toBeNull(); + + expect(getCoreRateLimit()!.remaining).toBe(1111); + expect(getGraphqlRateLimit()!.remaining).toBe(2222); + }); +}); + +// ── fetchRateLimitDetails — client override + cache behavior ───────────────── + +describe("fetchRateLimitDetails — signal updates with client override", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("successful call updates both signals and cache-hit restores them", async () => { + const resetEpoch = Math.floor(Date.now() / 1000) + 3600; + const mockClient = { + request: vi.fn().mockResolvedValue({ + data: { + resources: { + core: { limit: 5000, remaining: 3000, reset: resetEpoch }, + graphql: { limit: 5000, remaining: 2500, reset: resetEpoch }, + }, + }, + }), + }; + + // Fresh call: seeds cache and updates signals + const result = await fetchRateLimitDetails(mockClient as never); + expect(result).not.toBeNull(); + expect(result!.core.remaining).toBe(3000); + expect(result!.graphql.remaining).toBe(2500); + expect(getCoreRateLimit()!.remaining).toBe(3000); + expect(getGraphqlRateLimit()!.remaining).toBe(2500); + + // Overwrite signals (simulates hook.wrap updating mid-cycle) + updateRateLimitFromHeaders({ + "x-ratelimit-remaining": "999", + "x-ratelimit-reset": String(resetEpoch), + "x-ratelimit-limit": "5000", + }); + expect(getCoreRateLimit()!.remaining).toBe(999); + + // Cache-hit within 5s: restores authoritative values, no HTTP call + const cached = await fetchRateLimitDetails(mockClient as never); + expect(cached).not.toBeNull(); + expect(getCoreRateLimit()!.remaining).toBe(3000); + expect(getGraphqlRateLimit()!.remaining).toBe(2500); + expect(mockClient.request).toHaveBeenCalledTimes(1); + }); +}); diff --git a/tests/services/poll.test.ts b/tests/services/poll.test.ts index 87080234..8389485a 100644 --- a/tests/services/poll.test.ts +++ b/tests/services/poll.test.ts @@ -2,6 +2,7 @@ import "fake-indexeddb/auto"; import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { createRoot, createSignal } from "solid-js"; import { createPollCoordinator, disableNotifGate, resetPollState, type DashboardData } from "../../src/app/services/poll"; +import * as githubMod from "../../src/app/services/github"; // Mock pushError so we can spy on it const mockPushError = vi.fn(); @@ -32,6 +33,14 @@ vi.mock("../../src/app/lib/notifications", () => ({ _resetNotificationState: vi.fn(), })); +// Mock github module — fetchRateLimitDetails runs concurrently (fire-and-forget) in doFetch +vi.mock("../../src/app/services/github", () => ({ + getClient: vi.fn(() => null), + fetchRateLimitDetails: vi.fn(() => Promise.resolve(null)), + onApiRequest: vi.fn(), + initClientWatcher: vi.fn(), +})); + // Mock config so doFetch doesn't fail when accessing config.selectedRepos vi.mock("../../src/app/stores/config", () => ({ config: { @@ -41,6 +50,12 @@ vi.mock("../../src/app/stores/config", () => ({ }, })); +async function flushPromises(): Promise { + // doFetch() awaits fetchAll and fires fetchRateLimitDetails concurrently; + // 10 iterations ensures all chained microtasks settle regardless of depth. + for (let i = 0; i < 10; i++) await Promise.resolve(); +} + // ── Helpers ─────────────────────────────────────────────────────────────────── const emptyData: DashboardData = { @@ -108,12 +123,12 @@ describe("createPollCoordinator", () => { await createRoot(async (dispose) => { createPollCoordinator(makeGetInterval(60), fetchAll); - await Promise.resolve(); // initial fetch + await flushPromises(); // initial fetch // Advance 1 full interval (with jitter ±30s, 60s is within [30s, 90s]) // Use 90s to be safe and hit the interval regardless of jitter vi.advanceTimersByTime(90_000); - await Promise.resolve(); + await flushPromises(); expect(fetchAll.mock.calls.length).toBeGreaterThanOrEqual(2); dispose(); @@ -126,7 +141,7 @@ describe("createPollCoordinator", () => { await createRoot(async (dispose) => { createPollCoordinator(makeGetInterval(60), fetchAll); - await Promise.resolve(); // initial fetch + await flushPromises(); // initial fetch const callsAfterInit = fetchAll.mock.calls.length; @@ -135,7 +150,7 @@ describe("createPollCoordinator", () => { // Advance past the interval (60s with 0 jitter) vi.advanceTimersByTime(61_000); - await Promise.resolve(); + await flushPromises(); // Should have fetched while hidden (background refresh) expect(fetchAll.mock.calls.length).toBeGreaterThan(callsAfterInit); @@ -150,7 +165,7 @@ describe("createPollCoordinator", () => { await createRoot(async (dispose) => { createPollCoordinator(makeGetInterval(300), fetchAll); - await Promise.resolve(); // initial fetch + await flushPromises(); // initial fetch const callsAfterInit = fetchAll.mock.calls.length; @@ -162,7 +177,7 @@ describe("createPollCoordinator", () => { // Restore visibility setDocumentVisible(true); - await Promise.resolve(); + await flushPromises(); // Should have triggered at least a catch-up fetch on re-visible // (background polls may also have fired if interval < hidden duration) @@ -230,21 +245,21 @@ describe("createPollCoordinator", () => { await createRoot(async (dispose) => { createPollCoordinator(makeGetInterval(60), fetchAll); - await Promise.resolve(); // initial fetch + await flushPromises(); // initial fetch const callsAfterInit = fetchAll.mock.calls.length; // Hide for >2 min — background polls fire at 60s and 120s setDocumentVisible(false); vi.advanceTimersByTime(130_000); - await Promise.resolve(); + await flushPromises(); const callsWhileHidden = fetchAll.mock.calls.length; expect(callsWhileHidden).toBeGreaterThan(callsAfterInit); // Restore visibility — catch-up fetch fires + timer resets setDocumentVisible(true); - await Promise.resolve(); + await flushPromises(); const callsAfterRevisible = fetchAll.mock.calls.length; expect(callsAfterRevisible).toBeGreaterThan(callsWhileHidden); @@ -256,7 +271,7 @@ describe("createPollCoordinator", () => { // Advance another 31s (61s from reset) — timer fires vi.advanceTimersByTime(31_000); - await Promise.resolve(); + await flushPromises(); expect(fetchAll.mock.calls.length).toBeGreaterThan(callsAfterRevisible); dispose(); @@ -270,12 +285,12 @@ describe("createPollCoordinator", () => { await createRoot(async (dispose) => { const coordinator = createPollCoordinator(makeGetInterval(60), fetchAll); - await Promise.resolve(); // initial fetch + await flushPromises(); // initial fetch const callsAfterInit = fetchAll.mock.calls.length; coordinator.manualRefresh(); - await Promise.resolve(); + await flushPromises(); expect(fetchAll.mock.calls.length).toBe(callsAfterInit + 1); dispose(); @@ -353,6 +368,7 @@ describe("createPollCoordinator", () => { // During the in-flight fetch, isRefreshing should be true expect(coordinator.isRefreshing()).toBe(true); + await Promise.resolve(); // flush microtasks so fetchAll is called resolvePromise(); await Promise.resolve(); await Promise.resolve(); // allow finally block to run @@ -423,6 +439,7 @@ describe("createPollCoordinator", () => { // First fetch is in-flight (unresolved) expect(coordinator.isRefreshing()).toBe(true); + await Promise.resolve(); // flush microtasks so fetchAll is called expect(fetchAll).toHaveBeenCalledTimes(1); // Trigger a second fetch while the first is still in-flight @@ -599,13 +616,13 @@ describe("createPollCoordinator", () => { await createRoot(async (dispose) => { createPollCoordinator(makeGetInterval(60), fetchAll); - await Promise.resolve(); // initial fetch + await flushPromises(); // initial fetch const callsAfterInit = fetchAll.mock.calls.length; // Advance exactly past the deterministic 30s interval vi.advanceTimersByTime(30_001); - await Promise.resolve(); + await flushPromises(); expect(fetchAll.mock.calls.length).toBe(callsAfterInit + 1); dispose(); @@ -645,4 +662,19 @@ describe("createPollCoordinator", () => { dispose(); }); }); + + it("fetchRateLimitDetails is called exactly once per doFetch cycle", async () => { + const fetchRateLimitDetailsSpy = vi.mocked(githubMod.fetchRateLimitDetails); + fetchRateLimitDetailsSpy.mockClear(); + + const fetchAll = makeFetchAll(); + + await createRoot(async (dispose) => { + createPollCoordinator(makeGetInterval(0), fetchAll); + await flushPromises(); + + expect(fetchRateLimitDetailsSpy).toHaveBeenCalledTimes(1); + dispose(); + }); + }); });