diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 40b71c0b..0be99dbb 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -426,7 +426,7 @@ Each full refresh also checks GitHub's own status page for outages affecting Act A second, faster poll loop runs alongside the full refresh specifically for in-flight items. It targets: - **PRs with actively running (`running`) or stale (`checkStatusStale`) CI checks** — re-checks status until checks leave the running state (plus a 60-second window after an inline `Approve` or `Ready?` action to catch newly triggered workflows). PRs in `pending` (`Pending condition`) are re-checked for up to 3 minutes from when they were first seen pending on their current commit, which covers jobs waiting for a runner and gaps between dependent jobs; after that, PRs parked on a condition (such as review or draft status) wait for the next full refresh. A new commit or a return to `running` starts a fresh 3-minute window. PRs paused in `waiting` (`Pending action`) are not hot-polled. For PRs from forks whose CI runs only in the fork, the status is read from the fork's head commit, the same source the full refresh uses. -- **In-progress or queued workflow runs** — re-checks until the run completes or pauses on an environment approval gate (`waiting`) +- **In-progress or queued workflow runs** — re-checks until the run completes or pauses on an environment approval gate (`waiting`), using conditional requests (`ETag` / `304 Not Modified`) so unchanged runs do not consume REST rate-limit budget Default interval: **30 seconds** (configurable from 10 to 120 seconds in Settings). @@ -850,7 +850,7 @@ Encrypted credentials are sealed by this deployment's server and can only be imp The tracker uses GitHub's GraphQL and REST APIs. Each poll cycle consumes some of your 5,000 request hourly budget. Tracking many repos, tracked users, or having a short refresh interval increases consumption. Increasing the refresh interval or reducing the number of tracked repos will reduce API usage. -A 60-second events poll uses ETag conditional requests to detect changes at near-zero cost, triggering targeted per-repo refreshes only when needed. +A lightweight 60-second events poll (~1 REST request per cycle) detects new activity by comparing event IDs and triggers targeted per-repo refreshes only when needed. For detailed per-source API call counts, see Settings > API Usage. @@ -866,7 +866,7 @@ Your token does not have write permission for pull requests. If you are using a **Data looks stale after switching back to the tab.** -When a tab has been hidden for more than 2 minutes, a catch-up fetch fires automatically on return. The events poll continues running in background tabs using ETag conditional requests (zero rate-limit cost), so changes are detected even while the tab is hidden. +When a tab has been hidden for more than 2 minutes, a catch-up fetch fires automatically on return. The lightweight events poll (~1 REST request per 60-second cycle) continues running in background tabs, so changes are detected even while the tab is hidden without the cost of a full refresh. **I want to stop tracking a repository.** diff --git a/src/app/App.tsx b/src/app/App.tsx index f6b98798..83651389 100644 --- a/src/app/App.tsx +++ b/src/app/App.tsx @@ -1,10 +1,10 @@ -import { createSignal, createEffect, onMount, Show, ErrorBoundary, Suspense, lazy, type JSX } from "solid-js"; +import { createSignal, createEffect, on, onMount, onCleanup, Show, ErrorBoundary, Suspense, lazy, type JSX } from "solid-js"; import * as Sentry from "@sentry/solid"; import { Router, Route, Navigate, useNavigate } from "@solidjs/router"; import { isAuthenticated, validateToken, AUTH_STORAGE_KEY } from "./stores/auth"; import { config, initConfigPersistence, resolveTheme } from "./stores/config"; import { initViewPersistence, pruneStaleIgnoredItems } from "./stores/view"; -import { evictStaleEntries } from "./stores/cache"; +import { evictStaleEntries, evictKeysWithPrefix } from "./stores/cache"; import { initClientWatcher } from "./services/github"; import { initMcpRelay } from "./lib/mcp-relay"; import LoginPage from "./pages/LoginPage"; @@ -173,6 +173,24 @@ export default function App() { document.documentElement.dataset.density = config.viewDensity; }); + createEffect( + on( + () => config.selectedRepos.map((r) => r.fullName).join(","), + (currentCsv, prevCsv) => { + if (!prevCsv) return; + const currentSet = new Set(currentCsv.split(",").filter(Boolean)); + const removed = prevCsv + .split(",") + .filter((r) => r.length > 0 && !currentSet.has(r)); + if (removed.length > 0) { + evictKeysWithPrefix(removed.flatMap((r) => [`runs2:${r}:`, `run:${r}:`])).catch(() => { + // Non-fatal — cache eviction failure is acceptable + }); + } + }, + ), + ); + onMount(() => { // Listen for system theme changes so "auto" reacts immediately const mq = window.matchMedia("(prefers-color-scheme: dark)"); @@ -189,9 +207,20 @@ export default function App() { initClientWatcher(); initMcpRelay(); pruneStaleIgnoredItems(); - evictStaleEntries(24 * 60 * 60 * 1000).catch(() => { + + const staleCacheMaxAgeMs = 24 * 60 * 60 * 1000; + const staleEvictionIntervalMs = 60 * 60 * 1000; + evictStaleEntries(staleCacheMaxAgeMs).catch(() => { // Non-fatal — stale eviction failure is acceptable }); + const evictionInterval = setInterval(() => { + evictStaleEntries(staleCacheMaxAgeMs).catch(() => { + // Non-fatal — stale eviction failure is acceptable + }); + }, staleEvictionIntervalMs); + onCleanup(() => { + clearInterval(evictionInterval); + }); // Preload dashboard chunk in parallel with token validation to avoid // a sequential waterfall (validateToken → chunk fetch) diff --git a/src/app/components/dashboard/DashboardPage.tsx b/src/app/components/dashboard/DashboardPage.tsx index d454f5c6..8413975b 100644 --- a/src/app/components/dashboard/DashboardPage.tsx +++ b/src/app/components/dashboard/DashboardPage.tsx @@ -58,10 +58,13 @@ const ISSUE_FILTER_DEFAULTS = IssueFiltersSchema.parse({}); const PR_FILTER_DEFAULTS = PullRequestFiltersSchema.parse({}); const ACTIONS_FILTER_DEFAULTS = ActionsFiltersSchema.parse({}); +export const _testCounters = { exclusiveOwnershipRuns: 0 }; + /** * Build a scope matcher for a custom tab's org/repo scope. Shared between - * customTabData and tabCounts. An empty scope (no orgs, no repos) matches - * nothing — see isTabUnscoped, which flags this state for the UI. + * exclusiveCustomTabData, activeCustomTabData, and tabCounts. An empty scope + * (no orgs, no repos) matches nothing — see isTabUnscoped, which flags this + * state for the UI. */ function buildTabScopeMatcher(tab: CustomTab): (repoFullName: string) => boolean { const orgSet = tab.orgScope.length > 0 ? new Set(tab.orgScope.map((o) => o.toLowerCase())) : null; @@ -129,6 +132,9 @@ const [jiraIssues, setJiraIssues] = createSignal([]); const [jiraLoading, setJiraLoading] = createSignal(false); const [jiraKeyMap, setJiraKeyMap] = createSignal>(new Map()); let _jiraFetching = false; +let _inFlightJiraScope: string | null = null; +let _jiraPendingRerun = false; +let lastJiraFetchStartedAt = 0; // Dependency dashboard state — module-level for same reasons as jira state above const [abandonedDepsMap, setAbandonedDepsMap] = createSignal>(new Map()); @@ -380,6 +386,9 @@ onAuthCleared(() => { setJiraLoading(false); setJiraKeyMap(new Map()); _jiraFetching = false; + _inFlightJiraScope = null; + _jiraPendingRerun = false; + lastJiraFetchStartedAt = 0; setAbandonedDepsMap(new Map()); setDashboardIssueUrls(new Map()); setDepMeta(new Map()); @@ -695,12 +704,22 @@ export default function DashboardPage() { ); async function fetchJiraAssigned(): Promise { - if (_jiraFetching) return; + const scope = viewState.tabFilters.jiraAssigned?.scope ?? "assigned"; + if (_jiraFetching) { + if (scope !== _inFlightJiraScope) { + _jiraPendingRerun = true; + } + return; + } const client = jiraClient(); - if (!client) return; + if (!client) { + setJiraLoading(false); + return; + } _jiraFetching = true; + _inFlightJiraScope = scope; + lastJiraFetchStartedAt = Date.now(); setJiraLoading(true); - const scope = viewState.tabFilters.jiraAssigned?.scope ?? "assigned"; try { const result = await client.searchJql( jiraJqlForScope(scope), @@ -711,6 +730,12 @@ export default function DashboardPage() { if (missing.length > 0) console.info("[jira] issues missing issuetype:", missing.map((i) => `${i.key}: ${JSON.stringify(i.fields.issuetype)}`)); } if (!isJiraAuthenticated()) return; + const currentScope = viewState.tabFilters.jiraAssigned?.scope ?? "assigned"; + if (currentScope !== scope) { + _jiraPendingRerun = true; + return; + } + _jiraPendingRerun = false; setJiraIssues(result.issues); // Only prune when the result set is complete. The Enhanced JQL Search @@ -763,6 +788,7 @@ export default function DashboardPage() { } } } catch (err) { + lastJiraFetchStartedAt = 0; if (err instanceof JiraApiError) { if (err.status === 401) { clearJiraAuth(); @@ -782,7 +808,14 @@ export default function DashboardPage() { } } finally { _jiraFetching = false; - setJiraLoading(false); + _inFlightJiraScope = null; + if (_jiraPendingRerun && config.jira?.enabled && isJiraAuthenticated()) { + _jiraPendingRerun = false; + void fetchJiraAssigned().catch(handleJiraError); + } else { + _jiraPendingRerun = false; + setJiraLoading(false); + } } } @@ -1132,20 +1165,40 @@ export default function DashboardPage() { new Set(dependencyPullRequests().map((pr) => pr.id)) ); - // Eagerly compute scoped data for exclusive custom tabs (needed by exclusiveOwnership). - // Non-exclusive tabs only compute when they are the active tab. - const customTabData = createMemo(() => { + interface CustomTabScopedData { + issues: typeof dashboardData.issues; + pullRequests: typeof dashboardData.pullRequests; + workflowRuns: typeof dashboardData.workflowRuns; + } + + function computeCustomTabEntry(tab: CustomTab): CustomTabScopedData { + const matchesScope = buildTabScopeMatcher(tab); + return { + issues: tab.baseType === "issues" ? dashboardData.issues.filter((i) => matchesScope(i.repoFullName)) : [], + pullRequests: tab.baseType === "pullRequests" ? dashboardData.pullRequests.filter((p) => matchesScope(p.repoFullName)) : [], + workflowRuns: tab.baseType === "actions" ? dashboardData.workflowRuns.filter((w) => matchesScope(w.repoFullName)) : [], + }; + } + + // Scoped data for exclusive custom tabs only (does NOT depend on activeTab). + const exclusiveCustomTabData = createMemo(() => { + const result: Record = {}; + for (const tab of config.customTabs) { + if (!tab.exclusive) continue; + if (!config.enableActions && tab.baseType === "actions") continue; + result[tab.id] = computeCustomTabEntry(tab); + } + return result; + }); + + // Scoped data for the currently active non-exclusive custom tab (if any). + const activeCustomTabData = createMemo(() => { const currentTabId = activeTab(); - const result: Record = {}; + const result: Record = {}; for (const tab of config.customTabs) { + if (tab.exclusive || tab.id !== currentTabId) continue; if (!config.enableActions && tab.baseType === "actions") continue; - if (!tab.exclusive && tab.id !== currentTabId) continue; - const matchesScope = buildTabScopeMatcher(tab); - result[tab.id] = { - issues: tab.baseType === "issues" ? dashboardData.issues.filter((i) => matchesScope(i.repoFullName)) : [], - pullRequests: tab.baseType === "pullRequests" ? dashboardData.pullRequests.filter((p) => matchesScope(p.repoFullName)) : [], - workflowRuns: tab.baseType === "actions" ? dashboardData.workflowRuns.filter((w) => matchesScope(w.repoFullName)) : [], - }; + result[tab.id] = computeCustomTabEntry(tab); } return result; }); @@ -1154,6 +1207,8 @@ export default function DashboardPage() { // Only claims items matching the tab's baseType (an exclusive Issues tab must // not hide PRs or workflow runs from their respective tabs). const exclusiveOwnership = createMemo(() => { + _testCounters.exclusiveOwnershipRuns += 1; + const exclusiveData = exclusiveCustomTabData(); const issueOwner = new Map(); const prOwner = new Map(); const runOwner = new Map(); @@ -1164,7 +1219,7 @@ export default function DashboardPage() { } for (const tab of config.customTabs) { if (!tab.exclusive) continue; - const data = customTabData()[tab.id]; + const data = exclusiveData[tab.id]; if (!data) continue; if (tab.baseType === "issues") { for (const i of data.issues) if (!issueOwner.has(i.id)) issueOwner.set(i.id, tab.id); @@ -1220,20 +1275,12 @@ export default function DashboardPage() { const login = userLogin().toLowerCase(); const monitoredSet = new Set((config.monitoredRepos ?? []).map((r) => r.fullName)); const users = allUsers(); + const exclusiveData = exclusiveCustomTabData(); const customCounts: Record = {}; for (const tab of config.customTabs) { if (!config.enableActions && tab.baseType === "actions") continue; - // customTabData skips non-exclusive inactive tabs (perf optimization), - // so compute scope on demand for tabs absent from the memo. - let data = customTabData()[tab.id]; - if (!data) { - const matchesScope = buildTabScopeMatcher(tab); - data = { - issues: tab.baseType === "issues" ? dashboardData.issues.filter((i) => matchesScope(i.repoFullName)) : [], - pullRequests: tab.baseType === "pullRequests" ? dashboardData.pullRequests.filter((p) => matchesScope(p.repoFullName)) : [], - workflowRuns: tab.baseType === "actions" ? dashboardData.workflowRuns.filter((w) => matchesScope(w.repoFullName)) : [], - }; - } + // exclusiveCustomTabData caches exclusive tabs; compute scope on demand for non-exclusive tabs. + const data = exclusiveData[tab.id] ?? computeCustomTabEntry(tab); // Merge filter chain via shared helper (same as tab components) const preset = tab.filterPreset; if (tab.baseType === "issues") { @@ -1410,6 +1457,7 @@ export default function DashboardPage() { () => _coordinator()?.lastRefreshAt(), () => { if (!config.jira?.enabled || !isJiraAuthenticated()) return; + if (Date.now() - lastJiraFetchStartedAt < 20_000) return; fetchJiraAssigned().catch(handleJiraError); }, { defer: true } @@ -1785,7 +1833,7 @@ export default function DashboardPage() { `Could not approve ${pr.repoFullName}#${pr.number}: new commits were pushed — refreshing`, "warning" ); - _coordinator()?.manualRefresh(); + handleManualRefresh(); return; } const message = err instanceof Error ? err.message : "Unknown error"; @@ -1856,6 +1904,11 @@ export default function DashboardPage() { } } + function handleManualRefresh(): void { + lastJiraFetchStartedAt = 0; + _coordinator()?.manualRefresh(); + } + const isPartialSync = createMemo(() => getNotifications().some((n) => n.source === PR_ENRICHMENT_NOTIFICATION_SOURCE) ); @@ -1893,7 +1946,7 @@ export default function DashboardPage() { _coordinator()?.manualRefresh()} + onRefresh={handleManualRefresh} sortOptions={activeTab() === "dependencies" ? undefined : globalSortOptions} sortValue={activeTab() === "dependencies" ? undefined : viewState.globalSort.field} sortDirection={activeTab() === "dependencies" ? undefined : viewState.globalSort.direction} @@ -1950,7 +2003,7 @@ export default function DashboardPage() { rebaseLabel={config.dependencies.rebaseLabel} userLogin={userLogin()} trackedBotLogins={trackedBotLogins()} - onRefresh={() => _coordinator()?.manualRefresh()} + onRefresh={handleManualRefresh} onApprovePR={handleApprovePR} approvingPRIds={approvingPRIds()} onMarkPRReady={handleMarkPRReady} @@ -1993,13 +2046,14 @@ export default function DashboardPage() { {(tab) => { // Apply exclusivity: exclude items owned by OTHER exclusive tabs const data = createMemo(() => { - const raw = customTabData()[tab().id]; + const tabId = tab().id; + const raw = exclusiveCustomTabData()[tabId] ?? activeCustomTabData()[tabId]; if (!raw) return { issues: [] as typeof dashboardData.issues, pullRequests: [] as typeof dashboardData.pullRequests, workflowRuns: [] as typeof dashboardData.workflowRuns }; const ownership = exclusiveOwnership(); return { - issues: raw.issues.filter((i) => isItemVisibleOnTab(ownership.issues, i.id, tab().id)), - pullRequests: raw.pullRequests.filter((p) => isItemVisibleOnTab(ownership.pullRequests, p.id, tab().id)), - workflowRuns: raw.workflowRuns.filter((w) => isItemVisibleOnTab(ownership.actions, w.id, tab().id)), + issues: raw.issues.filter((i) => isItemVisibleOnTab(ownership.issues, i.id, tabId)), + pullRequests: raw.pullRequests.filter((p) => isItemVisibleOnTab(ownership.pullRequests, p.id, tabId)), + workflowRuns: raw.workflowRuns.filter((w) => isItemVisibleOnTab(ownership.actions, w.id, tabId)), }; }); return ( diff --git a/src/app/components/dashboard/DependenciesTab.tsx b/src/app/components/dashboard/DependenciesTab.tsx index 90e1c1fe..7ecec7dd 100644 --- a/src/app/components/dashboard/DependenciesTab.tsx +++ b/src/app/components/dashboard/DependenciesTab.tsx @@ -2,7 +2,7 @@ import { createMemo, createSignal, For, Show } from "solid-js"; import { config, updateConfig } from "../../stores/config"; import { viewState, setTabFilter, resetAllTabFilters, ignoreItem, unignoreItem, trackItem, untrackItem, DependencyFiltersSchema, setDependencyExpandedGroups } from "../../stores/view"; import IgnoreBadge from "./IgnoreBadge"; -import { isSafeGitHubUrl } from "../../lib/url"; +import { isSafeGitHubUrl, sizedAvatarUrl } from "../../lib/url"; import { canApprovePR, canMarkPRReady } from "../../lib/format"; import { isNonTerminalCheckStatus, type PullRequest } from "../../services/api"; import type { AbandonedDependency } from "../../lib/dependency-dashboard"; @@ -315,7 +315,13 @@ export default function DependenciesTab(props: DependenciesTabProps) { {(bot) => (
- {bot.login} + {bot.login} Dependency PRs from {bot.login} — track this bot for full coverage? diff --git a/src/app/components/dashboard/TrackedTab.tsx b/src/app/components/dashboard/TrackedTab.tsx index 1381b6e6..3ca6f29d 100644 --- a/src/app/components/dashboard/TrackedTab.tsx +++ b/src/app/components/dashboard/TrackedTab.tsx @@ -1,4 +1,4 @@ -import { For, Show, Switch, Match, createMemo } from "solid-js"; +import { For, Show, Switch, Match, createMemo, onCleanup } from "solid-js"; import { config } from "../../stores/config"; import { viewState, untrackItem, moveTrackedItem, untrackJiraItem, moveJiraItem } from "../../stores/view"; import type { TrackedItem } from "../../stores/view"; @@ -32,6 +32,12 @@ function TypeBadge(props: { type: TrackedItem["type"] }) { // FLIP animation: record positions before move, animate slide after DOM updates const itemRefs = new Map(); + +// Test-only accessor for the module-level itemRefs Map — lets tests verify +// row unmount and tab cleanup remove detached DOM references. +export function _getItemRefsCount(): number { + return itemRefs.size; +} const prefersReducedMotion = () => typeof window !== "undefined" && window.matchMedia("(prefers-reduced-motion: reduce)").matches; @@ -114,11 +120,20 @@ export default function TrackedTab(props: TrackedTabProps) { const isFirst = () => index() === 0; const isLast = () => index() === viewState.trackedItems.length - 1; const itemKey = `${item.type}:${item.id}`; + let rowEl: HTMLDivElement | undefined; + onCleanup(() => { + if (itemRefs.get(itemKey) === rowEl) { + itemRefs.delete(itemKey); + } + }); return (
{ itemRefs.set(itemKey, el); }} + ref={(el) => { + rowEl = el; + itemRefs.set(itemKey, el); + }} > {/* Reorder buttons */}
diff --git a/src/app/components/layout/Header.tsx b/src/app/components/layout/Header.tsx index 3b6865ca..f68eb706 100644 --- a/src/app/components/layout/Header.tsx +++ b/src/app/components/layout/Header.tsx @@ -2,6 +2,7 @@ import { createSignal, Show } from "solid-js"; import { useNavigate } from "@solidjs/router"; import { user, clearAuth } from "../../stores/auth"; import { getUnreadCount, markAllAsRead } from "../../lib/errors"; +import { sizedAvatarUrl } from "../../lib/url"; import NotificationDrawer from "../shared/NotificationDrawer"; import ToastContainer from "../shared/ToastContainer"; import { Tooltip } from "../shared/Tooltip"; @@ -46,7 +47,7 @@ export default function Header() { {(u) => (
{u().login} diff --git a/src/app/components/shared/UserAvatarBadge.tsx b/src/app/components/shared/UserAvatarBadge.tsx index 1650ee23..ebb181c2 100644 --- a/src/app/components/shared/UserAvatarBadge.tsx +++ b/src/app/components/shared/UserAvatarBadge.tsx @@ -1,4 +1,5 @@ import { createMemo, For, Show } from "solid-js"; +import { sizedAvatarUrl } from "../../lib/url"; import { Tooltip } from "./Tooltip"; export function buildSurfacedByUsers( @@ -37,8 +38,10 @@ export default function UserAvatarBadge(props: UserAvatarBadgeProps) { >
{u.login}
diff --git a/src/app/lib/notifications.ts b/src/app/lib/notifications.ts index 41ff6205..e48b3db0 100644 --- a/src/app/lib/notifications.ts +++ b/src/app/lib/notifications.ts @@ -22,6 +22,8 @@ export function canNotify(config: Config): boolean { // ── New item detection ──────────────────────────────────────────────────────── +export const SEEN_ID_CAP = 20_000; + // In-memory sets of seen IDs — not persisted across page loads const seenIssueIds = new Set(); const seenPrIds = new Set(); @@ -30,6 +32,33 @@ const seenRunIds = new Set(); // True after the first fetch completes — prevents flood on initial load let initialized = false; +function markIdSeen(set: Set, id: number): void { + set.delete(id); + set.add(id); + if (set.size > SEEN_ID_CAP) { + const oldest = set.values().next().value; + if (oldest !== undefined) { + set.delete(oldest); + } + } +} + +function collectNewAndMarkSeen(set: Set, items: T[]): T[] { + for (const item of items) { + if (set.has(item.id)) { + markIdSeen(set, item.id); + } + } + const newItems: T[] = []; + for (const item of items) { + if (!set.has(item.id)) { + newItems.push(item); + markIdSeen(set, item.id); + } + } + return newItems; +} + export interface NewItems { issues: Issue[]; pullRequests: PullRequest[]; @@ -37,40 +66,19 @@ export interface NewItems { } export function detectNewItems(current: DashboardData): NewItems { - const newIssues: Issue[] = []; - const newPrs: PullRequest[] = []; - const newRuns: WorkflowRun[] = []; - if (!initialized) { // First fetch: populate seen sets without notifying - for (const item of current.issues) seenIssueIds.add(item.id); - for (const item of current.pullRequests) seenPrIds.add(item.id); - for (const item of current.workflowRuns) seenRunIds.add(item.id); + for (const item of current.issues) markIdSeen(seenIssueIds, item.id); + for (const item of current.pullRequests) markIdSeen(seenPrIds, item.id); + for (const item of current.workflowRuns) markIdSeen(seenRunIds, item.id); initialized = true; console.debug(`[notifications] seeded: ${seenIssueIds.size} issues, ${seenPrIds.size} PRs, ${seenRunIds.size} runs`); return { issues: [], pullRequests: [], workflowRuns: [] }; } - for (const item of current.issues) { - if (!seenIssueIds.has(item.id)) { - seenIssueIds.add(item.id); - newIssues.push(item); - } - } - - for (const item of current.pullRequests) { - if (!seenPrIds.has(item.id)) { - seenPrIds.add(item.id); - newPrs.push(item); - } - } - - for (const item of current.workflowRuns) { - if (!seenRunIds.has(item.id)) { - seenRunIds.add(item.id); - newRuns.push(item); - } - } + const newIssues = collectNewAndMarkSeen(seenIssueIds, current.issues); + const newPrs = collectNewAndMarkSeen(seenPrIds, current.pullRequests); + const newRuns = collectNewAndMarkSeen(seenRunIds, current.workflowRuns); console.debug(`[notifications] detected: ${newIssues.length} new issues, ${newPrs.length} new PRs, ${newRuns.length} new runs (seen: ${seenIssueIds.size}/${seenPrIds.size}/${seenRunIds.size})`); return { issues: newIssues, pullRequests: newPrs, workflowRuns: newRuns }; @@ -84,6 +92,14 @@ export function _resetNotificationState(): void { initialized = false; } +export function _getSeenSizes(): { issues: number; pullRequests: number; workflowRuns: number } { + return { + issues: seenIssueIds.size, + pullRequests: seenPrIds.size, + workflowRuns: seenRunIds.size, + }; +} + // ── Notification dispatch ───────────────────────────────────────────────────── const BATCH_THRESHOLD = 5; diff --git a/src/app/lib/url.ts b/src/app/lib/url.ts index 016211eb..f49dd1b2 100644 --- a/src/app/lib/url.ts +++ b/src/app/lib/url.ts @@ -31,3 +31,24 @@ export function openGitHubUrl(url: string): void { if (!isSafeGitHubUrl(url)) return; window.open(url, "_blank", "noopener,noreferrer"); } + +/** + * Sets or replaces the `s` size query parameter on `https://avatars.githubusercontent.com/` + * URLs. Returns any other input (or invalid URLs) unchanged. + */ +export function sizedAvatarUrl(url: string, px: number): string { + try { + const parsed = new URL(url); + if ( + parsed.protocol === "https:" && + parsed.hostname === "avatars.githubusercontent.com" && + parsed.pathname.length > 1 + ) { + parsed.searchParams.set("s", String(px)); + return parsed.toString(); + } + return url; + } catch { + return url; + } +} diff --git a/src/app/services/api-usage.ts b/src/app/services/api-usage.ts index 44e42f51..d9c72e6f 100644 --- a/src/app/services/api-usage.ts +++ b/src/app/services/api-usage.ts @@ -222,11 +222,15 @@ export function deriveSource(info: ApiRequestInfo): ApiCallSource { return "rest"; } -// Register at module scope — intercepts every Octokit request automatically +// Register at module scope — intercepts every Octokit request automatically. +// HTTP 304 Not Modified responses do not consume GitHub rate-limit budget +// (actual cost = 0), so skip trackApiCall on 304 while still syncing resetAt. onApiRequest((info) => { - const source = deriveSource(info); - const pool: ApiPool = info.isGraphql ? "graphql" : "core"; - trackApiCall(source, pool, info.graphqlCost ?? 1); + if (info.status !== 304) { + const source = deriveSource(info); + const pool: ApiPool = info.isGraphql ? "graphql" : "core"; + 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 6b46730d..1db03e24 100644 --- a/src/app/services/api.ts +++ b/src/app/services/api.ts @@ -1,6 +1,7 @@ import { getClient, cachedRequest, updateGraphqlRateLimit } from "./github"; import * as Sentry from "@sentry/solid"; import { pushNotification, dismissNotificationBySource } from "../lib/errors"; +import { deleteCacheEntry } from "../stores/cache"; import type { ApiCallSource } from "./api-usage"; import type { TrackedUser } from "../stores/config"; import { VALID_REPO_NAME, VALID_TRACKED_LOGIN, SEARCH_RESULT_CAP, APPROVE_PR_MUTATION, MARK_PR_READY_MUTATION, GITHUB_NODE_ID_RE, GIT_SHA_RE } from "../../shared/validation"; @@ -42,7 +43,7 @@ interface RawRepo { archived?: boolean; } -interface RawWorkflowRun { +export interface RawWorkflowRun { id: number; name: string; status: string; @@ -62,6 +63,75 @@ interface RawWorkflowRun { actor: { login: string } | null; } +export interface RawWorkflowRunsResponse { + total_count: number; + workflow_runs: RawWorkflowRun[]; +} + +export type RawSingleWorkflowRun = Pick< + RawWorkflowRun, + "id" | "status" | "conclusion" | "updated_at" | "completed_at" +>; + +export function projectSingleRun(raw: RawSingleWorkflowRun): RawSingleWorkflowRun { + return { + id: raw.id, + status: raw.status, + conclusion: raw.conclusion ?? null, + updated_at: raw.updated_at, + completed_at: raw.completed_at ?? null, + }; +} + +export function projectRunsPage(raw: RawWorkflowRunsResponse): RawWorkflowRunsResponse { + return { + total_count: raw.total_count, + workflow_runs: (raw.workflow_runs ?? []).map((run) => ({ + id: run.id, + name: run.name, + status: run.status, + conclusion: run.conclusion, + event: run.event, + workflow_id: run.workflow_id, + head_sha: run.head_sha, + head_branch: run.head_branch, + run_number: run.run_number, + html_url: run.html_url, + created_at: run.created_at, + updated_at: run.updated_at, + run_started_at: run.run_started_at, + completed_at: run.completed_at ?? null, + run_attempt: run.run_attempt, + display_title: run.display_title, + actor: run.actor ? { login: run.actor.login } : null, + })), + }; +} + +export function mapWorkflowRun(run: RawWorkflowRun, repoFullName: string): WorkflowRun { + return { + id: run.id, + name: run.name, + status: run.status, + conclusion: run.conclusion, + event: run.event, + workflowId: run.workflow_id, + headSha: run.head_sha, + headBranch: run.head_branch, + runNumber: run.run_number, + htmlUrl: run.html_url, + createdAt: run.created_at, + updatedAt: run.updated_at, + repoFullName, + isPrRun: run.event === "pull_request", + runStartedAt: run.run_started_at, + completedAt: run.completed_at ?? null, + runAttempt: run.run_attempt, + displayTitle: run.display_title, + actorLogin: run.actor?.login ?? "", + }; +} + // ── Constants ──────────────────────────────────────────────────────────────── // Batch repos into chunks for search queries (keeps URL length manageable). @@ -246,7 +316,7 @@ const LIGHT_ISSUE_FRAGMENT = ` url createdAt updatedAt - author { login avatarUrl } + author { login avatarUrl(size: 40) } labels(first: 10) { nodes { name color } } assignees(first: 10) { nodes { login } } repository { nameWithOwner stargazerCount } @@ -283,7 +353,7 @@ const LIGHT_PR_FRAGMENT = ` url createdAt updatedAt - author { login avatarUrl } + author { login avatarUrl(size: 40) } repository { nameWithOwner stargazerCount } headRefName baseRefName @@ -2377,15 +2447,13 @@ export async function fetchWorkflowRuns( while (rawRuns.length < targetRunsPerRepo) { const result = await cachedRequest( octokit, - `runs:${repo.fullName}:p${page}`, + `runs2:${repo.fullName}:p${page}`, "GET /repos/{owner}/{repo}/actions/runs", - { owner: repo.owner, repo: repo.name, per_page: perPage, page } + { owner: repo.owner, repo: repo.name, per_page: perPage, page }, + { project: projectRunsPage } ); - const data = result.data as { - workflow_runs: RawWorkflowRun[]; - total_count: number; - }; + const data = result.data as RawWorkflowRunsResponse; const runs = data.workflow_runs ?? []; rawRuns.push(...runs); @@ -2424,27 +2492,7 @@ export async function fetchWorkflowRuns( (a, b) => a.created_at > b.created_at ? -1 : a.created_at < b.created_at ? 1 : 0 ); for (const run of sorted.slice(0, maxRuns)) { - allRuns.push({ - id: run.id, - name: run.name, - status: run.status, - conclusion: run.conclusion, - event: run.event, - workflowId: run.workflow_id, - headSha: run.head_sha, - headBranch: run.head_branch, - runNumber: run.run_number, - htmlUrl: run.html_url, - createdAt: run.created_at, - updatedAt: run.updated_at, - repoFullName: repo.fullName, - isPrRun: run.event === "pull_request", - runStartedAt: run.run_started_at, - completedAt: run.completed_at ?? null, - runAttempt: run.run_attempt, - displayTitle: run.display_title, - actorLogin: run.actor?.login ?? "", - }); + allRuns.push(mapWorkflowRun(run, repo.fullName)); } } }); @@ -2570,20 +2618,31 @@ export async function fetchWorkflowRunById( descriptor: { id: number; owner: string; repo: string } ): Promise { const { id, owner, repo } = descriptor; - const response = await octokit.request("GET /repos/{owner}/{repo}/actions/runs/{run_id}", { - owner, - repo, - run_id: id, - }); - // Octokit's generated type for this endpoint omits completed_at; cast to our full raw shape - const run = response.data as unknown as RawWorkflowRun; - return { + const cacheKey = `run:${owner}/${repo}:${id}`; + const result = await cachedRequest( + octokit, + cacheKey, + "GET /repos/{owner}/{repo}/actions/runs/{run_id}", + { + owner, + repo, + run_id: id, + }, + { project: projectSingleRun } + ); + // Octokit's generated type for this endpoint omits completed_at; cast to our projected raw shape + const run = result.data as RawSingleWorkflowRun; + const mapped: HotWorkflowRunUpdate = { id: run.id, status: run.status ?? "", conclusion: run.conclusion ?? null, updatedAt: run.updated_at, completedAt: run.completed_at ?? null, }; + if (mapped.status !== "in_progress" && mapped.status !== "queued") { + await deleteCacheEntry(cacheKey).catch(() => {}); + } + return mapped; } // ── User validation + upstream repo discovery ───────────────────────────────── diff --git a/src/app/services/github-status.ts b/src/app/services/github-status.ts index ce911a9e..e43740e8 100644 --- a/src/app/services/github-status.ts +++ b/src/app/services/github-status.ts @@ -199,7 +199,7 @@ export async function fetchGitHubStatus(): Promise { if (_fetchInProgress) return; // in-flight guard — avoid pile-up if refreshInterval is very short/0 or the endpoint is slow to respond _fetchInProgress = true; try { - const { data } = await cachedFetch(CACHE_KEY, async (headers) => { + const { data, fromCache } = await cachedFetch(CACHE_KEY, async (headers) => { const reqHeaders: Record = {}; if (headers.etag) reqHeaders["If-None-Match"] = headers.etag; const res = await fetch(STATUS_API_URL, { @@ -215,6 +215,10 @@ export async function fetchGitHubStatus(): Promise { return { data: json, etag: res.headers.get("ETag"), lastModified: res.headers.get("Last-Modified"), status: res.status }; }); + if (fromCache && _githubStatus() !== null && _consecutiveFailures === 0) { + return; + } + // Validate the raw shape before parsing — catches API drift (e.g. a field // renamed/removed upstream) as a distinct, loud log message instead of an // unexplained TypeError deep inside parseSummary. Treated the same as any diff --git a/src/app/services/github.ts b/src/app/services/github.ts index e8fbbb8e..c9663a45 100644 --- a/src/app/services/github.ts +++ b/src/app/services/github.ts @@ -3,7 +3,7 @@ import { Octokit } from "@octokit/core"; import { throttling } from "@octokit/plugin-throttling"; import { retry } from "@octokit/plugin-retry"; import { paginateRest } from "@octokit/plugin-paginate-rest"; -import { cachedFetch, type ConditionalHeaders } from "../stores/cache"; +import { cachedFetch, type CachedFetchOptions, type ConditionalHeaders } from "../stores/cache"; import { token } from "../stores/auth"; import { pushNotification } from "../lib/errors"; import { APPROVE_PR_MUTATION, MARK_PR_READY_MUTATION, GITHUB_NODE_ID_RE, GIT_SHA_RE, isReadOnlyGraphqlQuery } from "../../shared/validation"; @@ -402,12 +402,12 @@ export function createGitHubClient(authToken: string): GitHubOctokitInstance { // ── ETag-aware request wrapper ─────────────────────────────────────────────── -export async function cachedRequest( +export async function cachedRequest( octokit: GitHubOctokitInstance, cacheKey: string, route: string, params?: Record, - maxAge?: number + options?: number | CachedFetchOptions ): Promise<{ data: unknown; fromCache: boolean }> { return cachedFetch(cacheKey, async (cached: ConditionalHeaders) => { const conditionalHeaders: Record = {}; @@ -447,7 +447,7 @@ export async function cachedRequest( } throw err; } - }, maxAge); + }, options); } // ── Client singleton ───────────────────────────────────────────────────────── diff --git a/src/app/stores/cache.ts b/src/app/stores/cache.ts index 481ada0c..750c32a6 100644 --- a/src/app/stores/cache.ts +++ b/src/app/stores/cache.ts @@ -124,6 +124,22 @@ export async function evictStaleEntries(maxAgeMs: number): Promise { return count; } +export async function evictKeysWithPrefix(prefixes: string[]): Promise { + const activePrefixes = prefixes.filter((p) => p.length > 0); + if (activePrefixes.length === 0) return; + + const db = await getDb(); + const tx = db.transaction("cache", "readwrite"); + await Promise.all( + activePrefixes.map((prefix) => + tx.store.delete(IDBKeyRange.bound(prefix, `${prefix}￿`)) + ) + ); + await tx.done; +} + +export const STALE_TOUCH_THROTTLE_MS = 60 * 60 * 1000; + export interface FetchResult { data: unknown; etag: string | null; @@ -136,11 +152,19 @@ export interface ConditionalHeaders { lastModified: string | null; } -export async function cachedFetch( +export interface CachedFetchOptions { + maxAge?: number; + project?: (data: T) => T; +} + +export async function cachedFetch( key: string, fetchFn: (headers: ConditionalHeaders) => Promise, - maxAge?: number + options?: number | CachedFetchOptions ): Promise<{ data: unknown; fromCache: boolean }> { + const maxAge = typeof options === "number" ? options : options?.maxAge; + const project = typeof options === "object" && options !== null ? options.project : undefined; + const existing = await getCacheEntry(key); let conditionalHeaders: ConditionalHeaders = { etag: null, lastModified: null }; @@ -171,12 +195,22 @@ export async function cachedFetch( if (!existing || (conditionalHeaders.etag === null && conditionalHeaders.lastModified === null)) { throw new Error(`Received 304 but no valid cache entry for key: ${key}`); } + if (existing.maxAge === null && Date.now() - existing.fetchedAt >= STALE_TOUCH_THROTTLE_MS) { + await setCacheEntry( + key, + existing.data, + existing.etag, + undefined, + existing.lastModified + ).catch(() => {}); + } return { data: existing.data, fromCache: true }; } if (result.status === 200) { - await setCacheEntry(key, result.data, result.etag, maxAge, result.lastModified); - return { data: result.data, fromCache: false }; + const projectedData = project ? project(result.data as T) : result.data; + await setCacheEntry(key, projectedData, result.etag, maxAge, result.lastModified); + return { data: projectedData, fromCache: false }; } throw new Error(`Unexpected fetch status: ${result.status}`); diff --git a/tests/acceptance/steps/dependency-exclusions.steps.tsx b/tests/acceptance/steps/dependency-exclusions.steps.tsx index b5130865..ff18a8b2 100644 --- a/tests/acceptance/steps/dependency-exclusions.steps.tsx +++ b/tests/acceptance/steps/dependency-exclusions.steps.tsx @@ -142,6 +142,7 @@ vi.mock("../../../src/app/lib/errors", () => ({ vi.mock("../../../src/app/lib/url", () => ({ isSafeGitHubUrl: vi.fn(() => true), openGitHubUrl: vi.fn(), + sizedAvatarUrl: (url: string) => url, })); import { loadFeature, describeFeature } from "@amiceli/vitest-cucumber"; diff --git a/tests/app/cache-eviction.test.tsx b/tests/app/cache-eviction.test.tsx new file mode 100644 index 00000000..50c25ec9 --- /dev/null +++ b/tests/app/cache-eviction.test.tsx @@ -0,0 +1,114 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { render } from "@solidjs/testing-library"; + +vi.mock("../../src/app/stores/auth", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + isAuthenticated: () => true, + validateToken: vi.fn().mockResolvedValue(true), + }; +}); + +vi.mock("../../src/app/stores/config", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + initConfigPersistence: vi.fn(), + }; +}); + +vi.mock("../../src/app/stores/view", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + initViewPersistence: vi.fn(), + }; +}); + +vi.mock("../../src/app/services/github", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + initClientWatcher: vi.fn(), + getClient: vi.fn().mockReturnValue(null), + }; +}); + +vi.mock("../../src/app/stores/cache", () => ({ + evictStaleEntries: vi.fn().mockResolvedValue(0), + evictKeysWithPrefix: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("../../src/app/components/dashboard/DashboardPage", () => ({ + default: () =>
Dashboard
, +})); +vi.mock("../../src/app/components/onboarding/OnboardingWizard", () => ({ + default: () =>
Onboarding
, +})); +vi.mock("../../src/app/components/settings/SettingsPage", () => ({ + default: () =>
Settings
, +})); + +import * as configStore from "../../src/app/stores/config"; +import * as cacheStore from "../../src/app/stores/cache"; +import App from "../../src/app/App"; + +describe("App — cache eviction scheduling and deselected repo cleanup", () => { + beforeEach(() => { + vi.useFakeTimers(); + vi.clearAllMocks(); + vi.mocked(cacheStore.evictStaleEntries).mockResolvedValue(0); + vi.mocked(cacheStore.evictKeysWithPrefix).mockResolvedValue(undefined); + configStore.resetConfig(); + configStore.updateConfig({ onboardingComplete: true }); + if (window.location.pathname !== "/") { + window.history.pushState({}, "", "/"); + } + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it("runs evictStaleEntries(24h) on mount, again after 1 hour, and stops after unmount", async () => { + const twentyFourHoursMs = 24 * 60 * 60 * 1000; + const oneHourMs = 60 * 60 * 1000; + + const { unmount } = render(() => ); + + expect(cacheStore.evictStaleEntries).toHaveBeenCalledTimes(1); + expect(cacheStore.evictStaleEntries).toHaveBeenLastCalledWith(twentyFourHoursMs); + + await vi.advanceTimersByTimeAsync(oneHourMs); + expect(cacheStore.evictStaleEntries).toHaveBeenCalledTimes(2); + expect(cacheStore.evictStaleEntries).toHaveBeenLastCalledWith(twentyFourHoursMs); + + unmount(); + + await vi.advanceTimersByTimeAsync(oneHourMs); + expect(cacheStore.evictStaleEntries).toHaveBeenCalledTimes(2); + }); + + it("evicts runs2:: and run:: keys when a repo is deselected from config.selectedRepos", () => { + configStore.updateConfig({ + selectedRepos: [ + { owner: "a", name: "b", fullName: "a/b" }, + { owner: "c", name: "d", fullName: "c/d" }, + ], + }); + + const { unmount } = render(() => ); + expect(cacheStore.evictKeysWithPrefix).not.toHaveBeenCalled(); + + // Deselect a/b while keeping c/d + configStore.updateConfig({ + selectedRepos: [{ owner: "c", name: "d", fullName: "c/d" }], + }); + + expect(cacheStore.evictKeysWithPrefix).toHaveBeenCalledTimes(1); + expect(cacheStore.evictKeysWithPrefix).toHaveBeenCalledWith(["runs2:a/b:", "run:a/b:"]); + + unmount(); + }); +}); diff --git a/tests/components/App.test.tsx b/tests/components/App.test.tsx index 062aaa87..6d67be56 100644 --- a/tests/components/App.test.tsx +++ b/tests/components/App.test.tsx @@ -47,6 +47,7 @@ vi.mock("../../src/app/stores/cache", async (importOriginal) => { return { ...actual, evictStaleEntries: vi.fn().mockResolvedValue(undefined), + evictKeysWithPrefix: vi.fn().mockResolvedValue(undefined), }; }); diff --git a/tests/components/dashboard/ActionsTab.test.tsx b/tests/components/dashboard/ActionsTab.test.tsx index 27803271..b372242b 100644 --- a/tests/components/dashboard/ActionsTab.test.tsx +++ b/tests/components/dashboard/ActionsTab.test.tsx @@ -25,6 +25,7 @@ Object.defineProperty(globalThis, "localStorage", { vi.mock("../../../src/app/lib/url", () => ({ isSafeGitHubUrl: () => true, + sizedAvatarUrl: (url: string) => url, })); // ── Imports ─────────────────────────────────────────────────────────────────── diff --git a/tests/components/dashboard/DashboardPage.test.tsx b/tests/components/dashboard/DashboardPage.test.tsx index 1098e5b9..aa97e5b0 100644 --- a/tests/components/dashboard/DashboardPage.test.tsx +++ b/tests/components/dashboard/DashboardPage.test.tsx @@ -1,9 +1,12 @@ -import { describe, it, expect } from "vitest"; +import { describe, it, expect, vi, afterEach } from "vitest"; +import { createSignal } from "solid-js"; +import { render, fireEvent } from "@solidjs/testing-library"; import { rateLimitCssClass } from "../../../src/app/lib/format"; import { applyHotPollPRUpdates } from "../../../src/app/components/dashboard/DashboardPage"; -import { makePullRequest } from "../../helpers/index"; +import { makeIssue, makePullRequest } from "../../helpers/index"; import type { PullRequest } from "../../../src/shared/types"; import type { HotPRStatusUpdate } from "../../../src/app/services/api"; +import type { DashboardData } from "../../../src/app/services/poll"; describe("rateLimitCssClass", () => { it("remaining: 0 gives text-error", () => { @@ -161,3 +164,502 @@ describe("hot poll terminal PR splice logic", () => { expect(state.pullRequests[2].draft).toBe(true); }); }); + +// ── Jira duplicate-fetch deduplication on load ─────────────────────────────── + +describe("DashboardPage — Jira load fetch deduplication", () => { + afterEach(() => { + vi.useRealTimers(); + vi.resetModules(); + }); + + async function setupJiraDedupeMocks(mockSearchJql: ReturnType) { + vi.resetModules(); + + let identityEpochVal = 0; + const authClearCallbacks: (() => void)[] = []; + const [lastRefreshAt, setLastRefreshAt] = createSignal(null); + + vi.doMock("@solidjs/router", () => ({ + useNavigate: () => vi.fn(), + })); + + vi.doMock("../../../src/app/stores/auth", () => ({ + clearAuth: vi.fn(), + expireToken: vi.fn(), + token: () => "fake-token", + user: () => ({ login: "testuser", avatar_url: "", name: "Test User" }), + isAuthenticated: () => true, + onAuthCleared: vi.fn((cb: () => void) => { + authClearCallbacks.push(() => { + identityEpochVal++; + cb(); + }); + }), + DASHBOARD_STORAGE_KEY: "github-tracker:dashboard", + DEP_META_STORAGE_KEY: "github-tracker:dep-meta", + jiraAuth: vi.fn(() => ({ + cloudId: "cloud-123", + accessToken: "tok", + siteUrl: "https://test.atlassian.net", + siteName: "Test Site", + })), + isJiraAuthenticated: vi.fn(() => true), + setJiraAuth: vi.fn(), + clearJiraAuth: vi.fn(), + ensureJiraTokenValid: vi.fn().mockResolvedValue(true), + identityEpoch: () => identityEpochVal, + })); + + vi.doMock("../../../src/app/lib/dashboard-snapshot", () => ({ + loadSnapshot: vi.fn().mockResolvedValue(null), + writeSnapshot: vi.fn(), + flushSnapshot: vi.fn().mockResolvedValue(undefined), + clearSnapshot: vi.fn().mockResolvedValue(undefined), + })); + + const MockJiraClient = vi.fn(function (this: Record) { + this.searchJql = mockSearchJql; + this.bulkFetch = vi.fn().mockResolvedValue({ issues: [], errors: [] }); + }); + + vi.doMock("../../../src/app/services/jira-client", () => ({ + JiraClient: MockJiraClient, + JiraProxyClient: MockJiraClient, + JiraApiError: class JiraApiError extends Error { + status: number; + constructor(status: number, _body: unknown, message: string) { + super(message); + this.status = status; + } + }, + DEFAULT_FIELDS: ["summary", "status", "priority", "assignee", "project", "updated", "issuetype", "created"], + })); + + vi.doMock("../../../src/app/services/jira-keys", () => ({ + detectAndLookupJiraKeys: vi.fn().mockResolvedValue(new Map()), + clearJiraKeyCache: vi.fn(), + })); + + vi.doMock("../../../src/app/services/github", () => ({ + getCoreRateLimit: () => null, + getGraphqlRateLimit: () => null, + getClient: vi.fn(() => null), + updateGraphqlRateLimit: vi.fn(), + })); + + vi.doMock("../../../src/app/lib/notifications", () => ({ + detectNewItems: vi.fn(() => ({ issues: [], pullRequests: [], workflowRuns: [] })), + dispatchNotifications: vi.fn(), + _resetNotificationState: vi.fn(), + })); + + vi.doMock("../../../src/app/lib/errors", () => ({ + getErrors: vi.fn().mockReturnValue([]), + getNotifications: vi.fn().mockReturnValue([]), + getUnreadCount: vi.fn().mockReturnValue(0), + markAllAsRead: vi.fn(), + dismissError: vi.fn(), + dismissNotificationBySource: vi.fn(), + pushError: vi.fn(), + pushNotification: vi.fn(), + clearErrors: vi.fn(), + clearNotifications: vi.fn(), + addMutedSource: vi.fn(), + isMuted: vi.fn(() => false), + clearMutedSources: vi.fn(), + })); + + const mockFetchAllData = vi.fn().mockResolvedValue({ + issues: [], + pullRequests: [], + workflowRuns: [], + errors: [], + }); + let capturedFetchAll: (() => Promise) | null = null; + + vi.doMock("../../../src/app/services/poll", () => ({ + fetchAllData: mockFetchAllData, + createPollCoordinator: vi.fn().mockImplementation( + (_getInterval: unknown, fetchAll: () => Promise) => { + capturedFetchAll = fetchAll; + return { + isRefreshing: () => false, + lastRefreshAt, + manualRefresh: vi.fn(() => { + setLastRefreshAt(new Date()); + }), + destroy: vi.fn(), + }; + }, + ), + createHotPollCoordinator: vi.fn().mockImplementation(() => ({ destroy: vi.fn() })), + createEventsPollCoordinator: vi.fn().mockImplementation(() => ({ destroy: vi.fn() })), + rebuildHotSets: vi.fn(), + seedHotSetsFromTargeted: vi.fn(), + clearHotSets: vi.fn(), + getHotPollGeneration: vi.fn().mockReturnValue(0), + StaleIdentityError: class StaleIdentityError extends Error {}, + })); + + const freshView = await import("../../../src/app/stores/view"); + const freshConfig = await import("../../../src/app/stores/config"); + const freshDash = await import("../../../src/app/components/dashboard/DashboardPage"); + + freshView.resetViewState(); + freshConfig.resetConfig(); + freshConfig.updateJiraConfig({ + enabled: true, + siteUrl: "https://test.atlassian.net", + siteName: "Test Site", + authMethod: "oauth", + }); + + return { + DashboardPage: freshDash.default, + _testCounters: freshDash._testCounters, + freshConfig, + freshView, + mockFetchAllData, + triggerPoll: () => capturedFetchAll!(), + setLastRefreshAt, + triggerAuthCleared: () => { + for (const cb of authClearCallbacks) cb(); + }, + }; + } + + it("does not recompute exclusiveOwnership when switching between built-in and custom tabs and renders scoped custom-tab items", async () => { + const mockSearchJql = vi.fn().mockResolvedValue({ + issues: [], + maxResults: 100, + startAt: 0, + }); + + const { + DashboardPage: FreshDashboard, + _testCounters, + freshConfig, + freshView, + mockFetchAllData, + triggerPoll, + } = await setupJiraDedupeMocks(mockSearchJql); + + mockFetchAllData.mockResolvedValue({ + issues: [ + makeIssue({ id: 1, title: "Octocat Exclusive Issue", repoFullName: "octocat/Hello-World", userLogin: "testuser" }), + makeIssue({ id: 2, title: "Other Repo Issue", repoFullName: "other/Repo", userLogin: "testuser" }), + ], + pullRequests: [ + makePullRequest({ id: 10, title: "Octocat Team PR", repoFullName: "octocat/Hello-World", userLogin: "testuser" }), + makePullRequest({ id: 11, title: "Other Repo PR", repoFullName: "other/Repo", userLogin: "testuser" }), + ], + workflowRuns: [], + errors: [], + }); + + freshConfig.updateConfig({ + selectedRepos: [ + { owner: "octocat", name: "Hello-World", fullName: "octocat/Hello-World" }, + { owner: "other", name: "Repo", fullName: "other/Repo" }, + ], + customTabs: [ + { + id: "exclusive-issues", + name: "Exclusive Issues", + baseType: "issues", + orgScope: ["octocat"], + repoScope: [], + filterPreset: {}, + exclusive: true, + }, + { + id: "non-exclusive-prs", + name: "Team PRs", + baseType: "pullRequests", + orgScope: ["octocat"], + repoScope: [], + filterPreset: {}, + exclusive: false, + }, + ], + }); + freshView.updateViewState({ lastActiveTab: "issues" }); + freshView.setAllExpanded("issues", true); + freshView.setAllExpanded("pullRequests", true); + freshView.setAllExpanded("non-exclusive-prs", true); + freshView.setAllExpanded("exclusive-issues", true); + + _testCounters.exclusiveOwnershipRuns = 0; + + const { unmount, getByRole, queryByText } = render(() => ); + try { + await triggerPoll(); + + // Built-in Issues tab shows only the non-claimed issue + expect(queryByText("Other Repo Issue")).not.toBeNull(); + expect(queryByText("Octocat Exclusive Issue")).toBeNull(); + + const runsAfterLoad = _testCounters.exclusiveOwnershipRuns; + expect(runsAfterLoad).toBeGreaterThan(0); + + // Switch to Pull Requests tab via TabBar click and viewState + const prTabBtn = getByRole("tab", { name: /^Pull Requests/i }); + fireEvent.click(prTabBtn); + expect(freshView.viewState.lastActiveTab).toBe("pullRequests"); + expect(_testCounters.exclusiveOwnershipRuns).toBe(runsAfterLoad); + expect(queryByText("Octocat Team PR")).not.toBeNull(); + expect(queryByText("Other Repo PR")).not.toBeNull(); + + // Switch to non-exclusive custom tab ("Team PRs") — exercises activeCustomTabData()[tabId] + const teamPrsTabBtn = getByRole("tab", { name: /Team PRs/i }); + fireEvent.click(teamPrsTabBtn); + expect(freshView.viewState.lastActiveTab).toBe("non-exclusive-prs"); + expect(_testCounters.exclusiveOwnershipRuns).toBe(runsAfterLoad); + expect(queryByText("Octocat Team PR")).not.toBeNull(); + expect(queryByText("Other Repo PR")).toBeNull(); + + // Switch to exclusive custom tab ("Exclusive Issues") — exercises exclusiveCustomTabData()[tabId] + const exclusiveIssuesTabBtn = getByRole("tab", { name: /Exclusive Issues/i }); + fireEvent.click(exclusiveIssuesTabBtn); + expect(freshView.viewState.lastActiveTab).toBe("exclusive-issues"); + expect(_testCounters.exclusiveOwnershipRuns).toBe(runsAfterLoad); + expect(queryByText("Octocat Exclusive Issue")).not.toBeNull(); + expect(queryByText("Other Repo Issue")).toBeNull(); + + // Switch to Actions tab and back to Issues tab + const actionsTabBtn = getByRole("tab", { name: /Actions/i }); + fireEvent.click(actionsTabBtn); + expect(freshView.viewState.lastActiveTab).toBe("actions"); + expect(_testCounters.exclusiveOwnershipRuns).toBe(runsAfterLoad); + + const issuesTabBtn = getByRole("tab", { name: /^Issues/i }); + fireEvent.click(issuesTabBtn); + expect(freshView.viewState.lastActiveTab).toBe("issues"); + expect(_testCounters.exclusiveOwnershipRuns).toBe(runsAfterLoad); + expect(queryByText("Other Repo Issue")).not.toBeNull(); + expect(queryByText("Octocat Exclusive Issue")).toBeNull(); + } finally { + unmount(); + } + }); + + it("skips the duplicate post-refresh Jira fetch when the first poll completes 5 s after mount, and fetches on the next poll at +60 s", async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-11-01T12:00:00Z")); + + const mockSearchJql = vi.fn().mockResolvedValue({ + issues: [], + maxResults: 100, + startAt: 0, + }); + + const { DashboardPage: FreshDashboard, setLastRefreshAt } = await setupJiraDedupeMocks(mockSearchJql); + + const { unmount } = render(() => ); + try { + await vi.advanceTimersByTimeAsync(0); + // Immediate mount fetch ran once at t = 0 + expect(mockSearchJql).toHaveBeenCalledTimes(1); + + // First GitHub poll completes 5 s later (within the 20 s deduplication window) + await vi.advanceTimersByTimeAsync(5_000); + setLastRefreshAt(new Date()); + await vi.advanceTimersByTimeAsync(0); + + expect(mockSearchJql).toHaveBeenCalledTimes(1); + + // Next GitHub poll completes at +60 s (55 s later, outside the 20 s window) + await vi.advanceTimersByTimeAsync(55_000); + setLastRefreshAt(new Date()); + await vi.advanceTimersByTimeAsync(0); + + expect(mockSearchJql).toHaveBeenCalledTimes(2); + } finally { + unmount(); + } + }); + + it("resets lastJiraFetchStartedAt on logout so the first post-login poll within 20 s fetches Jira issues", async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-11-01T12:00:00Z")); + + const mockSearchJql = vi.fn().mockResolvedValue({ + issues: [], + maxResults: 100, + startAt: 0, + }); + + const { DashboardPage: FreshDashboard, setLastRefreshAt, triggerAuthCleared } = + await setupJiraDedupeMocks(mockSearchJql); + + const { unmount } = render(() => ); + try { + await vi.advanceTimersByTimeAsync(0); + expect(mockSearchJql).toHaveBeenCalledTimes(1); + + // Logout/login within 20 s (at +5 s) resets lastJiraFetchStartedAt to 0 + await vi.advanceTimersByTimeAsync(5_000); + triggerAuthCleared(); + + // Re-mount for the newly logged-in session or trigger the first post-login poll within 20 s + setLastRefreshAt(new Date()); + await vi.advanceTimersByTimeAsync(0); + + expect(mockSearchJql).toHaveBeenCalledTimes(2); + } finally { + unmount(); + } + }); + + it("retries Jira fetch on the first poll within 20 s if the initial mount fetch failed, and re-fetches on manual Refresh within 20 s", async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-11-01T12:00:00Z")); + + const mockSearchJql = vi + .fn() + .mockRejectedValueOnce(new Error("Transient network failure")) + .mockResolvedValue({ + issues: [], + maxResults: 100, + startAt: 0, + }); + + const { DashboardPage: FreshDashboard, setLastRefreshAt } = await setupJiraDedupeMocks(mockSearchJql); + + const { unmount, getByRole } = render(() => ); + try { + await vi.advanceTimersByTimeAsync(0); + expect(mockSearchJql).toHaveBeenCalledTimes(1); + + // First poll completes 5 s later -> retries because the mount fetch failed + await vi.advanceTimersByTimeAsync(5_000); + setLastRefreshAt(new Date()); + await vi.advanceTimersByTimeAsync(0); + expect(mockSearchJql).toHaveBeenCalledTimes(2); + + // User clicks manual Refresh 3 s later (within 20 s of the successful retry) -> not suppressed + await vi.advanceTimersByTimeAsync(3_000); + const refreshBtn = getByRole("button", { name: /Refresh data/i }); + fireEvent.click(refreshBtn); + await vi.advanceTimersByTimeAsync(0); + expect(mockSearchJql).toHaveBeenCalledTimes(3); + } finally { + unmount(); + } + }); + + it("discards stale-scope issues and re-runs fetchJiraAssigned for the new scope when scope changes while a Jira fetch is in flight", async () => { + const makeJiraIssue = (id: string, key: string, summary: string) => ({ + id, + key, + fields: { + summary, + status: { name: "To Do", statusCategory: { key: "new", name: "To Do" } }, + priority: { name: "High", id: "2" }, + assignee: { displayName: "Test User" }, + project: { key: "PROJ", name: "Project" }, + updated: "2026-11-01T12:00:00.000+0000", + created: "2026-11-01T10:00:00.000+0000", + issuetype: { name: "Bug", subtask: false }, + }, + }); + + let resolveFirstFetch!: (value: { issues: unknown[]; maxResults: number; startAt: number }) => void; + let resolveSecondFetch!: (value: { issues: unknown[]; maxResults: number; startAt: number }) => void; + const mockSearchJql = vi + .fn() + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveFirstFetch = resolve; + }), + ) + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveSecondFetch = resolve; + }), + ); + + const { DashboardPage: FreshDashboard, freshView } = await setupJiraDedupeMocks(mockSearchJql); + freshView.updateViewState({ lastActiveTab: "jiraAssigned" }); + freshView.setAllExpanded("jiraAssigned", true); + + const { unmount, queryByText } = render(() => ); + try { + // Mount fetch is in flight for default "assigned" scope + expect(mockSearchJql).toHaveBeenCalledTimes(1); + expect(mockSearchJql.mock.calls[0][0]).toContain("assignee = currentUser()"); + + // User switches scope to "reported" while the "assigned" fetch is still in flight + freshView.setTabFilter("jiraAssigned", "scope", "reported"); + + // Resolve the initial "assigned" fetch with a stale issue -> discarded and triggers a rerun for "reported" + resolveFirstFetch({ + issues: [makeJiraIssue("101", "PROJ-1", "Stale Assigned Issue")], + maxResults: 100, + startAt: 0, + }); + await new Promise((r) => setTimeout(r, 0)); + + expect(mockSearchJql).toHaveBeenCalledTimes(2); + expect(mockSearchJql.mock.calls[1][0]).toContain("reporter = currentUser()"); + expect(queryByText("Stale Assigned Issue")).toBeNull(); + + // Resolve the "reported" fetch -> renders only the reported scope's issue + resolveSecondFetch({ + issues: [makeJiraIssue("102", "PROJ-2", "Fresh Reported Issue")], + maxResults: 100, + startAt: 0, + }); + await new Promise((r) => setTimeout(r, 0)); + + expect(queryByText("Stale Assigned Issue")).toBeNull(); + expect(queryByText("Fresh Reported Issue")).not.toBeNull(); + } finally { + unmount(); + } + }); + + it("clears jiraLoading if a pending scope-change rerun finds jiraClient() is null", async () => { + let resolveFirstFetch!: (value: { issues: unknown[]; maxResults: number; startAt: number }) => void; + const mockSearchJql = vi.fn().mockImplementationOnce( + () => + new Promise((resolve) => { + resolveFirstFetch = resolve; + }), + ); + + const { DashboardPage: FreshDashboard, freshConfig, freshView } = await setupJiraDedupeMocks(mockSearchJql); + freshView.updateViewState({ lastActiveTab: "jiraAssigned" }); + freshView.setAllExpanded("jiraAssigned", true); + + const { unmount, queryByRole } = render(() => ); + try { + expect(mockSearchJql).toHaveBeenCalledTimes(1); + expect(queryByRole("status")).not.toBeNull(); + + // Switch scope while in-flight, and switch authMethod to "token" (no email -> createJiraClient returns null) + freshView.setTabFilter("jiraAssigned", "scope", "reported"); + freshConfig.updateJiraConfig({ + enabled: true, + siteUrl: "https://test.atlassian.net", + siteName: "Test Site", + authMethod: "token", + }); + + resolveFirstFetch({ + issues: [], + maxResults: 100, + startAt: 0, + }); + await new Promise((r) => setTimeout(r, 0)); + + expect(mockSearchJql).toHaveBeenCalledTimes(1); + expect(queryByRole("status")).toBeNull(); + } finally { + unmount(); + } + }); +}); diff --git a/tests/components/dashboard/DependenciesTab.test.tsx b/tests/components/dashboard/DependenciesTab.test.tsx index cdbd90a7..e97c15ed 100644 --- a/tests/components/dashboard/DependenciesTab.test.tsx +++ b/tests/components/dashboard/DependenciesTab.test.tsx @@ -21,7 +21,8 @@ Object.defineProperty(globalThis, "localStorage", { // ── Mocks ──────────────────────────────────────────────────────────────────── -vi.mock("../../../src/app/lib/url", () => ({ +vi.mock("../../../src/app/lib/url", async (importOriginal) => ({ + ...(await importOriginal()), isSafeGitHubUrl: (url: string) => typeof url === "string" && url.startsWith("https://github.com"), openGitHubUrl: vi.fn(), })); @@ -738,6 +739,10 @@ describe("DependenciesTab — unknown bot banner", () => { renderTab({ pullRequests: [pr] }); expect(screen.getByRole("button", { name: "Track bot" })).toBeDefined(); expect(screen.getByRole("button", { name: "Dismiss" })).toBeDefined(); + const img = screen.getByAltText("custom-dep-bot"); + expect(img.getAttribute("src")).toBe("https://avatars.githubusercontent.com/u/12345?s=40"); + expect(img.getAttribute("loading")).toBe("lazy"); + expect(img.getAttribute("decoding")).toBe("async"); }); it("does not show banner for known dep bots", () => { @@ -1214,6 +1219,10 @@ describe("DependenciesTab — unknown bot banner", () => { renderTab({ pullRequests: [pr] }); expect(screen.getByRole("button", { name: "Track bot" })).toBeDefined(); expect(screen.getByRole("button", { name: "Dismiss" })).toBeDefined(); + const img = screen.getByAltText("custom-dep-bot"); + expect(img.getAttribute("src")).toBe("https://avatars.githubusercontent.com/u/12345?s=40"); + expect(img.getAttribute("loading")).toBe("lazy"); + expect(img.getAttribute("decoding")).toBe("async"); }); it("does not show banner for known dep bots", () => { diff --git a/tests/components/dashboard/IssuesTab.test.tsx b/tests/components/dashboard/IssuesTab.test.tsx index 7d72af7e..e88f2c3a 100644 --- a/tests/components/dashboard/IssuesTab.test.tsx +++ b/tests/components/dashboard/IssuesTab.test.tsx @@ -25,6 +25,7 @@ Object.defineProperty(globalThis, "localStorage", { vi.mock("../../../src/app/lib/url", () => ({ isSafeGitHubUrl: () => true, + sizedAvatarUrl: (url: string) => url, })); // ── Imports ─────────────────────────────────────────────────────────────────── diff --git a/tests/components/dashboard/PullRequestsTab.test.tsx b/tests/components/dashboard/PullRequestsTab.test.tsx index c28c2edb..4b3e1cb2 100644 --- a/tests/components/dashboard/PullRequestsTab.test.tsx +++ b/tests/components/dashboard/PullRequestsTab.test.tsx @@ -25,6 +25,7 @@ Object.defineProperty(globalThis, "localStorage", { vi.mock("../../../src/app/lib/url", () => ({ isSafeGitHubUrl: () => true, + sizedAvatarUrl: (url: string) => url, })); // ── Imports ─────────────────────────────────────────────────────────────────── diff --git a/tests/components/dashboard/TrackedTab.test.tsx b/tests/components/dashboard/TrackedTab.test.tsx index b82b8e27..bc6a0fbe 100644 --- a/tests/components/dashboard/TrackedTab.test.tsx +++ b/tests/components/dashboard/TrackedTab.test.tsx @@ -24,12 +24,13 @@ Object.defineProperty(globalThis, "localStorage", { vi.mock("../../../src/app/lib/url", () => ({ isSafeGitHubUrl: () => true, + sizedAvatarUrl: (url: string) => url, })); // ── Imports ─────────────────────────────────────────────────────────────────── -import TrackedTab from "../../../src/app/components/dashboard/TrackedTab"; -import { viewState, resetViewState, updateViewState, untrackJiraItem } from "../../../src/app/stores/view"; +import TrackedTab, { _getItemRefsCount } from "../../../src/app/components/dashboard/TrackedTab"; +import { viewState, resetViewState, updateViewState, untrackItem, untrackJiraItem } from "../../../src/app/stores/view"; import { setConfig, resetConfig } from "../../../src/app/stores/config"; // ── Setup ───────────────────────────────────────────────────────────────────── @@ -636,3 +637,31 @@ describe("TrackedTab — auto-merge badge and Approve action", () => { expect(screen.getAllByText("Pending condition")).toHaveLength(1); }); }); + +describe("TrackedTab — itemRefs cleanup on untrack and unmount", () => { + it("tracks 3 item refs when rendered, drops to 2 when one item is untracked, and clears to 0 on unmount", () => { + const issue1 = makeIssue({ id: 1001, title: "Tracked 1" }); + const issue2 = makeIssue({ id: 1002, title: "Tracked 2" }); + const issue3 = makeIssue({ id: 1003, title: "Tracked 3" }); + + updateViewState({ + trackedItems: [ + makeTrackedItem({ id: 1001, type: "issue", title: "Tracked 1" }), + makeTrackedItem({ id: 1002, type: "issue", title: "Tracked 2" }), + makeTrackedItem({ id: 1003, type: "issue", title: "Tracked 3" }), + ], + }); + + const { unmount } = render(() => ( + + )); + + expect(_getItemRefsCount()).toBe(3); + + untrackItem(1002, "issue"); + expect(_getItemRefsCount()).toBe(2); + + unmount(); + expect(_getItemRefsCount()).toBe(0); + }); +}); diff --git a/tests/components/layout/Header.test.tsx b/tests/components/layout/Header.test.tsx index 3983bf84..019d7bab 100644 --- a/tests/components/layout/Header.test.tsx +++ b/tests/components/layout/Header.test.tsx @@ -184,4 +184,17 @@ describe("Header", () => { render(() =>
); expect(screen.getByLabelText("Critical GitHub service outage")).toBeDefined(); }); + + it("renders the user avatar with s=56 on avatars.githubusercontent.com URLs without lazy loading", () => { + vi.spyOn(authStore, "user").mockReturnValue({ + login: "octocat", + avatar_url: "https://avatars.githubusercontent.com/u/583231", + name: "The Octocat", + }); + render(() =>
); + const img = screen.getByAltText("octocat"); + expect(img.getAttribute("src")).toBe("https://avatars.githubusercontent.com/u/583231?s=56"); + expect(img.getAttribute("loading")).toBeNull(); + vi.mocked(authStore.user).mockRestore(); + }); }); diff --git a/tests/components/settings/ApiUsageSection.test.tsx b/tests/components/settings/ApiUsageSection.test.tsx index b63c4716..3dfacc24 100644 --- a/tests/components/settings/ApiUsageSection.test.tsx +++ b/tests/components/settings/ApiUsageSection.test.tsx @@ -78,6 +78,7 @@ vi.mock("../../../src/app/services/api", () => ({ vi.mock("../../../src/app/lib/url", () => ({ isSafeGitHubUrl: vi.fn(() => true), openGitHubUrl: vi.fn(), + sizedAvatarUrl: (url: string) => url, })); vi.mock("../../../src/app/lib/errors", () => ({ diff --git a/tests/components/settings/JiraSection.test.tsx b/tests/components/settings/JiraSection.test.tsx index 564dec2d..3cf2ec58 100644 --- a/tests/components/settings/JiraSection.test.tsx +++ b/tests/components/settings/JiraSection.test.tsx @@ -150,6 +150,7 @@ vi.mock("../../../src/app/lib/mcp-relay", () => ({ vi.mock("../../../src/app/lib/url", () => ({ isSafeGitHubUrl: vi.fn(() => true), openGitHubUrl: vi.fn(), + sizedAvatarUrl: (url: string) => url, })); const mockBuildJiraAuthorizeUrl = vi.fn(() => "https://auth.atlassian.com/authorize?mock=1"); diff --git a/tests/components/settings/SettingsPage.test.tsx b/tests/components/settings/SettingsPage.test.tsx index 18daa2bb..ebab9853 100644 --- a/tests/components/settings/SettingsPage.test.tsx +++ b/tests/components/settings/SettingsPage.test.tsx @@ -76,6 +76,7 @@ vi.mock("../../../src/app/services/api", () => ({ vi.mock("../../../src/app/lib/url", () => ({ isSafeGitHubUrl: vi.fn(() => true), openGitHubUrl: vi.fn(), + sizedAvatarUrl: (url: string) => url, })); vi.mock("../../../src/app/lib/errors", () => ({ diff --git a/tests/components/settings/actions-toggle.test.tsx b/tests/components/settings/actions-toggle.test.tsx index b48b7830..f8e2f427 100644 --- a/tests/components/settings/actions-toggle.test.tsx +++ b/tests/components/settings/actions-toggle.test.tsx @@ -56,6 +56,7 @@ vi.mock("../../../src/app/services/api", () => ({ vi.mock("../../../src/app/lib/url", () => ({ isSafeGitHubUrl: vi.fn(() => true), openGitHubUrl: vi.fn(), + sizedAvatarUrl: (url: string) => url, })); vi.mock("../../../src/app/lib/errors", () => ({ diff --git a/tests/components/shared/UserAvatarBadge.test.tsx b/tests/components/shared/UserAvatarBadge.test.tsx index 484a302e..6aa3a5dd 100644 --- a/tests/components/shared/UserAvatarBadge.test.tsx +++ b/tests/components/shared/UserAvatarBadge.test.tsx @@ -77,7 +77,7 @@ describe("UserAvatarBadge", () => { vi.useRealTimers(); }); - it("uses avatarUrl as img src", () => { + it("uses sizedAvatarUrl with s=40 and sets loading=lazy and decoding=async on img", () => { const avatarUrl = "https://avatars.githubusercontent.com/u/583231"; render(() => ( { /> )); const img = screen.getByRole("img"); - expect(img.getAttribute("src")).toBe(avatarUrl); + expect(img.getAttribute("src")).toBe("https://avatars.githubusercontent.com/u/583231?s=40"); + expect(img.getAttribute("loading")).toBe("lazy"); + expect(img.getAttribute("decoding")).toBe("async"); }); it("applies negative margin for stacked avatars when multiple users", () => { diff --git a/tests/integration/data-pipeline.test.ts b/tests/integration/data-pipeline.test.ts index ccbb5b72..c47562b8 100644 --- a/tests/integration/data-pipeline.test.ts +++ b/tests/integration/data-pipeline.test.ts @@ -104,7 +104,7 @@ describe("data pipeline: fetch → IDB cache → return", () => { expect(workflowRuns[0].repoFullName).toBe("octocat/Hello-World"); // IDB should have been written with the ETag - const cacheEntry = await getCacheEntry("runs:octocat/Hello-World:p1"); + const cacheEntry = await getCacheEntry("runs2:octocat/Hello-World:p1"); expect(cacheEntry).toBeDefined(); expect(cacheEntry!.etag).toBe('"etag-v1"'); expect((cacheEntry!.data as { workflow_runs: unknown[] }).workflow_runs).toHaveLength(1); @@ -172,11 +172,11 @@ describe("data pipeline: fetch → IDB cache → return", () => { ); // Verify it's in IDB - expect(await getCacheEntry("runs:octocat/Hello-World:p1")).toBeDefined(); + expect(await getCacheEntry("runs2:octocat/Hello-World:p1")).toBeDefined(); // Evict the cache await clearCache(); - expect(await getCacheEntry("runs:octocat/Hello-World:p1")).toBeUndefined(); + expect(await getCacheEntry("runs2:octocat/Hello-World:p1")).toBeUndefined(); // Second fetch should go back to API (no 304 this time) const updatedRun = { ...rawRun, id: 9002, run_number: 2 }; @@ -198,7 +198,7 @@ describe("data pipeline: fetch → IDB cache → return", () => { expect(workflowRuns[0].id).toBe(9002); // New ETag stored in IDB - const entry = await getCacheEntry("runs:octocat/Hello-World:p1"); + const entry = await getCacheEntry("runs2:octocat/Hello-World:p1"); expect(entry!.etag).toBe('"etag-v2"'); // Second call should NOT have sent If-None-Match (cache was empty) @@ -234,7 +234,7 @@ describe("data pipeline: fetch → IDB cache → return", () => { expect(errors[0].retryable).toBe(true); // IDB should NOT have been written - const entry = await getCacheEntry("runs:octocat/Hello-World:p1"); + const entry = await getCacheEntry("runs2:octocat/Hello-World:p1"); expect(entry).toBeUndefined(); }); }); diff --git a/tests/lib/notifications.test.ts b/tests/lib/notifications.test.ts index 75d03665..33c88701 100644 --- a/tests/lib/notifications.test.ts +++ b/tests/lib/notifications.test.ts @@ -3,7 +3,9 @@ import { canNotify, detectNewItems, dispatchNotifications, + SEEN_ID_CAP, _resetNotificationState, + _getSeenSizes, type NewItems, } from "../../src/app/lib/notifications"; import { ConfigSchema, type Config } from "../../src/app/stores/config"; @@ -180,6 +182,78 @@ describe("detectNewItems", () => { expect(result.pullRequests).toHaveLength(1); expect(result.workflowRuns).toHaveLength(1); }); + + it("caps seen IDs at SEEN_ID_CAP and evicts the oldest entries in insertion order", () => { + const initialIssues = Array.from({ length: SEEN_ID_CAP }, (_, i) => makeIssue(i + 1)); + detectNewItems(makeData(initialIssues)); + expect(_getSeenSizes().issues).toBe(SEEN_ID_CAP); + + // Add 10 new IDs past the cap -> oldest 1..10 should be evicted + const tenMore = Array.from({ length: 10 }, (_, i) => makeIssue(SEEN_ID_CAP + i + 1)); + const addedResult = detectNewItems(makeData(tenMore)); + expect(addedResult.issues).toHaveLength(10); + expect(_getSeenSizes().issues).toBe(SEEN_ID_CAP); + + // ID 11 should still be remembered (not reported as new), while ID 1 was evicted (reported as new) + const probeResult = detectNewItems(makeData([makeIssue(1), makeIssue(11)])); + expect(probeResult.issues.map((i) => i.id)).toEqual([1]); + }); + + it("refreshes an old ID's recency when re-seen so it is not evicted next", () => { + const initialIssues = Array.from({ length: SEEN_ID_CAP }, (_, i) => makeIssue(i + 1)); + detectNewItems(makeData(initialIssues)); + + // Re-see ID 1 so it moves to the most-recent position + const reseeResult = detectNewItems(makeData([makeIssue(1)])); + expect(reseeResult.issues).toHaveLength(0); + + // Push 1 new ID past the cap -> ID 2 (now oldest) should be evicted, while ID 1 remains + detectNewItems(makeData([makeIssue(SEEN_ID_CAP + 1)])); + expect(_getSeenSizes().issues).toBe(SEEN_ID_CAP); + + const probeResult = detectNewItems(makeData([makeIssue(1), makeIssue(2)])); + expect(probeResult.issues.map((i) => i.id)).toEqual([2]); + }); + + it("refreshes existing IDs in pass 1 before inserting new IDs and evicting in pass 2 within a single poll batch", () => { + const initialIssues = Array.from({ length: SEEN_ID_CAP }, (_, i) => makeIssue(i + 1)); + detectNewItems(makeData(initialIssues)); + + // Single poll batch with a new item ahead of the oldest seen item (ID 1): + // Pass 1 refreshes ID 1's recency before pass 2 inserts SEEN_ID_CAP + 1 and evicts ID 2. + const batchResult = detectNewItems(makeData([makeIssue(SEEN_ID_CAP + 1), makeIssue(1)])); + expect(batchResult.issues.map((i) => i.id)).toEqual([SEEN_ID_CAP + 1]); + expect(_getSeenSizes().issues).toBe(SEEN_ID_CAP); + + const probeResult = detectNewItems(makeData([makeIssue(1), makeIssue(2)])); + expect(probeResult.issues.map((i) => i.id)).toEqual([2]); + }); + + it("does not report an item as new when it disappears for several polls and reappears", () => { + detectNewItems(makeData([makeIssue(1), makeIssue(2)])); + + // Issue 2 disappears across several polls + expect(detectNewItems(makeData([makeIssue(1)])).issues).toHaveLength(0); + expect(detectNewItems(makeData([makeIssue(1)])).issues).toHaveLength(0); + expect(detectNewItems(makeData([])).issues).toHaveLength(0); + + // Issue 2 reappears -> still remembered, not reported as new + const result = detectNewItems(makeData([makeIssue(1), makeIssue(2)])); + expect(result.issues).toHaveLength(0); + }); + + it("does not re-notify existing workflow runs when enableActions is toggled off and back on", () => { + const runs = [makeRun(101), makeRun(102)]; + detectNewItems(makeData([makeIssue(1)], [], runs)); + + // Poll while enableActions is false (workflowRuns is empty) + const offPoll = detectNewItems(makeData([makeIssue(1)], [], [])); + expect(offPoll.workflowRuns).toHaveLength(0); + + // Poll after enableActions is turned back on -> existing runs are not reported as new + const onPoll = detectNewItems(makeData([makeIssue(1)], [], runs)); + expect(onPoll.workflowRuns).toHaveLength(0); + }); }); // ── dispatchNotifications ───────────────────────────────────────────────────── diff --git a/tests/lib/url.test.ts b/tests/lib/url.test.ts index 80c483e2..1b7f83d6 100644 --- a/tests/lib/url.test.ts +++ b/tests/lib/url.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import { isSafeGitHubUrl, isSafeJiraSiteUrl, openGitHubUrl } from "../../src/app/lib/url"; +import { isSafeGitHubUrl, isSafeJiraSiteUrl, openGitHubUrl, sizedAvatarUrl } from "../../src/app/lib/url"; describe("isSafeGitHubUrl", () => { it("returns true for a root GitHub URL", () => { @@ -107,3 +107,40 @@ describe("openGitHubUrl", () => { expect(window.open).not.toHaveBeenCalled(); }); }); + +describe("sizedAvatarUrl", () => { + it("adds the s query param to an avatars.githubusercontent.com URL", () => { + expect(sizedAvatarUrl("https://avatars.githubusercontent.com/u/583231", 40)).toBe( + "https://avatars.githubusercontent.com/u/583231?s=40", + ); + }); + + it("replaces an existing s query param while preserving other query params", () => { + expect(sizedAvatarUrl("https://avatars.githubusercontent.com/u/583231?v=4&s=200", 56)).toBe( + "https://avatars.githubusercontent.com/u/583231?v=4&s=56", + ); + }); + + it("returns non-GitHub avatar host URLs unchanged", () => { + const nonGitHub = "https://example.com/avatar.png"; + expect(sizedAvatarUrl(nonGitHub, 40)).toBe(nonGitHub); + expect(sizedAvatarUrl("https://github.com/images/error/octocat_happy.gif", 56)).toBe( + "https://github.com/images/error/octocat_happy.gif", + ); + }); + + it("returns non-HTTPS avatars.githubusercontent.com URLs unchanged", () => { + const httpUrl = "http://avatars.githubusercontent.com/u/583231"; + expect(sizedAvatarUrl(httpUrl, 40)).toBe(httpUrl); + }); + + it("returns bare root path on avatars.githubusercontent.com unchanged", () => { + const rootUrl = "https://avatars.githubusercontent.com/"; + expect(sizedAvatarUrl(rootUrl, 40)).toBe(rootUrl); + }); + + it("returns invalid URLs and empty strings unchanged", () => { + expect(sizedAvatarUrl("not-a-url", 40)).toBe("not-a-url"); + expect(sizedAvatarUrl("", 40)).toBe(""); + }); +}); diff --git a/tests/services/api-usage.test.ts b/tests/services/api-usage.test.ts index 0835493a..1f0c62d7 100644 --- a/tests/services/api-usage.test.ts +++ b/tests/services/api-usage.test.ts @@ -473,4 +473,38 @@ describe("deriveSource — URL pattern matching", () => { it("falls back to 'graphql' for unrecognized apiSource string", () => { expect(mod.deriveSource(makeInfo({ isGraphql: true, url: "/graphql", apiSource: "unknownLabel" }))).toBe("graphql"); }); + + it("skips trackApiCall on HTTP 304 Not Modified while still updating resetAt", async () => { + const githubMod = await import("../../src/app/services/github"); + const onApiRequestMock = vi.mocked(githubMod.onApiRequest); + const registeredCb = onApiRequestMock.mock.calls.at(-1)?.[0]; + expect(registeredCb).toBeDefined(); + + registeredCb!( + makeInfo({ + url: "/repos/foo/bar/actions/runs/12345", + status: 304, + resetEpochMs: 1_800_000_000_000, + }), + ); + + expect(mod.getUsageSnapshot()).toEqual([]); + expect(mod.getUsageResetAt()).toBe(1_800_000_000_000); + + registeredCb!( + makeInfo({ + url: "/repos/foo/bar/actions/runs/12345", + status: 200, + resetEpochMs: 1_800_000_000_000, + }), + ); + + expect(mod.getUsageSnapshot()).toEqual([ + expect.objectContaining({ + source: "hotRunStatus", + pool: "core", + count: 1, + }), + ]); + }); }); diff --git a/tests/services/github-status.test.ts b/tests/services/github-status.test.ts index 514ebf6d..746bc0c6 100644 --- a/tests/services/github-status.test.ts +++ b/tests/services/github-status.test.ts @@ -1,5 +1,6 @@ import "fake-indexeddb/auto"; import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { createEffect, createRoot } from "solid-js"; import { clearCache } from "../../src/app/stores/cache"; const mockPushNotification = vi.fn(); @@ -274,18 +275,101 @@ describe("fetchGitHubStatus", () => { expect(mockFetch).toHaveBeenCalledTimes(1); }); - it("304 cache-hit path: reflects parseSummary run against the cached JSON, not empty/null", async () => { + it("304 cache-hit path on cold start (_githubStatus() === null): reflects parseSummary run against the cached JSON, not empty/null", async () => { + const components = [...TRACKED_COMPONENT_NAMES].map((n) => + makeComponent(n, n === "Actions" ? "major_outage" : "operational") + ); const incidents = [makeIncident({ id: "cached-1", name: "Cached Incident", body: "Investigating", componentNames: ["Actions"] })]; - vi.stubGlobal("fetch", vi.fn().mockResolvedValue(jsonResponse(makeSummary({ incidents }), { etag: "etag-1" }))); + vi.stubGlobal("fetch", vi.fn().mockResolvedValue(jsonResponse(makeSummary({ components, incidents }), { etag: "etag-1" }))); await fetchGitHubStatus(); expect(getGitHubStatus()!.incidents).toHaveLength(1); + // Simulate a cold start / page reload where IndexedDB retains the ETag + JSON + // while in-memory _githubStatus() is reset to null: + resetGitHubStatusState(); + mockPushNotification.mockClear(); + expect(getGitHubStatus()).toBeNull(); + vi.stubGlobal("fetch", vi.fn().mockResolvedValue(new Response(null, { status: 304 }))); await fetchGitHubStatus(); const result = getGitHubStatus(); expect(result!.incidents).toHaveLength(1); expect(result!.incidents[0].name).toBe("Cached Incident"); + expect(mockPushNotification).toHaveBeenCalledWith("github-status", "Cached Incident", "error", false); + }); + + it("short-circuits on a consecutive 304 without re-setting the signal or re-running notifyTransitions", async () => { + const incidents = [makeIncident({ id: "cached-1", name: "Cached Incident", body: "Investigating", componentNames: ["Actions"] })]; + vi.stubGlobal("fetch", vi.fn().mockResolvedValue(jsonResponse(makeSummary({ incidents }), { etag: "etag-1" }))); + await fetchGitHubStatus(); + + const firstSummary = getGitHubStatus(); + expect(firstSummary).not.toBeNull(); + + let signalEmissions = 0; + const dispose = createRoot((d) => { + createEffect(() => { + getGitHubStatus(); + signalEmissions++; + }); + return d; + }); + + try { + expect(signalEmissions).toBe(1); + mockPushNotification.mockClear(); + mockDismissNotificationBySource.mockClear(); + + vi.stubGlobal("fetch", vi.fn().mockResolvedValue(new Response(null, { status: 304 }))); + await fetchGitHubStatus(); + + // Signal was not written again and notification transitions did not re-run + expect(signalEmissions).toBe(1); + expect(getGitHubStatus()).toBe(firstSummary); + expect(mockPushNotification).not.toHaveBeenCalled(); + expect(mockDismissNotificationBySource).not.toHaveBeenCalled(); + } finally { + dispose(); + } + }); + + it("re-runs parseSummary and notifyTransitions on a 304 after 3 consecutive failures so a dismissed ongoing incident is re-shown", async () => { + const components = [...TRACKED_COMPONENT_NAMES].map((n) => + makeComponent(n, n === "Actions" ? "major_outage" : "operational") + ); + const incidents = [makeIncident({ id: "inc-recover", name: "Actions Outage", body: "Investigating", componentNames: ["Actions"] })]; + vi.stubGlobal("fetch", vi.fn().mockResolvedValue(jsonResponse(makeSummary({ components, incidents }), { etag: "etag-inc" }))); + await fetchGitHubStatus(); + const initialSummary = getGitHubStatus(); + expect(mockPushNotification).toHaveBeenCalledWith("github-status", "Actions Outage", "error", false); + + // 3 consecutive failures dismiss the active incident notification + vi.stubGlobal("fetch", vi.fn().mockRejectedValue(new TypeError("Failed to fetch"))); + mockDismissNotificationBySource.mockClear(); + await fetchGitHubStatus(); + await fetchGitHubStatus(); + await fetchGitHubStatus(); + expect(mockDismissNotificationBySource).toHaveBeenCalledWith("github-status"); + + mockPushNotification.mockClear(); + mockDismissNotificationBySource.mockClear(); + + // Recovery answered by 304 must take the full path and re-push the incident + vi.stubGlobal("fetch", vi.fn().mockResolvedValue(new Response(null, { status: 304 }))); + await fetchGitHubStatus(); + + expect(mockPushNotification).toHaveBeenCalledTimes(1); + expect(mockPushNotification).toHaveBeenCalledWith("github-status", "Actions Outage", "error", false); + const recoveredSummary = getGitHubStatus(); + expect(recoveredSummary).not.toBe(initialSummary); + expect(recoveredSummary!.incidents[0].name).toBe("Actions Outage"); + + // Subsequent 304 now short-circuits normally because _consecutiveFailures was reset to 0 + mockPushNotification.mockClear(); + await fetchGitHubStatus(); + expect(mockPushNotification).not.toHaveBeenCalled(); + expect(getGitHubStatus()).toBe(recoveredSummary); }); it("keeps the prior value on a single fetch failure and does not dismiss the notification (CR-002)", async () => { diff --git a/tests/services/hot-poll.test.ts b/tests/services/hot-poll.test.ts index 6e6bbd56..c456d9a9 100644 --- a/tests/services/hot-poll.test.ts +++ b/tests/services/hot-poll.test.ts @@ -16,14 +16,31 @@ Object.defineProperty(window, "location", { // Mock github module so getClient returns our fake octokit const mockGetClient = vi.fn(); +const defaultCachedRequestImpl = async ( + octokit: { request: (route: string, params?: Record) => Promise<{ data: unknown }> }, + _key: string, + route: string, + params?: Record, + options?: { project?: (data: never) => unknown }, +) => { + const response = await octokit.request(route, params); + const data = options?.project ? options.project(response.data as never) : response.data; + return { data, fromCache: false }; +}; +const mockCachedRequest = vi.fn(defaultCachedRequestImpl); vi.mock("../../src/app/services/github", () => ({ getClient: () => mockGetClient(), - cachedRequest: vi.fn(), + cachedRequest: (...args: Parameters) => mockCachedRequest(...args), updateGraphqlRateLimit: vi.fn(), updateRateLimitFromHeaders: vi.fn(), onApiRequest: vi.fn(), })); +const mockDeleteCacheEntry = vi.fn().mockResolvedValue(undefined); +vi.mock("../../src/app/stores/cache", () => ({ + deleteCacheEntry: (...args: unknown[]) => mockDeleteCacheEntry(...args), +})); + // Mock errors/notifications so poll.ts module doesn't crash vi.mock("../../src/app/lib/errors", () => ({ pushError: vi.fn(), @@ -84,6 +101,7 @@ import { import { fetchHotPRStatus, fetchWorkflowRunById, + projectSingleRun, isUnauthorizedError, } from "../../src/app/services/api"; @@ -441,8 +459,18 @@ describe("fetchHotPRStatus", () => { }); describe("fetchWorkflowRunById", () => { + beforeEach(() => { + mockCachedRequest.mockReset(); + mockDeleteCacheEntry.mockClear(); + }); + + afterEach(() => { + mockCachedRequest.mockImplementation(defaultCachedRequestImpl); + }); + it("maps snake_case response to camelCase", async () => { - const octokit = makeOctokit(() => Promise.resolve({ + const octokit = makeOctokit(); + mockCachedRequest.mockResolvedValue({ data: { id: 100, status: "in_progress", @@ -450,8 +478,8 @@ describe("fetchWorkflowRunById", () => { updated_at: "2026-03-29T10:00:00Z", completed_at: null, }, - headers: {}, - })); + fromCache: false, + }); const result = await fetchWorkflowRunById(octokit as never, { id: 100, @@ -464,19 +492,43 @@ describe("fetchWorkflowRunById", () => { expect(result.conclusion).toBeNull(); expect(result.updatedAt).toBe("2026-03-29T10:00:00Z"); expect(result.completedAt).toBeNull(); + expect(mockDeleteCacheEntry).not.toHaveBeenCalled(); }); - it("calls octokit.request with correct route and params", async () => { - const octokit = makeOctokit(() => Promise.resolve({ + it("calls cachedRequest with key/route/params/project and deletes cache entry on terminal or waiting status", async () => { + const octokit = makeOctokit(); + mockCachedRequest.mockResolvedValue({ data: { id: 200, status: "completed", conclusion: "success", updated_at: "2026-01-01T00:00:00Z", completed_at: "2026-01-01T00:05:00Z" }, - headers: {}, - })); + fromCache: false, + }); await fetchWorkflowRunById(octokit as never, { id: 200, owner: "myorg", repo: "myrepo" }); - expect(octokit.request).toHaveBeenCalledWith( + expect(mockCachedRequest).toHaveBeenCalledWith( + octokit, + "run:myorg/myrepo:200", "GET /repos/{owner}/{repo}/actions/runs/{run_id}", { owner: "myorg", repo: "myrepo", run_id: 200 }, + { project: projectSingleRun }, ); + expect(mockDeleteCacheEntry).toHaveBeenCalledWith("run:myorg/myrepo:200"); + }); + + it("still returns mapped completed update when deleteCacheEntry rejects", async () => { + const octokit = makeOctokit(); + mockCachedRequest.mockResolvedValue({ + data: { id: 201, status: "completed", conclusion: "failure", updated_at: "2026-01-01T00:00:00Z", completed_at: "2026-01-01T00:05:00Z" }, + fromCache: false, + }); + mockDeleteCacheEntry.mockRejectedValueOnce(new Error("IDB delete failed")); + + const result = await fetchWorkflowRunById(octokit as never, { id: 201, owner: "myorg", repo: "myrepo" }); + expect(result).toEqual({ + id: 201, + status: "completed", + conclusion: "failure", + updatedAt: "2026-01-01T00:00:00Z", + completedAt: "2026-01-01T00:05:00Z", + }); }); // NOTE: updateRateLimitFromHeaders is now handled by the Octokit hook.wrap diff --git a/tests/services/workflow-run-conditional.test.ts b/tests/services/workflow-run-conditional.test.ts new file mode 100644 index 00000000..b54fcab5 --- /dev/null +++ b/tests/services/workflow-run-conditional.test.ts @@ -0,0 +1,172 @@ +import "fake-indexeddb/auto"; +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { clearCache, getCacheEntry } from "../../src/app/stores/cache"; +import { fetchWorkflowRunById } from "../../src/app/services/api"; + +vi.mock("../../src/app/lib/dashboard-snapshot", () => ({ + clearSnapshot: vi.fn().mockResolvedValue(undefined), + loadSnapshot: vi.fn().mockResolvedValue(null), + writeSnapshot: vi.fn(), + flushSnapshot: vi.fn().mockResolvedValue(undefined), +})); + +describe("fetchWorkflowRunById conditional ETag requests", () => { + beforeEach(async () => { + await clearCache(); + vi.clearAllMocks(); + }); + + it("sends no If-None-Match on the first call, projects the cached payload, sends the stored ETag on the second, returns the cached mapped update on 304, and deletes the cache entry on completed status", async () => { + const rawInProgress = { + id: 101, + status: "in_progress", + conclusion: null, + updated_at: "2026-11-01T15:52:22Z", + completed_at: null, + repository: { id: 1, full_name: "org/repo" }, + head_repository: { id: 1, full_name: "org/repo" }, + actor: { login: "octocat", id: 99 }, + jobs_url: "https://api.github.com/repos/org/repo/actions/runs/101/jobs", + }; + + const requestFn = vi.fn().mockResolvedValueOnce({ + data: rawInProgress, + headers: { etag: 'W/"etag-run-101"' }, + status: 200, + }); + const octokit = { request: requestFn } as never; + const descriptor = { id: 101, owner: "org", repo: "repo" }; + const cacheKey = "run:org/repo:101"; + + // 1. First call: no If-None-Match header sent; ETag + projected payload stored in IndexedDB + const first = await fetchWorkflowRunById(octokit, descriptor); + expect(requestFn).toHaveBeenCalledTimes(1); + expect(requestFn).toHaveBeenNthCalledWith( + 1, + "GET /repos/{owner}/{repo}/actions/runs/{run_id}", + { + owner: "org", + repo: "repo", + run_id: 101, + headers: {}, + }, + ); + expect(first).toEqual({ + id: 101, + status: "in_progress", + conclusion: null, + updatedAt: "2026-11-01T15:52:22Z", + completedAt: null, + }); + + const cachedAfterFirst = await getCacheEntry(cacheKey); + expect(cachedAfterFirst).toBeDefined(); + expect(cachedAfterFirst?.etag).toBe('W/"etag-run-101"'); + expect(cachedAfterFirst?.data).toEqual({ + id: 101, + status: "in_progress", + conclusion: null, + updated_at: "2026-11-01T15:52:22Z", + completed_at: null, + }); + + // 2. Second call: sends stored If-None-Match; 304 returns the cached mapped update + const err304 = Object.assign(new Error("Not Modified"), { status: 304 }); + requestFn.mockRejectedValueOnce(err304); + + const second = await fetchWorkflowRunById(octokit, descriptor); + expect(requestFn).toHaveBeenCalledTimes(2); + expect(requestFn).toHaveBeenNthCalledWith( + 2, + "GET /repos/{owner}/{repo}/actions/runs/{run_id}", + { + owner: "org", + repo: "repo", + run_id: 101, + headers: { "If-None-Match": 'W/"etag-run-101"' }, + }, + ); + expect(second).toEqual({ + id: 101, + status: "in_progress", + conclusion: null, + updatedAt: "2026-11-01T15:52:22Z", + completedAt: null, + }); + expect(await getCacheEntry(cacheKey)).toBeDefined(); + + // 3. Third call: run completes with 200 -> returns mapped completed update and deletes cache entry + requestFn.mockResolvedValueOnce({ + data: { + id: 101, + status: "completed", + conclusion: "success", + updated_at: "2026-11-01T15:54:33Z", + completed_at: "2026-11-01T15:54:33Z", + }, + headers: { etag: 'W/"etag-run-101-completed"' }, + status: 200, + }); + + const third = await fetchWorkflowRunById(octokit, descriptor); + expect(requestFn).toHaveBeenCalledTimes(3); + expect(requestFn).toHaveBeenNthCalledWith( + 3, + "GET /repos/{owner}/{repo}/actions/runs/{run_id}", + { + owner: "org", + repo: "repo", + run_id: 101, + headers: { "If-None-Match": 'W/"etag-run-101"' }, + }, + ); + expect(third).toEqual({ + id: 101, + status: "completed", + conclusion: "success", + updatedAt: "2026-11-01T15:54:33Z", + completedAt: "2026-11-01T15:54:33Z", + }); + + // Terminal status deletes the entry from IndexedDB + expect(await getCacheEntry(cacheKey)).toBeUndefined(); + }); + + it("deletes the run: cache entry when a run transitions to waiting status", async () => { + const requestFn = vi + .fn() + .mockResolvedValueOnce({ + data: { + id: 202, + status: "queued", + conclusion: null, + updated_at: "2026-11-01T15:50:00Z", + completed_at: null, + }, + headers: { etag: 'W/"etag-run-202"' }, + status: 200, + }) + .mockResolvedValueOnce({ + data: { + id: 202, + status: "waiting", + conclusion: null, + updated_at: "2026-11-01T15:51:00Z", + completed_at: null, + }, + headers: { etag: 'W/"etag-run-202-waiting"' }, + status: 200, + }); + + const octokit = { request: requestFn } as never; + const descriptor = { id: 202, owner: "org", repo: "repo" }; + const cacheKey = "run:org/repo:202"; + + await fetchWorkflowRunById(octokit, descriptor); + expect(await getCacheEntry(cacheKey)).toBeDefined(); + + const waitingUpdate = await fetchWorkflowRunById(octokit, descriptor); + expect(waitingUpdate.status).toBe("waiting"); + expect(await getCacheEntry(cacheKey)).toBeUndefined(); + }); +}); diff --git a/tests/stores/cache.test.ts b/tests/stores/cache.test.ts index 88fd169a..6cb2ee0b 100644 --- a/tests/stores/cache.test.ts +++ b/tests/stores/cache.test.ts @@ -8,8 +8,14 @@ import { deleteCacheEntry, clearCache, evictStaleEntries, + evictKeysWithPrefix, cachedFetch, } from "../../src/app/stores/cache"; +import { + projectRunsPage, + mapWorkflowRun, + type RawWorkflowRunsResponse, +} from "../../src/app/services/api"; vi.mock("@sentry/solid", () => ({ captureException: vi.fn(), @@ -374,4 +380,164 @@ describe("cachedFetch", () => { "Unexpected fetch status: 500" ); }); + + it("projects workflow run payloads before storing on 200 and preserves mapper equivalence across 200 and 304", async () => { + const fullRun = { + id: 4201, + name: "CI", + status: "completed", + conclusion: "success", + event: "push", + workflow_id: 77, + head_sha: "abc1234def5678", + head_branch: "main", + run_number: 18, + html_url: "https://github.com/octocat/Hello-World/actions/runs/4201", + created_at: "2026-11-01T10:00:00Z", + updated_at: "2026-11-01T10:05:00Z", + run_started_at: "2026-11-01T10:00:05Z", + completed_at: "2026-11-01T10:05:00Z", + run_attempt: 1, + display_title: "Run CI suite", + actor: { + login: "octocat", + id: 1, + avatar_url: "https://avatars.githubusercontent.com/u/1?v=4", + html_url: "https://github.com/octocat", + }, + repository: { + id: 1296269, + name: "Hello-World", + full_name: "octocat/Hello-World", + }, + head_repository: { + id: 1296269, + name: "Hello-World", + full_name: "octocat/Hello-World", + }, + jobs_url: "https://api.github.com/repos/octocat/Hello-World/actions/runs/4201/jobs", + }; + + const fullPage: RawWorkflowRunsResponse = { + total_count: 1, + workflow_runs: [fullRun], + }; + + const fetch200 = vi.fn().mockResolvedValue({ + data: fullPage, + etag: '"runs-etag-1"', + lastModified: null, + status: 200, + }); + + const res200 = await cachedFetch( + "runs2:octocat/Hello-World:p1", + fetch200, + { project: projectRunsPage } + ); + + const returned200Page = res200.data as RawWorkflowRunsResponse; + const returnedRun = returned200Page.workflow_runs[0] as unknown as Record; + expect(returnedRun.repository).toBeUndefined(); + expect(returnedRun.head_repository).toBeUndefined(); + expect(returnedRun.jobs_url).toBeUndefined(); + expect(returnedRun.actor).toEqual({ login: "octocat" }); + + const stored = await getCacheEntry("runs2:octocat/Hello-World:p1"); + expect(stored).toBeDefined(); + const storedPage = stored!.data as RawWorkflowRunsResponse; + const storedRun = storedPage.workflow_runs[0] as unknown as Record; + expect(storedRun.repository).toBeUndefined(); + expect(storedRun.head_repository).toBeUndefined(); + expect(storedRun.actor).toEqual({ login: "octocat" }); + + // Mapper output on projected run deep-equals mapper output on full run + const mappedFromFull = mapWorkflowRun(fullRun, "octocat/Hello-World"); + const mappedFrom200 = mapWorkflowRun(returned200Page.workflow_runs[0], "octocat/Hello-World"); + expect(mappedFrom200).toEqual(mappedFromFull); + + // Subsequent 304 returns cached projected data that maps identically + const fetch304 = vi.fn().mockResolvedValue({ + data: null, + etag: null, + lastModified: null, + status: 304, + }); + + const res304 = await cachedFetch( + "runs2:octocat/Hello-World:p1", + fetch304, + { project: projectRunsPage } + ); + + expect(res304.fromCache).toBe(true); + const returned304Page = res304.data as RawWorkflowRunsResponse; + expect(mapWorkflowRun(returned304Page.workflow_runs[0], "octocat/Hello-World")).toEqual( + mappedFromFull + ); + }); + + it("refreshes fetchedAt on 304 only when the entry is at least 1 hour old so hourly 24h eviction does not drop active keys", async () => { + const db = await getDb(); + const initialFetchedAt = Date.now() - 2 * 60 * 60 * 1000; // 2 hours ago (>= 1h throttle) + await db.put("cache", { + key: "runs2:octocat/Hello-World:p1", + data: { total_count: 0, workflow_runs: [] }, + etag: '"etag-old"', + lastModified: null, + fetchedAt: initialFetchedAt, + maxAge: null, + }); + + const fetch304 = vi.fn().mockResolvedValue({ + data: null, + etag: null, + lastModified: null, + status: 304, + }); + + const firstRes = await cachedFetch("runs2:octocat/Hello-World:p1", fetch304); + expect(firstRes.fromCache).toBe(true); + const refreshedEntry = await getCacheEntry("runs2:octocat/Hello-World:p1"); + expect(refreshedEntry).toBeDefined(); + expect(refreshedEntry!.fetchedAt).toBeGreaterThan(initialFetchedAt); + + // An immediate second 304 within the 1h throttle window does not rewrite fetchedAt + const recentFetchedAt = Date.now() - 5 * 60 * 1000; // 5 minutes ago (< 1h throttle) + await db.put("cache", { + ...refreshedEntry!, + fetchedAt: recentFetchedAt, + }); + await cachedFetch("runs2:octocat/Hello-World:p1", fetch304); + const afterSecond304 = await getCacheEntry("runs2:octocat/Hello-World:p1"); + expect(afterSecond304!.fetchedAt).toBe(recentFetchedAt); + }); +}); + +describe("evictKeysWithPrefix", () => { + it("deletes only keys starting with any of the provided prefixes using ranged deletes", async () => { + await setCacheEntry("runs2:a/b:p1", { page: 1 }, "e1"); + await setCacheEntry("runs2:a/b:p2", { page: 2 }, "e2"); + await setCacheEntry("run:a/b:101", { id: 101 }, "e-run"); + await setCacheEntry("runs2:a/b", { boundary: "no-trailing-colon" }, "e-boundary1"); + await setCacheEntry("runs2:a/bc:p1", { boundary: "longer-repo-name" }, "e-boundary2"); + await setCacheEntry("runs2:c/d:p1", { page: 1 }, "e3"); + await setCacheEntry("other:key", { other: true }, "e4"); + + await evictKeysWithPrefix(["runs2:a/b:", "run:a/b:"]); + + expect(await getCacheEntry("runs2:a/b:p1")).toBeUndefined(); + expect(await getCacheEntry("runs2:a/b:p2")).toBeUndefined(); + expect(await getCacheEntry("run:a/b:101")).toBeUndefined(); + expect(await getCacheEntry("runs2:a/b")).toBeDefined(); + expect(await getCacheEntry("runs2:a/bc:p1")).toBeDefined(); + expect(await getCacheEntry("runs2:c/d:p1")).toBeDefined(); + expect(await getCacheEntry("other:key")).toBeDefined(); + }); + + it("is a no-op when passed an empty prefix list", async () => { + await setCacheEntry("runs2:a/b:p1", { page: 1 }, "e1"); + await evictKeysWithPrefix([]); + expect(await getCacheEntry("runs2:a/b:p1")).toBeDefined(); + }); });